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.

Sign up for GitHub

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

Use a map instead of a hook to jump to grep result #5149

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

Use a map instead of a hook to jump to grep result #5149

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions rc/tools/grep.kak
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,7 @@ hook -group grep-highlight global WinSetOption filetype=grep %{
}

hook global WinSetOption filetype=grep %{
hook buffer -group grep-hooks NormalKey <ret> jump
hook -once -always window WinSetOption filetype=.* %{ remove-hooks buffer grep-hooks }
map buffer normal <ret> :jump<ret>
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I was also surprised why we use a hook here instead of a mapping.

Turns out there is a behavioral difference; consider

map global normal <c-j> <ret>

This mapping will fire NormalKey <ret> hooks, thus triggering the existing mapping.
It would not trigger the proposed mapping.

I wonder if we should move away from relying on this subtle difference.
One could use execute-keys -with-maps instead.

There is a related problem; this remapping is currently broken in that it doesn't add to prompt history:

map global prompt <c-j> %{<a-semicolon>:execute-keys -with-hooks -with-maps <lt>ret><ret>}

luckily I don't need maps/hooks in prompt mode so I can use the simple remapping.

There is the argument that if we used NormalKey hooks everywhere,
then the simple remapping (map global normal <c-j> <ret>) would work, no need for execute-keys.
But of course it's surprising that normal mode key-aliases are mapped differently than for other modes.
I'm not yet sure what we should do.
Of course we should be less inconsistent in the end.

Either way we still want to remove the mapping when the filetype changes.
A naive

hook -once -always window WinSetOption filetype=.* %{ unmap buffer <ret> :jump<ret> }

will have problems; particularly if we switch from filetype=grep to another
filetype that adds this mapping via a filetype hook.
Since hooks run in definition order, the unmap hook will run last and remove
the wrong mapping.

We can solve that by making the maps unique (map buffer <ret> ':jump<ret> # grep').

I wonder if instead of mapping <ret> everywhere, we should define a buffer-scoped option (or alias).
That might make it easier to map keys like <c-j> to do the same as <ret> but it kinda goes against our conventions (<ret> is just that, there is no other name), so I'd not go there.


define-command -hidden grep-jump %{
Expand Down