After discussing this with the team, the `gh config` changes to display `accessible_colors` have been removed from this branch being outside of acceptance criteria. This will be moved to a separate issue along with any other work needed to finalize the public preview such as `gh help` entries for `GH_ACCESSIBLE_COLORS` environment variable.
List commands that use ColorScheme.Gray have been updated to use ColorScheme.Muted.
This commit brings the `gh issue comment` and `gh pr comment` tests in line with one another while also addressing some corner cases that weren't previously tested.
There is still a bit of work to get the gh pr comment tests in order, however this goes a way towards fixing the issue along with acceptance tests.
Also, it turns out some of the issue acceptance tests were really running `pr` tests.
This commit brings this code more into alignment with other places:
1. Using a local variable for upstream remote name instead of hardcoding everywhere
2. Raising error raised by setting default remote
3. Removing condition for displaying upstream default message
4. Bring message logic more inline with other places within function
Due to the refactor of BranchConfig, the tests for `pr create` no longer
require a handful of stubbed git commands.
Additionally, I noticed some overlap between `pr create`'s existing logic
with the new finder code. I suspect that the finder's new ParsePRRefs
helper could be leveraged here as well, which would allow us to respect
non-centralized workflows pr create in the same way we are respecting them
in `gh pr view` and `gh pr status`
There was a lot of copy-pasta code between the finder and pr status. After
some investigation it was clear that the prSelectorForCurrentBranch code
was really just a duplicate of the finder's code without actually making
the API call for the PR. Since the ParsePRRefs helper had already
extracted much of the logic for determining a PR's head ref branch, I was
able to reuse it in gh pr status with a small refactor.
I've been struggling horribly to reason through all of this code, and
after much mental gymnastics I identified the culprit as the overloaded
"branch" string returned by parseCurrentBranch.
This value was either the name of the branch that the PR we're looking for
is associated with, or that name prepended with the owner's name and a :
if we're on a branch, so:
PR branch: featureBranch
branch == "featureBranch"
If on Fork belonging to "ForkOwner"
branch == "ForkOwner:featureBranch"
Since this extra information was bundled up into this single string, it
complicated the responsibilities of parseCurrentBranch's "branch" return
value. Thus, I've teased out "branch" into the new PRRefs struct:
type PRRefs struct{
BranchName string
HeadRepo ghrepo.Interface
BaseRepo ghrepo.Interface
}
This allows the new parsePRRefs function to move all the previous
"branch" string's information into structured data, and allows for a new
method on PRRefs, GetPRLabel(), to create the string that "branch"
previously held to pass into its downstream consumer, namely
findForBranch.
This also allowed for better test coverage, directly connecting the PRRefs
fields to the values contained in the git config. Overall, I am now
confident that this is doing what its supposed to do with respect to my
understanding of the various central and triangular git workflows we are
addressing.
Latest Glamour has slightly changed logic in line length / wrapping,
resulting test failures due to string mismatch. Update tests and bump
dependency to v0.8.0, and others to the bare minimal level as generated
by `go mod tidy`.
This was detected then building the GitHub cli package `gh` in Debian
started to fail with src:golang-github-charmbracelet-glamour 0.8.0-1.
Closes: #10179
Go 1.24 introduces stricter checks for format string validation.
This commit fixes instances where non-constant format strings were
used in calls to functions like `fmt.Errorf`, `fmt.Printf`, and similar.
Signed-off-by: Mikel Olasagasti Uranga <mikel@olasagasti.info>
A recent refactor caused the API for ReadBranchConfig to change, resulting
in changes for its consumers. Additionally, there was a large refactor of
the tests associated with ReadBranchConfig and its consumers to gain
confidence in this refactor. This commit attempts to resolve the
conflicts between the refactor and this effort as well as massage the
changes introduced here to reflect the refactor.
The refactor PR can be found here: https://github.com/cli/cli/pull/10197
I'll note that there are still a few failing tests in status_test.go. I
haven't had a chance to fully grok while they are failing, yet, and
suspect that some insights from the original PR author may be helpful
here.
Full disclaimer: I haven't verified any of this is working locally yet.
My primary motivation is to get these new changes working together in a
manner that unblocks further iteration on this effort.
* trunk: (79 commits)
Enhance help docs on ext upgrade notices
chore: fix some function names in comment
Expand docs on cleaning extension update dir
Simplifying cleanExtensionUpdateDir logic
Separate logic for checking updates
Capture greater detail on updaterEnabled
Restore old error functionality of prSelectorForCurrentBranch
Change error handling on ReadBranchConfig to respect git Exit Codes
fix: add back colon that I removed
fix: actually read how MaxFunc work and simplify the code
fix: padded display
Collapse dryrun checks in ext bin upgrade
Bump github.com/mattn/go-colorable from 0.1.13 to 0.1.14
Rename test user in tests
Change pr number in test
Surface and handle error from ReadBranchConfig in parseCurrentBranch
Directly stub headBranchConfig in Test_tryDetermineTrackingRef
Refactor error handling in ReadBranchConfig to avoid panic
Refine error handling of ReadBranchConfig
Add test for empty BranchConfig in prSelectorForCurrentBranch
...
