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

Add custom godef jump stack (fixes #667) #776

Merged
merged 2 commits into from
Apr 11, 2016
Merged

Conversation

flowchartsman
Copy link
Contributor

Unified and updated PR

@flowchartsman
Copy link
Contributor Author

How does this look so far? The UI integration took the longest and required the deepest modifications, but the result is much cleaner. Obviously, it needs some more testing, and I will continue to push changes if I find any issues. Also, I'm not terribly keen on having the current line and cursor highlighted in the non-interactive mode, and would like to change that to behave more light a pager. Thoughts/feedback?

@fatih
Copy link
Owner

fatih commented Mar 28, 2016

Hi @alaska

I was absent on the weekend. I'll look into this soon!

vsplit
else
" We didn't get a mode, so we start the process of pushing to the
" jumpstack
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So does it mean when I open a split view, we'll not going to use the jumpstack ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's correct. I was trying to avoid inconsistent behavior, but I have just thought of a better way to do this that will push the location AFTER the jump, so that each new window gets its own stack that starts from the first location visited.

autoload/go/def.vim:
- Fix Jump stack to work across windows.  That is, if you issue a split
  or tab jump, you will enter a new window with a new jump stack
  starting from where you were
- Changes to remove :GoDefStack. go#def#StackUI() is always interactive
  now. Add more informative help message and mappings

autoload/go/ui.vim:
- defaulting to content length in all cases
- add an echon to remove confusing message from command line

doc/vim-go.txt:
- Changes to remove :GoDefStack.
- Move :GoDefJump to directly after :GoDef, because it now shares the
  job of describing the stack

ftplugin/go/commands.vim:
- remove :GoDefStack
@flowchartsman
Copy link
Contributor Author

All requested changes have been made. In this version, the jumpstack starts new in new splits/tabs with one entry, which is the location you left. Do you think this is the proper behavior, or should the stack be empty?

@fatih
Copy link
Owner

fatih commented Apr 11, 2016

Hi @alaska

This seems to be working well. I'm using it now. Probably going to merge it today/tomorrowy and then will do some changes to make it compatible with the new guru changes. Thanks for all the work, this all looks great 👍

@flowchartsman
Copy link
Contributor Author

Thanks! I'm still on the fence about whether the stack should be empty in a new split/tab or not. I'm leaning towards not, since the location you jumped from would still be in the window you left, and popping would then leave you with two splits on the same buffer, but I can see how some folks might find it useful.

@fatih
Copy link
Owner

fatih commented Apr 11, 2016

About that. I think we need a command to clear the stack. Let's do it manual. Basically we should be able to set the entry point ourselves. Best way is doing it like:

  • :GoDefJump should be renamed to :GoDefStack
  • :GoDefStackClear is a new command that clears the stack, with an appropriate mapping

The reason is that split or tab is not a boundary. For example I do a lot ctrl+w v which opens a new split for the same file. However I still would like to use the previous stack for that file. It's a little bit trickt. To solve tricky problems, we should just provide primitives (such as :GoDefStackClear) and then let the user do the rest.

@fatih
Copy link
Owner

fatih commented Apr 11, 2016

There is a weird error. Suppose you called :GoDef 3-4 times, after that if you call :GoDefJump and you press the upper arrow case, it tries to insert the character A. I'm not sure if it's my settings or not, but looking into it.

@fatih
Copy link
Owner

fatih commented Apr 11, 2016

@alaska I'm merging this and going to open another PR inside the guru pr to fix some of the style changes and co. That would make it easier for us. Thanks a lot again for the idea and working on this 👍

@fatih fatih merged commit 80f6cf8 into fatih:master Apr 11, 2016
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.

None yet

2 participants