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

Jump by sign names #8

Closed
brendanator opened this issue Sep 20, 2018 · 17 comments
Closed

Jump by sign names #8

brendanator opened this issue Sep 20, 2018 · 17 comments
Labels
enhancement New feature or request
Milestone

Comments

@brendanator
Copy link

It would be nice to be able to set up a mapping something like

nnoremap ]d signjump#next_sign(123, 234)

which would only jump to signs 123 and 234 instead of all signs

@ZeroKnight
Copy link
Owner

ZeroKnight commented Sep 20, 2018 via email

@brendanator
Copy link
Author

Hi!

Maybe I didnt understand enough about signs but I spotted a usage here:
https://github.com/alfredodeza/coveragepy.vim/blob/master/ftplugin/python/coveragepy.vim#L32

Having looked a bit closer at this https://github.com/w0rp/ale/blob/06132954b12ba87c7bba8fb7d5a40e4ff8b774e8/autoload/ale/sign.vim#L126 it would seem that signs with different ids can be of the same type.

I guess ideally the mapping would be able to take a list of sign names instead of ids then

@ZeroKnight
Copy link
Owner

Maybe I didnt understand enough about signs

Actually, that would be me in this case :) When writing the plugin, I had only read enough about them as I needed to implement what I have now. I didn't realize that the sign IDs were manually set.

Thank you for the examples as well, I can see how this would be useful.

@ZeroKnight ZeroKnight added the enhancement New feature or request label Sep 21, 2018
@ZeroKnight
Copy link
Owner

ZeroKnight commented Sep 21, 2018

So my current thinking is to allow passing a list of sign IDs to next_sign and prev_sign, which will only attempt to jump to signs that match an ID in the list. This way, you could define extra mappings something like so:

noremap ]d @=signjump#next_sign([123, 456, 789])<CR>

Keep in mind that different signs can have the same ID, so in this way plugins that utilize specific IDs for their signs could be filtered to be the only ones jumped to. Is this roughly what you had in mind?

@brendanator
Copy link
Author

I think having read a little more about it, it should work by sign names instead of sign ids. That way I could have something like

noremap ]e @=signjump#next_sign(['ALEErrorSign', 'ALEErrorStyleSign'])<CR>
noremap ]w @=signjump#next_sign(['ALEWarningSign', 'ALEWarningStyleSign'])<CR>

@brendanator brendanator changed the title Jump by sign ids Jump by sign names Sep 21, 2018
@ZeroKnight
Copy link
Owner

Yeah, I'm liking this idea. I'll work on adding this later in the day.

Thank you for your interest!

@ZeroKnight
Copy link
Owner

ZeroKnight commented Sep 30, 2018

@brendanator Hi there! I've pushed some commits to master that adds this functionality. You can create your mappings by hand, however I highly recommend that you use the experimental helper function signjump#add_name_map(), as it will take care of creating both a normal and visual mode mapping, as well as ensuring that passing a count isn't broken.

As an example, you could add the following to your configuration:

call signjump#add_name_map(']e', 'next', ['ALEErrorSign', 'ALEErrorStyleSign'])
call signjump#add_name_map(']w', 'next', ['ALEWarningSign', 'ALEWarningStyleSign'])

This would do the same thing as the example you gave in your last comment, with the addition of a visual mode mapping as well. In fact, the sign names are interpreted as patterns, so you could even do:

call signjump#add_name_map(']e', 'next', ['ALEError'])
call signjump#add_name_map(']w', 'next', ['ALEWarning'])

Please let me know what you think, and if you have any further suggestions.

@brendanator
Copy link
Author

Looks good! Thanks

Adding the mapping directly works for me

nmap [r @=signjump#prev_sign(['ALEErrorSign'], 1)<cr>
xmap [r @=signjump#prev_sign(['ALEErrorSign'], 1)<cr>

but using call signjump#add_name_map(']r', 'next', ['ALEError']) gives error Unknown function: signjump#add_name_map

I suspect this can be easily fixed by moving signjump#add_name_map into the autoload file

I also spotted that if there's no matching signs the cursor is moved to the beginning of the line. Ideally it would be left where it is

@ZeroKnight
Copy link
Owner

I can't reproduce either of those problems with a fresh user and Vim config using vim-plug. Are you attempting to call signjump#add_name_map before the plugin is loaded? Have you started a new (n)vim session after updating?

@ZeroKnight ZeroKnight reopened this Oct 1, 2018
@ZeroKnight ZeroKnight added the need more info More information or steps to reproduce are needed label Oct 1, 2018
@brendanator
Copy link
Author

I call it after plug#end() so I assume the plugin is loaded at the point. Yep, I restarted nvim and get that error on load

Error detected while processing /Users/brendan/.config/nvim/init.vim:
line  395:
E117: Unknown function: signjump#add_name_map
line  396:
E117: Unknown function: signjump#add_name_map
line  397:
E117: Unknown function: signjump#add_name_map
line  398:
E117: Unknown function: signjump#add_name_map
Press ENTER or type command to continue

Can confirm the jump to start of line issue too

@ZeroKnight
Copy link
Owner

Hmm...I don't know why I can't reproduce this; perhaps I'm missing something. Could you share your nvim configuration, please? Could you also try to reproduce this behavior with a minimal configuration?

@ZeroKnight
Copy link
Owner

I also spotted that if there's no matching signs the cursor is moved to the beginning of the line

I just realized you wrote line and not file. 😓 Yeah, I can reproduce that; that's not supposed to happen. I'm still trying to figure out why add_name_map is available to me on startup, but not you. You aren't doing using vim-plug's on-demand loading by chance, are you?

I should probably just make add_name_map a global function (or a command, if I can wrestle the argument passing correctly, the escaping is weird), as I don't think my using of signjump# is correct in the plugin/ directory anyway.

@ZeroKnight
Copy link
Owner

ZeroKnight commented Oct 2, 2018

I pushed a small commit bdc30b9 that makes the map function global, which should solve the unknown function problem (I'd still like to know the reason behind its cause, but oh well).

@brendanator
Copy link
Author

That doesn't work either. The problem is the load order of scripts.

  1. call plug#end() changes the rtp by adding in the path to each of the plugins
  2. The rest of init.vim/.vimrc is run
  3. call signjump#add_name_map(...) or call SignJumpCreateMap(...) try to call the function but it hasn't been loaded yet. In the case of call signjump#add_name_map(..) vim will try autoload directories in the rtp.
  4. Each script in <rtp>/plugin is loaded and then the above functions are available

The solution is to put signjump#add_name_map(...) into autoload/signjump.vim. Try :h autoload for details

@ZeroKnight
Copy link
Owner

I was under the impression that after plug#end(), plugin/ directories would have been loaded, but as you outlined, that isn't the case. I could move it to autoload, but then I can't use s:map(). Though I suppose I don't need the checking provided by it, since I could offload that responsibility to the user. I think I'll go with that then.

Thank you for your patience, it seems there's many pitfalls when it comes to Vim scripting/plugin authoring.

@ZeroKnight
Copy link
Owner

ZeroKnight commented Oct 2, 2018

Pushed commit 8594031 that should finally address this. I'm afraid I'll have to address #12 after I get some sleep, however.

The function is now signjump#create_map.

@brendanator
Copy link
Author

Great! This works. Thank you

@ZeroKnight ZeroKnight removed the need more info More information or steps to reproduce are needed label Oct 2, 2018
@ZeroKnight ZeroKnight added this to the Version 0.7 milestone Oct 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants