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

Return from symbol #19

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Conversation

ColonelPhantom
Copy link

This PR implements a function that allows going back to where the cursor was before going to a symbol. Personally I think this is quite convenient for going back and forth through a call-stack.

For now, I only have a very rudimentary implementation. Some things I'm looking for feedback/suggestions on are:

  • working across files (should it close the newly opened file? Keep it open? Currently it does not work across files at all)
  • handling updated code elsewhere (e.g. line numbers shifting around; not sure how I could deal with that)
  • command naming and keyboard shortcuts
  • general code clarity

@jgmdev
Copy link
Member

jgmdev commented Jan 11, 2022

Looking at the code makes me think the functionality is pretty similar to the navigate plugin that I wrote which also lets you jump back and forward.

working across files (should it close the newly opened file? Keep it open? Currently it does not work across files at all)

By this you mean to keep a history of symbol jumps per file? The navigate plugin already keeps a back and forward global history of cursor positions but not a per file one.

handling updated code elsewhere (e.g. line numbers shifting around; not sure how I could deal with that)

Maybe by storing the current id of internal lite-xl undo/redo stack and comparing latest changes to the change when the goto declaration was triggered... Not sure how complicated this would be and if worth the effort.

command naming and keyboard shortcuts

By default the navigate plugin binds to alt+left and alt+right

Maybe the navigate plugin provides what you are trying to implement here, give it a try if haven't yet and let me know.

@ColonelPhantom
Copy link
Author

It seems like I missed the navigate plugin, thanks for pointing me towards it 🙂

It's pretty close to what I want and definitely very convenient, although it does also push when I'm jumping around code by hand. That makes it sometimes a little fine-grained, whereas the LSP go-back is more coarse (only on an LSP goto).

Perhaps making it possible to override navigate's update mechanism would do the job, or something like it, I'm not sure. I do personally think the coarser navigation is more convenient, since the finer navigation would have me hitting alt+left over and over again.

@jgmdev
Copy link
Member

jgmdev commented Jan 12, 2022

I do personally think the coarser navigation is more convenient, since the finer navigation would have me hitting alt+left over and over again.

That is true, if you would like to continue implementing this on the LSP here are my comments to some of your previous questions:

  • Your choice of lsp:go-back seems right with me, can't think of any other future use case for that command name.
  • I would change the alt-z default shortcut to alt+shift+left
  • You can take some ideas on the navigate plugin to properly get the go-back jumps working across files by making use of a dedicated table inside the lsp namespace to store document jump locations.
  • About tracking document changes to jump into proper location on subsequent document edits... I still don't know what would be the most efficient and easier route in case this is desired, other solution that comes to mind (besides the one I already mentioned on my first response) may be counting the occurrences of the selected symbol on the document and storing both the index position and symbol text in order to be able to properly jump back to it by performing a text search, so jumping back even if the source code changes would still work, but this may be slow on really big files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants