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

Improve editor history navigation: merge locations #29027

Closed
bpasero opened this issue Jun 19, 2017 · 9 comments
Closed

Improve editor history navigation: merge locations #29027

bpasero opened this issue Jun 19, 2017 · 9 comments
Assignees
Labels
feature-request Request for new features or functionality on-testplan ux User experience issues

Comments

@bpasero
Copy link
Member

bpasero commented Jun 19, 2017

There are some scenarios where our editor history navigation is not ideal.

Example: Goto definition

  1. go to definition to jump somewhere else and produce a history entry
  2. from the place where you end up, click around a little bit (not more than 5 lines away)
  3. jump back
  4. jump forward

=> you end up at the original place (the definition) instead of the last location you were before going back

Example: Exploring code

  1. click into a line
  2. click 3 lines below
  3. click 3 lines below
  4. jump back

=> you end up not in the original line from 1. but in the one from 2.

Both issues can be solved by smartly merging a history entry with the current position if it does not qualify for a new location.

/cc @egamma

@bpasero bpasero added editor feature-request Request for new features or functionality help wanted Issues identified as good community contribution opportunities labels Jun 19, 2017
@bpasero bpasero added this to the On Deck milestone Jun 19, 2017
@bpasero bpasero self-assigned this Jun 19, 2017
@egamma
Copy link
Member

egamma commented Jun 20, 2017

@bpasero here is some more feedback and the scenario from Anders.

The difference here is that VS collapses any number of consecutive cursor navigation commands into a single nav history event. That’s the thing that is missing in VS Code. The current 5 line threshold has no intuitive meaning so it just seems like the cursor is randomly hopping back through code you’ve visited.

Here’s a common scenario: I’m on a call to some method and I want to see what it does. So I go to definition using F12. Then I (randomly) cursor around in the method reading the code and possibly F12 again. This may repeat several levels. When I have finally seen enough I want to quickly navigate back to the original spot. I absolutely don’t care about the individual cursor moves between the F12 jumps. In fact, they’re nothing but noise. But VS Code insists on taking me through them (or rather some indeterminate subset of them). That’s irritating. The VS behavior is much more sensible here.

@bpasero
Copy link
Member Author

bpasero commented Jun 21, 2017

The difference here is that VS collapses any number of consecutive cursor navigation commands into a single nav history event. That’s the thing that is missing in VS Code. The current 5 line threshold has no intuitive meaning so it just seems like the cursor is randomly hopping back through code you’ve visited.

@egamma I am not sure that is the desired behaviour though. If I understand correctly, VS folds multiple goto definition commands happening consecutively into one navigation event. But what if the user wants to navigate back to one of those navigation stops in between?

@stevencl can you share more insights how VS navigation works?

@bpasero bpasero added the ux User experience issues label Jun 21, 2017
@cristianhosu
Copy link
Contributor

@bpasero if I may, from what I understand, actually VS just ignores cursor movement and keeps in the history only navigation events (through Go to definition. I believe this would be a desired behavior, as I also feel this frustration.
@egamma When was this feature added to VS? I am currently working with VS 2015 and it does not have it :)

I could start a PR regarding this if we can clarify how exactly it should behave

@bpasero
Copy link
Member Author

bpasero commented Jun 22, 2017

@cristianhosu it seems the logic is more complex. I see that there is no history entries added when moving around in a file but as soon as I edit the file, all those locations are being added. Could it be that only if you edit a file, a lot more history entries pop up? That is why I would like to read a documentation on this feature rather then finding out by trying :)

@cristianhosu
Copy link
Contributor

cristianhosu commented Jun 22, 2017 via email

@bpasero
Copy link
Member Author

bpasero commented Jun 30, 2017

Related from #5208: only store a position in history from a mouseclick but not from keyboard navigation.

@bpasero bpasero modified the milestones: September 2017, On Deck Sep 7, 2017
@bpasero bpasero removed the help wanted Issues identified as good community contribution opportunities label Sep 7, 2017
@bpasero bpasero closed this as completed in 499086d Sep 7, 2017
@bpasero
Copy link
Member Author

bpasero commented Sep 7, 2017

Pushed a couple of changes:

  • threshold is now 10 lines (like VS)
  • locations get merged when navigation is not hitting the threshold
  • we no longer aggressively reveal the location in center to avoid scrolling spam

@Chillee
Copy link

Chillee commented Sep 26, 2017

@bpasero Just wanted to note that this solves a couple of fairly annoying issues on the VSCode side. VSCodeVim/Vim#1688, VSCodeVim/Vim#1850, and VSCodeVim/Vim#1933.

Thanks!

@bpasero
Copy link
Member Author

bpasero commented Sep 26, 2017

@Chillee happy to hear that! Let me know if there are still rough edges with the history.

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality on-testplan ux User experience issues
Projects
None yet
Development

No branches or pull requests

4 participants