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

Render hyperlinks in the command log output panel #3911

Closed
wants to merge 1 commit into from

Conversation

gmlexx
Copy link
Contributor

@gmlexx gmlexx commented Sep 14, 2024

  • PR Description

Git output might contain a useful http links call for actions, for example:

remote:
remote: To create a merge request for <branch>, visit:
remote:   https://gitlab.<company>.com/<project>/-/merge_requests/new?merge_request%5Bsource_branch%5D=<branch>
remote:
  • Please check if the PR fulfills these requirements
  • Cheatsheets are up-to-date (run go generate ./...)
  • Code has been formatted (see here)
  • Tests have been added/updated (see here for the integration test guide)
  • Text is internationalised (see here)
  • If a new UserConfig entry was added, make sure it can be hot-reloaded (see here)
  • Docs have been updated if necessary
  • You've read through your own file changes for silly mistakes etc

@gmlexx
Copy link
Contributor Author

gmlexx commented Sep 14, 2024

Hey @stefanhaller, I've added changes based on your suggestion here
Could you have a look please?

@stefanhaller
Copy link
Collaborator

Thanks for working on this!

I feel bad about having sent you in a wrong direction, but after looking at this PR and thinking about it a bit more, I no longer think this is the best approach. For two reasons:

  • we need to find all places that print something to the Extras view, and manually call underlineLinks for each one. That's error-prone (you missed a few in this PR 😄)
  • this approach relies on the assumption that the output of commands always comes in chunks of entire lines; this doesn't have to be the case, depending on how the output of the command is buffered. So it could theoretically happen that one call to prefixWriter.Write receives To create a merge request for <branch>, visit: htt, and the next call receives ps://etc. Probably unlikely in practice, but possible.

To solve both of these issues, I think it's better to build this functionality right into gocui, so that you can turn it on with a flag in the view. I tried this in a separate PR, and I think it looks promising. Please have a look: #3914

@gmlexx
Copy link
Contributor Author

gmlexx commented Sep 15, 2024

Thanks Stefan, no worries! Closing this PR.

@gmlexx gmlexx closed this Sep 15, 2024
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.

2 participants