Fix Git completion filenames, remotes, and push options

custom-completions/git/git-completions.nu

@@ -66,19 +66,39 @@ module git-completion-utils {
     $result
   }
 
-  # Get changed files which can be restored by `git checkout --`
-  export def get-changed-files []: nothing -> list<string> {
-    ^git status -uno --porcelain=2 | lines
-    | where $it =~ '^1 [.MD]{2}'
-    | each { split row ' ' -n 9 | last }
+  # Porcelain v2's -z form contains literal pathnames, including quotes and newlines.
+  # Ordinary changes use type 1; every unmerged XY state uses type u.
+  def status-paths [--include-unmerged]: nothing -> list<string> {
+    let pattern = if $include_unmerged {
+      '(?s)^(?:1 [.MD]{2} (?:\S+ ){6}|u (?:DD|AU|UD|UA|DU|AA|UU) (?:\S+ ){8})(?<value>.+)$'
+    } else {
+      '(?s)^1 [.MD]{2} (?:\S+ ){6}(?<value>.+)$'
+    }
+    ^git --no-optional-locks status -uno --porcelain=2 -z
+    | split row "\u{0}"
+    | generate {|record, skip_old_name|
+        if $skip_old_name {
+          {next: false}
+        } else if $record starts-with '2 ' {
+          # A renamed/copied entry owns the next NUL field: its original pathname.
+          {next: true}
+        } else {
+          {out: $record, next: false}
+        }
+      } false
+    | parse --regex $pattern
+    | get value
   }
 
-  # Get files which can be retrieved from a branch/commit by `git checkout <tree-ish>`
-  export def get-checkoutable-files []: nothing -> list<string> {
-    # Relevant statuses are .M", "MM", "MD", ".D", "UU"
-    ^git status -uno --porcelain=2 | lines
-    | where $it =~ '^1 ([.MD]{2}|UU)'
-    | each { split row ' ' -n 9 | last }
+  # Get changed files which can be restored by `git checkout --`.
+  export def get-changed-files []: nothing -> list<string> { status-paths }
+
+  # Include unmerged paths when checking out a tree or choosing a conflict side.
+  export def get-checkoutable-files []: nothing -> list<string> { status-paths --include-unmerged }
+
+  # Completion values are pasted as Nu source; display the literal path separately.
+  export def format-checkout-files []: list<string> -> list<record<value: string, display_override: string, description: string, style: string>> {
+    $in | each {|path| {value: ($path | to nuon), display_override: $path, description: File, style: green} }
   }
 
   export def get-all-git-local-refs []: nothing -> list<record<ref: string, obj: string, upstream: string, subject: string>> {
@@ -103,7 +123,8 @@ def "nu-complete git available upstream" [] {
 
 def "nu-complete git remotes" [] {
   ^git remote --verbose
-  | parse --regex '(?<value>\S+)\s+(?<description>\S+)'
+  | lines
+  | parse --regex '^(?<value>\S+)\s+(?<description>.+) \((?:fetch|push)\)$'
   | uniq-by value # Deduplicate where fetch and push remotes are the same
 }
 
@@ -171,22 +192,22 @@ def "nu-complete git checkout" [buffer: string, place?: record] {
   # - First arg is a branch
   # If before '--' is just 'git checkout' (or its alias), we suggest "dirty" files only (user is about to reset file).
   if $prev_tokens.2? == '--' {
-    return (get-changed-files)
+    return (get-changed-files | format-checkout-files)
   }
   if '--' in $prev_tokens {
-    return (get-checkoutable-files)
+    return (get-checkoutable-files | format-checkout-files)
   }
   # Already typed first argument.
   if ($prev_tokens | length) > 2 and $preceding ends-with ' ' {
     # If we are creating a new branch, we may want to specify a start point
     if ("-b" not-in $prev_tokens) and ("-B" not-in $prev_tokens) and ("--orphan" not-in $prev_tokens) {
-      return (get-checkoutable-files)
+      return (get-checkoutable-files | format-checkout-files)
     }
   }
   # The first argument can be local branches, remote branches, files and commits
   # Get local and remote branches
   let branches = get-mergable-sources
-  let files = (get-checkoutable-files) | wrap value | insert description 'File' | insert style green
+  let files = get-checkoutable-files | format-checkout-files
   let commits = ^git rev-list -n 400 --remotes --oneline | lines | split column -n 2 ' ' value description | upsert description {|x| $'Commit, ($x.value) ($x.description)' } | insert style 'light_cyan_dimmed'
   {
     options: {
@@ -403,33 +424,62 @@ export extern "git fetch" [
 def "nu-complete git push" [buffer: string, place: record] {
   use git-completion-utils *
   let preceding = $buffer | str substring ..$place.cursor
-  let tokens = $preceding | str trim | args-split | where ($it not-in $GIT_SKIPABLE_FLAGS)
-
-  # Check if we have a remote argument (2nd token, 1st is 'git', 2nd is 'push', 3rd is remote)
-  # BUT, args-split might be different depending on how it's called.
-  # "git push origin" -> ["git", "push", "origin"]
-  # If we have at least 3 tokens, the 3rd one IS likely the remote.
-  # We should double check if the 3rd token is actually a remote.
-
+  let tokens = $preceding | str trim | args-split
+  let push_matches = $tokens | enumerate | where item == push
+  let push_index = $push_matches.0?.index? | default 1
   mut remote = ""
-  if ($tokens | length) >= 3 {
-    $remote = $tokens.2
+  mut option_value = ""
+  mut after_separator = false
+  mut deleting = false
+  # Scan through refspecs too: Git accepts flags before or after the remote.
+  for token in ($tokens | skip ($push_index + 1)) {
+    if ($option_value | is-not-empty) {
+      if $option_value == '--repo' { $remote = $token }
+      $option_value = ""
+      continue
+    }
+    if not $after_separator {
+      if $token == '--' {
+        $after_separator = true
+        continue
+      }
+      if $token in ['--delete' '-d'] {
+        $deleting = true
+        continue
+      }
+      if $token == '--no-delete' {
+        $deleting = false
+        continue
+      }
+      if $token == '--repo' {
+        $option_value = $token
+        continue
+      }
+      if $token starts-with '--repo=' {
+        $remote = $token | str substring 7..
+        continue
+      }
+      if $token in ['--exec' '--push-option' '-o' '--receive-pack' '--recurse-submodules'] {
+        $option_value = $token
+        continue
+      }
+      if $token starts-with '-' { continue }
+    }
+    if ($remote | is-empty) { $remote = $token }
   }
 
   let local_branches = (nu-complete git local branches)
-
   if ($remote | is-empty) {
     return $local_branches
   }
 
-  # If we have a remote, find branches for that remote
-  # Use plumbing command to get remote branches, excluding HEAD
-  let remote_branches = (^git for-each-ref --format='%(refname:lstrip=3)' $'refs/remotes/($remote)' | lines | where $it != 'HEAD')
-
-  # Prefix them with :
-  let deletion_candidates = ($remote_branches | each { |it| $":($it)" })
-
-  $local_branches | append $deletion_candidates
+  # The trailing slash excludes similarly named remotes such as upstream2.
+  let prefix = $'refs/remotes/($remote)/'
+  let remote_branches = (^git for-each-ref --format='%(refname)' $prefix | lines | each {|ref| $ref | str replace $prefix '' } | where $it != 'HEAD')
+  if $deleting {
+    return $remote_branches
+  }
+  $local_branches | append ($remote_branches | each { |branch| $":($branch)" })
 }
 
 # Push changes

custom-completions/git/tests/git-completions.nu

@@ -0,0 +1,214 @@
+# Run with `nu custom-completions/git/tests/git-completions.nu`.
+# Set GIT_EXPECT_CUSTOM_COMPLETIONS=false only on older Nu versions whose engine
+# falls back to built-in paths; direct provider assertions still run there.
+use std/assert
+source ../git-completions.nu
+use git-completion-utils *
+
+def with-repo [test: closure] {
+  let original = pwd
+  let dir = mktemp -d
+  try {
+    cd $dir
+    ^git init -q -b main
+    ^git config user.email tests@example.com
+    ^git config user.name tests
+    ^git config commit.gpgsign false
+    ^git config core.hooksPath ($dir | path join disabled-hooks)
+    "base" | save base.txt
+    ^git add base.txt
+    ^git commit -q -m base
+    do $test
+    cd $original
+    rm -r $dir
+  } catch {|error|
+    cd $original
+    rm -r $dir
+    error make $error
+  }
+}
+
+def "changed paths are literal and conflicts are checkoutable" [] {
+  with-repo {||
+    let names = if $nu.os-info.family == windows {
+      ["annual report.md" "dollar $ and [glob].txt"]
+    } else {
+      ["annual report.md" "dollar $ and [glob].txt" "it's a \"draft\".md" "line\nbreak.txt"]
+    }
+    for name in $names {
+      "original" | save $name
+      ^git add -- $name
+    }
+    ^git commit -q -m paths
+    for name in $names { "changed" | save -f $name }
+    let changed = get-changed-files
+    let checkoutable = get-checkoutable-files
+    for name in $names {
+      assert ($name in $changed)
+      assert ($name in $checkoutable)
+    }
+    # Every checkout path slot needs a single, quoted Nu argument as its value.
+    for line in ['git checkout -- ' 'git checkout main -- ' 'git checkout main ' 'git checkout '] {
+      let result = nu-complete git checkout $line {cursor: ($line | str length)}
+      let rows = if (($result | describe) | str starts-with 'record') { $result.completions } else { $result }
+      for name in $names {
+        let matches = $rows | where {|row| $row.display_override? == $name }
+        assert equal ($matches | length) 1
+        assert equal ($matches.0.value | from nuon) $name
+      }
+      # Require the custom provider by default so a fallback cannot hide a failure.
+      let detailed = $line | commandline complete --detailed
+      let custom_rows = $detailed | where {|row| $row.description? == File }
+      if ($env.GIT_EXPECT_CUSTOM_COMPLETIONS? | default 'true') == 'true' {
+        assert equal ($custom_rows | length) ($names | length)
+      }
+      for name in $names {
+        let matches = $custom_rows | where {|row| $row.display_override? == $name }
+        if ($matches | is-not-empty) {
+          assert equal ($matches.0.value | from nuon) $name
+        }
+      }
+    }
+    let detailed = 'git checkout -- annual' | commandline complete --detailed
+    let custom = $detailed | where {|row| $row.display_override? == 'annual report.md' }
+    if ($env.GIT_EXPECT_CUSTOM_COMPLETIONS? | default 'true') == 'true' {
+      assert equal ($custom | length) 1
+    }
+    if ($custom | is-not-empty) {
+      assert equal ($custom.0.value | from nuon) 'annual report.md'
+      assert equal $custom.0.span.start ('git checkout -- ' | str length)
+      assert equal $custom.0.span.end ('git checkout -- annual' | str length)
+    }
+
+    ^git branch side
+    "main" | save -f base.txt
+    ^git commit -q -am main
+    ^git switch -q side
+    "side" | save -f base.txt
+    ^git commit -q -am side
+    ^git switch -q main
+    let merge = (^git merge --no-edit side | complete)
+    assert equal $merge.exit_code 1
+    assert ("base.txt" in (get-checkoutable-files))
+    assert ("base.txt" not-in (get-changed-files))
+  }
+}
+
+def "add-add conflicts are checkoutable but not ordinary changes" [] {
+  with-repo {||
+    ^git branch side
+    "main" | save both.txt
+    ^git add both.txt
+    ^git commit -q -m main-add
+    ^git switch -q side
+    "side" | save both.txt
+    ^git add both.txt
+    ^git commit -q -m side-add
+    ^git switch -q main
+    let merge = (^git merge --no-edit side | complete)
+    assert equal $merge.exit_code 1
+    assert ("both.txt" in (get-checkoutable-files))
+    assert ("both.txt" not-in (get-changed-files))
+  }
+}
+
+def "rename source cannot masquerade as a status record" [] {
+  with-repo {||
+    let old = '1 .M N... 100644 100644 100644 abcdef abcdef phantom'
+    "original" | save $old
+    ^git add -- $old
+    ^git commit -q -m old-name
+    ^git mv -- $old renamed.txt
+    let records = ^git status -uno --porcelain=2 -z | split row "\u{0}"
+    assert ($records.0 starts-with '2 R.')
+    assert equal (get-changed-files) []
+    assert equal (get-checkoutable-files) []
+  }
+}
+
+def "remote rows stay on their own lines" [] {
+  with-repo {||
+    ^git remote add origin '/tmp/a remote with spaces'
+    ^git remote add plain 'https://example.invalid/repo.git'
+    let rows = nu-complete git remotes
+    assert equal ($rows | length) 2
+    assert equal ($rows | get value | sort) [origin plain]
+    assert equal ($rows | where value == origin | get 0.description) '/tmp/a remote with spaces'
+    assert equal ($rows | where value == plain | get 0.description) 'https://example.invalid/repo.git'
+  }
+}
+
+def "push delete offers remote names with flags in context" [] {
+  with-repo {||
+    for ref in [refs/remotes/upstream/main refs/remotes/upstream/feature/a refs/remotes/upstream2/decoy] {
+      ^git update-ref $ref HEAD
+    }
+    ^git symbolic-ref refs/remotes/upstream/HEAD refs/remotes/upstream/main
+    for line in ['git push --delete upstream ' 'git push -d upstream ' 'git push --force upstream --delete ' 'git push --push-option value --delete upstream ' 'git push --repo upstream --delete ' 'git push --repo=upstream --delete '] {
+      let actual = nu-complete git push $line {cursor: ($line | str length)}
+      assert equal ($actual | sort) ['feature/a' 'main']
+    }
+    let midline = 'git push --delete upstream --force'
+    assert equal ((nu-complete git push $midline {cursor: 27}) | sort) ['feature/a' 'main']
+    let normal = 'git push --force upstream '
+    let actual = nu-complete git push $normal {cursor: ($normal | str length)}
+    assert ('main' in $actual)
+    assert (':feature/a' in $actual)
+    assert (':decoy' not-in $actual)
+    # These flags must not turn an option value or a refspec into a remote or --delete.
+    for line in [
+      'git push --signed upstream '
+      'git push --signed=if-asked upstream '
+      'git push --repo upstream '
+      'git push --repo=upstream '
+      'git push --push-option --delete upstream '
+      'git push upstream --push-option --delete '
+      'git push -- upstream '
+      'git push upstream -- --delete '
+      'git push --delete --no-delete upstream '
+    ] {
+      let actual = nu-complete git push $line {cursor: ($line | str length)}
+      assert ('main' in $actual)
+      assert (':feature/a' in $actual)
+      assert ('feature/a' not-in $actual)
+    }
+  }
+}
+
+def "push strips a slash-containing remote name exactly" [] {
+  with-repo {||
+    ^git remote add team/upstream ../unused-remote
+    for ref in [refs/remotes/team/upstream/main refs/remotes/team/upstream/feature/a refs/remotes/team/upstream/foo/HEAD refs/remotes/team/upstream2/decoy] {
+      ^git update-ref $ref HEAD
+    }
+    ^git symbolic-ref refs/remotes/team/upstream/HEAD refs/remotes/team/upstream/main
+    let deleted = 'git push --delete team/upstream '
+    assert equal ((nu-complete git push $deleted {cursor: ($deleted | str length)}) | sort) ['feature/a' 'foo/HEAD' 'main']
+    let normal = 'git push team/upstream '
+    let rows = nu-complete git push $normal {cursor: ($normal | str length)}
+    assert (':feature/a' in $rows)
+    assert (':foo/HEAD' in $rows)
+    assert (':decoy' not-in $rows)
+  }
+}
+
+match ($env.GIT_COMPLETION_CASE? | default all) {
+  changed => {
+    changed paths are literal and conflicts are checkoutable
+    add-add conflicts are checkoutable but not ordinary changes
+    rename source cannot masquerade as a status record
+  }
+  remotes => { remote rows stay on their own lines }
+  push => {
+    push delete offers remote names with flags in context
+    push strips a slash-containing remote name exactly
+  }
+  all => {
+    changed paths are literal and conflicts are checkoutable
+    add-add conflicts are checkoutable but not ordinary changes
+    rename source cannot masquerade as a status record
+    remote rows stay on their own lines
+    push delete offers remote names with flags in context
+    push strips a slash-containing remote name exactly
+  }
+}