Skip to content
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

custom-completions: git: Include remote branches #407

Merged
merged 1 commit into from
Mar 29, 2023

Conversation

sgasse
Copy link
Contributor

@sgasse sgasse commented Mar 12, 2023

This commit adds remote branches (e.g. origin/main, github/feature-a etc.) to the output of the helper command nu-complete git switchable branches.

Addresses #406

@fdncred
Copy link
Collaborator

fdncred commented Mar 13, 2023

I'm a little worried about this because the regexes are so radically different. Will this get remote branches too? I wonder since the nu-complete git remotes part was removed. I haven't test it myself.

@sgasse
Copy link
Contributor Author

sgasse commented Mar 16, 2023

Thanks for asking again @fdncred 🙂 - turns out there was a subtle difference in the output which I found while writing an explanation. Actually, I think we need different completions for git switch and e.g. git rebase --onto. Here is the output of three different completion commands in the latest version:

/home/simon/workspace/nu_scripts〉nu-complete git branches                                                              03/16/2023 08:54:07 
╭───┬────────────────────────────────╮
│ 0 │ feature/git_rebase             │
│ 1 │ feature/remote_in_git_branches │
│ 2 │ main                           │
╰───┴────────────────────────────────╯
/home/simon/workspace/nu_scripts〉nu-complete git switchable branches                                                   03/16/2023 08:54:13 
╭───┬────────────────────────────────╮
│ 0 │ feature/git_rebase             │
│ 1 │ feature/remote_in_git_branches │
│ 2 │ main                           │
│ 3 │ fix-since-last                 │
╰───┴────────────────────────────────╯
/home/simon/workspace/nu_scripts〉nu-complete git local and remote branches                                             03/16/2023 08:54:22 
╭───┬───────────────────────────────────────╮
│ 0 │ feature/git_rebase                    │
│ 1 │ feature/remote_in_git_branches        │
│ 2 │ main                                  │
│ 3 │ origin/main                           │
│ 4 │ origin/feature/git_rebase             │
│ 5 │ origin/feature/remote_in_git_branches │
│ 6 │ upstream/fix-since-last               │
│ 7 │ upstream/main                         │
╰───┴───────────────────────────────────────╯

Here are some differences:

  • The remote upsteam has a branch called fix-since-last. Running git switch fix-since-last is a valid command and will create a local branch fix-since-last from upstream/fix-since-last and switch to it.
  • Trying to run e.g. git rebase --onto fix-since-last 2024d529 will throw an error because the local branch fix-since-last does not exist.
  • Rebasing on a remote branch with prefix which does not exist locally is totally fine, e.g. git rebase --onto upstream/main 2024d529 is a very usual command.
  • Trying to switch to a remote branch with git switch upstream/main throws an error because it would require the --detach flag.
  • git checkout upstream/main is fine.

I added the completion command now separately instead of replacing the "switchable branches" and updated the argument completions accordingly. Does it make sense like this?

@sgasse sgasse force-pushed the feature/remote_in_git_branches branch from fb24a45 to fcc8e38 Compare March 16, 2023 20:17
| parse "{value}"
| insert description Branch
| append (nu-complete git commits)
}

# Check out git branches and files
export extern "git checkout" [
...targets: string@"nu-complete git switchable branches" # name of the branch or files to checkout
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do we do here? Add yet another completion command or concatenate the output of two and make it unique?
The below commands are all valid

  • git checkout local-branch
  • git checkout remote-branch-without-prefix
  • git checkout upstream/remote-branch-without-prefix
  • git checkout 234commit456hash

Copy link
Collaborator

@fdncred fdncred Mar 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I'm wrong here, but I'm thinking we'd concatenate and unique the list.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK that would work 👍 One further question: Should the completion suggestions be a flat list (as in the current version of git switch) or a table with value/description pairs? The flat list makes better use of limited terminal space, but especially commit suggestions may be useless without any description alongside.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you maybe use a parameter to allow one to choose --verbose to show the value/description version?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm but how exactly would that work? A parameter to using the completion, or a parameter in the git command? 🤔

@fdncred
Copy link
Collaborator

fdncred commented Mar 16, 2023

Thanks for the thorough explanation @sgasse.

@sgasse sgasse force-pushed the feature/remote_in_git_branches branch from fcc8e38 to 514b55c Compare March 27, 2023 20:55
@sgasse sgasse requested a review from a team as a code owner March 27, 2023 20:55
The different commands such as `git checkout`, `git switch`, `git
cherry-pick` and `git rebase` all accept slightly differing refspecs.
This commit separates the extraction of refspecs and combines them in
individual completion commands for the different external commands.

All git commands complete with a table of `value | description` now.

Addresses nushell#406
@sgasse sgasse force-pushed the feature/remote_in_git_branches branch from 514b55c to 6b289e2 Compare March 27, 2023 20:57
@sgasse
Copy link
Contributor Author

sgasse commented Mar 27, 2023

Hi, sorry for the wait - I was busy with some stuff. Took another stab at it and I think it makes sense to separate the individual completion parts into different functions. Here are five different parts:

  1. Local branches
  2. Remote branches without prefix that do not exist locally (e.g. feature-a derived from origin/feature-a)
  3. Remote branches with prefix (e.g. upstream/feature-b)
  4. Commits of the current branch
  5. All commits known to the local git repo

Here is who accepts what:

  • git checkout: 1,2,3,4,5
  • git switch: 1,2
  • git cherry-pick: 4,5
  • git rebase (--onto): 1,3,4,5

I personally would never need 5 (all commits) in git rebase and would prefer to yield only 4 (the commits of the current branch as cut-off point), but restricting it would not be completely right.

I also saw that there were some changes and rebased on them updating namings where needed. Could you take another look at how you like it @fdncred ? Below are screenshots of how it looks on the current version.

git_cherry_pick
git_rebase
git_checkout
git_switch

Copy link
Collaborator

@fdncred fdncred left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for the changes.

@fdncred fdncred merged commit 9f480c4 into nushell:main Mar 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants