Add hyperlinks #2322
Add hyperlinks #2322
Conversation
|
I suggest you look at https://sw.kovidgoyal.net/kitty/kittens/hyperlinked_grep/ In particular By default, this kitten adds hyperlinks for several parts of ripgrep output: the per-file header, match context lines, and match lines. You can control which items are linked with a --kitten hyperlink flag. For example, --kitten hyperlink=matching_lines will only add hyperlinks to the match lines. --kitten hyperlink=file_headers,context_lines will link file headers and context lines but not match lines. --kitten hyperlink=none will cause the command line to be passed to directly to rg so no hyperlinking will be performed. --kitten hyperlink may be specified multiple times. I am pretty sure users will want to customize what parts of the output are hyperlinked. |
|
I thought about adding a It looks like hyperlinked_grep is able to point the editor directly at the matched line, which is awesome. Unfortunately, that doesn't seem to be possible to achieve in a standard way with just the |
|
On Mon, Oct 03, 2022 at 02:20:23AM -0700, Lucas Trzesniewski wrote:
I thought about adding a `--hyperlink` command line option, but wasn't sure what exactly what options I should put there. That's something that should be discussed I guess.
Yes, indeed, it should be discussed.
It looks like hyperlinked_grep is able to point the editor directly at the matched line, which is *awesome*. Unfortunately, that doesn't seem to be possible to achieve in a *standard* way with just the `file://` protocol alone. I took a quick glance at your code, and you're adding the line number in the fragment part of the URI (`file://host/file#line`), which, you guessed it... doesn't work on Windows. 😭
I feel for you, maybe come over to the dark side :)) On a serious note,
there is nothing preventing a windows based terminal from parsing the
fragment part of the URL and either discarding it or using it, as it
sees fit.
|
I have WSL on Windows, so I'm getting the best of both worlds. 😜
That's not really how things work on Windows, and I don't think Microsoft would change their terminal that way. This would need special-casing the Let me explain: URIs are handled by the shell (a system component), and applications can be associated with URI schemes. For instance, This could be worked around with a custom URI scheme such as |
|
On Mon, Oct 03, 2022 at 05:47:38AM -0700, Lucas Trzesniewski wrote:
That's not really how things work on Windows, and I don't think Microsoft would change their terminal that way. This would need special-casing the `file://` protocol in the terminal, enhancing it with `#line` support, then adding configurable apps by file types with custom command line argument patterns. They won't do such a thing.
No, they can just strip the fragment from the URL before passing it to
the shell. If they want to remain backwards that's their lookout, I don't
see why the rest of the world should suffer for it.
Let me explain: URIs are handled by the shell (a system component), and applications can be associated with URI schemes. For instance, `https://` would be associated with the web browser. So, the shell decides how to handle a particular URI, not the terminal app. That way, you get an integrated UI experience which is consistent across applications. I'm not sure how exactly `file://` is set up, but it must be tied to something that decodes the URI to a file path, and chooses the application to open based on the file extension, which is configurable in the system settings. But this configuration doesn't let you specify additional parameters to pass to the target application, such as the file line.
Umm, I am pretty sure if you pass a http:// url with a fragment via
the shell, the browser will handle the fragment just fine. I dont see
why the shell cant pass file:// urls with fragments to applications, all
it requires is an extra check box in their configuration UI, to indicate
to the shell not to modify file urls when passed to the specified
application.
Or just use a non-Microsoft terminal if you must use windows.
Regardless, I am very strongly against crippling hyperlink support in rg
to satisfy the limitations of Microsoft's software. If you really want
to do it, add an option to control whether to add line numbers to the
file URLs, or better yet create a template system with a default
template that is file://{hostname}{path}#{line}
Then users of rg can adapt the hyperlink to use whatever scheme and
level of usefulness they require.
|
Like I said before, the behavior can be slightly different on Windows and Linux. You don't need to suffer anything. Do you know if other terminals can interpret the fragment part of the URI as a line number, and act accordingly?
Because of the intermediate step in the Windows shell which translates the URI to a file path, which apparently cannot handle it. The target appliction is passed the file path, not the URI. "all it requires" is an obscure change at the OS level. That won't happen.
I want this feature to work by default. Otherwise, there's no point. It also happens I want it to work with the terminal I use every day. And finally, please stop being aggressive. I'm just trying to add a feature here, in good faith. I'm not trying to cripple anything. It doesn't need to be perfect right from the start. Remember it can always be enhanced afterwards. In the meantime, your kitten can continue to fill the gaps. |
|
On Mon, Oct 03, 2022 at 07:11:31AM -0700, Lucas Trzesniewski wrote:
> I don't see why the rest of the world should suffer for it.
Like I said before, the behavior can be slightly different on Windows and Linux. You don't need to suffer anything.
ripgrep cannot know if its output is being interpreted on linux or
windows. It can be running over ssh on a linux machine but its output
can be interpreted on a windows machine. Or vice versa. You really
cannot include compile time tests to control output in a terminal application.
Do you know if other terminals can interpret the fragment part of the URI as a line number, and act accordingly?
No clue. Probably not, if I had to guess.
> I dont see why the shell cant pass file:// urls with fragments to applications, all it requires is an extra check box in their configuration UI, to indicate to the shell not to modify file urls when passed to the specified application.
Because of the intermediate step *in the Windows shell* which translates the URI to a file path, which apparently cannot handle it. The target appliction is passed the *file path*, not the URI.
Yes, hence the suggestion to have a checkbox to pass the URI *instead*
of the file path.
*"all it requires"* is an obscure change at the OS level. That won't happen.
I already know it wont happen. I was simply pointing out that it is not hard to do.
> Or just use a non-Microsoft terminal if you must use windows.
I want this feature to work *by default*. Otherwise, there's no point. It also happens I want it to work with the terminal I use every day.
You want it to work by default on *your terminal* in *your use case*,
breaking it by default for other people and other usecases. And you
picked the terminal and usecase that is the *most limited*. No remote
hosts and no line numbers. There is simply nothing stopping your
favorite terminal from stripping fragments and hostnames if it doesnt
like them. But no, according to you instead by default ripgrep must
produce crippled output, so that it works by default for *you*. I don't
consider clicking on a search result and being taken to a random point
in the file (as will happen if the editor remembers last opened
position) or the top of the file *working by default*.
And finally, please stop being aggressive. I'm just trying to *add a feature* here, in good faith. I'm not trying to cripple anything. It doesn't need to be perfect right from the start. Remember it can always be enhanced afterwards. In the meantime, your kitten can continue to fill the gaps.
You are adding the feature *wrong*. I thought you were asking for
feedback, so I wasted my precious time giving you some, my mistake. But
OK, whatever. I will continue to work around this in hyperlinked_grep.
Sorry for wasting your time and good luck. You wont be hearing from me
again.
|
|
Sigh. Looks like you don't understand what I've been saying in the issue thread.
Yes. And SSH support will work fine if I include the hostname on Linux. Because there's no Linux to Windows SSH.
So, in other words, you're trying to change the default behavior of ripgrep to nonstandard links just for kitty support.
Well, of course I want to make it work for myself. But I won't be breaking anyone else. On the contrary, I want this to work for most people out of the box. You're trying to make me generate nonstandard links which only your terminal understands. Now that is breaking it by default for other people, like me.
Yes. That's the lowest common denominator which works for everyone.
ripgrep must produce standard output.
Then, as you're fond of saying, change your settings. Or continue to use hyperlinked_grep. Just don't break it for other people.
This is your opinion. I've heard it. I just don't agree with it. Thanks for your feedback. |
Sigh. Looks like you dont have a clue what you are doing.
Yes there is. Google is your friend.
No, I am trying to make the output of ripgrep fit for purpose. The purpose here being, clicking on a search result and navigating to that result in an editor.
It breaks only on terminals that pass file URLs without removing hostnames, fragments, queries etc from them onto systems which cant handle those. AKA terminals that dont sanitize untrusted input before passing it on. I feel for you that you are wedded to such a terminal.
Yes, you will. You have conveniently ignored the fact that output from a search tool when clicked should go to the search result inside the file, not just the file. By leaving off line numbers you are breaking that extremely reasonable expectation for everybody, forcing them to use wrappers to work around your lack.
Non standard? Which standard are you referring to? Last I checked fragments are a part of every standard that determines what constitutes a valid URL.
Except, it doesn't actually work.
Is it really your position that you think clicking on search results should take you to random positions in a file?
Again, what standard?
You mean you and your input unsanitizing, crippled terminal.
You are most welcome. |
|
That discussion leads to nowhere. Now please stop insulting me and stick to your word:
|
|
@kovidgoyal You are coming across as pretty agressive and it is a VERY POOR way to get your point across. Stop it or you will no longer be welcome here. The overall temperature of this conversation should be reduced considerably. There is no reason for it to be heated. |
|
Sure, happy to leave. I dont know why I bothered. Good luck. |
|
I think this should now be ready for a first review, but I'm keeping the PR marked as draft as it can't be merged yet:
Also, sorry for the drama in this thread. |
# Conflicts: # Cargo.lock # crates/printer/Cargo.toml
This ensures the termcolor version from my other PR is available. BurntSushi/termcolor#65
|
Superseded by #2483 |
This PR adds hyperlinks to the file paths for terminals which support it:
It's just a draft for now, there's still work to do, but I'm opening this in case anyone would like to provide some early feedback. I'm still new to Rust, so this surely looks like garbage right now, but I'd like to make the code more idiomatic, bring it to the adequate speed and quality level, and learn in the process.
I
still need to addadded hostnames on Unix for better SSH support, but that requires approval from @BurntSushi.This depends on BurntSushi/termcolor#65,
which will certainly need to be rewritten (because of the clunky API it currently introduces), or at the very least enhanced with stuff likesupports_hyperlinks().Fixes #665
This is what I have in mind for this PR:
file://hostname/path/file.txtfile:///C:/path/file.txtfile://wsl$/distribution/path/file.txtHostnames on Unix would be useful for using ripgrep through SSH (SSH is mostly used to connect to Unix destinations). They could also be added on Windows after microsoft/terminal#14116 is fixed.
Preview builds can be found here.