Skip to content

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 stefanhaller mentioned this pull request Sep 15, 2024
7 tasks
@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
stefanhaller added a commit that referenced this pull request Sep 28, 2024
- **PR Description**

Add a facility to gocui.View to enable auto-rendering of https
hyperlinks. Then, use it for the Command Log panel (as an alternative
approach to #3911), and also in the status view and in confirmation
popups to get rid of some code that used to do this manually.

- **Please check if the PR fulfills these requirements**

* [x] Cheatsheets are up-to-date (run `go generate ./...`)
* [x] Code has been formatted (see
[here](https://github.com/jesseduffield/lazygit/blob/master/CONTRIBUTING.md#code-formatting))
* [ ] Tests have been added/updated (see
[here](https://github.com/jesseduffield/lazygit/blob/master/pkg/integration/README.md)
for the integration test guide)
* [ ] Text is internationalised (see
[here](https://github.com/jesseduffield/lazygit/blob/master/CONTRIBUTING.md#internationalisation))
* [ ] If a new UserConfig entry was added, make sure it can be
hot-reloaded (see
[here](https://github.com/jesseduffield/lazygit/blob/master/docs/dev/Codebase_Guide.md#using-userconfig))
* [ ] Docs have been updated if necessary
* [x] You've read through your own file changes for silly mistakes etc
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