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 a --hyperlink-only-matches flag to hyperlinked_grep #5428

Merged
merged 3 commits into from Aug 30, 2022

Conversation

groves
Copy link
Contributor

@groves groves commented Aug 26, 2022

If it's active, only matching lines get links, not file headers or context lines.

I have a kitten that steps through the links it finds to open them like "Find Next" when searching for text in a browser. It's way more useful if the links are only to the lines the search hits, which is what this flag enables.

If it's active, only matching lines get links, not file headers or context lines.
@kovidgoyal
Copy link
Owner

Add some docs for this to hyperlinked_grep.rst

@page-down
Copy link
Contributor

I would suggest adding a prefix to the kitten options, just like the ssh kitten.
Reduce the possibility of adding options with the same name from upstream.

It would be nice if this option could accept more than one value. (all,file_path,context_line,match_line,match_text)
For example, you can add hyperlinks to file paths and matching lines.
The default is to add hyperlinks to all lines.

kitty +kitten hyperlinked_grep --kitten hyperlink=file_path --kitten hyperlink=match_text

If there are examples in the documentation, then the user will know how to use it with other ripgrep options, such as rg --context.

@groves
Copy link
Contributor Author

groves commented Aug 28, 2022

Add some docs for this to hyperlinked_grep.rst

Done.

I would suggest adding a prefix to the kitten options, just like the ssh kitten. Reduce the possibility of adding options with the same name from upstream.

I was thinking the hyperlink prefix was serving the same purpose of avoiding collisions. I prefer it as it makes parsing and cleanup simpler.

It would be nice if this option could accept more than one value. (all,file_path,context_line,match_line,match_text) For example, you can add hyperlinks to file paths and matching lines.

I can't think of a reason why I'd want those other options. Do you have a use in mind?

@page-down
Copy link
Contributor

I am interested in file paths and matching lines for hyperlinks.
I can open and locate the beginning by clicking on the file path. There are times when I need to look for keywords and start at the beginning of the file.

When pressing ctrl+shift+p>y, it will only match what is needed, e.g. only the file path, making the selection easier.

@page-down
Copy link
Contributor

I was thinking the hyperlink prefix was serving the same purpose of avoiding collisions.

I don't think so. What prevents conflicts is the -only-matches part.
In the docs:

Hopefully, someday this functionality will make it into some upstream grep program ...

So someday --hyperlink might be introduced upstream like ls --hyperlink.

This boolean set option is too limited and cannot be extended.

For all kitten wrapping another program, having uniform parameters for configuring additional options would reduce the user's cognitive and ease of use, and would not require each kitten to implement its own argument parsing logic.

So the user clearly knows that it's for kitten, not ripgrep.

@groves groves force-pushed the hyperlinked-grep-only-matches branch from e8b3cd1 to db5b4da Compare August 29, 2022 17:23
@groves
Copy link
Contributor Author

groves commented Aug 29, 2022

Added three options for hyperlinking: matching_lines, context_lines, and file_headers. They're toggled on with a --kitten option as @page-down suggested.

@kovidgoyal kovidgoyal merged commit 49f8c0e into kovidgoyal:master Aug 30, 2022
@kovidgoyal
Copy link
Owner

Merged, with a few minor changes.

@groves groves deleted the hyperlinked-grep-only-matches branch August 30, 2022 17:45
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

3 participants