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 ergonomics of multiple grep/git buffers #5105
Draft
krobelus
wants to merge
10
commits into
mawww:master
Choose a base branch
from
krobelus:jump
base: master
Could not load branches
Branch not found: {{ refName }}
Could not load tags
Nothing to show
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
grep-jump and make-jump[*] support Windows-style paths like C:\path. However grep-next-match and make-next-error don't, which suggests that no one uses this feature. IIRC Cygwin mounts Windows drives in proper Unix paths like /mnt/drive_c. Let's remove it for simplicity and consistency. This reverts commit 6c47b20 (Support windows style path in grep output, 2014-11-11). [*]: Though make-jump recently regressed in 8e5ca3f (rc/make.kak introduce a new option to be back compatible, 2023-10-27) by failing to capture the "C:" prefix.
If a user modifies a grep buffer, we can end up in weird situations where we try match a filename over multiple lines. Let's rule out newlines in filenames here. There is an argument this is a case of GIGO but we already do this for the corresponding highlighters. We also do it in make.kak, see ca225ad (Cleanup make.kak and optimize the make-next/make-prev regexes, 2016-12-09). There is one case left where a filename would theoretically span multiple lines. Fix and optimize this too.
Given make[1]: Entering directory '/home/johannes/git/kakoune/src' main.cc:66:9: error: expected ‘}’ before ‘asdf’ If I select the whole second line, including the newline, make-jump fails with an enigmatic "no such file or directory main.cc". This is because `gl` does not move the cursor away from the newline. Fix it. This appears to have regressed in 80d661e (rc/: More consistent uses of regex syntax, 2017-09-29).
Since the default make error pattern spans until the end of the line, make-jump select the whole line, moving the cursor to the end. This is especially bad when the error message is very long and hence the cursor movement scrolls the viewport so the file:line:col is no longer visible. Stop moving the cursor in `*make*` and `*grep*` buffers. We already have highlighting to indicate the current line. --- While at it -- since this commit already adds a draft context -- also add a "," to fix the case where grep-jump/make-jump is run with multiple selections. Today, this "half-succeeds" in that it selects the file:line:col part on each line of each selection and updates "jump_current_line" but does not actually jump or focus the jumpclient. Fix this by jumping to the location at the main cursor's line. This enables workflows like :grep<ret> %s needle<ret> )<ret> )<ret> )<ret>
`grep-next-match` works only on the `*grep*` buffer so it can't be used on buffers that were preserved by renaming or on other grep-flavored buffers created by 3rd party plugins kakoune-find and kakoune-lsp, like `*find*` and `*references*`. Let's generalize `grep-next-match` with a `jump-next` command that takes a buffer argument. This renames some options but I think they're not commonly used. kakoune-lsp is an exception that uses grep_current_line but it's no big deal, things will fail loud and early if options are missing. Since grep.kak and friends now depend on jump.kak, move the jumpclient declaration there as well. --- This makes it easier to create a [DWIM command] to go to the next/previous location in the relevant grep-like buffer. As a user, I normally don't care about the distinction between `*grep*`, `*find*` and `*references*`; I want to use a single mapping to navigate either (usually whichever was created last). Same goes for `*make*` (and `*lint*` and `*clang-output*` which I haven't really used yet). For navigating those buffers I tend to use a different mapping than for grep-like buffers. [DWIM command]: https://discuss.kakoune.com/t/single-command-for-grep-next-match-in-similar-buffers-lsp-make-find/1215 --- The make filetype has a different (and as of recently, configurable) error pattern. For now I have not added a configurable location pattern to jump.kak, because I don't remember ever needing one outside of `*make*`. Instead, override the pattern using buffer-local aliases in make-type buffers. Besides *make*, also *lint* and *clang-output* have the make filetype. Today their `lint-next-message` and `clang-diagnostics-next` do not respect `%opt{make_error_pattern}` but hardcode mostly equivalent patterns. The new `jump-next *lint*` and `jump-next *clang-output*` commands do, which seems reasonable.
As mentioned in the previous patch, I want to run grep lsp-references jump-next And have `jump-next` use the `*references*` buffer. Make it so. The difficult question is how to identify the buffer. I see three options 1. buffer name 2. buffer filetype 3. a magic buffer-local option 4. leave it to the user Let's compare the approaches while considering some goals: 1. 3rd party plugins like kakoune-lsp want their buffers covered by "jump-next" by default. 2. A following patch suggests to make `:grep` and `:git` create new buffers instead of replacing old ones. To support this (possibly optional) behavior, `jump-next` should be able to identify the right buffer at any time, even after some buffers have been deleted or renamed by adding something like a "-1"-suffix. So I'm not sure about hardcoding a buffer name. 3. On top of 2, it should be easy to switch to the buffer that "jump-next" would use. I'd like to generalize the solution; it should also allow to switch to the latest *git* buffer. With my current approach this is easy in most cases where it's simply called *git*, however if that buffer is deleted it's something like `*git-123*`. I don't think this is goal is very important, we can probably ignore it. --- 1. search by buffer name This is probably the best because it's most user-visible. Though it's not clear what the command arguments should look like. It be a list of buffer names, or a regex jump-next \*(grep|references)(-\d*)?\* but that makes the common interactive case worse ("jump-next \*grep\*"). I guess we could add an option jump-next -regex \*(grep|references)(-\d*)?\* To address goal 1, we would do # jump.kak declare-option regex jump_buffer_names # lsp.kak set-option -add global jump_buffer_names |\*(references|symbols)\* which is ugly but acceptable. Another advantage of this approach is that it scales better (in case we have hundreds of buffers) because we only need to get buflist once. If we go with this apprach, we could probably add more things like arrange-buffers -regex buffer-next -regex buffer-previous -regex delete-buffer -regex --- 2. search by buffer filetype We can pass a regex: jump-next grep|lsp-goto If we can give all relevant buffers the "grep" filetype, the default could simply be jump-next grep Plugins like kakoune-find and kakoune-lsp generate grep-like buffers. Today, they don't share a filetype, because a. Some file:line:col lines in LSP goto-buffers are indented, so their regex in `lsp-jump` skips over leading spaces. b. `lsp-jump` prepends a buffer-scoped root directory option to relative paths. We could add this functionality to jump.kak, so `lsp-goto` buffers can use the `grep` filetype instead. Or kakoune-lsp could add hacks to `grep` filetype to still alias `jump` in that buffer but I'd rather not go there. If we don't consolidate things to a single filetype, this is probably strictly worse than option 1. --- 3. search for a magic buffer-local option This is what's implemented in the current version. It is the cleanest implementation of goal 1 but probably very obscure to users. I'd only keep this approach if we add minor modes in one way or another. 4. leave it to the user to find the relevant buffer This is the least opinionated approach. We could define a special option that the user may populate with a buffer name to use in jump-next and others. For jump-next alone that special option would not be worth it (the user can already define wrapper commands instead) but the next patch will use it in a "jump-pop" that can be very useful but it's not trivial to come up with. This approach is worse for goal 1 and regarding room 2 there's a lot of room for user error.
This flag allows us to rename a buffer in situations where the user does not want to provide a completely unique name. For example, it allows to save `*grep*` buffers with minimal effort.
Now that jump-next and friends default to the most recent `*grep* buffer, we can keep around multiple of those buffers. This makes `:grep` not destroy state created by previous invocations. To-do: perhaps limit the stack size, or provide a nice way to clear grep buffers?
This is especially useful with `git blame-jump`; I can quickly run a sequence of recursive `git blame-jump` commands, and go back at any time with `delete-buffer`, without having to think about saving/renaming buffers. --- Also tag git buffers, so I can add a mapping to go to the last git buffer buffer-with-last git buffer This is experimental, probably we'll end up identifying buffers by a name regex instead, see the earlier patch that adds buffer-with-last.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
(Also submitted as https://lists.sr.ht/~mawww/kakoune/%3C20240225101444.906545-2-aclopte%40gmail.com%3E)
I frequently want to run
In particular, when I'm navigating a call graph.
I think we should make this workflow easier if not ship by default.
I've removed the friction from my workflow with a buffer-agnostic
grep-next-match and a stack of grep buffers.
Recently both have been added to boost which signals that there is some interest in using them.
I have cleaned up these changes; still WIP but the intended workflows should be stable.
The first 4-5 patches are improvements on their own.
Patch 5/10 extracts
jump{,-next,-previous}
commands to navigateany grep-like buffer.
Patch 6 ("rc grep/jump: default to the last grep-like buffer")
introduces a convention to identify grep-like buffers. I'll probably
change it to find the buffer by name.
Patch 7 introduces a
jump-pop
command to implement the opening example.It is most useful with automatic buffer-renaming in
patch 9 ("rc grep: keep multiple
*grep*
buffers")Finally, improve recursive Git blame workflows with
patch 10 ("rc git: keep multiple
*git*
buffers").To test with
kakoune-{lsp,boost}
, use theirjump
branches.