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

feat: hyperlink support in engine + hyperlink for path segment #307

Merged
merged 1 commit into from
Jan 12, 2021

Conversation

lnu
Copy link
Contributor

@lnu lnu commented Jan 2, 2021

Prerequisites

  • I have read and understand the CONTRIBUTING guide
  • The commit message follows the conventional commits guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

Description

Hyperlink in git and path segment. Supported in windows terminal 1.5(in preview now and available this month for the stable one)

@lnu lnu force-pushed the feat/hyperlink branch 2 times, most recently from 5e00ed6 to 455d5fa Compare January 2, 2021 11:00
@lnu
Copy link
Contributor Author

lnu commented Jan 2, 2021

Work in progress. First draft to see how it works. I have to check how to integrate this cleanly.

@JanDeDobbeleer
Copy link
Owner

The goal is that you can click the branch/tag/commit and link to the repo?

@lnu
Copy link
Contributor Author

lnu commented Jan 2, 2021

The goal is that you can click the branch/tag/commit and link to the repo?

Yes it's a first draft to see how it looks like.
Right now the ansi sequence is embedded in the segment for testing purpose. I was thinking of using the markdown syntax for url to decouple the ansi rendering from the segment. I have a more or less working regex but it's not easy to cover all scenarios.

src/segment_git.go Outdated Show resolved Hide resolved
@tradiff
Copy link
Contributor

tradiff commented Jan 2, 2021

Supported in windows terminal 1.5(in preview now and available this month for the stable one)

BTW I'm using windows terminal 1.4.3243.0 and the clickable link works for me ¯\_(ツ)_/¯

src/segment_git.go Outdated Show resolved Hide resolved
@lnu lnu force-pushed the feat/hyperlink branch 5 times, most recently from 0a41fc7 to 9b65dd3 Compare January 5, 2021 12:22
@lnu
Copy link
Contributor Author

lnu commented Jan 5, 2021

I still have an issue for path not rendered correctly when mapped_locations is used and a unicode character is set.
Example:

{
          "type": "path",
          "style": "powerline",
          "powerline_symbol": "\uE0B0",
          "foreground": "#ffffff",
          "background": "#ff479c",
          "properties": {
            "prefix": " \uE5FF ",
            "style": "full",
            "mapped_locations": [
              ["D:\\dev", "\uf962 "]
            ],
            "mapped_locations_enabled": true,
            "enable_hyperlink": true
          }
}

rendering:
image

With something else:
image

I think it can be related to that windows terminal issue: microsoft/terminal#3546

@lnu
Copy link
Contributor Author

lnu commented Jan 5, 2021

and support for opening uri is coming:microsoft/terminal#7526
we're going too fast 🤣

to track hyperlink progress: microsoft/terminal#5001

@lnu
Copy link
Contributor Author

lnu commented Jan 5, 2021

And to test hyperlink in the terminal:
printf '\e]8;;https://github.com/lnu/oh-my-posh3.git\e\\main\e]8;;\e\\'

@JanDeDobbeleer
Copy link
Owner

JanDeDobbeleer commented Jan 5, 2021

@lnu I was looking for that unicode bug the other day. Sometimes I think this whole repo is "too soon" for MS as we're constantly battling with incomplete features whereas users expect all of this to "just work". Or, we're right on time and will provide appropriate focus 😅

@lnu
Copy link
Contributor Author

lnu commented Jan 6, 2021

tested in gnome terminal and everything works(uri + http), and no glitch:
image

src/segment_git.go Outdated Show resolved Hide resolved
src/segment_git.go Outdated Show resolved Hide resolved
src/segment_git.go Outdated Show resolved Hide resolved
src/segment_git.go Outdated Show resolved Hide resolved
src/segment_git.go Outdated Show resolved Hide resolved
@lnu lnu force-pushed the feat/hyperlink branch 3 times, most recently from 25fa84e to 6b2b315 Compare January 9, 2021 08:12
@JanDeDobbeleer
Copy link
Owner

@lnu I would then maybe, for the sake of simplicity, only limit to the engine and path segment for 1. Leave git on a separate track.

@lnu lnu force-pushed the feat/hyperlink branch 3 times, most recently from 78acc70 to 0e77ba2 Compare January 9, 2021 15:18
@lnu lnu changed the title feat: hyperlink for git and path segments feat: hyperlink support in engine + hyperlink for path segment Jan 9, 2021
@lnu lnu marked this pull request as ready for review January 9, 2021 15:20
src/ansi_formats.go Outdated Show resolved Hide resolved
src/engine.go Outdated Show resolved Hide resolved
src/engine.go Outdated Show resolved Hide resolved
src/settings.go Outdated Show resolved Hide resolved
@lnu lnu force-pushed the feat/hyperlink branch 3 times, most recently from c51de38 to 31e4af1 Compare January 9, 2021 21:44
@lnu
Copy link
Contributor Author

lnu commented Jan 9, 2021

Still have to change the call in engine.go to generateHyperlink. Calling through color is a bit strange.

@JanDeDobbeleer
Copy link
Owner

JanDeDobbeleer commented Jan 10, 2021

Calling through color is a bit strange

I agree, but ansi_formats.go makes sense.

Copy link
Owner

@JanDeDobbeleer JanDeDobbeleer left a comment

Choose a reason for hiding this comment

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

What I'm missing is an addition to the docs about this like colors. Because I can create a text segment/prefix/postfix, that contains a link which will be rendered when hyperlinks are enabled.

@lnu
Copy link
Contributor Author

lnu commented Jan 10, 2021

What I'm missing is an addition to the docs about this like colors. Because I can create a text segment/prefix/postfix, that contains a link which will be rendered when hyperlinks are enabled.

Yes but be careful that right now the engine replaces only hyperlink in segment text, not prefix or postfix.

@JanDeDobbeleer
Copy link
Owner

Yes but be careful that right now the engine replaces only hyperlink in segment text, not prefix or postfix.

True. Which is fine I guess, this needs to be more contextual anyways. It's not because we can that we should.

@lnu lnu force-pushed the feat/hyperlink branch 2 times, most recently from e3f9ea1 to 832ba79 Compare January 11, 2021 21:32
An hyperlink can be added using markdown syntax and will be detected by the engine.
Initial implementation for path segments.
@lnu lnu requested a review from JanDeDobbeleer January 11, 2021 21:37
@@ -37,6 +37,7 @@ Display the current path.
is set to `true`)
- mapped_locations_enabled: `boolean` - replace known locations in the path with the replacements before applying the
style. defaults to `true`
- enable_hyperlink: `boolean` - displays an hyperlink for the path - defaults to `false`
Copy link
Owner

Choose a reason for hiding this comment

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

Can't believe I missed this, do we want to keep it at this level or higher up at Settings?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now, the engine detects url from any segments. Hyperlink can be enabled/disabled for each segment and it takes care of rendering the text or the url markdown.
On the other side, it could be enabled at global level and implying that all segments supporting url will render them.
Both have their pros and cons.
It’s a matter of taste :).

Copy link
Owner

Choose a reason for hiding this comment

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

I believe both can be an option, but let's keep it like this for now. Amazing work.

@JanDeDobbeleer JanDeDobbeleer merged commit 866c44e into JanDeDobbeleer:main Jan 12, 2021

#### Supported terminals

- [Terminal list](thttps://gist.github.com/egmontkob/eb114294efbcd5adb1944c9f3cb5feda)

Choose a reason for hiding this comment

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

t

Copy link
Owner

Choose a reason for hiding this comment

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

fixed, thx.

@lnu lnu deleted the feat/hyperlink branch January 13, 2021 05:50
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.

4 participants