completion: wrap __git_refs() for better option parsing

__git_refs() currently accepts two optional positional parameters: a remote and a flag for 'git checkout's tracking DWIMery. To fix a minor bug, and, more importantly, for faster refs completion, this series will add three more parameters: a prefix, the current word to be completed and a suffix, i.e. the options accepted by __gitcomp() & friends, and will change __git_refs() to list only refs matching that given current word and to add that given prefix and suffix to the listed refs. However, __git_refs() is the helper function that is most likely used in users' custom completion scriptlets for their own git commands, and we don't want to break those, so - we can't change __git_refs()'s default output format, i.e. we can't by default append a trailing space to every listed ref, meaning that the suffix parameter containing the default trailing space would have to be specified on every invocation, and - we can't change the position of existing positional parameters either, so there would have to be plenty of set-but-empty placeholder positional parameters all over the completion script. Furthermore, with five positional parameters it would be really hard to remember which position means what. To keep callsites simple, add the new wrapper function __git_complete_refs() around __git_refs(), which: - instead of positional parameters accepts real '--opt=val'-style options and with minimalistic option parsing translates them to __git_refs()'s and __gitcomp_nl()'s positional parameters, and - includes the '__gitcomp_nl "$(__git_refs ...)" ...' command substitution to make its behavior match its name and the behavior of other __git_complete_* functions, and to limit future changes in this series to __git_refs() and this new wrapper function. Call this wrapper function instead of __git_refs() wherever possible throughout the completion script, i.e. when __git_refs()'s output is fed to __gitcomp_nl() right away without further processing, which means all callsites except a single one in the __git_refs2() helper. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

SZEDER Gábor committed Mar 23, 2017 at 16:29 UTC 15b4a163950c2e8660a7797ce3975ccea8705f80
2 files changed +173 -35
contrib/completion/git-completion.bash
+67 -35
@@ -354,6 +354,8 @@ __git_tags ()
354 # Can be the name of a configured remote, a path, or a URL.
355 # 2: In addition to local refs, list unique branches from refs/remotes/ for
356 # 'git checkout's tracking DWIMery (optional; ignored, if set but empty).
357 +#
358 +# Use __git_complete_refs() instead.
359 __git_refs ()
360 {
361 local i hash dir track="${2-}"
@@ -446,6 +448,36 @@ __git_refs ()
448 esac
449 }
450
451 +# Completes refs, short and long, local and remote, symbolic and pseudo.
452 +#
453 +# Usage: __git_complete_refs [<option>]...
454 +# --remote=<remote>: The remote to list refs from, can be the name of a
455 +# configured remote, a path, or a URL.
456 +# --track: List unique remote branches for 'git checkout's tracking DWIMery.
457 +# --pfx=<prefix>: A prefix to be added to each ref.
458 +# --cur=<word>: The current ref to be completed. Defaults to the current
459 +# word to be completed.
460 +# --sfx=<suffix>: A suffix to be appended to each ref instead of the default
461 +# space.
462 +__git_complete_refs ()
463 +{
464 + local remote track pfx cur_="$cur" sfx=" "
465 +
466 + while test $# != 0; do
467 + case "$1" in
468 + --remote=*) remote="${1##--remote=}" ;;
469 + --track) track="yes" ;;
470 + --pfx=*) pfx="${1##--pfx=}" ;;
471 + --cur=*) cur_="${1##--cur=}" ;;
472 + --sfx=*) sfx="${1##--sfx=}" ;;
473 + *) return 1 ;;
474 + esac
475 + shift
476 + done
477 +
478 + __gitcomp_nl "$(__git_refs "$remote" "$track")" "$pfx" "$cur_" "$sfx"
479 +}
480 +
481 # __git_refs2 requires 1 argument (to pass to __git_refs)
482 __git_refs2 ()
483 {
@@ -554,15 +586,15 @@ __git_complete_revlist_file ()
586 *...*)
587 pfx="${cur_%...*}..."
588 cur_="${cur_#*...}"
557 - __gitcomp_nl "$(__git_refs)" "$pfx" "$cur_"
589 + __git_complete_refs --pfx="$pfx" --cur="$cur_"
590 ;;
591 *..*)
592 pfx="${cur_%..*}.."
593 cur_="${cur_#*..}"
562 - __gitcomp_nl "$(__git_refs)" "$pfx" "$cur_"
594 + __git_complete_refs --pfx="$pfx" --cur="$cur_"
595 ;;
596 *)
565 - __gitcomp_nl "$(__git_refs)"
597 + __git_complete_refs
598 ;;
599 esac
600 }
@@ -649,21 +681,21 @@ __git_complete_remote_or_refspec ()
681 if [ $lhs = 1 ]; then
682 __gitcomp_nl "$(__git_refs2 "$remote")" "$pfx" "$cur_"
683 else
652 - __gitcomp_nl "$(__git_refs)" "$pfx" "$cur_"
684 + __git_complete_refs --pfx="$pfx" --cur="$cur_"
685 fi
686 ;;
687 pull|remote)
688 if [ $lhs = 1 ]; then
657 - __gitcomp_nl "$(__git_refs "$remote")" "$pfx" "$cur_"
689 + __git_complete_refs --remote="$remote" --pfx="$pfx" --cur="$cur_"
690 else
659 - __gitcomp_nl "$(__git_refs)" "$pfx" "$cur_"
691 + __git_complete_refs --pfx="$pfx" --cur="$cur_"
692 fi
693 ;;
694 push)
695 if [ $lhs = 1 ]; then
664 - __gitcomp_nl "$(__git_refs)" "$pfx" "$cur_"
696 + __git_complete_refs --pfx="$pfx" --cur="$cur_"
697 else
666 - __gitcomp_nl "$(__git_refs "$remote")" "$pfx" "$cur_"
698 + __git_complete_refs --remote="$remote" --pfx="$pfx" --cur="$cur_"
699 fi
700 ;;
701 esac
@@ -1061,7 +1093,7 @@ _git_bisect ()
1093
1094 case "$subcommand" in
1095 bad|good|reset|skip|start)
1064 - __gitcomp_nl "$(__git_refs)"
1096 + __git_complete_refs
1097 ;;
1098 *)
1099 ;;
@@ -1083,7 +1115,7 @@ _git_branch ()
1115
1116 case "$cur" in
1117 --set-upstream-to=*)
1086 - __gitcomp_nl "$(__git_refs)" "" "${cur##--set-upstream-to=}"
1118 + __git_complete_refs --cur="${cur##--set-upstream-to=}"
1119 ;;
1120 --*)
1121 __gitcomp "
@@ -1097,7 +1129,7 @@ _git_branch ()
1129 if [ $only_local_ref = "y" -a $has_r = "n" ]; then
1130 __gitcomp_nl "$(__git_heads)"
1131 else
1100 - __gitcomp_nl "$(__git_refs)"
1132 + __git_complete_refs
1133 fi
1134 ;;
1135 esac
@@ -1140,18 +1172,18 @@ _git_checkout ()
1172 *)
1173 # check if --track, --no-track, or --no-guess was specified
1174 # if so, disable DWIM mode
1143 - local flags="--track --no-track --no-guess" track=1
1175 + local flags="--track --no-track --no-guess" track_opt="--track"
1176 if [ -n "$(__git_find_on_cmdline "$flags")" ]; then
1145 - track=''
1177 + track_opt=''
1178 fi
1147 - __gitcomp_nl "$(__git_refs '' $track)"
1179 + __git_complete_refs $track_opt
1180 ;;
1181 esac
1182 }
1183
1184 _git_cherry ()
1185 {
1154 - __gitcomp_nl "$(__git_refs)"
1186 + __git_complete_refs
1187 }
1188
1189 _git_cherry_pick ()
@@ -1166,7 +1198,7 @@ _git_cherry_pick ()
1198 __gitcomp "--edit --no-commit --signoff --strategy= --mainline"
1199 ;;
1200 *)
1169 - __gitcomp_nl "$(__git_refs)"
1201 + __git_complete_refs
1202 ;;
1203 esac
1204 }
@@ -1216,7 +1248,7 @@ _git_commit ()
1248 {
1249 case "$prev" in
1250 -c|-C)
1219 - __gitcomp_nl "$(__git_refs)"
1251 + __git_complete_refs
1252 return
1253 ;;
1254 esac
@@ -1229,7 +1261,7 @@ _git_commit ()
1261 ;;
1262 --reuse-message=*|--reedit-message=*|\
1263 --fixup=*|--squash=*)
1232 - __gitcomp_nl "$(__git_refs)" "" "${cur#*=}"
1264 + __git_complete_refs --cur="${cur#*=}"
1265 return
1266 ;;
1267 --untracked-files=*)
@@ -1267,7 +1299,7 @@ _git_describe ()
1299 "
1300 return
1301 esac
1270 - __gitcomp_nl "$(__git_refs)"
1302 + __git_complete_refs
1303 }
1304
1305 __git_diff_algorithms="myers minimal patience histogram"
@@ -1453,7 +1485,7 @@ _git_grep ()
1485 ;;
1486 esac
1487
1456 - __gitcomp_nl "$(__git_refs)"
1488 + __git_complete_refs
1489 }
1490
1491 _git_help ()
@@ -1621,7 +1653,7 @@ _git_merge ()
1653 --rerere-autoupdate --no-rerere-autoupdate --abort --continue"
1654 return
1655 esac
1624 - __gitcomp_nl "$(__git_refs)"
1656 + __git_complete_refs
1657 }
1658
1659 _git_mergetool ()
@@ -1646,7 +1678,7 @@ _git_merge_base ()
1678 return
1679 ;;
1680 esac
1649 - __gitcomp_nl "$(__git_refs)"
1681 + __git_complete_refs
1682 }
1683
1684 _git_mv ()
@@ -1684,7 +1716,7 @@ _git_notes ()
1716 ,*)
1717 case "$prev" in
1718 --ref)
1687 - __gitcomp_nl "$(__git_refs)"
1719 + __git_complete_refs
1720 ;;
1721 *)
1722 __gitcomp "$subcommands --ref"
@@ -1693,7 +1725,7 @@ _git_notes ()
1725 ;;
1726 add,--reuse-message=*|append,--reuse-message=*|\
1727 add,--reedit-message=*|append,--reedit-message=*)
1696 - __gitcomp_nl "$(__git_refs)" "" "${cur#*=}"
1728 + __git_complete_refs --cur="${cur#*=}"
1729 ;;
1730 add,--*|append,--*)
1731 __gitcomp '--file= --message= --reedit-message=
@@ -1712,7 +1744,7 @@ _git_notes ()
1744 -m|-F)
1745 ;;
1746 *)
1715 - __gitcomp_nl "$(__git_refs)"
1747 + __git_complete_refs
1748 ;;
1749 esac
1750 ;;
@@ -1750,10 +1782,10 @@ __git_complete_force_with_lease ()
1782 --*=)
1783 ;;
1784 *:*)
1753 - __gitcomp_nl "$(__git_refs)" "" "${cur_#*:}"
1785 + __git_complete_refs --cur="${cur_#*:}"
1786 ;;
1787 *)
1756 - __gitcomp_nl "$(__git_refs)" "" "$cur_"
1788 + __git_complete_refs --cur="$cur_"
1789 ;;
1790 esac
1791 }
@@ -1829,7 +1861,7 @@ _git_rebase ()
1861
1862 return
1863 esac
1832 - __gitcomp_nl "$(__git_refs)"
1864 + __git_complete_refs
1865 }
1866
1867 _git_reflog ()
@@ -1840,7 +1872,7 @@ _git_reflog ()
1872 if [ -z "$subcommand" ]; then
1873 __gitcomp "$subcommands"
1874 else
1843 - __gitcomp_nl "$(__git_refs)"
1875 + __git_complete_refs
1876 fi
1877 }
1878
@@ -1986,7 +2018,7 @@ _git_config ()
2018 return
2019 ;;
2020 branch.*.merge)
1989 - __gitcomp_nl "$(__git_refs)"
2021 + __git_complete_refs
2022 return
2023 ;;
2024 branch.*.rebase)
@@ -2460,7 +2492,7 @@ _git_remote ()
2492
2493 _git_replace ()
2494 {
2463 - __gitcomp_nl "$(__git_refs)"
2495 + __git_complete_refs
2496 }
2497
2498 _git_reset ()
@@ -2473,7 +2505,7 @@ _git_reset ()
2505 return
2506 ;;
2507 esac
2476 - __gitcomp_nl "$(__git_refs)"
2508 + __git_complete_refs
2509 }
2510
2511 _git_revert ()
@@ -2489,7 +2521,7 @@ _git_revert ()
2521 return
2522 ;;
2523 esac
2492 - __gitcomp_nl "$(__git_refs)"
2524 + __git_complete_refs
2525 }
2526
2527 _git_rm ()
@@ -2597,7 +2629,7 @@ _git_stash ()
2629 ;;
2630 branch,*)
2631 if [ $cword -eq 3 ]; then
2600 - __gitcomp_nl "$(__git_refs)";
2632 + __git_complete_refs
2633 else
2634 __gitcomp_nl "$(__git stash list \
2635 | sed -n -e 's/:.*//p')"
@@ -2755,7 +2787,7 @@ _git_tag ()
2787 fi
2788 ;;
2789 *)
2758 - __gitcomp_nl "$(__git_refs)"
2790 + __git_complete_refs
2791 ;;
2792 esac
2793
t/t9902-completion.sh
+106
@@ -775,6 +775,112 @@ test_expect_success '__git_refs - unique remote branches for git checkout DWIMer
775 test_cmp expected "$actual"
776 '
777
778 +test_expect_success '__git_complete_refs - simple' '
779 + sed -e "s/Z$//" >expected <<-EOF &&
780 + HEAD Z
781 + master Z
782 + matching-branch Z
783 + other/branch-in-other Z
784 + other/master-in-other Z
785 + matching-tag Z
786 + EOF
787 + (
788 + cur= &&
789 + __git_complete_refs &&
790 + print_comp
791 + ) &&
792 + test_cmp expected out
793 +'
794 +
795 +test_expect_success '__git_complete_refs - matching' '
796 + sed -e "s/Z$//" >expected <<-EOF &&
797 + matching-branch Z
798 + matching-tag Z
799 + EOF
800 + (
801 + cur=mat &&
802 + __git_complete_refs &&
803 + print_comp
804 + ) &&
805 + test_cmp expected out
806 +'
807 +
808 +test_expect_success '__git_complete_refs - remote' '
809 + sed -e "s/Z$//" >expected <<-EOF &&
810 + HEAD Z
811 + branch-in-other Z
812 + master-in-other Z
813 + EOF
814 + (
815 + cur=
816 + __git_complete_refs --remote=other &&
817 + print_comp
818 + ) &&
819 + test_cmp expected out
820 +'
821 +
822 +test_expect_success '__git_complete_refs - track' '
823 + sed -e "s/Z$//" >expected <<-EOF &&
824 + HEAD Z
825 + master Z
826 + matching-branch Z
827 + other/branch-in-other Z
828 + other/master-in-other Z
829 + matching-tag Z
830 + branch-in-other Z
831 + master-in-other Z
832 + EOF
833 + (
834 + cur=
835 + __git_complete_refs --track &&
836 + print_comp
837 + ) &&
838 + test_cmp expected out
839 +'
840 +
841 +test_expect_success '__git_complete_refs - current word' '
842 + sed -e "s/Z$//" >expected <<-EOF &&
843 + matching-branch Z
844 + matching-tag Z
845 + EOF
846 + (
847 + cur="--option=mat" &&
848 + __git_complete_refs --cur="${cur#*=}" &&
849 + print_comp
850 + ) &&
851 + test_cmp expected out
852 +'
853 +
854 +test_expect_success '__git_complete_refs - prefix' '
855 + sed -e "s/Z$//" >expected <<-EOF &&
856 + v1.0..matching-branch Z
857 + v1.0..matching-tag Z
858 + EOF
859 + (
860 + cur=v1.0..mat &&
861 + __git_complete_refs --pfx=v1.0.. --cur=mat &&
862 + print_comp
863 + ) &&
864 + test_cmp expected out
865 +'
866 +
867 +test_expect_success '__git_complete_refs - suffix' '
868 + cat >expected <<-EOF &&
869 + HEAD.
870 + master.
871 + matching-branch.
872 + other/branch-in-other.
873 + other/master-in-other.
874 + matching-tag.
875 + EOF
876 + (
877 + cur= &&
878 + __git_complete_refs --sfx=. &&
879 + print_comp
880 + ) &&
881 + test_cmp expected out
882 +'
883 +
884 test_expect_success 'teardown after ref completion' '
885 git branch -d matching-branch &&
886 git tag -d matching-tag &&