Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Standardize selection of upstream branches #4295

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ChrisMcD1
Copy link
Contributor

  • PR Description

Changes the behavior of all upstream related commands I could find to behave similar to how multi-remote pull requests currently behave:

Changes: Pull, Push, Set Upstream, Open Pull Request

The new flow behaves as I describe in the comment I wrote:

Creates a series of two prompts that will gather an upstream remote and branch from the user.
The first prompt gathers the remote name, with a pre-filled default of origin, or the first remote in the list.
After selecting a remote, only branches present on that remote will be displayed.
If there is only one remote in the current repository, the remote prompt will be skipped.

For users who only work on single-remote repositories, this should be basically unchanged, with the exception of the prompt they are displayed. (They no longer have the option to enter a remote at all, since they only have 1 choice).
For users who work in multi-remote repositories, I hope this flow should be quite smooth! The most controversial choice I made was pre-filling the default repository. In my head, users are dealing with a small number of remotes (<5), and probably still dealing with the primary more often than not. Therefore, having it prefilled can save them a keystroke in many cases, and when they need other remotes, they can press tab and arrow down.

If we want to change the behavior so the default remote is never filled, I am totally open to that option!

Currently have in a draft because I know I need to do some internationalization.

Implements #4292

  • 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

more things!

some more refactorings

Pull remote 0 optimization into core helper

Remove remote from initial option

Move over open pull request

fixing tests

foo

Added ability to dynamically do title of second tab
@ChrisMcD1 ChrisMcD1 marked this pull request as draft February 20, 2025 05:30
@stefanhaller
Copy link
Collaborator

I'm a bit torn on this. I guess I would have to use it for a while to see if I could get used to this, but right now, pushing a new branch is shift-P enter for me (yes, even in repos where I have more than one remote. It is very rare for me that I create branches on remotes other than origin). So this is one extra key stroke, every single time. Maybe it's not so bad, but I'm not sure yet. I do see how this improves it for people who regularly need to choose the remote to push to.

Your argument that we have the same flow for opening PRs, so it must be ok, doesn't really count for me, because I pretty much never use shift-O to open PRs; I use o, which goes straight to origin without additional keystrokes.

@stefanhaller
Copy link
Collaborator

Ok, I tried it for a bit, and yes, this is a nice improvement. The extra enter keystroke that I now need is an acceptable price to pay for it, so I'm fine with the change.

However, the UX for choosing the remote is weird. This shouldn't be a prompt with suggestions, but simply a menu listing all remotes, with origin at the top. If you want to get fancy and allow some keyboard convenience, you could assign keys 1 through 9 to the first 9 remotes, like we did here.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants