-
Notifications
You must be signed in to change notification settings - Fork 8k
Description
Describe the bug
-
issue
gh browsewith a arg of <commit SHA or number> and a--branchflag generates invalid URLs.
As a result, 404 page is opened in browser.
It seems a backward compatibility issue, because It happens in v2.25.0 and v2.25.1, but not v2.24.3. -
gh version: between 2.25.0 and 2.25.1(current latest version)
$ ./gh_2.25.1 version
gh version 2.25.1 (2023-03-21)
https://github.com/cli/cli/releases/tag/v2.25.1
Steps to reproduce the behavior
- v2.25.1 > invalid
$ ./gh_2.25.1 browse de07febc26e19000f8c9e821207f3bc34a3c8038 -R cli/cli -b trunk
Opening github.com/cli/cli/tree/trunk/de07febc26e19000f8c9e821207f3bc34a3c8038 in your browser.
$ ./gh_2.25.1 browse 1 -R cli/cli -b trunk
Opening github.com/cli/cli/tree/trunk/1 in your browser.
- v2.25.0 > invalid the same as v2.25.1
$ ./gh_2.25.0 browse de07febc26e19000f8c9e821207f3bc34a3c8038 -R cli/cli -b trunk
Opening github.com/cli/cli/tree/trunk/de07febc26e19000f8c9e821207f3bc34a3c8038 in your browser.
$ ./gh_2.25.0 browse 1 -R cli/cli -b trunk
Opening github.com/cli/cli/tree/trunk/1 in your browser.
- v2.24.3 > valid !!
$ ./gh_2.24.3 browse de07febc26e19000f8c9e821207f3bc34a3c8038 -R cli/cli -b trunk
Opening github.com/cli/cli/commit/de07febc26e19000f8c9e821207f3bc34a3c8038 in your browser.
$ ./gh_2.24.3 browse 1 -R cli/cli -b trunk
Opening github.com/cli/cli/issues/1 in your browser.
Expected vs actual behavior
I thought two idea to fix this as follows.
-
Adding commit SHA and number args to
cmdutil.MutualExclusive()below. As a result command exits with error when commit SHA or number are used with some flags including--branchbelow.
Lines 115 to 124 in de07feb
if err := cmdutil.MutuallyExclusive( "specify only one of `--branch`, `--commit`, `--projects`, `--releases`, `--settings`, or `--wiki`", opts.Branch != "", opts.Commit != "", opts.ProjectsFlag, opts.ReleasesFlag, opts.SettingsFlag, opts.WikiFlag, ); err != nil { return err -
Ignoring
--branchflags when commit SHA or number is specified with the flag, prioritizing backward compatibility with v2.24.3.
I assume the former(No.1) is better because it explicitly indicates that branch does not relate to URL of commit and issue(or PR) page.