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
...
I've only added the one test for parseCurrentBranch because the function
appears to be largely exercised by TestFind. There's definitely an
opportunity for a bigger refactor of the tests, here, but I want to avoid
scope creep as I propagate the ReadBranchConfig api changes throughout the
codebase
When using a push.default = current triangular workflow, apart from
using @{push} to determine the remote branch name, we should also follow
the
1. branch.<name>.pushRemote
2. remote.pushDefault
3. branch.<name>.remote
...list to determine which remote Git pushes to.
When push.default is not 'upstream' / 'tracking' (or 'nothing'), we can
expect local and remote branch names to be the same and solely rely on
@{push} or RemoteURL.
This fixes the wrong error message 'no pull requests found for branch
"<target branch>"' when the local branch is not pushed in the
push.default = simple / current and upstream = <target branch> setup.
When using a push.default = current central workflow [1], we should use
@{push} instead to locate the remote branch.
In fact, @{push} covers most cases in push.default = upstream too. The
branch.<name>.merge is probably only needed when using RemoteURL and
different remote / local branch names.
[1] https://github.com/tpope/vim-fugitive/issues/1172#issuecomment-522301607
This commit reverts the previous color changes in the prompt UI. While color highlighting could potentially improve the visual appealing of the prompt
using the existing color library (mguz/ansi) with the prompt library (AlecAivazis/survey) caused unintended side effects.
It reset the bold text style for the selected option. We decide to that
bold text style would have a higher priority than the color text ,for
three reasons:
1. To maintain consistency with other prompts in the UI and prioritize accessibility
2. While color can enhance the user experience, according to Primer Design Guidelines, color should not be relied upon to convey essential information.
3. visual indicator of the selected option, especially crucial when dealing with long PR titles or branch names.
As a future improvement, we may consider a separate issue or PR to address the color library issue and explore controlled color usage in prompts. This could potentially allow for more nuanced visual differentiation while avoiding unintended style resets.
Co-authored-by: Kynan Ware <47394200+BagToad@users.noreply.github.com>
Improve the interactive PR selection UI by
- prefix the PR number with hashcode #
- perserve the text formatting (bold) upon an option is hovered
- add the PR head label
Technical changes:
- Replace \033[0m with \033[39m for maintaining text formatting
Currently, the prompter doesn't enforce the requirement of a title for PR
and Issue creation. However, that creates the UX where a user can enter a
blank title, spend time filling out a detailed body, then fail to create
the issue or pr when the request is sent to the api because the title is
blank. This attempts to handle that before sending a request to the api,
enforcing that, when prompting, the title of an issue is not blank.
Allows multiple issues or PRs to be edited in parallel, and querying for shared fields once to reduce network requests.
Co-authored-by: Sam Coe <samcoe@users.noreply.github.com>
