-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Open pull requests in browser with extra leading slashes removed #5018
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
Open pull requests in browser with extra leading slashes removed #5018
Conversation
stefanhaller
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am a little bit reluctant about this change; those don't look like valid URLs to me (I find this hard to tell though, it's difficult to find official documentation about this). Wouldn't it be better to submit bug fixes to whatever software has trouble parsing them? (I don't know what "bitbake fetcher" is...)
On the other hand, there's the "be liberal in what you accept" thing, and stripping the slashes doesn't seem harmful, so why not. I guess I could be persuaded.
| var defaultUrlRegexStrings = []string{ | ||
| `^(?:https?|ssh)://[^/]+/(?P<owner>.*)/(?P<repo>.*?)(?:\.git)?$`, | ||
| `^.*?@.*:(?P<owner>.*)/(?P<repo>.*?)(?:\.git)?$`, | ||
| `^.*?@.*:/*(?P<owner>[^/].*)/(?P<repo>.*?)(?:\.git)?$`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is the added [^/] needed? The /* before the owner is a greedy match, so it will consume all the leading slashes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question, i'll check again whether it is needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You were right, those were not needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stefanhaller I tried the mentioned: regoio.herokuapp.com from line 4 to debug the regex, but that link seems to be dead, do you mind if I also update the url to something working like https://regex101.com/, which also has support for Golang regex?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You were right, those were not needed.
Good. Please squash the second commit into the first; see https://github.com/jesseduffield/lazygit/blob/master/CONTRIBUTING.md#commit-history.
do you mind if I also update the url to something working like https://regex101.com/,
Yes please, that's a good change (separate commit of course). I use regex101 myself most of the time, it's great.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stefanhaller Done as requested, Thanks you for your review.
I totally agree on that, but the release one sometimes has to work with may not be maintained anymore so a bug report is pointless, the issue is most likely already fixed in newer versions. |
|
Just for the reference, bitbake from yocto-3.0.0 produces the mentioned warning here: The issue is probably still presetn as the warning is still present on the master branch: |
e33099f to
8063f50
Compare
This allows for having extra slashes in git urls, for example to avoid warnings with older bitbake fetcher implementations in yocto. Which warns about a missing / in git urls
8063f50 to
f729e2c
Compare
|
Excellent, thanks! |
This MR contains the following updates: | Package | Update | Change | |---|---|---| | [jesseduffield/lazygit](https://github.com/jesseduffield/lazygit) | minor | `v0.56.0` -> `v0.57.0` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>jesseduffield/lazygit (jesseduffield/lazygit)</summary> ### [`v0.57.0`](https://github.com/jesseduffield/lazygit/releases/tag/v0.57.0) [Compare Source](jesseduffield/lazygit@v0.56.0...v0.57.0) <!-- Release notes generated using configuration in .github/release.yml at v0.57.0 --> #### What's Changed ##### Enhancements 🔥 - Open pull requests in browser with extra leading slashes removed by [@​hrzlgnm](https://github.com/hrzlgnm) in [#​5018](jesseduffield/lazygit#5018) - Allow using SelectedSubmodule in CustomCommands by [@​rlkandela](https://github.com/rlkandela) in [#​5015](jesseduffield/lazygit#5015) - Don't allow empty input in most prompts by [@​stefanhaller](https://github.com/stefanhaller) in [#​5043](jesseduffield/lazygit#5043) - Suppress output from background fetch (unless there were errors) by [@​stefanhaller](https://github.com/stefanhaller) in [#​5044](jesseduffield/lazygit#5044) - feat: add fork remote command by [@​karolzwolak](https://github.com/karolzwolak) in [#​4831](jesseduffield/lazygit#4831) - Trigger immediate background fetch when switching repos by [@​stefanhaller](https://github.com/stefanhaller) in [#​5047](jesseduffield/lazygit#5047) ##### Fixes 🔧 - Keep cursor at top/bottom when navigating by page by [@​RaphaeleL](https://github.com/RaphaeleL) in [#​5019](jesseduffield/lazygit#5019) - Switch to branches view when checking out a commit by [@​stefanhaller](https://github.com/stefanhaller) in [#​5048](jesseduffield/lazygit#5048) - Fix deleting a remote tag when a remote branch with the same name exists, or vice versa by [@​stefanhaller](https://github.com/stefanhaller) in [#​5075](jesseduffield/lazygit#5075) - Show fixup base commits in correct order in ctrl-f error message by [@​stefanhaller](https://github.com/stefanhaller) in [#​5073](jesseduffield/lazygit#5073) - Band-aid fix for rare crashes when refreshing files by [@​stefanhaller](https://github.com/stefanhaller) in [#​5074](jesseduffield/lazygit#5074) - Fix to support creating MRs for repositories cloned with SSH alias by [@​roveo](https://github.com/roveo) in [#​5082](jesseduffield/lazygit#5082) ##### Maintenance ⚙️ - Keep config and schema unchanged during a release by [@​stefanhaller](https://github.com/stefanhaller) in [#​5010](jesseduffield/lazygit#5010) - Modernize all codes by [@​phanen](https://github.com/phanen) in [#​5036](jesseduffield/lazygit#5036) - Bump golang.org/x/crypto from 0.37.0 to 0.45.0 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​5054](jesseduffield/lazygit#5054) - Don't use "HEADLESS" environment variable for running tests by [@​stefanhaller](https://github.com/stefanhaller) in [#​5064](jesseduffield/lazygit#5064) - refactor: use strings.Builder and strings.Repeat to simplify code by [@​boqishan](https://github.com/boqishan) in [#​5068](jesseduffield/lazygit#5068) - chore: fix function name in comment by [@​riyueguang](https://github.com/riyueguang) in [#​4481](jesseduffield/lazygit#4481) ##### Docs 📖 - Add a "Commit history" section to CONTRIBUTING.md by [@​stefanhaller](https://github.com/stefanhaller) in [#​5022](jesseduffield/lazygit#5022) - Update table of contents in README.md by [@​madscientist16](https://github.com/madscientist16) in [#​5045](jesseduffield/lazygit#5045) - Update docs and schema for release by [@​stefanhaller](https://github.com/stefanhaller) in [#​5091](jesseduffield/lazygit#5091) ##### I18n 🌎 - Update translations from Crowdin by [@​stefanhaller](https://github.com/stefanhaller) in [#​5090](jesseduffield/lazygit#5090) ##### Performance Improvements 📊 - Fix and speed up the file list for the "Enter path to filter by" feature by [@​stefanhaller](https://github.com/stefanhaller) in [#​5056](jesseduffield/lazygit#5056) #### New Contributors - [@​hrzlgnm](https://github.com/hrzlgnm) made their first contribution in [#​5018](jesseduffield/lazygit#5018) - [@​RaphaeleL](https://github.com/RaphaeleL) made their first contribution in [#​5019](jesseduffield/lazygit#5019) - [@​rlkandela](https://github.com/rlkandela) made their first contribution in [#​5015](jesseduffield/lazygit#5015) - [@​phanen](https://github.com/phanen) made their first contribution in [#​5036](jesseduffield/lazygit#5036) - [@​madscientist16](https://github.com/madscientist16) made their first contribution in [#​5045](jesseduffield/lazygit#5045) - [@​karolzwolak](https://github.com/karolzwolak) made their first contribution in [#​4831](jesseduffield/lazygit#4831) - [@​boqishan](https://github.com/boqishan) made their first contribution in [#​5068](jesseduffield/lazygit#5068) - [@​riyueguang](https://github.com/riyueguang) made their first contribution in [#​4481](jesseduffield/lazygit#4481) - [@​roveo](https://github.com/roveo) made their first contribution in [#​5082](jesseduffield/lazygit#5082) **Full Changelog**: <jesseduffield/lazygit@v0.56.0...v0.57.0> </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0Mi40Ny4wIiwidXBkYXRlZEluVmVyIjoiNDIuNDcuMCIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiUmVub3ZhdGUgQm90Il19-->
PR Description
This allows for having extra slashes in git URLs. Especially in submodules. Those may be used to avoid warnings with older bitbake fetcher implementations in yocto. Which warns about relative urls when not having a slash in git urls.
But when trying to open a PR from lazygit with an url like [email protected]:/project/repo.git leads to https://bitbucket.org//project/repo being opened and one has to stare at a blank page unless one removes the extra / manualy.
This PR updates the regex used for matching git url fragments in a way, that the extra
/is ignored.Please check if the PR fulfills these requirements
go generate ./...)