Add hyperlinks for directories in path #668

Rycieos · 2021-05-01T19:56:09Z

Add a _lp_create_link() function that turns a URL and text into a operating system command sequence for a clickable hyperlink in the
terminal. See https://github.com/mintty/mintty/wiki/CtrlSeqs#hyperlinks for details.

Add sections of the current path as hyperlinks, with the "file:https://" protocol.

Remaining questions:

Should links be added for other objects in the prompt? I can't think of any good ideas, other than VCS branches linking to the remote site copy of the branch. Or maybe, since Python virtualenvs IDs are nothing more than their path, they could be done the same as the path sections.
Should there be ENABLE config options for each type of link?
Should it be enabled or disabled by default?
Should it be scoped to controlled by themes, or by Liquidprompt internals? (this current implementation uses it only in internals, not at the default theme level)
Needs docs written.
Needs specific test coverage.

Resolves #659

nojhan · 2021-05-02T07:42:20Z

Should links be added for other objects in the prompt? I can't think of any good ideas, other than VCS branches linking to the remote site copy of the branch.

The username may be used as a link toward $HOME.
The hostname may be a sftp:https:// link (maybe useless for localhost, thus fallback to file:https://?).

Also, many sensors may be links toward Linux' state file system? Maybe a little bit too much, but kinda fun way to discover the OS internals…

Or maybe, since Python virtualenvs IDs are nothing more than their path, they could be done the same as the path sections.

Yes,

Should there be ENABLE config options for each type of link?

Definitely yes. For instance, many terminal emulator have feature allowing to add a term link on text matching a regexp, in order to run any command (I added one to Terminator myself, so as to be able to open a file in my editor at a line/column, just by clicking on a compiler output). Such a feature may come in the way, hence the necessary declutching option.

Should it be enabled or disabled by default?

I would still say enabled by default, or else many users are not going to discover it.

Should it be scoped to controlled by themes, or by Liquidprompt internals? (this current implementation uses it only in internals, not at the default theme level)

It's difficult to say, but I would tend to see themes as a first step toward finding common modules that would fit as internals. So if it's easy to make a modular and generic feature within the path function (which seems already quite modular, at least I was able to use it in my own theme easily), it's OK to put it in there, else, maybe consider making it a theme first, then see how to make it a common feature.

nojhan

Awesome.

ismith · 2021-08-27T16:44:24Z

I can't think of any good ideas, other than VCS branches linking to the remote site copy of the branch.

Personally, I'd be more interested in github branches linking to the PR opened for that branch - which is probably infeasible. Eg gh pr view --json url --template '{{.url}}' does the right thing, cool, but that's a full network round trip, and 400+ms seems like too much latency for my prompt.

Rycieos · 2021-08-27T16:59:46Z

Yeah, that much latency is a deal breaker. I think a safe rule is that the prompt should never make a network request.

I think we could probably make a few guesses and hardcode some GitHub URL patterns and make it work without network requests, but it would take some work to make it stable.

ismith · 2021-08-27T21:14:13Z

I think we could probably make a few guesses and hardcode some GitHub URL patterns and make it work without network requests, but it would take some work to make it stable.

I don't think you can, actually - a PR is identified in-url by an id number, which is not the branch name. That mapping would have to be done (AFAIK) server-side.

Oh well.

Rycieos · 2021-08-28T02:59:06Z

I don't think you can, actually - a PR is identified in-url by an id number, which is not the branch name. That mapping would have to be done (AFAIK) server-side.

True. The best we could do is link to a search, like for this PR:

https://github.com/nojhan/liquidprompt/pulls?q=is%3Apr+7470b5fa566fda8da939c2767417a7d36e8ce05a

But we could always link to the branch, and Github might link to it?

https://github.com/nojhan/liquidprompt/tree/feature/url-links

Or link to the branch search, which would show a matching PR if it exists:

https://github.com/nojhan/liquidprompt/branches/all?query=feature%2Furl-links

nojhan · 2022-10-29T16:34:39Z

Thinking about that, it may be tricky to have links toward any git-related website, given that not everybody use Github (at least Gitlab is quite a competitor, not even talking about other options).

I guess we need to:

Parse the git branch -vv command, to get the remote, then the git remote show command, to get the URL. Then we need to bet that the remote URL follows the same scheme for HTTP (which seems to be a fairly common case, but nothing guaranteed). However, that only links to the repository, not the branch. For the branch we would need to:
Let the user configure their own LP_GIT_LINK_BRANCH_TEMPLATE="https://github.com/nojhan/liquidprompt/tree/%s" (and co.).

Rycieos · 2022-11-01T17:33:59Z

Thinking about that, it may be tricky to have links toward any git-related website, given that not everybody use Github

Exactly, and guessing is all but guaranteed to fail at some point. I could see implementing your second idea with templates, but that would greatly reduce the amount of users that would configure to enable it.

nojhan · 2022-11-02T08:29:04Z

A compromise would be to provide a default to Github, or to add the most known templates as comments in the shipped preset.

nojhan · 2022-11-02T08:33:15Z

I implemented something (cherry-picking the aforementioned code to be up-to-date with the current master) on feat+url-links.

nojhan · 2022-11-04T08:37:30Z

I'm quite loosing track of my work, so if that's OK, I will force-push feat+url-links on this PR (it contains @Rycieos' commit, and this PR was too old to be merged anyway). Then it will be easier to review.

nojhan · 2022-11-05T06:02:18Z

@Rycieos I cannot assign you a review on a PR you started, but I certainly hope you will request some changes 😄

Rycieos

I can't requests changes on my own PR either 😑 But I can submit a review.

Looking pretty good. I need to go over the whole thing again in depth before I can feel confident with it, but these suggestions are at least a start.

liquidprompt

tests/test_utils.sh

nojhan · 2022-12-04T19:53:36Z

As of now, path elements are linked toward $PWD, because only the path label is passed to _lp_create_link_path, but ideally, the absolute path down to the current label would be passed, so that we can build up a link to this specific directory. However, _lp_path_format being such a complex function, I am not at ease at adding such a feature there.

nojhan · 2022-12-23T16:49:06Z

@Rycieos Probably needs a review.

Rycieos

Request for changes

Sorry for the silence recently; work got busy, and then the holidays.

In general looking good. I want to see a bit better test coverage, since lp_path_format is so complicated and impossible to understand (sorry).

Ignore the failing jobs, they are fixed with commits c8722f2 and 6c403a0. A rebase would fix the failures.

liquidprompt

tests/test_utils.sh

liquidprompt

nojhan · 2023-01-19T20:44:06Z

@Rycieos: Good for another review.

Rycieos

Looks good. One easy tests issue.

This has conflicts with your prompt_comparison file, you will need to rebase to master to fix those before I can merge.

tests/test_utils.sh

nojhan · 2023-02-23T10:16:46Z

@Rycieos Review fixes done and squashed.

Add a _lp_create_link() function that turns a URL and text into a operating system command sequence for a clickable hyperlink in the terminal. See https://github.com/mintty/mintty/wiki/CtrlSeqs#hyperlinks for details. Add sections of the current path as hyperlinks, with the "file:https://" protocol. Resolves #659

Linking change if we are connected through SSH: - If the connection is SSH: - The hostname links to "ssh:https://<user>@<fqdn>/<path>" - The path elements to "sftp:https://<user>@<fqdn>/<path> - If the connection is su or local: - The path elements to "file:https://<user>@<fqdn>/<path> Use typeset instead of local in tests. Uo-authored-by: Mark Vander Stel <[email protected]>

nojhan · 2023-02-24T07:46:29Z

GG 🍻

Rycieos added enhancement Feature request path Related to current working directory lookup or display labels May 1, 2021

Rycieos added this to the v2.1 milestone May 1, 2021

Rycieos requested a review from nojhan May 1, 2021 19:56

Rycieos added the reviews wanted Looking for reviews from the community label May 1, 2021

nojhan approved these changes May 2, 2021

View reviewed changes

Rycieos modified the milestones: v2.1, v2.2 May 25, 2022

nojhan force-pushed the feature/url-links branch 2 times, most recently from 4278f3c to e161ce0 Compare November 5, 2022 05:51

nojhan marked this pull request as ready for review November 5, 2022 06:00

Rycieos commented Nov 5, 2022

View reviewed changes

liquidprompt Outdated Show resolved Hide resolved

liquidprompt Outdated Show resolved Hide resolved

liquidprompt Outdated Show resolved Hide resolved

tests/test_utils.sh Outdated Show resolved Hide resolved

nojhan self-requested a review December 23, 2022 16:43

Rycieos commented Dec 26, 2022

View reviewed changes

liquidprompt Outdated Show resolved Hide resolved

tests/test_utils.sh Show resolved Hide resolved

liquidprompt Show resolved Hide resolved

nojhan force-pushed the feature/url-links branch from efd98c5 to 7ce5d14 Compare January 19, 2023 20:40

nojhan assigned Rycieos Jan 19, 2023

Rycieos commented Jan 21, 2023

View reviewed changes

tests/test_utils.sh Outdated Show resolved Hide resolved

nojhan force-pushed the feature/url-links branch from 7ce5d14 to 5e3e916 Compare February 23, 2023 10:15

Rycieos and others added 2 commits February 23, 2023 13:24

Rycieos force-pushed the feature/url-links branch from 5e3e916 to a9b6956 Compare February 23, 2023 18:24

Rycieos merged commit 19d2b55 into master Feb 23, 2023

Rycieos deleted the feature/url-links branch February 23, 2023 18:28

nojhan mentioned this pull request Feb 24, 2023

Hyperlinks should depends on path elements #768

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add hyperlinks for directories in path #668

Add hyperlinks for directories in path #668

Rycieos commented May 1, 2021 •

edited by nojhan

Loading

nojhan commented May 2, 2021 •

edited

Loading

nojhan left a comment

ismith commented Aug 27, 2021

Rycieos commented Aug 27, 2021

ismith commented Aug 27, 2021

Rycieos commented Aug 28, 2021

nojhan commented Oct 29, 2022

Rycieos commented Nov 1, 2022

nojhan commented Nov 2, 2022

nojhan commented Nov 2, 2022

nojhan commented Nov 4, 2022

nojhan commented Nov 5, 2022

Rycieos left a comment

nojhan commented Dec 4, 2022

nojhan commented Dec 23, 2022

Rycieos left a comment

nojhan commented Jan 19, 2023

Rycieos left a comment

nojhan commented Feb 23, 2023

nojhan commented Feb 24, 2023

Add hyperlinks for directories in path #668

Add hyperlinks for directories in path #668

Conversation

Rycieos commented May 1, 2021 • edited by nojhan Loading

nojhan commented May 2, 2021 • edited Loading

nojhan left a comment

Choose a reason for hiding this comment

ismith commented Aug 27, 2021

Rycieos commented Aug 27, 2021

ismith commented Aug 27, 2021

Rycieos commented Aug 28, 2021

nojhan commented Oct 29, 2022

Rycieos commented Nov 1, 2022

nojhan commented Nov 2, 2022

nojhan commented Nov 2, 2022

nojhan commented Nov 4, 2022

nojhan commented Nov 5, 2022

Rycieos left a comment

Choose a reason for hiding this comment

nojhan commented Dec 4, 2022

nojhan commented Dec 23, 2022

Rycieos left a comment

Choose a reason for hiding this comment

nojhan commented Jan 19, 2023

Rycieos left a comment

Choose a reason for hiding this comment

nojhan commented Feb 23, 2023

nojhan commented Feb 24, 2023

Rycieos commented May 1, 2021 •

edited by nojhan

Loading

nojhan commented May 2, 2021 •

edited

Loading