Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue with dynamic content providers and reference ui (and more) #11360

Closed
eamodio opened this issue Sep 1, 2016 · 9 comments
Closed

Issue with dynamic content providers and reference ui (and more) #11360

eamodio opened this issue Sep 1, 2016 · 9 comments
Assignees
Labels
api *question Issue represents a question, should be posted to StackOverflow (VS Code)

Comments

@eamodio
Copy link
Contributor

eamodio commented Sep 1, 2016

  • VSCode Version: Version 1.5.0-insider (1.5.0-insider) 1de4d83
  • OS Version: OS X El Capitan Version 10.11.6 (15G31)

I'm working on a new extension to provide CodeLens for Git across most supported vscode languages (assuming they implement a DocumentSymbolProvider. But I've run into a few bugs as well as API (and UI) limitations.

You can find my GitLens extension on GitHub with the latest release here. I haven't yet wanted to publish it to the marketplace until I get some of these issues resolved.

Here are a couple of screenshots to give you an idea of the UI:

Provides CodeLens with the author and date of the last check-in.

CodeLens

Clicking on a CodeLens opens a blame "explorer" with the commits and changed lines in the right pane and the commit (file) contents on the left.

Blame Explorer

Let me first say that I know I am really stretching the limits (read: hijacking) the reference UI, but I think it does come pretty close to something pretty cool for this.

The first biggest issue I've been running into is using a TextDocumentContentProvider to provide the content for a reference location url seems to time out after a minute or so -- seems like it is getting garbage collection, but I'm not sure how to stop that.

Next issue is I would like to provide decorators (blame line indication) for the content provided by my TextDocumentContentProvider. I've been able to hack this the first time the my GitBlameContentProvider is loaded for a particular uri, but once it is cached there doesn't seem to be any published event that I can trap to trigger a decoration. It would be really nice if there was an even when a reference was selected in the reference UI. It would also be great if there was a way to officially find a particular uri in the visible set of editors -- right now I have this awful hack.

I would also love to be able to control the order (and initial selection) of locations provided to the reference UI -- right now they are sorted alphabetically, so I am resorting to crafting my uris with an embedded index -- which is less than ideal. Also the initial selection of the reference UI seems to be the location closest to the trigger point in the document, but I would love to be able to control that -- even for real references it is arguable if that is ideal.

I thought I was also seeing race conditions with the Omnisharp-vscode extension, where the C# DocumentSymbolProvider wouldn't respond when my CodeLens would try to execute vscode.executeDocumentSymbolProvider. I haven't been able to really reproduce this lately, so it could be gone now.

Thank you for the awesome product that vscode is and I would greatly appreciate any help or guidance on these issues!

Thanks,
Eric Amodio

@dbaeumer dbaeumer added the api label Sep 1, 2016
@jrieken jrieken added this to the September 2016 milestone Sep 1, 2016
@jrieken jrieken added *question Issue represents a question, should be posted to StackOverflow (VS Code) feature-request Request for new features or functionality labels Sep 1, 2016
@jrieken
Copy link
Member

jrieken commented Sep 20, 2016

Let me first say that I know I am really stretching the limits (read: hijacking) the reference UI, but I think it does come pretty close to something pretty cool for this.

Yes you are. We will not bend the reference search UI in funny ways but we are exploring to allow custom peek rendered peek widgets this milestone and are likely to have something real in October (see #3220). Feel free to add ideas/requirements etc to #3220.

The first biggest issue I've been running into is using a TextDocumentContentProvider to provide the content for a reference location url seems to time out after a minute

We do that and you cannot stop it. Periodically we check if a document is still being shown and dispose it iff not. Your provider should always be able to re-create a document from a uri.

It would also be great if there was a way to officially find a particular uri in the visible set of editors

It's looping over the set of visible editor and checking the document uri. I think that's fair to ask but note that multiple editors might show the same uri. Also note that in your hack you are not using the official API but some _-fields for which we don't guarantee anything. Please use the API defined here vscode.d.ts

@jrieken jrieken closed this as completed Sep 20, 2016
@jrieken jrieken removed the feature-request Request for new features or functionality label Sep 20, 2016
@eamodio
Copy link
Contributor Author

eamodio commented Sep 20, 2016

@jrieken custom peek sounds cool -- will take a look at the noted issues.

As for the TextDocumentContentProvider timing out -- that seems like a bug that shouldn't be ignored. If I am currently looking at the content from that provider -- it shouldn't just disappear after a timeout.

@jrieken
Copy link
Member

jrieken commented Sep 21, 2016

If I am currently looking at the content from that provider

You mean it's open/visible in an editor and closes? Is this in peek?

@eamodio
Copy link
Contributor Author

eamodio commented Sep 21, 2016

@jrieken Yes, the content is visible in the peek and will just go blank after ~1 minute.

@eamodio
Copy link
Contributor Author

eamodio commented Sep 21, 2016

@jrieken Also while I understand the desire to not "bend the reference search UI in funny ways" one enhancement -- provide an event when the selection changes in the peek UI -- doesn't feel (to me at least) as much bending. Right now because there is no focus change on switching in the peek UI there is no onDidChangeActiveTextEditor event (or any other that I could see) so there is just no way at all to know that the content in the peek has changed.

Any chance something like that would be acceptable?

@jrieken
Copy link
Member

jrieken commented Sep 21, 2016

It actual should fire onDidChangeActiveTextEditor - it's a bug iff not

@eamodio
Copy link
Contributor Author

eamodio commented Sep 21, 2016

@jrieken I believe it only fires it once when the peek opens -- but doesn't as you switch between the references unless you select a ref and then click inside the content (left pane) to give it focus.

@eamodio
Copy link
Contributor Author

eamodio commented Oct 6, 2016

@jrieken should I open 2 new specific issues for these bugs?

1 - for the TextDocumentContentProvider timing out even when visible in the peek UI
2 - for onDidChangeActiveTextEditor not firing when changing selection in the right pane of the peek UI

@jrieken
Copy link
Member

jrieken commented Oct 6, 2016

I have on my list but not yet created issues for it - feel free to go ahead with it

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api *question Issue represents a question, should be posted to StackOverflow (VS Code)
Projects
None yet
Development

No branches or pull requests

3 participants