git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH 00/14] completion: a bunch of updates
@ 2019-06-21 22:30 Felipe Contreras
  2019-06-21 22:30 ` [PATCH 01/14] completion: zsh: fix __gitcomp_direct() Felipe Contreras
                   ` (14 more replies)
  0 siblings, 15 replies; 58+ messages in thread
From: Felipe Contreras @ 2019-06-21 22:30 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, SZEDER Gábor,
	Nguyễn Thái Ngọc Duy, Felipe Contreras

Hi,

Here's another try at completion fixes, cleanups, and more tests. Some
of these have already been sent.

Felipe Contreras (14):
  completion: zsh: fix __gitcomp_direct()
  completion: zsh: fix for directories with spaces
  completion: remove zsh hack
  completion: zsh: improve main function selection
  completion: prompt: fix color for Zsh
  completion: bash: cleanup cygwin check
  completion: zsh: update installation instructions
  completion: bash: remove old compat wrappers
  completion: bash: remove zsh wrapper
  completion: zsh: trivial cleanups
  test: completion: tests for __gitcomp regression
  test: completion: use global config
  completion: add default options
  completion: add default merge strategies

 contrib/completion/git-completion.bash | 202 +++++++++++++------------
 contrib/completion/git-completion.zsh  |  53 +++----
 contrib/completion/git-prompt.sh       |  10 +-
 t/t9902-completion.sh                  |  37 +++--
 4 files changed, 161 insertions(+), 141 deletions(-)

-- 
2.22.0


^ permalink raw reply	[flat|nested] 58+ messages in thread

* [PATCH 01/14] completion: zsh: fix __gitcomp_direct()
  2019-06-21 22:30 [PATCH 00/14] completion: a bunch of updates Felipe Contreras
@ 2019-06-21 22:30 ` Felipe Contreras
  2019-06-22 15:03   ` Felipe Contreras
  2019-06-21 22:30 ` [PATCH 02/14] completion: zsh: fix for directories with spaces Felipe Contreras
                   ` (13 subsequent siblings)
  14 siblings, 1 reply; 58+ messages in thread
From: Felipe Contreras @ 2019-06-21 22:30 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, SZEDER Gábor,
	Nguyễn Thái Ngọc Duy, Felipe Contreras

Many callers append a space suffix, but zsh automatically appends a
space, making the completion add two spaces, for example:

  git log ma<tab>

Will complete 'master  '.

Let's remove that extra space.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.bash | 2 +-
 contrib/completion/git-completion.zsh  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 9f71bcde96..a65d5956c1 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -3009,7 +3009,7 @@ if [[ -n ${ZSH_VERSION-} ]] &&
 
 		local IFS=$'\n'
 		compset -P '*[=:]'
-		compadd -Q -- ${=1} && _ret=0
+		compadd -Q -- ${${=1}% } && _ret=0
 	}
 
 	__gitcomp_nl ()
diff --git a/contrib/completion/git-completion.zsh b/contrib/completion/git-completion.zsh
index 886bf95d1f..0d66c27366 100644
--- a/contrib/completion/git-completion.zsh
+++ b/contrib/completion/git-completion.zsh
@@ -73,7 +73,7 @@ __gitcomp_direct ()
 
 	local IFS=$'\n'
 	compset -P '*[=:]'
-	compadd -Q -- ${=1} && _ret=0
+	compadd -Q -- ${${=1}% } && _ret=0
 }
 
 __gitcomp_nl ()
-- 
2.22.0


^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [PATCH 02/14] completion: zsh: fix for directories with spaces
  2019-06-21 22:30 [PATCH 00/14] completion: a bunch of updates Felipe Contreras
  2019-06-21 22:30 ` [PATCH 01/14] completion: zsh: fix __gitcomp_direct() Felipe Contreras
@ 2019-06-21 22:30 ` Felipe Contreras
  2019-06-21 22:30 ` [PATCH 03/14] completion: remove zsh hack Felipe Contreras
                   ` (12 subsequent siblings)
  14 siblings, 0 replies; 58+ messages in thread
From: Felipe Contreras @ 2019-06-21 22:30 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, SZEDER Gábor,
	Nguyễn Thái Ngọc Duy, Felipe Contreras

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.zsh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.zsh b/contrib/completion/git-completion.zsh
index 0d66c27366..034cfa9e8f 100644
--- a/contrib/completion/git-completion.zsh
+++ b/contrib/completion/git-completion.zsh
@@ -30,7 +30,7 @@ if [ -z "$script" ]; then
 	local -a locations
 	local e
 	locations=(
-		$(dirname ${funcsourcetrace[1]%:*})/git-completion.bash
+		"$(dirname ${funcsourcetrace[1]%:*})"/git-completion.bash
 		'/etc/bash_completion.d/git' # fedora, old debian
 		'/usr/share/bash-completion/completions/git' # arch, ubuntu, new debian
 		'/usr/share/bash-completion/git' # gentoo
-- 
2.22.0


^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [PATCH 03/14] completion: remove zsh hack
  2019-06-21 22:30 [PATCH 00/14] completion: a bunch of updates Felipe Contreras
  2019-06-21 22:30 ` [PATCH 01/14] completion: zsh: fix __gitcomp_direct() Felipe Contreras
  2019-06-21 22:30 ` [PATCH 02/14] completion: zsh: fix for directories with spaces Felipe Contreras
@ 2019-06-21 22:30 ` Felipe Contreras
  2019-06-21 22:30 ` [PATCH 04/14] completion: zsh: improve main function selection Felipe Contreras
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 58+ messages in thread
From: Felipe Contreras @ 2019-06-21 22:30 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, SZEDER Gábor,
	Nguyễn Thái Ngọc Duy, Felipe Contreras,
	Mark Lodato

We don't want to override the 'complete()' function in zsh, which can be
used by bashcomp.

Reported-by: Mark Lodato <lodato@google.com>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.bash | 1 +
 contrib/completion/git-completion.zsh  | 6 ------
 2 files changed, 1 insertion(+), 6 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index a65d5956c1..676b19a983 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -3066,6 +3066,7 @@ __git_func_wrap ()
 # This is NOT a public function; use at your own risk.
 __git_complete ()
 {
+	test -n "$ZSH_VERSION" && return
 	local wrapper="__git_wrap${2}"
 	eval "$wrapper () { __git_func_wrap $2 ; }"
 	complete -o bashdefault -o default -o nospace -F $wrapper $1 2>/dev/null \
diff --git a/contrib/completion/git-completion.zsh b/contrib/completion/git-completion.zsh
index 034cfa9e8f..aade33ec9f 100644
--- a/contrib/completion/git-completion.zsh
+++ b/contrib/completion/git-completion.zsh
@@ -16,12 +16,6 @@
 #
 #  fpath=(~/.zsh $fpath)
 
-complete ()
-{
-	# do nothing
-	return 0
-}
-
 zstyle -T ':completion:*:*:git:*' tag-order && \
 	zstyle ':completion:*:*:git:*' tag-order 'common-commands'
 
-- 
2.22.0


^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [PATCH 04/14] completion: zsh: improve main function selection
  2019-06-21 22:30 [PATCH 00/14] completion: a bunch of updates Felipe Contreras
                   ` (2 preceding siblings ...)
  2019-06-21 22:30 ` [PATCH 03/14] completion: remove zsh hack Felipe Contreras
@ 2019-06-21 22:30 ` Felipe Contreras
  2019-06-21 22:30 ` [PATCH 05/14] completion: prompt: fix color for Zsh Felipe Contreras
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 58+ messages in thread
From: Felipe Contreras @ 2019-06-21 22:30 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, SZEDER Gábor,
	Nguyễn Thái Ngọc Duy, Felipe Contreras

Sometimes we want to use the function directly (e.g. _git_checkout), for
example when zsh has the option 'complete_aliases', this way, we can do
something like:

  compdef _git gco=git_checkout

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.zsh | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.zsh b/contrib/completion/git-completion.zsh
index aade33ec9f..2801f2f7c8 100644
--- a/contrib/completion/git-completion.zsh
+++ b/contrib/completion/git-completion.zsh
@@ -226,8 +226,10 @@ _git ()
 
 	if (( $+functions[__${service}_zsh_main] )); then
 		__${service}_zsh_main
-	else
+	elif (( $+functions[__${service}_main] )); then
 		emulate ksh -c __${service}_main
+	elif (( $+functions[_${service}] )); then
+		emulate ksh -c _${service}
 	fi
 
 	let _ret && _default && _ret=0
-- 
2.22.0


^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [PATCH 05/14] completion: prompt: fix color for Zsh
  2019-06-21 22:30 [PATCH 00/14] completion: a bunch of updates Felipe Contreras
                   ` (3 preceding siblings ...)
  2019-06-21 22:30 ` [PATCH 04/14] completion: zsh: improve main function selection Felipe Contreras
@ 2019-06-21 22:30 ` Felipe Contreras
  2019-06-21 22:30 ` [PATCH 06/14] completion: bash: cleanup cygwin check Felipe Contreras
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 58+ messages in thread
From: Felipe Contreras @ 2019-06-21 22:30 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, SZEDER Gábor,
	Nguyễn Thái Ngọc Duy, Felipe Contreras

We don't need PROMPT_COMMAND in Zsh; we are already using %F{color} %f,
which in turn use %{ and %}, which are the equivalent of Bash's
\[ and \].

We can use as many colors as we want and output directly into PS1
(or RPS1) without the risk of buffer wrapping issues.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-prompt.sh | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 983e419d2b..b57a9c96cb 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -88,7 +88,7 @@
 # If you would like a colored hint about the current dirty state, set
 # GIT_PS1_SHOWCOLORHINTS to a nonempty value. The colors are based on
 # the colored output of "git status -sb" and are available only when
-# using __git_ps1 for PROMPT_COMMAND or precmd.
+# using __git_ps1 for PROMPT_COMMAND in Bash, but always available in Zsh.
 #
 # If you would like __git_ps1 to do nothing in the case when the current
 # directory is set up to be ignored by git, then set
@@ -506,9 +506,11 @@ __git_ps1 ()
 
 	local z="${GIT_PS1_STATESEPARATOR-" "}"
 
-	# NO color option unless in PROMPT_COMMAND mode
-	if [ $pcmode = yes ] && [ -n "${GIT_PS1_SHOWCOLORHINTS-}" ]; then
-		__git_ps1_colorize_gitstring
+	# NO color option unless in PROMPT_COMMAND mode or it's Zsh
+	if [ -n "${GIT_PS1_SHOWCOLORHINTS-}" ]; then
+		if [ $pcmode = yes ] || [ -n "${ZSH_VERSION-}" ]; then
+			__git_ps1_colorize_gitstring
+		fi
 	fi
 
 	b=${b##refs/heads/}
-- 
2.22.0


^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [PATCH 06/14] completion: bash: cleanup cygwin check
  2019-06-21 22:30 [PATCH 00/14] completion: a bunch of updates Felipe Contreras
                   ` (4 preceding siblings ...)
  2019-06-21 22:30 ` [PATCH 05/14] completion: prompt: fix color for Zsh Felipe Contreras
@ 2019-06-21 22:30 ` Felipe Contreras
  2019-06-21 22:31 ` [PATCH 07/14] completion: zsh: update installation instructions Felipe Contreras
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 58+ messages in thread
From: Felipe Contreras @ 2019-06-21 22:30 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, SZEDER Gábor,
	Nguyễn Thái Ngọc Duy, Felipe Contreras

Avoid Yoda conditions, and use $OSTYPE.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.bash | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 676b19a983..dba822d0e7 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -3092,6 +3092,6 @@ __git_complete gitk __gitk_main
 # when the user has tab-completed the executable name and consequently
 # included the '.exe' suffix.
 #
-if [ Cygwin = "$(uname -o 2>/dev/null)" ]; then
-__git_complete git.exe __git_main
+if [ "$OSTYPE" = "Cygwin" ]; then
+	__git_complete git.exe __git_main
 fi
-- 
2.22.0


^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [PATCH 07/14] completion: zsh: update installation instructions
  2019-06-21 22:30 [PATCH 00/14] completion: a bunch of updates Felipe Contreras
                   ` (5 preceding siblings ...)
  2019-06-21 22:30 ` [PATCH 06/14] completion: bash: cleanup cygwin check Felipe Contreras
@ 2019-06-21 22:31 ` Felipe Contreras
  2019-06-21 22:31 ` [PATCH 08/14] completion: bash: remove old compat wrappers Felipe Contreras
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 58+ messages in thread
From: Felipe Contreras @ 2019-06-21 22:31 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, SZEDER Gábor,
	Nguyễn Thái Ngọc Duy, Felipe Contreras

Commit 0e5ed7cca3 wrongly changed the extension of the bash script
to .zsh. The extension doesn't really matter, but it confuses people.

I've changed the text to make it clear that your zsh script goes to
~/.zsh/_git, and the bash script to ~/.contrib/completion/git-completion.bash (or wherever
you want).

Also, update the default locations of the system bash-completion,
including the default bash-completion location for user scripts, and the
recommended way to find the system location (with pkg-config)

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.zsh | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/contrib/completion/git-completion.zsh b/contrib/completion/git-completion.zsh
index 2801f2f7c8..7f614d5854 100644
--- a/contrib/completion/git-completion.zsh
+++ b/contrib/completion/git-completion.zsh
@@ -4,17 +4,19 @@
 #
 # Copyright (c) 2012-2013 Felipe Contreras <felipe.contreras@gmail.com>
 #
-# You need git's bash completion script installed somewhere, by default it
-# would be the location bash-completion uses.
-#
-# If your script is somewhere else, you can configure it on your ~/.zshrc:
-#
-#  zstyle ':completion:*:*:git:*' script ~/.git-completion.zsh
-#
 # The recommended way to install this script is to copy to '~/.zsh/_git', and
 # then add the following to your ~/.zshrc file:
 #
 #  fpath=(~/.zsh $fpath)
+#
+# You need git's bash completion script installed. By default it will use
+# bash-completion's script.
+#
+# If your bash completion script is somewhere else, you can configure it on
+# your ~/.zshrc:
+#
+#  zstyle ':completion:*:*:git:*' script ~/.git-completion.bash
+#
 
 zstyle -T ':completion:*:*:git:*' tag-order && \
 	zstyle ':completion:*:*:git:*' tag-order 'common-commands'
@@ -25,9 +27,10 @@ if [ -z "$script" ]; then
 	local e
 	locations=(
 		"$(dirname ${funcsourcetrace[1]%:*})"/git-completion.bash
-		'/etc/bash_completion.d/git' # fedora, old debian
-		'/usr/share/bash-completion/completions/git' # arch, ubuntu, new debian
-		'/usr/share/bash-completion/git' # gentoo
+		"$HOME/.local/share/bash-completion/completions/git"
+		"$(pkg-config --variable=completionsdir bash-completion)"/git
+		'/usr/share/bash-completion/completions/git'
+		'/etc/bash_completion.d/git' # old debian
 		)
 	for e in $locations; do
 		test -f $e && script="$e" && break
-- 
2.22.0


^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [PATCH 08/14] completion: bash: remove old compat wrappers
  2019-06-21 22:30 [PATCH 00/14] completion: a bunch of updates Felipe Contreras
                   ` (6 preceding siblings ...)
  2019-06-21 22:31 ` [PATCH 07/14] completion: zsh: update installation instructions Felipe Contreras
@ 2019-06-21 22:31 ` Felipe Contreras
  2019-06-21 22:31 ` [PATCH 09/14] completion: bash: remove zsh wrapper Felipe Contreras
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 58+ messages in thread
From: Felipe Contreras @ 2019-06-21 22:31 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, SZEDER Gábor,
	Nguyễn Thái Ngọc Duy, Felipe Contreras

It's been seven years, probably more than enough time to move on.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.bash | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index dba822d0e7..1f9b833913 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -3073,18 +3073,6 @@ __git_complete ()
 		|| complete -o default -o nospace -F $wrapper $1
 }
 
-# wrapper for backwards compatibility
-_git ()
-{
-	__git_wrap__git_main
-}
-
-# wrapper for backwards compatibility
-_gitk ()
-{
-	__git_wrap__gitk_main
-}
-
 __git_complete git __git_main
 __git_complete gitk __gitk_main
 
-- 
2.22.0


^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [PATCH 09/14] completion: bash: remove zsh wrapper
  2019-06-21 22:30 [PATCH 00/14] completion: a bunch of updates Felipe Contreras
                   ` (7 preceding siblings ...)
  2019-06-21 22:31 ` [PATCH 08/14] completion: bash: remove old compat wrappers Felipe Contreras
@ 2019-06-21 22:31 ` Felipe Contreras
  2019-06-21 22:31 ` [PATCH 10/14] completion: zsh: trivial cleanups Felipe Contreras
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 58+ messages in thread
From: Felipe Contreras @ 2019-06-21 22:31 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, SZEDER Gábor,
	Nguyễn Thái Ngọc Duy, Felipe Contreras

It has been deprecated for more than seven years. It's time to move on.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.bash | 84 +-------------------------
 1 file changed, 2 insertions(+), 82 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 1f9b833913..d3ee6c7dc2 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2969,88 +2969,8 @@ __gitk_main ()
 	__git_complete_revlist
 }
 
-if [[ -n ${ZSH_VERSION-} ]] &&
-   # Don't define these functions when sourced from 'git-completion.zsh',
-   # it has its own implementations.
-   [[ -z ${GIT_SOURCING_ZSH_COMPLETION-} ]]; then
-	echo "WARNING: this script is deprecated, please see git-completion.zsh" 1>&2
-
-	autoload -U +X compinit && compinit
-
-	__gitcomp ()
-	{
-		emulate -L zsh
-
-		local cur_="${3-$cur}"
-
-		case "$cur_" in
-		--*=)
-			;;
-		*)
-			local c IFS=$' \t\n'
-			local -a array
-			for c in ${=1}; do
-				c="$c${4-}"
-				case $c in
-				--*=*|*.) ;;
-				*) c="$c " ;;
-				esac
-				array[${#array[@]}+1]="$c"
-			done
-			compset -P '*[=:]'
-			compadd -Q -S '' -p "${2-}" -a -- array && _ret=0
-			;;
-		esac
-	}
-
-	__gitcomp_direct ()
-	{
-		emulate -L zsh
-
-		local IFS=$'\n'
-		compset -P '*[=:]'
-		compadd -Q -- ${${=1}% } && _ret=0
-	}
-
-	__gitcomp_nl ()
-	{
-		emulate -L zsh
-
-		local IFS=$'\n'
-		compset -P '*[=:]'
-		compadd -Q -S "${4- }" -p "${2-}" -- ${=1} && _ret=0
-	}
-
-	__gitcomp_file_direct ()
-	{
-		emulate -L zsh
-
-		local IFS=$'\n'
-		compset -P '*[=:]'
-		compadd -f -- ${=1} && _ret=0
-	}
-
-	__gitcomp_file ()
-	{
-		emulate -L zsh
-
-		local IFS=$'\n'
-		compset -P '*[=:]'
-		compadd -p "${2-}" -f -- ${=1} && _ret=0
-	}
-
-	_git ()
-	{
-		local _ret=1 cur cword prev
-		cur=${words[CURRENT]}
-		prev=${words[CURRENT-1]}
-		let cword=CURRENT-1
-		emulate ksh -c __${service}_main
-		let _ret && _default && _ret=0
-		return _ret
-	}
-
-	compdef _git git gitk
+if [[ -n ${ZSH_VERSION-} && -z ${GIT_SOURCING_ZSH_COMPLETION-} ]]; then
+	echo "ERROR: this script is obsolete, please see git-completion.zsh" 1>&2
 	return
 fi
 
-- 
2.22.0


^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [PATCH 10/14] completion: zsh: trivial cleanups
  2019-06-21 22:30 [PATCH 00/14] completion: a bunch of updates Felipe Contreras
                   ` (8 preceding siblings ...)
  2019-06-21 22:31 ` [PATCH 09/14] completion: bash: remove zsh wrapper Felipe Contreras
@ 2019-06-21 22:31 ` Felipe Contreras
  2019-06-21 22:31 ` [PATCH 11/14] test: completion: tests for __gitcomp regression Felipe Contreras
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 58+ messages in thread
From: Felipe Contreras @ 2019-06-21 22:31 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, SZEDER Gábor,
	Nguyễn Thái Ngọc Duy, Felipe Contreras

We don't need to override IFS, zsh has a native way of splitting by new
lines: the expansion flag (f).

Also, we don't need to split files by ':' or '='; that's only for words.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.zsh | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/contrib/completion/git-completion.zsh b/contrib/completion/git-completion.zsh
index 7f614d5854..317f5bd80a 100644
--- a/contrib/completion/git-completion.zsh
+++ b/contrib/completion/git-completion.zsh
@@ -68,44 +68,38 @@ __gitcomp_direct ()
 {
 	emulate -L zsh
 
-	local IFS=$'\n'
 	compset -P '*[=:]'
-	compadd -Q -- ${${=1}% } && _ret=0
+	compadd -Q -- ${${(f)1}% } && _ret=0
 }
 
 __gitcomp_nl ()
 {
 	emulate -L zsh
 
-	local IFS=$'\n'
 	compset -P '*[=:]'
-	compadd -Q -S "${4- }" -p "${2-}" -- ${=1} && _ret=0
+	compadd -Q -S "${4- }" -p "${2-}" -- ${(f)1} && _ret=0
 }
 
 __gitcomp_nl_append ()
 {
 	emulate -L zsh
 
-	local IFS=$'\n'
-	compadd -Q -S "${4- }" -p "${2-}" -- ${=1} && _ret=0
+	compset -P '*[=:]'
+	compadd -Q -S "${4- }" -p "${2-}" -- ${(f)1} && _ret=0
 }
 
 __gitcomp_file_direct ()
 {
 	emulate -L zsh
 
-	local IFS=$'\n'
-	compset -P '*[=:]'
-	compadd -f -- ${=1} && _ret=0
+	compadd -f -- ${(f)1} && _ret=0
 }
 
 __gitcomp_file ()
 {
 	emulate -L zsh
 
-	local IFS=$'\n'
-	compset -P '*[=:]'
-	compadd -p "${2-}" -f -- ${=1} && _ret=0
+	compadd -f -p "${2-}" -- ${(f)1} && _ret=0
 }
 
 __git_zsh_bash_func ()
-- 
2.22.0


^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [PATCH 11/14] test: completion: tests for __gitcomp regression
  2019-06-21 22:30 [PATCH 00/14] completion: a bunch of updates Felipe Contreras
                   ` (9 preceding siblings ...)
  2019-06-21 22:31 ` [PATCH 10/14] completion: zsh: trivial cleanups Felipe Contreras
@ 2019-06-21 22:31 ` Felipe Contreras
  2019-07-03 17:38   ` Junio C Hamano
  2019-07-03 17:49   ` SZEDER Gábor
  2019-06-21 22:31 ` [PATCH 12/14] test: completion: use global config Felipe Contreras
                   ` (3 subsequent siblings)
  14 siblings, 2 replies; 58+ messages in thread
From: Felipe Contreras @ 2019-06-21 22:31 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, SZEDER Gábor,
	Nguyễn Thái Ngọc Duy, Felipe Contreras

There's a regression in the completion since the introduction of
__gitcomp.

Go to any directory that doesn't contain a git repository, like /tmp.
Then type the following:

  git checkout --<tab>

You will see nothing. That's because
`git checkout --git-completion-helper` fails when you run it outside a
git repository.

You might change to a directory that has a git repository, but it's too
late, because the empty options have been cached.

It's unclear how many commands are affected, but this patch attempts to
at least detect some already in the testing framework.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 t/t9902-completion.sh | 37 ++++++++++++++++++++++++++++---------
 1 file changed, 28 insertions(+), 9 deletions(-)

diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 43cf313a1c..7bef41eaf5 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -122,6 +122,15 @@ test_gitcomp_nl ()
 	test_cmp expected out
 }
 
+offgit ()
+{
+	GIT_CEILING_DIRECTORIES="$ROOT" &&
+	export GIT_CEILING_DIRECTORIES &&
+	test_when_finished "ROOT='$ROOT'; cd '$TRASH_DIRECTORY'; unset GIT_CEILING_DIRECTORIES" &&
+	ROOT="$ROOT"/non-repo &&
+	cd "$ROOT"
+}
+
 invalid_variable_name='${foo.bar}'
 
 actual="$TRASH_DIRECTORY/actual"
@@ -358,10 +367,8 @@ test_expect_success SYMLINKS '__git_find_repo_path - resulting path avoids symli
 '
 
 test_expect_success '__git_find_repo_path - not a git repository' '
+	offgit &&
 	(
-		cd non-repo &&
-		GIT_CEILING_DIRECTORIES="$ROOT" &&
-		export GIT_CEILING_DIRECTORIES &&
 		test_must_fail __git_find_repo_path &&
 		printf "$__git_repo_path" >"$actual"
 	) &&
@@ -1388,6 +1395,7 @@ test_expect_success '__git_pretty_aliases' '
 '
 
 test_expect_success 'basic' '
+	offgit &&
 	run_completion "git " &&
 	# built-in
 	grep -q "^add \$" out &&
@@ -1401,6 +1409,7 @@ test_expect_success 'basic' '
 '
 
 test_expect_success 'double dash "git" itself' '
+	offgit &&
 	test_completion "git --" <<-\EOF
 	--paginate Z
 	--no-pager Z
@@ -1419,7 +1428,8 @@ test_expect_success 'double dash "git" itself' '
 	EOF
 '
 
-test_expect_success 'double dash "git checkout"' '
+test_expect_failure 'double dash "git checkout"' '
+	offgit &&
 	test_completion "git checkout --" <<-\EOF
 	--quiet Z
 	--detach Z
@@ -1442,6 +1452,7 @@ test_expect_success 'double dash "git checkout"' '
 '
 
 test_expect_success 'general options' '
+	offgit &&
 	test_completion "git --ver" "--version " &&
 	test_completion "git --hel" "--help " &&
 	test_completion "git --exe" <<-\EOF &&
@@ -1460,6 +1471,7 @@ test_expect_success 'general options' '
 '
 
 test_expect_success 'general options plus command' '
+	offgit &&
 	test_completion "git --version check" "checkout " &&
 	test_completion "git --paginate check" "checkout " &&
 	test_completion "git --git-dir=foo check" "checkout " &&
@@ -1480,11 +1492,13 @@ test_expect_success 'general options plus command' '
 '
 
 test_expect_success 'git --help completion' '
+	offgit &&
 	test_completion "git --help ad" "add " &&
 	test_completion "git --help core" "core-tutorial "
 '
 
-test_expect_success 'completion.commands removes multiple commands' '
+test_expect_failure 'completion.commands removes multiple commands' '
+	offgit &&
 	test_config completion.commands "-cherry -mergetool" &&
 	git --list-cmds=list-mainporcelain,list-complete,config >out &&
 	! grep -E "^(cherry|mergetool)$" out
@@ -1547,9 +1561,10 @@ test_expect_success 'complete tree filename with metacharacters' '
 	EOF
 '
 
-test_expect_success PERL 'send-email' '
-	test_completion "git send-email --cov" "--cover-letter " &&
-	test_completion "git send-email ma" "master "
+test_expect_failure PERL 'send-email' '
+	test_completion "git send-email ma" "master " &&
+	offgit &&
+	test_completion "git send-email --cov" "--cover-letter "
 '
 
 test_expect_success 'complete files' '
@@ -1649,6 +1664,7 @@ test_expect_success 'completion used <cmd> completion for alias: !f() { : git <c
 '
 
 test_expect_success 'completion without explicit _git_xxx function' '
+	offgit &&
 	test_completion "git version --" <<-\EOF
 	--build-options Z
 	--no-build-options Z
@@ -1699,13 +1715,15 @@ do
 done
 
 test_expect_success 'sourcing the completion script clears cached commands' '
+	offgit &&
 	__git_compute_all_commands &&
 	verbose test -n "$__git_all_commands" &&
 	. "$GIT_BUILD_DIR/contrib/completion/git-completion.bash" &&
 	verbose test -z "$__git_all_commands"
 '
 
-test_expect_success 'sourcing the completion script clears cached merge strategies' '
+test_expect_failure 'sourcing the completion script clears cached merge strategies' '
+	offgit &&
 	GIT_TEST_GETTEXT_POISON= &&
 	__git_compute_merge_strategies &&
 	verbose test -n "$__git_merge_strategies" &&
@@ -1714,6 +1732,7 @@ test_expect_success 'sourcing the completion script clears cached merge strategi
 '
 
 test_expect_success 'sourcing the completion script clears cached --options' '
+	offgit &&
 	__gitcomp_builtin checkout &&
 	verbose test -n "$__gitcomp_builtin_checkout" &&
 	__gitcomp_builtin notes_edit &&
-- 
2.22.0


^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [PATCH 12/14] test: completion: use global config
  2019-06-21 22:30 [PATCH 00/14] completion: a bunch of updates Felipe Contreras
                   ` (10 preceding siblings ...)
  2019-06-21 22:31 ` [PATCH 11/14] test: completion: tests for __gitcomp regression Felipe Contreras
@ 2019-06-21 22:31 ` Felipe Contreras
  2019-07-03 17:22   ` Junio C Hamano
  2019-06-21 22:31 ` [PATCH 13/14] completion: add default options Felipe Contreras
                   ` (2 subsequent siblings)
  14 siblings, 1 reply; 58+ messages in thread
From: Felipe Contreras @ 2019-06-21 22:31 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, SZEDER Gábor,
	Nguyễn Thái Ngọc Duy, Felipe Contreras

When appropriate.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 t/t9902-completion.sh | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 7bef41eaf5..3dbfef6960 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1497,9 +1497,9 @@ test_expect_success 'git --help completion' '
 	test_completion "git --help core" "core-tutorial "
 '
 
-test_expect_failure 'completion.commands removes multiple commands' '
+test_expect_success 'completion.commands removes multiple commands' '
 	offgit &&
-	test_config completion.commands "-cherry -mergetool" &&
+	test_config_global completion.commands "-cherry -mergetool" &&
 	git --list-cmds=list-mainporcelain,list-complete,config >out &&
 	! grep -E "^(cherry|mergetool)$" out
 '
@@ -1637,7 +1637,7 @@ test_expect_success 'complete files' '
 '
 
 test_expect_success "completion uses <cmd> completion for alias: !sh -c 'git <cmd> ...'" '
-	test_config alias.co "!sh -c '"'"'git checkout ...'"'"'" &&
+	test_config_global alias.co "!sh -c '"'"'git checkout ...'"'"'" &&
 	test_completion "git co m" <<-\EOF
 	master Z
 	mybranch Z
@@ -1646,7 +1646,7 @@ test_expect_success "completion uses <cmd> completion for alias: !sh -c 'git <cm
 '
 
 test_expect_success 'completion uses <cmd> completion for alias: !f () { VAR=val git <cmd> ... }' '
-	test_config alias.co "!f () { VAR=val git checkout ... ; } f" &&
+	test_config_global alias.co "!f () { VAR=val git checkout ... ; } f" &&
 	test_completion "git co m" <<-\EOF
 	master Z
 	mybranch Z
@@ -1655,7 +1655,7 @@ test_expect_success 'completion uses <cmd> completion for alias: !f () { VAR=val
 '
 
 test_expect_success 'completion used <cmd> completion for alias: !f() { : git <cmd> ; ... }' '
-	test_config alias.co "!f() { : git checkout ; if ... } f" &&
+	test_config_global alias.co "!f() { : git checkout ; if ... } f" &&
 	test_completion "git co m" <<-\EOF
 	master Z
 	mybranch Z
-- 
2.22.0


^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [PATCH 13/14] completion: add default options
  2019-06-21 22:30 [PATCH 00/14] completion: a bunch of updates Felipe Contreras
                   ` (11 preceding siblings ...)
  2019-06-21 22:31 ` [PATCH 12/14] test: completion: use global config Felipe Contreras
@ 2019-06-21 22:31 ` Felipe Contreras
  2019-06-22  3:01   ` Duy Nguyen
  2019-06-21 22:31 ` [PATCH 14/14] completion: add default merge strategies Felipe Contreras
  2019-07-03 17:50 ` [PATCH 00/14] completion: a bunch of updates Junio C Hamano
  14 siblings, 1 reply; 58+ messages in thread
From: Felipe Contreras @ 2019-06-21 22:31 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, SZEDER Gábor,
	Nguyễn Thái Ngọc Duy, Felipe Contreras

Versions of Git older than v2.17 don't know about
--git-completion-helper, so provide some defaults for them.

Also, some commands fail if there's no Git repository available.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.bash | 97 +++++++++++++++++++++++++-
 t/t9902-completion.sh                  |  4 +-
 2 files changed, 98 insertions(+), 3 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index d3ee6c7dc2..922ba5f925 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -377,6 +377,100 @@ else
 	unset $(compgen -v __gitcomp_builtin_)
 fi
 
+__gitcomp_builtin_add_default=" --dry-run --verbose --interactive --patch --edit --force --update --renormalize --intent-to-add --all --ignore-removal --refresh --ignore-errors --ignore-missing --chmod= --no-dry-run -- --no-verbose --no-interactive --no-patch --no-edit --no-force --no-update --no-renormalize --no-intent-to-add --no-all --no-ignore-removal --no-refresh --no-ignore-errors --no-ignore-missing --no-chmod"
+__gitcomp_builtin_am_default=" --interactive --3way --quiet --signoff --utf8 --keep --keep-non-patch --message-id --keep-cr --no-keep-cr --scissors --whitespace= --ignore-space-change --ignore-whitespace --directory= --exclude= --include= --patch-format= --reject --resolvemsg= --continue --resolved --skip --abort --quit --show-current-patch --committer-date-is-author-date --ignore-date --rerere-autoupdate --gpg-sign -- --no-interactive --no-3way --no-quiet --no-signoff --no-utf8 --no-keep --no-keep-non-patch --no-message-id --no-scissors --no-whitespace --no-ignore-space-change --no-ignore-whitespace --no-directory --no-exclude --no-include --no-patch-format --no-reject --no-resolvemsg --no-committer-date-is-author-date --no-ignore-date --no-rerere-autoupdate --no-gpg-sign"
+__gitcomp_builtin_apply_default=" --exclude= --include= --no-add --stat --numstat --summary --check --index --intent-to-add --cached --apply --3way --build-fake-ancestor= --whitespace= --ignore-space-change --ignore-whitespace --reverse --unidiff-zero --reject --allow-overlap --verbose --inaccurate-eof --recount --directory= --add -- --no-stat --no-numstat --no-summary --no-check --no-index --no-intent-to-add --no-cached --no-apply --no-3way --no-build-fake-ancestor --no-whitespace --no-ignore-space-change --no-ignore-whitespace --no-reverse --no-unidiff-zero --no-reject --no-allow-overlap --no-verbose --no-inaccurate-eof --no-recount --no-directory"
+__gitcomp_builtin_archive_default=" --output= --remote= --exec= --no-output -- --no-remote --no-exec"
+__gitcomp_builtin_bisect__helper_default=" --next-all --write-terms --bisect-clean-state --check-expected-revs --bisect-reset --bisect-write --check-and-set-terms --bisect-next-check --bisect-terms --bisect-start --no-checkout --no-log --checkout --log"
+__gitcomp_builtin_blame_default=" --incremental --root --show-stats --progress --score-debug --show-name --show-number --porcelain --line-porcelain --show-email --color-lines --color-by-age --indent-heuristic --minimal --contents= --abbrev --no-incremental -- --no-root --no-show-stats --no-progress --no-score-debug --no-show-name --no-show-number --no-porcelain --no-line-porcelain --no-show-email --no-color-lines --no-color-by-age --no-minimal --no-contents --no-abbrev"
+__gitcomp_builtin_branch_default=" --verbose --quiet --track --set-upstream-to= --unset-upstream --color --remotes --contains --no-contains --abbrev --all --delete --move --copy --list --show-current --create-reflog --edit-description --merged --no-merged --column --sort= --points-at= --ignore-case --format= -- --no-verbose --no-quiet --no-track --no-set-upstream-to --no-unset-upstream --no-color --no-remotes --no-abbrev --no-all --no-delete --no-move --no-copy --no-list --no-show-current --no-create-reflog --no-edit-description --no-column --no-points-at --no-ignore-case --no-format"
+__gitcomp_builtin_cat_file_default=" --textconv --filters --path= --allow-unknown-type --buffer --batch --batch-check --follow-symlinks --batch-all-objects --unordered --no-path -- --no-allow-unknown-type --no-buffer --no-follow-symlinks --no-batch-all-objects --no-unordered"
+__gitcomp_builtin_check_attr_default=" --all --cached --stdin --no-all -- --no-cached --no-stdin"
+__gitcomp_builtin_check_ignore_default=" --quiet --verbose --stdin --non-matching --no-index --index -- --no-quiet --no-verbose --no-stdin --no-non-matching"
+__gitcomp_builtin_check_mailmap_default=" --stdin --no-stdin"
+__gitcomp_builtin_checkout_default=" --quiet --detach --track --orphan= --ours --theirs --merge --conflict= --patch --ignore-skip-worktree-bits --no-guess --ignore-other-worktrees --recurse-submodules --progress --overlay --guess -- --no-quiet --no-detach --no-track --no-orphan --no-merge --no-conflict --no-patch --no-ignore-skip-worktree-bits --no-ignore-other-worktrees --no-recurse-submodules --no-progress --no-overlay"
+__gitcomp_builtin_checkout_index_default=" --all --force --quiet --no-create --index --stdin --temp --prefix= --stage= --create -- --no-all --no-force --no-quiet --no-index --no-stdin --no-temp --no-prefix"
+__gitcomp_builtin_cherry_default=" --abbrev --verbose --no-abbrev -- --no-verbose"
+__gitcomp_builtin_cherry_pick_default=" --quit --continue --abort --cleanup= --no-commit --edit --signoff --mainline= --rerere-autoupdate --strategy= --strategy-option= --gpg-sign --ff --allow-empty --allow-empty-message --keep-redundant-commits --commit -- --no-cleanup --no-edit --no-signoff --no-mainline --no-rerere-autoupdate --no-strategy --no-strategy-option --no-gpg-sign --no-ff --no-allow-empty --no-allow-empty-message --no-keep-redundant-commits"
+__gitcomp_builtin_clean_default=" --quiet --dry-run --interactive --exclude= --no-quiet -- --no-dry-run --no-interactive"
+__gitcomp_builtin_clone_default=" --verbose --quiet --progress --no-checkout --bare --mirror --local --no-hardlinks --shared --recursive --recurse-submodules --jobs= --template= --reference= --reference-if-able= --dissociate --origin= --branch= --upload-pack= --depth= --shallow-since= --shallow-exclude= --single-branch --no-tags --shallow-submodules --separate-git-dir= --config= --server-option= --ipv4 --ipv6 --filter= --checkout --hardlinks --tags -- --no-verbose --no-quiet --no-progress --no-bare --no-mirror --no-local --no-shared --no-recursive --no-recurse-submodules --no-jobs --no-template --no-reference --no-reference-if-able --no-dissociate --no-origin --no-branch --no-upload-pack --no-depth --no-shallow-since --no-shallow-exclude --no-single-branch --no-shallow-submodules --no-separate-git-dir --no-config --no-server-option --no-ipv4 --no-ipv6 --no-filter"
+__gitcomp_builtin_column_default=" --command= --mode --raw-mode= --width= --indent= --nl= --padding= --no-command -- --no-mode --no-raw-mode --no-width --no-indent --no-nl --no-padding"
+__gitcomp_builtin_commit_default=" --quiet --verbose --file= --author= --date= --message= --reedit-message= --reuse-message= --fixup= --squash= --reset-author --signoff --template= --edit --cleanup= --status --gpg-sign --all --include --interactive --patch --only --no-verify --dry-run --short --branch --ahead-behind --porcelain --long --null --amend --no-post-rewrite --untracked-files --verify --post-rewrite -- --no-quiet --no-verbose --no-file --no-author --no-date --no-message --no-reedit-message --no-reuse-message --no-fixup --no-squash --no-reset-author --no-signoff --no-template --no-edit --no-cleanup --no-status --no-gpg-sign --no-all --no-include --no-interactive --no-patch --no-only --no-dry-run --no-short --no-branch --no-ahead-behind --no-porcelain --no-long --no-null --no-amend --no-untracked-files"
+__gitcomp_builtin_commit_graph_default=" --object-dir= --no-object-dir"
+__gitcomp_builtin_config_default=" --global --system --local --worktree --file= --blob= --get --get-all --get-regexp --get-urlmatch --replace-all --add --unset --unset-all --rename-section --remove-section --list --edit --get-color --get-colorbool --type= --bool --int --bool-or-int --path --expiry-date --null --name-only --includes --show-origin --default= --no-global -- --no-system --no-local --no-worktree --no-file --no-blob --no-get --no-get-all --no-get-regexp --no-get-urlmatch --no-replace-all --no-add --no-unset --no-unset-all --no-rename-section --no-remove-section --no-list --no-edit --no-get-color --no-get-colorbool --no-type --no-null --no-name-only --no-includes --no-show-origin --no-default"
+__gitcomp_builtin_count_objects_default=" --verbose --human-readable --no-verbose -- --no-human-readable"
+__gitcomp_builtin_describe_default=" --contains --debug --all --tags --long --first-parent --abbrev --exact-match --candidates= --match= --exclude= --always --dirty --broken --no-contains -- --no-debug --no-all --no-tags --no-long --no-first-parent --no-abbrev --no-exact-match --no-candidates --no-match --no-exclude --no-always --no-dirty --no-broken"
+__gitcomp_builtin_difftool_default=" --gui --dir-diff --no-prompt --symlinks --tool= --tool-help --trust-exit-code --extcmd= --no-index -- --no-gui --no-dir-diff --no-symlinks --no-tool --no-tool-help --no-trust-exit-code --no-extcmd"
+__gitcomp_builtin_fast_export_default=" --progress= --signed-tags= --tag-of-filtered-object= --export-marks= --import-marks= --fake-missing-tagger --full-tree --use-done-feature --no-data --refspec= --anonymize --reference-excluded-parents --show-original-ids --data -- --no-progress --no-signed-tags --no-tag-of-filtered-object --no-export-marks --no-import-marks --no-fake-missing-tagger --no-full-tree --no-use-done-feature --no-refspec --no-anonymize --no-reference-excluded-parents --no-show-original-ids"
+__gitcomp_builtin_fetch_default=" --verbose --quiet --all --append --upload-pack= --force --multiple --tags --jobs= --prune --prune-tags --recurse-submodules --dry-run --keep --update-head-ok --progress --depth= --shallow-since= --shallow-exclude= --deepen= --unshallow --update-shallow --refmap= --server-option= --ipv4 --ipv6 --negotiation-tip= --filter= --no-verbose -- --no-quiet --no-all --no-append --no-upload-pack --no-force --no-multiple --no-tags --no-jobs --no-prune --no-prune-tags --no-recurse-submodules --no-dry-run --no-keep --no-update-head-ok --no-progress --no-depth --no-shallow-since --no-shallow-exclude --no-deepen --no-update-shallow --no-server-option --no-ipv4 --no-ipv6 --no-negotiation-tip --no-filter"
+__gitcomp_builtin_fmt_merge_msg_default=" --log --message= --file= --no-log -- --no-message --no-file"
+__gitcomp_builtin_for_each_ref_default=" --shell --perl --python --tcl --count= --format= --color --sort= --points-at= --merged --no-merged --contains --no-contains --ignore-case -- --no-shell --no-perl --no-python --no-tcl --no-count --no-format --no-color --no-points-at --no-ignore-case"
+__gitcomp_builtin_format_patch_default=" --numbered --no-numbered --signoff --stdout --cover-letter --numbered-files --suffix= --start-number= --reroll-count= --rfc --subject-prefix= --output-directory= --keep-subject --no-binary --zero-commit --ignore-if-in-upstream --no-stat --add-header= --to= --cc= --from --in-reply-to= --attach --inline --thread --signature= --base= --signature-file= --quiet --progress --interdiff= --range-diff= --creation-factor= --binary -- --no-numbered --no-signoff --no-stdout --no-cover-letter --no-numbered-files --no-suffix --no-start-number --no-reroll-count --no-zero-commit --no-ignore-if-in-upstream --no-add-header --no-to --no-cc --no-from --no-in-reply-to --no-attach --no-thread --no-signature --no-base --no-signature-file --no-quiet --no-progress --no-interdiff --no-range-diff --no-creation-factor"
+__gitcomp_builtin_fsck_default=" --verbose --unreachable --dangling --tags --root --cache --reflogs --full --connectivity-only --strict --lost-found --progress --name-objects --no-verbose -- --no-unreachable --no-dangling --no-tags --no-root --no-cache --no-reflogs --no-full --no-connectivity-only --no-strict --no-lost-found --no-progress --no-name-objects"
+__gitcomp_builtin_fsck_objects_default=" --verbose --unreachable --dangling --tags --root --cache --reflogs --full --connectivity-only --strict --lost-found --progress --name-objects --no-verbose -- --no-unreachable --no-dangling --no-tags --no-root --no-cache --no-reflogs --no-full --no-connectivity-only --no-strict --no-lost-found --no-progress --no-name-objects"
+__gitcomp_builtin_gc_default=" --quiet --prune --aggressive --keep-largest-pack --no-quiet -- --no-prune --no-aggressive --no-keep-largest-pack"
+__gitcomp_builtin_grep_default=" --cached --no-index --untracked --exclude-standard --recurse-submodules --invert-match --ignore-case --word-regexp --text --textconv --recursive --max-depth= --extended-regexp --basic-regexp --fixed-strings --perl-regexp --line-number --column --full-name --files-with-matches --name-only --files-without-match --only-matching --count --color --break --heading --context= --before-context= --after-context= --threads= --show-function --function-context --and --or --not --quiet --all-match --index -- --no-cached --no-untracked --no-exclude-standard --no-recurse-submodules --no-invert-match --no-ignore-case --no-word-regexp --no-text --no-textconv --no-recursive --no-extended-regexp --no-basic-regexp --no-fixed-strings --no-perl-regexp --no-line-number --no-column --no-full-name --no-files-with-matches --no-name-only --no-files-without-match --no-only-matching --no-count --no-color --no-break --no-heading --no-context --no-before-context --no-after-context --no-threads --no-show-function --no-function-context --no-or --no-quiet --no-all-match"
+__gitcomp_builtin_hash_object_default=" --stdin --stdin-paths --no-filters --literally --path= --filters -- --no-stdin --no-stdin-paths --no-literally --no-path"
+__gitcomp_builtin_help_default=" --all --guides --config --man --web --info --verbose --no-all -- --no-guides --no-config --no-man --no-web --no-info --no-verbose"
+__gitcomp_builtin_init_default=" --template= --bare --shared --quiet --separate-git-dir= --no-template -- --no-bare --no-quiet --no-separate-git-dir"
+__gitcomp_builtin_init_db_default=" --template= --bare --shared --quiet --separate-git-dir= --no-template -- --no-bare --no-quiet --no-separate-git-dir"
+__gitcomp_builtin_interpret_trailers_default=" --in-place --trim-empty --where= --if-exists= --if-missing= --only-trailers --only-input --unfold --parse --no-divider --trailer= --divider -- --no-in-place --no-trim-empty --no-where --no-if-exists --no-if-missing --no-only-trailers --no-only-input --no-unfold --no-trailer"
+__gitcomp_builtin_log_default=" --quiet --source --use-mailmap --decorate-refs= --decorate-refs-exclude= --decorate --no-quiet -- --no-source --no-use-mailmap --no-decorate-refs --no-decorate-refs-exclude --no-decorate"
+__gitcomp_builtin_ls_files_default=" --cached --deleted --modified --others --ignored --stage --killed --directory --eol --empty-directory --unmerged --resolve-undo --exclude= --exclude-from= --exclude-per-directory= --exclude-standard --full-name --recurse-submodules --error-unmatch --with-tree= --abbrev --debug --no-cached -- --no-deleted --no-modified --no-others --no-ignored --no-stage --no-killed --no-directory --no-eol --no-empty-directory --no-unmerged --no-resolve-undo --no-exclude-per-directory --no-recurse-submodules --no-error-unmatch --no-with-tree --no-abbrev --no-debug"
+__gitcomp_builtin_ls_remote_default=" --quiet --upload-pack= --tags --heads --refs --get-url --sort= --symref --server-option= --no-quiet -- --no-upload-pack --no-tags --no-heads --no-refs --no-get-url --no-symref --no-server-option"
+__gitcomp_builtin_ls_tree_default=" --long --name-only --name-status --full-name --full-tree --abbrev --no-long -- --no-name-only --no-name-status --no-full-name --no-full-tree --no-abbrev"
+__gitcomp_builtin_merge_default=" --stat --summary --log --squash --commit --edit --cleanup= --ff --ff-only --rerere-autoupdate --verify-signatures --strategy= --strategy-option= --message= --file --verbose --quiet --abort --continue --allow-unrelated-histories --progress --gpg-sign --overwrite-ignore --signoff --verify --no-stat -- --no-summary --no-log --no-squash --no-commit --no-edit --no-cleanup --no-ff --no-rerere-autoupdate --no-verify-signatures --no-strategy --no-strategy-option --no-message --no-verbose --no-quiet --no-abort --no-continue --no-allow-unrelated-histories --no-progress --no-gpg-sign --no-overwrite-ignore --no-signoff --no-verify"
+__gitcomp_builtin_merge_base_default=" --all --octopus --independent --is-ancestor --fork-point --no-all"
+__gitcomp_builtin_merge_file_default=" --stdout --diff3 --ours --theirs --union --marker-size= --quiet --no-stdout -- --no-diff3 --no-ours --no-theirs --no-union --no-marker-size --no-quiet"
+__gitcomp_builtin_mktree_default=" --missing --batch --no-missing -- --no-batch"
+__gitcomp_builtin_multi_pack_index_default=" --object-dir= --no-object-dir"
+__gitcomp_builtin_mv_default=" --verbose --dry-run --no-verbose -- --no-dry-run"
+__gitcomp_builtin_name_rev_default=" --name-only --tags --refs= --exclude= --all --stdin --undefined --always --no-name-only -- --no-tags --no-refs --no-exclude --no-all --no-stdin --no-undefined --no-always"
+__gitcomp_builtin_notes_default=" --ref= --no-ref"
+__gitcomp_builtin_pack_objects_default=" --quiet --progress --all-progress --all-progress-implied --index-version= --max-pack-size= --local --incremental --window= --window-memory= --depth= --reuse-delta --reuse-object --delta-base-offset --threads= --non-empty --revs --unpacked --all --reflog --indexed-objects --stdout --include-tag --keep-unreachable --pack-loose-unreachable --unpack-unreachable --sparse --thin --shallow --honor-pack-keep --keep-pack= --compression= --keep-true-parents --use-bitmap-index --write-bitmap-index --filter= --missing= --exclude-promisor-objects --delta-islands --no-quiet -- --no-progress --no-all-progress --no-all-progress-implied --no-local --no-incremental --no-window --no-depth --no-reuse-delta --no-reuse-object --no-delta-base-offset --no-threads --no-non-empty --no-revs --no-stdout --no-include-tag --no-keep-unreachable --no-pack-loose-unreachable --no-unpack-unreachable --no-sparse --no-thin --no-shallow --no-honor-pack-keep --no-keep-pack --no-compression --no-keep-true-parents --no-use-bitmap-index --no-write-bitmap-index --no-filter --no-exclude-promisor-objects --no-delta-islands"
+__gitcomp_builtin_pack_refs_default=" --all --prune --no-all -- --no-prune"
+__gitcomp_builtin_pickaxe_default=" --incremental --root --show-stats --progress --score-debug --show-name --show-number --porcelain --line-porcelain --show-email --color-lines --color-by-age --indent-heuristic --minimal --contents= --abbrev --no-incremental -- --no-root --no-show-stats --no-progress --no-score-debug --no-show-name --no-show-number --no-porcelain --no-line-porcelain --no-show-email --no-color-lines --no-color-by-age --no-minimal --no-contents --no-abbrev"
+__gitcomp_builtin_prune_default=" --dry-run --verbose --progress --expire= --exclude-promisor-objects --no-dry-run -- --no-verbose --no-progress --no-expire --no-exclude-promisor-objects"
+__gitcomp_builtin_prune_packed_default=" --dry-run --quiet --no-dry-run -- --no-quiet"
+__gitcomp_builtin_pull_default=" --verbose --quiet --progress --recurse-submodules --rebase --stat --log --signoff --squash --commit --edit --cleanup= --ff --ff-only --verify-signatures --autostash --strategy= --strategy-option= --gpg-sign --allow-unrelated-histories --all --append --upload-pack= --force --tags --prune --jobs --dry-run --keep --depth= --unshallow --update-shallow --refmap= --ipv4 --ipv6 --no-verbose -- --no-quiet --no-progress --no-recurse-submodules --no-rebase --no-stat --no-log --no-signoff --no-squash --no-commit --no-edit --no-cleanup --no-ff --no-verify-signatures --no-autostash --no-strategy --no-strategy-option --no-gpg-sign --no-allow-unrelated-histories --no-all --no-append --no-upload-pack --no-force --no-tags --no-prune --no-jobs --no-dry-run --no-keep --no-depth --no-update-shallow --no-ipv4 --no-ipv6"
+__gitcomp_builtin_push_default=" --verbose --quiet --repo= --all --mirror --delete --tags --dry-run --porcelain --force --force-with-lease --recurse-submodules --receive-pack= --exec= --set-upstream --progress --prune --no-verify --follow-tags --signed --atomic --push-option= --ipv4 --ipv6 --verify -- --no-verbose --no-quiet --no-repo --no-all --no-mirror --no-delete --no-tags --no-dry-run --no-porcelain --no-force --no-force-with-lease --no-recurse-submodules --no-receive-pack --no-exec --no-set-upstream --no-progress --no-prune --no-follow-tags --no-signed --no-atomic --no-push-option --no-ipv4 --no-ipv6"
+__gitcomp_builtin_range_diff_default=" --creation-factor= --no-dual-color --patch --no-patch --unified --function-context --raw --patch-with-raw --patch-with-stat --numstat --shortstat --dirstat --cumulative --dirstat-by-file --check --summary --name-only --name-status --stat --stat-width= --stat-name-width= --stat-graph-width= --stat-count= --compact-summary --binary --full-index --color --ws-error-highlight= --abbrev --src-prefix= --dst-prefix= --line-prefix= --no-prefix --inter-hunk-context= --output-indicator-new= --output-indicator-old= --output-indicator-context= --break-rewrites --find-renames --irreversible-delete --find-copies --find-copies-harder --no-renames --rename-empty --follow --minimal --ignore-all-space --ignore-space-change --ignore-space-at-eol --ignore-cr-at-eol --ignore-blank-lines --indent-heuristic --patience --histogram --diff-algorithm= --anchored= --word-diff --word-diff-regex= --color-words --color-moved --color-moved-ws= --relative --text --exit-code --quiet --ext-diff --textconv --ignore-submodules --submodule --ita-invisible-in-index --ita-visible-in-index --pickaxe-all --pickaxe-regex --find-object= --diff-filter= --output= --dual-color -- --no-creation-factor --no-function-context --no-compact-summary --no-full-index --no-color --no-abbrev --no-find-copies-harder --no-rename-empty --no-follow --no-minimal --no-indent-heuristic --no-color-moved --no-color-moved-ws --no-text --no-exit-code --no-quiet --no-ext-diff --no-textconv"
+__gitcomp_builtin_read_tree_default=" --index-output= --empty --verbose --trivial --aggressive --reset --prefix= --exclude-per-directory= --dry-run --no-sparse-checkout --debug-unpack --recurse-submodules --quiet --sparse-checkout -- --no-empty --no-verbose --no-trivial --no-aggressive --no-reset --no-dry-run --no-debug-unpack --no-recurse-submodules --no-quiet"
+__gitcomp_builtin_rebase_default=" --onto= --no-verify --quiet --verbose --no-stat --signoff --ignore-whitespace --committer-date-is-author-date --ignore-date --whitespace= --force-rebase --no-ff --continue --skip --abort --quit --edit-todo --show-current-patch --merge --interactive --preserve-merges --rerere-autoupdate --keep-empty --autosquash --gpg-sign --autostash --exec= --allow-empty-message --rebase-merges --fork-point --strategy= --strategy-option= --root --reschedule-failed-exec --verify --stat --ff -- --no-onto --no-quiet --no-verbose --no-signoff --no-ignore-whitespace --no-committer-date-is-author-date --no-ignore-date --no-whitespace --no-force-rebase --no-preserve-merges --no-rerere-autoupdate --no-keep-empty --no-autosquash --no-gpg-sign --no-autostash --no-exec --no-allow-empty-message --no-rebase-merges --no-fork-point --no-strategy --no-strategy-option --no-root --no-reschedule-failed-exec"
+__gitcomp_builtin_rebase__interactive_default=" --ff --keep-empty --allow-empty-message --rebase-merges --rebase-cousins --autosquash --signoff --verbose --continue --skip --edit-todo --show-current-patch --shorten-ids --expand-ids --check-todo-list --rearrange-squash --add-exec-commands --onto= --restrict-revision= --squash-onto= --upstream= --head-name= --gpg-sign --strategy= --strategy-opts= --switch-to= --onto-name= --cmd= --rerere-autoupdate --reschedule-failed-exec --no-ff -- --no-keep-empty --no-allow-empty-message --no-rebase-merges --no-rebase-cousins --no-autosquash --no-signoff --no-verbose --no-head-name --no-gpg-sign --no-strategy --no-strategy-opts --no-switch-to --no-onto-name --no-cmd --no-rerere-autoupdate --no-reschedule-failed-exec"
+__gitcomp_builtin_receive_pack_default=" --quiet --no-quiet"
+__gitcomp_builtin_reflog_default=" --quiet --source --use-mailmap --decorate-refs= --decorate-refs-exclude= --decorate --no-quiet -- --no-source --no-use-mailmap --no-decorate-refs --no-decorate-refs-exclude --no-decorate"
+__gitcomp_builtin_remote_default=" --verbose --no-verbose"
+__gitcomp_builtin_repack_default=" --quiet --local --write-bitmap-index --delta-islands --unpack-unreachable= --keep-unreachable --window= --window-memory= --depth= --threads= --max-pack-size= --pack-kept-objects --keep-pack= --no-quiet -- --no-local --no-write-bitmap-index --no-delta-islands --no-unpack-unreachable --no-keep-unreachable --no-window --no-window-memory --no-depth --no-threads --no-max-pack-size --no-pack-kept-objects --no-keep-pack"
+__gitcomp_builtin_replace_default=" --list --delete --edit --graft --convert-graft-file --raw --format= --no-raw -- --no-format"
+__gitcomp_builtin_rerere_default=" --rerere-autoupdate --no-rerere-autoupdate"
+__gitcomp_builtin_reset_default=" --quiet --mixed --soft --hard --merge --keep --recurse-submodules --patch --intent-to-add --no-quiet -- --no-mixed --no-soft --no-hard --no-merge --no-keep --no-recurse-submodules --no-patch --no-intent-to-add"
+__gitcomp_builtin_revert_default=" --quit --continue --abort --cleanup= --no-commit --edit --signoff --mainline= --rerere-autoupdate --strategy= --strategy-option= --gpg-sign --commit -- --no-cleanup --no-edit --no-signoff --no-mainline --no-rerere-autoupdate --no-strategy --no-strategy-option --no-gpg-sign"
+__gitcomp_builtin_rm_default=" --dry-run --quiet --cached --ignore-unmatch --no-dry-run -- --no-quiet --no-cached --no-ignore-unmatch"
+__gitcomp_builtin_send_pack_default=" --verbose --quiet --receive-pack= --exec= --remote= --all --dry-run --mirror --force --signed --push-option= --progress --thin --atomic --stateless-rpc --stdin --helper-status --force-with-lease --no-verbose -- --no-quiet --no-receive-pack --no-exec --no-remote --no-all --no-dry-run --no-mirror --no-force --no-signed --no-push-option --no-progress --no-thin --no-atomic --no-stateless-rpc --no-stdin --no-helper-status --no-force-with-lease"
+__gitcomp_builtin_shortlog_default=" --committer --numbered --summary --email --no-committer -- --no-numbered --no-summary --no-email"
+__gitcomp_builtin_show_default=" --quiet --source --use-mailmap --decorate-refs= --decorate-refs-exclude= --decorate --no-quiet -- --no-source --no-use-mailmap --no-decorate-refs --no-decorate-refs-exclude --no-decorate"
+__gitcomp_builtin_show_branch_default=" --all --remotes --color --more --list --no-name --current --sha1-name --merge-base --independent --topo-order --topics --sparse --date-order --reflog --name -- --no-all --no-remotes --no-color --no-more --no-list --no-current --no-sha1-name --no-merge-base --no-independent --no-topo-order --no-topics --no-sparse --no-date-order"
+__gitcomp_builtin_show_index_default=""
+__gitcomp_builtin_show_ref_default=" --tags --heads --verify --head --dereference --hash --abbrev --quiet --exclude-existing --no-tags -- --no-heads --no-verify --no-head --no-dereference --no-hash --no-abbrev --no-quiet"
+__gitcomp_builtin_stage_default=" --dry-run --verbose --interactive --patch --edit --force --update --renormalize --intent-to-add --all --ignore-removal --refresh --ignore-errors --ignore-missing --chmod= --no-dry-run -- --no-verbose --no-interactive --no-patch --no-edit --no-force --no-update --no-renormalize --no-intent-to-add --no-all --no-ignore-removal --no-refresh --no-ignore-errors --no-ignore-missing --no-chmod"
+__gitcomp_builtin_stash_default=""
+__gitcomp_builtin_status_default=" --verbose --short --branch --show-stash --ahead-behind --porcelain --long --null --untracked-files --ignored --ignore-submodules --column --no-renames --find-renames --renames -- --no-verbose --no-short --no-branch --no-show-stash --no-ahead-behind --no-porcelain --no-long --no-null --no-untracked-files --no-ignored --no-ignore-submodules --no-column"
+__gitcomp_builtin_stripspace_default=" --strip-comments --comment-lines"
+__gitcomp_builtin_symbolic_ref_default=" --quiet --delete --short --no-quiet -- --no-delete --no-short"
+__gitcomp_builtin_tag_default=" --list --delete --verify --annotate --message= --file= --edit --sign --cleanup= --local-user= --force --create-reflog --column --contains --no-contains --merged --no-merged --sort= --points-at --format= --color --ignore-case -- --no-annotate --no-file --no-edit --no-sign --no-cleanup --no-local-user --no-force --no-create-reflog --no-column --no-points-at --no-format --no-color --no-ignore-case"
+__gitcomp_builtin_update_index_default=" --ignore-submodules --add --replace --remove --unmerged --refresh --really-refresh --cacheinfo --chmod= --assume-unchanged --no-assume-unchanged --skip-worktree --no-skip-worktree --info-only --force-remove --stdin --index-info --unresolve --again --ignore-missing --verbose --clear-resolve-undo --index-version= --split-index --untracked-cache --test-untracked-cache --force-untracked-cache --force-write-index --fsmonitor --fsmonitor-valid --no-fsmonitor-valid -- --no-ignore-submodules --no-add --no-replace --no-remove --no-unmerged --no-info-only --no-force-remove --no-ignore-missing --no-verbose --no-index-version --no-split-index --no-untracked-cache --no-test-untracked-cache --no-force-untracked-cache --no-force-write-index --no-fsmonitor"
+__gitcomp_builtin_update_ref_default=" --no-deref --stdin --create-reflog --deref -- --no-stdin --no-create-reflog"
+__gitcomp_builtin_update_server_info_default=" --force --no-force"
+__gitcomp_builtin_upload_pack_default=" --stateless-rpc --advertise-refs --strict --timeout= --no-stateless-rpc -- --no-advertise-refs --no-strict --no-timeout"
+__gitcomp_builtin_verify_commit_default=" --verbose --raw --no-verbose -- --no-raw"
+__gitcomp_builtin_verify_pack_default=" --verbose --stat-only --no-verbose -- --no-stat-only"
+__gitcomp_builtin_verify_tag_default=" --verbose --raw --format= --no-verbose -- --no-raw --no-format"
+__gitcomp_builtin_version_default=" --build-options --no-build-options"
+__gitcomp_builtin_whatchanged_default=" --quiet --source --use-mailmap --decorate-refs= --decorate-refs-exclude= --decorate --no-quiet -- --no-source --no-use-mailmap --no-decorate-refs --no-decorate-refs-exclude --no-decorate"
+__gitcomp_builtin_write_tree_default=" --missing-ok --prefix= --no-missing-ok -- --no-prefix"
+__gitcomp_builtin_send_email_default=" --numbered --no-numbered --signoff --stdout --cover-letter --numbered-files --suffix= --start-number= --reroll-count= --rfc --subject-prefix= --output-directory= --keep-subject --no-binary --zero-commit --ignore-if-in-upstream --no-stat --add-header= --to= --cc= --from --in-reply-to= --attach --inline --thread --signature= --base= --signature-file= --quiet --progress --interdiff= --range-diff= --creation-factor= --binary -- --no-numbered --no-signoff --no-stdout --no-cover-letter --no-numbered-files --no-suffix --no-start-number --no-reroll-count --no-zero-commit --no-ignore-if-in-upstream --no-add-header --no-to --no-cc --no-from --no-in-reply-to --no-attach --no-thread --no-signature --no-base --no-signature-file --no-quiet --no-progress --no-interdiff --no-range-diff --no-creation-factor"
+
 # This function is equivalent to
 #
 #    __gitcomp "$(git xxx --git-completion-helper) ..."
@@ -400,7 +494,8 @@ __gitcomp_builtin ()
 	if [ -z "$options" ]; then
 		# leading and trailing spaces are significant to make
 		# option removal work correctly.
-		options=" $incl $(__git ${cmd/_/ } --git-completion-helper) "
+		options=" $incl $(__git ${cmd/_/ } --git-completion-helper) " ||
+			eval "options=\" $incl \$${var}_default \""
 		for i in $excl; do
 			options="${options/ $i / }"
 		done
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 3dbfef6960..14598bfbec 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1428,7 +1428,7 @@ test_expect_success 'double dash "git" itself' '
 	EOF
 '
 
-test_expect_failure 'double dash "git checkout"' '
+test_expect_success 'double dash "git checkout"' '
 	offgit &&
 	test_completion "git checkout --" <<-\EOF
 	--quiet Z
@@ -1561,7 +1561,7 @@ test_expect_success 'complete tree filename with metacharacters' '
 	EOF
 '
 
-test_expect_failure PERL 'send-email' '
+test_expect_success PERL 'send-email' '
 	test_completion "git send-email ma" "master " &&
 	offgit &&
 	test_completion "git send-email --cov" "--cover-letter "
-- 
2.22.0


^ permalink raw reply related	[flat|nested] 58+ messages in thread

* [PATCH 14/14] completion: add default merge strategies
  2019-06-21 22:30 [PATCH 00/14] completion: a bunch of updates Felipe Contreras
                   ` (12 preceding siblings ...)
  2019-06-21 22:31 ` [PATCH 13/14] completion: add default options Felipe Contreras
@ 2019-06-21 22:31 ` Felipe Contreras
  2019-06-24 17:23   ` Junio C Hamano
  2019-07-03 17:50 ` [PATCH 00/14] completion: a bunch of updates Junio C Hamano
  14 siblings, 1 reply; 58+ messages in thread
From: Felipe Contreras @ 2019-06-21 22:31 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, SZEDER Gábor,
	Nguyễn Thái Ngọc Duy, Felipe Contreras

In case the command fails.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.bash | 4 +++-
 t/t9902-completion.sh                  | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 922ba5f925..91b87eb558 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -936,6 +936,7 @@ __git_list_merge_strategies ()
 	}'
 }
 
+__git_merge_strategies_default='octopus ours recursive resolve subtree'
 __git_merge_strategies=
 # 'git merge -s help' (and thus detection of the merge strategy
 # list) fails, unfortunately, if run outside of any git working
@@ -945,7 +946,8 @@ __git_merge_strategies=
 __git_compute_merge_strategies ()
 {
 	test -n "$__git_merge_strategies" ||
-	__git_merge_strategies=$(__git_list_merge_strategies)
+	{ __git_merge_strategies=$(__git_list_merge_strategies);
+		__git_merge_strategies="${__git_merge_strategies:-__git_merge_strategies_default}"; }
 }
 
 __git_merge_strategy_options="ours theirs subtree subtree= patience
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 14598bfbec..f4453ce70d 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1722,7 +1722,7 @@ test_expect_success 'sourcing the completion script clears cached commands' '
 	verbose test -z "$__git_all_commands"
 '
 
-test_expect_failure 'sourcing the completion script clears cached merge strategies' '
+test_expect_success 'sourcing the completion script clears cached merge strategies' '
 	offgit &&
 	GIT_TEST_GETTEXT_POISON= &&
 	__git_compute_merge_strategies &&
-- 
2.22.0


^ permalink raw reply related	[flat|nested] 58+ messages in thread

* Re: [PATCH 13/14] completion: add default options
  2019-06-21 22:31 ` [PATCH 13/14] completion: add default options Felipe Contreras
@ 2019-06-22  3:01   ` Duy Nguyen
  2019-06-22  4:36     ` Felipe Contreras
  2019-06-24 17:22     ` Junio C Hamano
  0 siblings, 2 replies; 58+ messages in thread
From: Duy Nguyen @ 2019-06-22  3:01 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Git Mailing List, Junio C Hamano, SZEDER Gábor

On Sat, Jun 22, 2019 at 5:31 AM Felipe Contreras
<felipe.contreras@gmail.com> wrote:
>
> Versions of Git older than v2.17 don't know about
> --git-completion-helper, so provide some defaults for them.
>
> Also, some commands fail if there's no Git repository available.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  contrib/completion/git-completion.bash | 97 +++++++++++++++++++++++++-
>  t/t9902-completion.sh                  |  4 +-
>  2 files changed, 98 insertions(+), 3 deletions(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index d3ee6c7dc2..922ba5f925 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -377,6 +377,100 @@ else
>         unset $(compgen -v __gitcomp_builtin_)
>  fi
>
> +__gitcomp_builtin_add_default=" --dry-run --verbose --interactive --patch --edit --force --update --renormalize --intent-to-add --all --ignore-
removal --refresh --ignore-errors --ignore-missing --chmod=
--no-dry-run -- --no-verbose --no-interactive --no-patch --no-edit
--no-force --no-update --no-renormalize --no-intent-to-add --no-all
--no-ignore-removal --no-refresh --no-ignore-errors
--no-ignore-missing --no-chmod"

And who's going to keep these uptodate? If you do this, might as well
delete --git-completion-helper

A more acceptable option might be regenerate git-completion.bash and
run --git-completion-helper to generate these, or make
git-completion.bash source a generated file. But that might need some
more build infrastructure, and people who just one to copy the file
might not like it.
-- 
Duy

^ permalink raw reply	[flat|nested] 58+ messages in thread

* Re: [PATCH 13/14] completion: add default options
  2019-06-22  3:01   ` Duy Nguyen
@ 2019-06-22  4:36     ` Felipe Contreras
  2019-06-24 17:22     ` Junio C Hamano
  1 sibling, 0 replies; 58+ messages in thread
From: Felipe Contreras @ 2019-06-22  4:36 UTC (permalink / raw)
  To: Duy Nguyen; +Cc: Git Mailing List, Junio C Hamano, SZEDER Gábor

On Fri, Jun 21, 2019 at 10:02 PM Duy Nguyen <pclouds@gmail.com> wrote:
>
> On Sat, Jun 22, 2019 at 5:31 AM Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
> >
> > Versions of Git older than v2.17 don't know about
> > --git-completion-helper, so provide some defaults for them.
> >
> > Also, some commands fail if there's no Git repository available.
> >
> > Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> > ---
> >  contrib/completion/git-completion.bash | 97 +++++++++++++++++++++++++-
> >  t/t9902-completion.sh                  |  4 +-
> >  2 files changed, 98 insertions(+), 3 deletions(-)
> >
> > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> > index d3ee6c7dc2..922ba5f925 100644
> > --- a/contrib/completion/git-completion.bash
> > +++ b/contrib/completion/git-completion.bash
> > @@ -377,6 +377,100 @@ else
> >         unset $(compgen -v __gitcomp_builtin_)
> >  fi
> >
> > +__gitcomp_builtin_add_default=" --dry-run --verbose --interactive --patch --edit --force --update --renormalize --intent-to-add --all --ignore-
> removal --refresh --ignore-errors --ignore-missing --chmod=
> --no-dry-run -- --no-verbose --no-interactive --no-patch --no-edit
> --no-force --no-update --no-renormalize --no-intent-to-add --no-all
> --no-ignore-removal --no-refresh --no-ignore-errors
> --no-ignore-missing --no-chmod"
>
> And who's going to keep these uptodate?

The same people that kept them up-to-date before git-completion-helper.

> If you do this, might as well delete --git-completion-helper

They serve two different purposes. Say you install the completion of
Git v2.22, but a while later you have Git v2.25; you will get the
updated commands thanks to git-completion-helper, and all the
__gitcomp_builtin_*_default will be ignored.

Granted; that's not the typical situation, as many people get the Git
completion through their distribution in tandem with their Git
version. But remember that these completion scripts are part of
contrib; they are not part of official Git (`make install` doesn't
install them).

When a) most people have a version of git that has
git-completion-helper, and b) most of the issues running commands
outside of a Git repo are resolved, they could be removed. But right
now they do serve a purpose.

> A more acceptable option might be regenerate git-completion.bash and
> run --git-completion-helper to generate these, or make
> git-completion.bash source a generated file. But that might need some
> more build infrastructure, and people who just one to copy the file
> might not like it.

Indeed, I wrote a script to generate these, but I manually copied
them. I could write a script that automatically generates this file if
it's agreed that this is indeed the way we want to go.

But even if these were not up-to-date--as historically has been the
case for most options--and a) you are running a version of Git that
doesn't have git-completion-helper, or b) you run a command that
requires a Git repo; it's better to get outdated options than to get
*nothing*, which is what we get now.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 58+ messages in thread

* Re: [PATCH 01/14] completion: zsh: fix __gitcomp_direct()
  2019-06-21 22:30 ` [PATCH 01/14] completion: zsh: fix __gitcomp_direct() Felipe Contreras
@ 2019-06-22 15:03   ` Felipe Contreras
  0 siblings, 0 replies; 58+ messages in thread
From: Felipe Contreras @ 2019-06-22 15:03 UTC (permalink / raw)
  To: Git
  Cc: Junio C Hamano, SZEDER Gábor,
	Nguyễn Thái Ngọc Duy

On Fri, Jun 21, 2019 at 5:31 PM Felipe Contreras
<felipe.contreras@gmail.com> wrote:
>
> Many callers append a space suffix, but zsh automatically appends a
> space, making the completion add two spaces, for example:

> --- a/contrib/completion/git-completion.zsh
> +++ b/contrib/completion/git-completion.zsh
> @@ -73,7 +73,7 @@ __gitcomp_direct ()
>
>         local IFS=$'\n'
>         compset -P '*[=:]'
> -       compadd -Q -- ${=1} && _ret=0
> +       compadd -Q -- ${${=1}% } && _ret=0

This is better actually:

compadd -Q -S '' -- ${=1} && _ret=0

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 58+ messages in thread

* Re: [PATCH 13/14] completion: add default options
  2019-06-22  3:01   ` Duy Nguyen
  2019-06-22  4:36     ` Felipe Contreras
@ 2019-06-24 17:22     ` Junio C Hamano
  2019-06-25  1:38       ` Felipe Contreras
  1 sibling, 1 reply; 58+ messages in thread
From: Junio C Hamano @ 2019-06-24 17:22 UTC (permalink / raw)
  To: Duy Nguyen; +Cc: Felipe Contreras, Git Mailing List, SZEDER Gábor

Duy Nguyen <pclouds@gmail.com> writes:

> On Sat, Jun 22, 2019 at 5:31 AM Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>>
>> Versions of Git older than v2.17 don't know about
>> --git-completion-helper, so provide some defaults for them.
> ...
>> +__gitcomp_builtin_add_default=" --dry-run --verbose --interactive --patch --edit --force --update --renormalize --intent-to-add --all --ignore-
> removal --refresh --ignore-errors --ignore-missing --chmod=
> --no-dry-run -- --no-verbose --no-interactive --no-patch --no-edit
> --no-force --no-update --no-renormalize --no-intent-to-add --no-all
> --no-ignore-removal --no-refresh --no-ignore-errors
> --no-ignore-missing --no-chmod"
>
> And who's going to keep these uptodate? If you do this, might as well
> delete --git-completion-helper
>
> A more acceptable option might be regenerate git-completion.bash and
> run --git-completion-helper to generate these, or make
> git-completion.bash source a generated file.

Nicely analysed and summarized.  What kind of target audience are we
talking about?  What's the payoff vs cost comparison trying to
catering to those who install more recent completion script that
requires the --git-completion-helper option without using antient
Git?

If the cutoff boundary is 2.17, that is more than year ago, and the
boundary gets further and further in the past as time goes by. Also,
depending on how old the version of Git the target user runs, these
hardcoded and manually listed options may not yet even exist in
their binary.

^ permalink raw reply	[flat|nested] 58+ messages in thread

* Re: [PATCH 14/14] completion: add default merge strategies
  2019-06-21 22:31 ` [PATCH 14/14] completion: add default merge strategies Felipe Contreras
@ 2019-06-24 17:23   ` Junio C Hamano
  2019-06-25  1:11     ` Felipe Contreras
  0 siblings, 1 reply; 58+ messages in thread
From: Junio C Hamano @ 2019-06-24 17:23 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: git, SZEDER Gábor, Nguyễn Thái Ngọc Duy

Felipe Contreras <felipe.contreras@gmail.com> writes:

> In case the command fails.

It is unclear what you wanted to say with this.  What command?
After "git merge" fails?

>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  contrib/completion/git-completion.bash | 4 +++-
>  t/t9902-completion.sh                  | 2 +-
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 922ba5f925..91b87eb558 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -936,6 +936,7 @@ __git_list_merge_strategies ()
>  	}'
>  }
>  
> +__git_merge_strategies_default='octopus ours recursive resolve subtree'
>  __git_merge_strategies=
>  # 'git merge -s help' (and thus detection of the merge strategy
>  # list) fails, unfortunately, if run outside of any git working
> @@ -945,7 +946,8 @@ __git_merge_strategies=
>  __git_compute_merge_strategies ()
>  {
>  	test -n "$__git_merge_strategies" ||
> -	__git_merge_strategies=$(__git_list_merge_strategies)
> +	{ __git_merge_strategies=$(__git_list_merge_strategies);
> +		__git_merge_strategies="${__git_merge_strategies:-__git_merge_strategies_default}"; }
>  }
>  
>  __git_merge_strategy_options="ours theirs subtree subtree= patience
> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
> index 14598bfbec..f4453ce70d 100755
> --- a/t/t9902-completion.sh
> +++ b/t/t9902-completion.sh
> @@ -1722,7 +1722,7 @@ test_expect_success 'sourcing the completion script clears cached commands' '
>  	verbose test -z "$__git_all_commands"
>  '
>  
> -test_expect_failure 'sourcing the completion script clears cached merge strategies' '
> +test_expect_success 'sourcing the completion script clears cached merge strategies' '
>  	offgit &&
>  	GIT_TEST_GETTEXT_POISON= &&
>  	__git_compute_merge_strategies &&

^ permalink raw reply	[flat|nested] 58+ messages in thread

* Re: [PATCH 14/14] completion: add default merge strategies
  2019-06-24 17:23   ` Junio C Hamano
@ 2019-06-25  1:11     ` Felipe Contreras
  2019-06-25  1:43       ` Junio C Hamano
  2019-07-03 17:14       ` SZEDER Gábor
  0 siblings, 2 replies; 58+ messages in thread
From: Felipe Contreras @ 2019-06-25  1:11 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Git, SZEDER Gábor, Nguyễn Thái Ngọc Duy

On Mon, Jun 24, 2019 at 12:24 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
> > In case the command fails.
>
> It is unclear what you wanted to say with this.  What command?
> After "git merge" fails?

Yes. The command that __git_list_merge_strategies() uses.

 % cd /tmp
 % git merge -s help
fatal: not a git repository (or any parent up to mount point /)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 58+ messages in thread

* Re: [PATCH 13/14] completion: add default options
  2019-06-24 17:22     ` Junio C Hamano
@ 2019-06-25  1:38       ` Felipe Contreras
  2019-06-25  3:32         ` Duy Nguyen
  0 siblings, 1 reply; 58+ messages in thread
From: Felipe Contreras @ 2019-06-25  1:38 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Duy Nguyen, Git Mailing List, SZEDER Gábor

On Mon, Jun 24, 2019 at 12:22 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Duy Nguyen <pclouds@gmail.com> writes:
>
> > On Sat, Jun 22, 2019 at 5:31 AM Felipe Contreras
> > <felipe.contreras@gmail.com> wrote:
> >>
> >> Versions of Git older than v2.17 don't know about
> >> --git-completion-helper, so provide some defaults for them.
> > ...
> >> +__gitcomp_builtin_add_default=" --dry-run --verbose --interactive --patch --edit --force --update --renormalize --intent-to-add --all --ignore-
> > removal --refresh --ignore-errors --ignore-missing --chmod=
> > --no-dry-run -- --no-verbose --no-interactive --no-patch --no-edit
> > --no-force --no-update --no-renormalize --no-intent-to-add --no-all
> > --no-ignore-removal --no-refresh --no-ignore-errors
> > --no-ignore-missing --no-chmod"
> >
> > And who's going to keep these uptodate? If you do this, might as well
> > delete --git-completion-helper
> >
> > A more acceptable option might be regenerate git-completion.bash and
> > run --git-completion-helper to generate these, or make
> > git-completion.bash source a generated file.
>
> Nicely analysed and summarized.  What kind of target audience are we
> talking about?

The people that install their completion independently of their
distribution. A quick search in Stack Overflow shows hundreds of
questions, many related to Homebrew and Cygwin.

> What's the payoff vs cost comparison trying to
> catering to those who install more recent completion script that
> requires the --git-completion-helper option without using antient
> Git?

You use the adjective "ancient", but is it really? The current Ubuntu
LTS release uses
2.17.1, the previous one (supported until 2021) uses 2.7.4, the
current Debian stable uses 2.11.0, and the previous RHEL uses 2.3.5.

Travis CI runs 2.15.1 by default.

If you are going to call these "ancient" what would you call the
current version in Debian oldstable? 2.1.4.

Not everyone is a privileged as us.

> If the cutoff boundary is 2.17, that is more than year ago, and the
> boundary gets further and further in the past as time goes by. Also,
> depending on how old the version of Git the target user runs, these
> hardcoded and manually listed options may not yet even exist in
> their binary.

Indeed, the need for these defaults will diminish over time, but
*right now* people are running versions of Git older than 2.17, for
sure.

And yes, it's possible that the completion will return an option that
doesn't exist yet in the user's version of Git, but historically that
has always been the case regarding Git completions (at least until
git-completion-helper).

So what would you rather have? a) return potentially non-existent
completions, or b) don't complete anything?

Another idea I had is to have a separate 'git completion-helper'
command that could act as a proxy for `git cmd
--git-completion-helper` and `git --list-cmds`. The completion would
throw a warning if such command is missing, then, the person that
installed the completion would have to find a suitable `git
completion-helper` command. We could provide an "example" script that
contains all these defaults. People could modify this to generate the
correct options for different Git versions. Realistically though, most
people will just use the defaults for the latest version, but at least
the responsibility is not on your side.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 58+ messages in thread

* Re: [PATCH 14/14] completion: add default merge strategies
  2019-06-25  1:11     ` Felipe Contreras
@ 2019-06-25  1:43       ` Junio C Hamano
  2019-07-03 17:14       ` SZEDER Gábor
  1 sibling, 0 replies; 58+ messages in thread
From: Junio C Hamano @ 2019-06-25  1:43 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Git, SZEDER Gábor, Nguyễn Thái Ngọc Duy

Felipe Contreras <felipe.contreras@gmail.com> writes:

> On Mon, Jun 24, 2019 at 12:24 PM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>
>> > In case the command fails.
>>
>> It is unclear what you wanted to say with this.  What command?
>> After "git merge" fails?
>
> Yes. The command that __git_list_merge_strategies() uses.

Next round, write that in the proposed log message, please.  An
issue in the proposed commit log message that puzzles reviewers is
something we expect future readers of "git log" to also stumble on.

>  % cd /tmp
>  % git merge -s help
> fatal: not a git repository (or any parent up to mount point /)
> Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).

I think the recent <20190612085606.12144-1-pclouds@gmail.com>
established a good pattern we should follow; when a command we run
to get list of things to use in completion fails, we refrain from
caching that broken output, and arrange so that we will try again.
It looks to me that "git merge -s help" barfing outside a repository
is a good candidate to follow that pattern.  Outside a repository,
the user will not be able to perform a merge with any strategy, so
not completing the command line when the user say "git merge -s
<TAB>" outside a repository is not the end of the world, as long as
we follow the right codepath to grab the available strategies once
the user goes into a repository where "git merge -s help" works, no?

Thanks.

^ permalink raw reply	[flat|nested] 58+ messages in thread

* Re: [PATCH 13/14] completion: add default options
  2019-06-25  1:38       ` Felipe Contreras
@ 2019-06-25  3:32         ` Duy Nguyen
  0 siblings, 0 replies; 58+ messages in thread
From: Duy Nguyen @ 2019-06-25  3:32 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Junio C Hamano, Git Mailing List, SZEDER Gábor

On Tue, Jun 25, 2019 at 8:38 AM Felipe Contreras
<felipe.contreras@gmail.com> wrote:
>
> On Mon, Jun 24, 2019 at 12:22 PM Junio C Hamano <gitster@pobox.com> wrote:
> >
> > Duy Nguyen <pclouds@gmail.com> writes:
> >
> > > On Sat, Jun 22, 2019 at 5:31 AM Felipe Contreras
> > > <felipe.contreras@gmail.com> wrote:
> > >>
> > >> Versions of Git older than v2.17 don't know about
> > >> --git-completion-helper, so provide some defaults for them.
> > > ...
> > >> +__gitcomp_builtin_add_default=" --dry-run --verbose --interactive --patch --edit --force --update --renormalize --intent-to-add --all --ignore-
> > > removal --refresh --ignore-errors --ignore-missing --chmod=
> > > --no-dry-run -- --no-verbose --no-interactive --no-patch --no-edit
> > > --no-force --no-update --no-renormalize --no-intent-to-add --no-all
> > > --no-ignore-removal --no-refresh --no-ignore-errors
> > > --no-ignore-missing --no-chmod"
> > >
> > > And who's going to keep these uptodate? If you do this, might as well
> > > delete --git-completion-helper
> > >
> > > A more acceptable option might be regenerate git-completion.bash and
> > > run --git-completion-helper to generate these, or make
> > > git-completion.bash source a generated file.
> >
> > Nicely analysed and summarized.  What kind of target audience are we
> > talking about?
>
> The people that install their completion independently of their
> distribution. A quick search in Stack Overflow shows hundreds of
> questions, many related to Homebrew and Cygwin.

Which could be answered with installing the right completion version.
I don't think we make any promise of supporting "old" versions anyway
even if used to work.

I could see we add support to source/preload some generated shell
script, so that it works without --git-completion-helper [1]. But
that's about it, the generated scripts that contain all these
__gitcomp_ variables can be packaged and maintained separately. Then
you could even have multiple completion packages for different git
versions if you want. But I'd rather we (git.git devs) do not maintain
these generated variables by ourselves.

[1] which may even gain interest from Windows crowd because there are
fewer processes to run.
-- 
Duy

^ permalink raw reply	[flat|nested] 58+ messages in thread

* Re: [PATCH 14/14] completion: add default merge strategies
  2019-06-25  1:11     ` Felipe Contreras
  2019-06-25  1:43       ` Junio C Hamano
@ 2019-07-03 17:14       ` SZEDER Gábor
  1 sibling, 0 replies; 58+ messages in thread
From: SZEDER Gábor @ 2019-07-03 17:14 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Junio C Hamano, Git, Nguyễn Thái Ngọc Duy

On Mon, Jun 24, 2019 at 08:11:40PM -0500, Felipe Contreras wrote:
> On Mon, Jun 24, 2019 at 12:24 PM Junio C Hamano <gitster@pobox.com> wrote:
> >
> > Felipe Contreras <felipe.contreras@gmail.com> writes:
> >
> > > In case the command fails.
> >
> > It is unclear what you wanted to say with this.  What command?
> > After "git merge" fails?
> 
> Yes. The command that __git_list_merge_strategies() uses.
> 
>  % cd /tmp
>  % git merge -s help
> fatal: not a git repository (or any parent up to mount point /)
> Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).

If that command behind __git_list_merge_strategies() fails, then 'git
merge -s <TAB>' won't simply list any merge strategies.  However,
that's not a big deal, because the command won't work without a
repository anyway, so I don't see the point in adding a hard-coded
list of merge strategies.  And in this case $__git_merge_strategies
will remain empty, so the next time the user attempts to complete a
strategies while in a repository, then it will Just Work (unlike the
undesired caching of options without a repository that is fixed in
69702523af (completion: do not cache if --git-completion-helper fails,
2019-06-12).


^ permalink raw reply	[flat|nested] 58+ messages in thread

* Re: [PATCH 12/14] test: completion: use global config
  2019-06-21 22:31 ` [PATCH 12/14] test: completion: use global config Felipe Contreras
@ 2019-07-03 17:22   ` Junio C Hamano
  0 siblings, 0 replies; 58+ messages in thread
From: Junio C Hamano @ 2019-07-03 17:22 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: git, SZEDER Gábor, Nguyễn Thái Ngọc Duy

Felipe Contreras <felipe.contreras@gmail.com> writes:

> When appropriate.

It is unclear what makes these (but not other use of test_config)
appropriate.

>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  t/t9902-completion.sh | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
> index 7bef41eaf5..3dbfef6960 100755
> --- a/t/t9902-completion.sh
> +++ b/t/t9902-completion.sh
> @@ -1497,9 +1497,9 @@ test_expect_success 'git --help completion' '
>  	test_completion "git --help core" "core-tutorial "
>  '
>  
> -test_expect_failure 'completion.commands removes multiple commands' '
> +test_expect_success 'completion.commands removes multiple commands' '
>  	offgit &&
> -	test_config completion.commands "-cherry -mergetool" &&
> +	test_config_global completion.commands "-cherry -mergetool" &&

This feels more like fixing a bug introduced by step 11/14 in that
(besides doing "offgit" that affects global test environment outside
a subshell) we want to do this test outside a repository so there is
no appropriate "local" configuration "git config" (hence test_config)
can touch.  IOW, shouldn't this have been done in the step 11/14 when
"offgit" was added?

^ permalink raw reply	[flat|nested] 58+ messages in thread

* Re: [PATCH 11/14] test: completion: tests for __gitcomp regression
  2019-06-21 22:31 ` [PATCH 11/14] test: completion: tests for __gitcomp regression Felipe Contreras
@ 2019-07-03 17:38   ` Junio C Hamano
  2019-07-03 17:49   ` SZEDER Gábor
  1 sibling, 0 replies; 58+ messages in thread
From: Junio C Hamano @ 2019-07-03 17:38 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: git, SZEDER Gábor, Nguyễn Thái Ngọc Duy

Felipe Contreras <felipe.contreras@gmail.com> writes:

> +offgit ()
> +{

Style: opening brace comes on the same line, like

	offgit () {


> +	GIT_CEILING_DIRECTORIES="$ROOT" &&
> +	export GIT_CEILING_DIRECTORIES &&
> +	test_when_finished "ROOT='$ROOT'; cd '$TRASH_DIRECTORY'; unset GIT_CEILING_DIRECTORIES" &&
> +	ROOT="$ROOT"/non-repo &&
> +	cd "$ROOT"
> +}

All of these means that anytime some test uses offgit outside a
subshell, all the subsequent test will start from outside a
repository, with nonstandard GIT_CEILING_DIRECTORIES settings.

The test should avoid using this outside a subshell when able (and
if it apparently cannot easily, we should try to find a way).

> @@ -358,10 +367,8 @@ test_expect_success SYMLINKS '__git_find_repo_path - resulting path avoids symli
>  '
>  
>  test_expect_success '__git_find_repo_path - not a git repository' '
> +	offgit &&
>  	(
> -		cd non-repo &&
> -		GIT_CEILING_DIRECTORIES="$ROOT" &&
> -		export GIT_CEILING_DIRECTORIES &&
>  		test_must_fail __git_find_repo_path &&
>  		printf "$__git_repo_path" >"$actual"
>  	) &&
> @@ -1388,6 +1395,7 @@ test_expect_success '__git_pretty_aliases' '
>  '
>  
>  test_expect_success 'basic' '
> +	offgit &&
>  	run_completion "git " &&

Adding "offgit" everywhere like this patch does means that this
"basic" test, for example, no longer is performed in the condition
we have been testing the completion script for, doesn't it?  If so,
the patch is trading test coverage outside repo with coverage inside
repo, which is not a very good tradeoff.



^ permalink raw reply	[flat|nested] 58+ messages in thread

* Re: [PATCH 11/14] test: completion: tests for __gitcomp regression
  2019-06-21 22:31 ` [PATCH 11/14] test: completion: tests for __gitcomp regression Felipe Contreras
  2019-07-03 17:38   ` Junio C Hamano
@ 2019-07-03 17:49   ` SZEDER Gábor
  1 sibling, 0 replies; 58+ messages in thread
From: SZEDER Gábor @ 2019-07-03 17:49 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: git, Junio C Hamano, Nguyễn Thái Ngọc Duy

On Fri, Jun 21, 2019 at 05:31:04PM -0500, Felipe Contreras wrote:
> There's a regression in the completion since the introduction of
> __gitcomp.
> 
> Go to any directory that doesn't contain a git repository, like /tmp.
> Then type the following:
> 
>   git checkout --<tab>
> 
> You will see nothing. That's because
> `git checkout --git-completion-helper` fails when you run it outside a
> git repository.
> 
> You might change to a directory that has a git repository, but it's too
> late, because the empty options have been cached.

This will get outdated rather soonish, as soon as 69702523af
(completion: do not cache if --git-completion-helper fails,
2019-06-12) graduates to master.

> It's unclear how many commands are affected, but this patch attempts to
> at least detect some already in the testing framework.

It seems that several changes in this patch modify tests in a way that
defeats the purpose of the given test, e.g. the tests
'completion.commands removes multiple commands' or 'sourcing the
completion script clears cached merge strategies'

I would rather see tests specifically focusing on the
__gitcomp_builtin() helper function, including test cases when it's
excersized outside of a repository and when it gets additional
parameters to include and exclude some options.

> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  t/t9902-completion.sh | 37 ++++++++++++++++++++++++++++---------
>  1 file changed, 28 insertions(+), 9 deletions(-)
> 
> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
> index 43cf313a1c..7bef41eaf5 100755
> --- a/t/t9902-completion.sh
> +++ b/t/t9902-completion.sh
> @@ -122,6 +122,15 @@ test_gitcomp_nl ()
>  	test_cmp expected out
>  }
>  
> +offgit ()
> +{
> +	GIT_CEILING_DIRECTORIES="$ROOT" &&
> +	export GIT_CEILING_DIRECTORIES &&
> +	test_when_finished "ROOT='$ROOT'; cd '$TRASH_DIRECTORY'; unset GIT_CEILING_DIRECTORIES" &&
> +	ROOT="$ROOT"/non-repo &&
> +	cd "$ROOT"

I think fiddling with $ROOT is unnecessary here.

> +}
> +
>  invalid_variable_name='${foo.bar}'
>  
>  actual="$TRASH_DIRECTORY/actual"
> @@ -358,10 +367,8 @@ test_expect_success SYMLINKS '__git_find_repo_path - resulting path avoids symli
>  '
>  
>  test_expect_success '__git_find_repo_path - not a git repository' '
> +	offgit &&
>  	(
> -		cd non-repo &&
> -		GIT_CEILING_DIRECTORIES="$ROOT" &&
> -		export GIT_CEILING_DIRECTORIES &&
>  		test_must_fail __git_find_repo_path &&
>  		printf "$__git_repo_path" >"$actual"
>  	) &&
> @@ -1388,6 +1395,7 @@ test_expect_success '__git_pretty_aliases' '
>  '
>  
>  test_expect_success 'basic' '
> +	offgit &&
>  	run_completion "git " &&
>  	# built-in
>  	grep -q "^add \$" out &&
> @@ -1401,6 +1409,7 @@ test_expect_success 'basic' '
>  '
>  
>  test_expect_success 'double dash "git" itself' '
> +	offgit &&
>  	test_completion "git --" <<-\EOF
>  	--paginate Z
>  	--no-pager Z
> @@ -1419,7 +1428,8 @@ test_expect_success 'double dash "git" itself' '
>  	EOF
>  '
>  
> -test_expect_success 'double dash "git checkout"' '
> +test_expect_failure 'double dash "git checkout"' '
> +	offgit &&
>  	test_completion "git checkout --" <<-\EOF
>  	--quiet Z
>  	--detach Z
> @@ -1442,6 +1452,7 @@ test_expect_success 'double dash "git checkout"' '
>  '
>  
>  test_expect_success 'general options' '
> +	offgit &&
>  	test_completion "git --ver" "--version " &&
>  	test_completion "git --hel" "--help " &&
>  	test_completion "git --exe" <<-\EOF &&
> @@ -1460,6 +1471,7 @@ test_expect_success 'general options' '
>  '
>  
>  test_expect_success 'general options plus command' '
> +	offgit &&
>  	test_completion "git --version check" "checkout " &&
>  	test_completion "git --paginate check" "checkout " &&
>  	test_completion "git --git-dir=foo check" "checkout " &&
> @@ -1480,11 +1492,13 @@ test_expect_success 'general options plus command' '
>  '
>  
>  test_expect_success 'git --help completion' '
> +	offgit &&
>  	test_completion "git --help ad" "add " &&
>  	test_completion "git --help core" "core-tutorial "
>  '
>  
> -test_expect_success 'completion.commands removes multiple commands' '
> +test_expect_failure 'completion.commands removes multiple commands' '
> +	offgit &&
>  	test_config completion.commands "-cherry -mergetool" &&
>  	git --list-cmds=list-mainporcelain,list-complete,config >out &&
>  	! grep -E "^(cherry|mergetool)$" out
> @@ -1547,9 +1561,10 @@ test_expect_success 'complete tree filename with metacharacters' '
>  	EOF
>  '
>  
> -test_expect_success PERL 'send-email' '
> -	test_completion "git send-email --cov" "--cover-letter " &&
> -	test_completion "git send-email ma" "master "
> +test_expect_failure PERL 'send-email' '
> +	test_completion "git send-email ma" "master " &&
> +	offgit &&
> +	test_completion "git send-email --cov" "--cover-letter "
>  '
>  
>  test_expect_success 'complete files' '
> @@ -1649,6 +1664,7 @@ test_expect_success 'completion used <cmd> completion for alias: !f() { : git <c
>  '
>  
>  test_expect_success 'completion without explicit _git_xxx function' '
> +	offgit &&
>  	test_completion "git version --" <<-\EOF
>  	--build-options Z
>  	--no-build-options Z
> @@ -1699,13 +1715,15 @@ do
>  done
>  
>  test_expect_success 'sourcing the completion script clears cached commands' '
> +	offgit &&
>  	__git_compute_all_commands &&
>  	verbose test -n "$__git_all_commands" &&
>  	. "$GIT_BUILD_DIR/contrib/completion/git-completion.bash" &&
>  	verbose test -z "$__git_all_commands"
>  '
>  
> -test_expect_success 'sourcing the completion script clears cached merge strategies' '
> +test_expect_failure 'sourcing the completion script clears cached merge strategies' '
> +	offgit &&
>  	GIT_TEST_GETTEXT_POISON= &&
>  	__git_compute_merge_strategies &&
>  	verbose test -n "$__git_merge_strategies" &&
> @@ -1714,6 +1732,7 @@ test_expect_success 'sourcing the completion script clears cached merge strategi
>  '
>  
>  test_expect_success 'sourcing the completion script clears cached --options' '
> +	offgit &&
>  	__gitcomp_builtin checkout &&
>  	verbose test -n "$__gitcomp_builtin_checkout" &&
>  	__gitcomp_builtin notes_edit &&
> -- 
> 2.22.0
> 

^ permalink raw reply	[flat|nested] 58+ messages in thread

* Re: [PATCH 00/14] completion: a bunch of updates
  2019-06-21 22:30 [PATCH 00/14] completion: a bunch of updates Felipe Contreras
                   ` (13 preceding siblings ...)
  2019-06-21 22:31 ` [PATCH 14/14] completion: add default merge strategies Felipe Contreras
@ 2019-07-03 17:50 ` Junio C Hamano
  2019-07-03 19:06   ` SZEDER Gábor
  2020-10-25  3:46   ` Felipe Contreras
  14 siblings, 2 replies; 58+ messages in thread
From: Junio C Hamano @ 2019-07-03 17:50 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: git, SZEDER Gábor, Nguyễn Thái Ngọc Duy

Felipe Contreras <felipe.contreras@gmail.com> writes:

> Here's another try at completion fixes, cleanups, and more tests. Some
> of these have already been sent.
>
> Felipe Contreras (14):
>   completion: zsh: fix __gitcomp_direct()
>   completion: zsh: fix for directories with spaces
>   completion: remove zsh hack
>   completion: zsh: improve main function selection
>   completion: prompt: fix color for Zsh
>   completion: bash: cleanup cygwin check
>   completion: zsh: update installation instructions
>   completion: bash: remove old compat wrappers
>   completion: bash: remove zsh wrapper
>   completion: zsh: trivial cleanups
>   test: completion: tests for __gitcomp regression
>   test: completion: use global config
>   completion: add default options
>   completion: add default merge strategies
>
>  contrib/completion/git-completion.bash | 202 +++++++++++++------------
>  contrib/completion/git-completion.zsh  |  53 +++----
>  contrib/completion/git-prompt.sh       |  10 +-
>  t/t9902-completion.sh                  |  37 +++--
>  4 files changed, 161 insertions(+), 141 deletions(-)

Having scanned the discussion threads so far, I think the last four
patches are going against the list consensus of (1) it is OK to rely
on --git-completion-helper; using ancient Git with new completion
script won't obviously work, but that is "if it hurts, don't". (2)
some subcommands will fail the --git-completion-helper request
(e.g. outside a repository), but as long as the output from failed
request is not cached, it is OK.

But we haven't seen any response to the earlier zsh specific
patches.  Does it mean that nobody other than Felipe cares about
having a working Git completion for zsh?  Or does it mean that all
users other than Felipe are happy with the current Git completion
for zsh and it works very well for them already?  Or somewhere in
between?

What I am trying to get at is if we would want to keep the earlier
zsh parts of the series, but with nobody seemingly interested in, it
is hard for me to justify queuing them.

Thanks.

^ permalink raw reply	[flat|nested] 58+ messages in thread

* Re: [PATCH 00/14] completion: a bunch of updates
  2019-07-03 17:50 ` [PATCH 00/14] completion: a bunch of updates Junio C Hamano
@ 2019-07-03 19:06   ` SZEDER Gábor
  2020-10-25  3:51     ` Felipe Contreras
  2020-10-25  3:46   ` Felipe Contreras
  1 sibling, 1 reply; 58+ messages in thread
From: SZEDER Gábor @ 2019-07-03 19:06 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Felipe Contreras, git, Nguyễn Thái Ngọc Duy

On Wed, Jul 03, 2019 at 10:50:26AM -0700, Junio C Hamano wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
> 
> > Here's another try at completion fixes, cleanups, and more tests. Some
> > of these have already been sent.
> >
> > Felipe Contreras (14):
> >   completion: zsh: fix __gitcomp_direct()
> >   completion: zsh: fix for directories with spaces
> >   completion: remove zsh hack
> >   completion: zsh: improve main function selection
> >   completion: prompt: fix color for Zsh
> >   completion: bash: cleanup cygwin check
> >   completion: zsh: update installation instructions
> >   completion: bash: remove old compat wrappers
> >   completion: bash: remove zsh wrapper
> >   completion: zsh: trivial cleanups
> >   test: completion: tests for __gitcomp regression
> >   test: completion: use global config
> >   completion: add default options
> >   completion: add default merge strategies
> >
> >  contrib/completion/git-completion.bash | 202 +++++++++++++------------
> >  contrib/completion/git-completion.zsh  |  53 +++----
> >  contrib/completion/git-prompt.sh       |  10 +-
> >  t/t9902-completion.sh                  |  37 +++--
> >  4 files changed, 161 insertions(+), 141 deletions(-)
> 
> Having scanned the discussion threads so far, I think the last four
> patches are going against the list consensus of (1) it is OK to rely
> on --git-completion-helper; using ancient Git with new completion
> script won't obviously work, but that is "if it hurts, don't". (2)
> some subcommands will fail the --git-completion-helper request
> (e.g. outside a repository), but as long as the output from failed
> request is not cached, it is OK.
> 
> But we haven't seen any response to the earlier zsh specific
> patches.  Does it mean that nobody other than Felipe cares about
> having a working Git completion for zsh?  Or does it mean that all
> users other than Felipe are happy with the current Git completion
> for zsh and it works very well for them already?  Or somewhere in
> between?
> 
> What I am trying to get at is if we would want to keep the earlier
> zsh parts of the series, but with nobody seemingly interested in, it
> is hard for me to justify queuing them.

I'm not a Zsh user and am mostly unfamiliar with its antics, but
FWIW...

Zsh has its own git completion routines, which are in some aspects
more advanced than what can be achieved with Bash's completion
facilities (or more wasteful in screen real estate, depending on your
preferences :), e.g. Zsh's completion shows a short description for
each completeable --option and whatnot).  I suppose that the avarage
Zsh & Git user uses Zsh's own git completion instead of our Bash
completion script wrapped for Zsh.

Having said that, I applied the first 7 patches in my tree and then
followed the updated installation instructions, and it finally worked.
I remember trying it in the past once or twice, to check whether some
of my bigger completion updates break something in Zsh, but it never
worked.  So these patches (and perhaps patch 10 as well) seem to be a
definite improvement (though admittedly I haven't tested them
thoroughly).

As for the latter part of the series, I think the more hard-coded
options we can get rid of the better we are off, and I would rather
not see them making a comeback.  I don't really have an opinion about
patches 8 and 9 (that old wrapper is probably just bitrotting away,
but I just tried to source our bash completion script from Zsh, and
apart from the deprecation warning it appeared to work).


^ permalink raw reply	[flat|nested] 58+ messages in thread

* Re: [PATCH 00/14] completion: a bunch of updates
  2019-07-03 17:50 ` [PATCH 00/14] completion: a bunch of updates Junio C Hamano
  2019-07-03 19:06   ` SZEDER Gábor
@ 2020-10-25  3:46   ` Felipe Contreras
  2020-10-27 20:23     ` Junio C Hamano
  1 sibling, 1 reply; 58+ messages in thread
From: Felipe Contreras @ 2020-10-25  3:46 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Git, SZEDER Gábor, Nguyễn Thái Ngọc Duy

On Wed, Jul 3, 2019 at 12:50 PM Junio C Hamano <gitster@pobox.com> wrote:

> But we haven't seen any response to the earlier zsh specific
> patches.  Does it mean that nobody other than Felipe cares about
> having a working Git completion for zsh?  Or does it mean that all
> users other than Felipe are happy with the current Git completion
> for zsh and it works very well for them already?  Or somewhere in
> between?

The answer is obvious: the set of zsh users and the set of git
developers don't overlap.

> What I am trying to get at is if we would want to keep the earlier
> zsh parts of the series, but with nobody seemingly interested in, it
> is hard for me to justify queuing them.

You are asking in the wrong forum.

I would gladly point you to *dozens* of issues reported in Stack
Overflow and Oh-My-Zsh if you don't believe me.

Or you could just install zsh and see the issues for yourself.

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 58+ messages in thread

* Re: [PATCH 00/14] completion: a bunch of updates
  2019-07-03 19:06   ` SZEDER Gábor
@ 2020-10-25  3:51     ` Felipe Contreras
  0 siblings, 0 replies; 58+ messages in thread
From: Felipe Contreras @ 2020-10-25  3:51 UTC (permalink / raw)
  To: SZEDER Gábor
  Cc: Junio C Hamano, Git, Nguyễn Thái Ngọc Duy

On Wed, Jul 3, 2019 at 2:06 PM SZEDER Gábor <szeder.dev@gmail.com> wrote:

> As for the latter part of the series, I think the more hard-coded
> options we can get rid of the better we are off, and I would rather
> not see them making a comeback.  I don't really have an opinion about
> patches 8 and 9 (that old wrapper is probably just bitrotting away,
> but I just tried to source our bash completion script from Zsh, and
> apart from the deprecation warning it appeared to work).

I just added those patches to highlight the issue, which is very real,
and you can see by running the added tests.

I will maintain those patches separately in a branch named "hacks" in
my project git-completion. If anybody wants the latest completion with
an old version of Git they can use my project instead.

https://github.com/felipec/git-completion/commits/hacks

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 58+ messages in thread

* Re: [PATCH 00/14] completion: a bunch of updates
  2020-10-25  3:46   ` Felipe Contreras
@ 2020-10-27 20:23     ` Junio C Hamano
  2020-10-27 22:19       ` Felipe Contreras
  0 siblings, 1 reply; 58+ messages in thread
From: Junio C Hamano @ 2020-10-27 20:23 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Git, SZEDER Gábor, Nguyễn Thái Ngọc Duy

Felipe Contreras <felipe.contreras@gmail.com> writes:

>> What I am trying to get at is if we would want to keep the earlier
>> zsh parts of the series, but with nobody seemingly interested in, it
>> is hard for me to justify queuing them.
>
> You are asking in the wrong forum.
>
> I would gladly point you to *dozens* of issues reported in Stack
> Overflow and Oh-My-Zsh if you don't believe me.

Oh, no, there is no "believing" needed.

Have you fed your patches to those folks who have dozens of issues
and the patches made their life better?  It does not help much to
make me look at these forums; we need some way to make those in
these forums aware of your improvements, try them out and report
success, to help the wider range of users who are not even in these
forums and struggling with their zsh-completion use (they will get
their zsh/git completion from their distros---I am assuming that the
distros get theirs from us in contrib/completion/).

> Or you could just install zsh and see the issues for yourself.

No, thanks.  I am not a zsh user, and have no plan to become one ;-)

^ permalink raw reply	[flat|nested] 58+ messages in thread

* Re: [PATCH 00/14] completion: a bunch of updates
  2020-10-27 20:23     ` Junio C Hamano
@ 2020-10-27 22:19       ` Felipe Contreras
  2020-10-27 23:32         ` Junio C Hamano
  0 siblings, 1 reply; 58+ messages in thread
From: Felipe Contreras @ 2020-10-27 22:19 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Git, SZEDER Gábor, Nguyễn Thái Ngọc Duy

On Tue, Oct 27, 2020 at 2:23 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Felipe Contreras <felipe.contreras@gmail.com> writes:

> > You are asking in the wrong forum.
> >
> > I would gladly point you to *dozens* of issues reported in Stack
> > Overflow and Oh-My-Zsh if you don't believe me.
>
> Oh, no, there is no "believing" needed.
>
> Have you fed your patches to those folks who have dozens of issues
> and the patches made their life better?

Yes.

> It does not help much to
> make me look at these forums; we need some way to make those in
> these forums aware of your improvements, try them out and report
> success,

They already are, and they already have. In those forums.

> (they will get
> their zsh/git completion from their distros---I am assuming that the
> distros get theirs from us in contrib/completion/).

I don't know of anyone that relies on the zsh completion shared by
their distribution.

> > Or you could just install zsh and see the issues for yourself.
>
> No, thanks.  I am not a zsh user, and have no plan to become one ;-)

You don't need to become a zsh user to test a patch. I often test bash
completion patches on bash, even though I'm not a bash user.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 58+ messages in thread

* Re: [PATCH 00/14] completion: a bunch of updates
  2020-10-27 22:19       ` Felipe Contreras
@ 2020-10-27 23:32         ` Junio C Hamano
  2020-10-28  0:06           ` Felipe Contreras
  0 siblings, 1 reply; 58+ messages in thread
From: Junio C Hamano @ 2020-10-27 23:32 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Git, SZEDER Gábor, Nguyễn Thái Ngọc Duy

Felipe Contreras <felipe.contreras@gmail.com> writes:

>> Have you fed your patches to those folks who have dozens of issues
>> and the patches made their life better?
>
> Yes.

That's good.

>> (they will get
>> their zsh/git completion from their distros---I am assuming that the
>> distros get theirs from us in contrib/completion/).
>
> I don't know of anyone that relies on the zsh completion shared by
> their distribution.

Hmph.  If the real users don't get the completion scripts from their
distribution, is there still a point in having them in my tree?  You
are certainly not suggesting me to remove contrib/completion/ at
least for zsh part, but then it is unclear what you want.

Are you saying that by adding the latest and greatest, these real
users who so far couldn't rely on distros now can start to do so,
and we'll make their life easier by updating the 29-patch series
(which I presume is the v2 of this 14-patch series)?

In any case, some Zsh users, even though they are not active
developers for the completion script, may have something good to
say, now the 29-patch series has been posted to the list and queued.
I didn't look at the zsh part, but I didn't find anything glaringly
wrong in the changes to the bash completion.

Thanks.

^ permalink raw reply	[flat|nested] 58+ messages in thread

* Re: [PATCH 00/14] completion: a bunch of updates
  2020-10-27 23:32         ` Junio C Hamano
@ 2020-10-28  0:06           ` Felipe Contreras
  2020-10-28  9:09             ` Stefan Haller
  0 siblings, 1 reply; 58+ messages in thread
From: Felipe Contreras @ 2020-10-28  0:06 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Git, SZEDER Gábor, Nguyễn Thái Ngọc Duy

On Tue, Oct 27, 2020 at 5:32 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Felipe Contreras <felipe.contreras@gmail.com> writes:

> >> (they will get
> >> their zsh/git completion from their distros---I am assuming that the
> >> distros get theirs from us in contrib/completion/).
> >
> > I don't know of anyone that relies on the zsh completion shared by
> > their distribution.
>
> Hmph.  If the real users don't get the completion scripts from their
> distribution, is there still a point in having them in my tree?  You
> are certainly not suggesting me to remove contrib/completion/ at
> least for zsh part, but then it is unclear what you want.

I didn't say the users didn't get the scripts from the distribution, I
said I didn't know of anyone that did. I just checked the installation
instructions of Homebrew, and they do seem to install the zsh
completion from contrib, whoever, by the time I see the bug reports,
those users already downloaded the most recent version from GitHub
[1].

On the other hand my distribution (Arch Linux) does not enable the zsh
script by default, it just lies dormant in /usr/share/git/completion,
which nobody uses. So users in Arch Linux naturally would download the
latest version from GitHub [1] as well.

So, which distributions package and enable the zsh script by default? Who knows.

I suggested you to graduate those scripts out of contrib so
distributions would trust the scripts enough to enable them by
default, but you refused.

What you do with the scripts is up to you, I only know what would
happen depending on what you do. 1) If you leave them as is, some
distributions would enable them, others don't, and people will keep
downloading the scripts from git's GitHub [1]. 2) If you graduate
them, more--if not all--distributions would enable them by default,
and less people would download them. 3) If you remove them, people
would look for another git repository to download those scripts from.

> Are you saying that by adding the latest and greatest, these real
> users who so far couldn't rely on distros now can start to do so,
> and we'll make their life easier by updating the 29-patch series
> (which I presume is the v2 of this 14-patch series)?

No. Many of them will keep downloading the completion from git's
mirror in GitHub [1].

Unless you graduate the scripts out of contrib.

> In any case, some Zsh users, even though they are not active
> developers for the completion script, may have something good to
> say, now the 29-patch series has been posted to the list and queued.
> I didn't look at the zsh part, but I didn't find anything glaringly
> wrong in the changes to the bash completion.

That's great. Although I have v3 already, since I found a couple of issues.

I'll send those.

Cheers.

[1] https://github.com/git/git/blob/master/contrib/completion/git-completion.zsh

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 58+ messages in thread

* Re: [PATCH 00/14] completion: a bunch of updates
  2020-10-28  0:06           ` Felipe Contreras
@ 2020-10-28  9:09             ` Stefan Haller
  2020-10-28 16:31               ` Felipe Contreras
  0 siblings, 1 reply; 58+ messages in thread
From: Stefan Haller @ 2020-10-28  9:09 UTC (permalink / raw)
  To: Felipe Contreras, Junio C Hamano
  Cc: Git, SZEDER Gábor, Nguyễn Thái Ngọc Duy

On 28.10.20 1:06, Felipe Contreras wrote:
> On Tue, Oct 27, 2020 at 5:32 PM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Felipe Contreras <felipe.contreras@gmail.com> writes:
> 
>>>> (they will get
>>>> their zsh/git completion from their distros---I am assuming that the
>>>> distros get theirs from us in contrib/completion/).
>>>
>>> I don't know of anyone that relies on the zsh completion shared by
>>> their distribution.
>>
>> Hmph.  If the real users don't get the completion scripts from their
>> distribution, is there still a point in having them in my tree?  You
>> are certainly not suggesting me to remove contrib/completion/ at
>> least for zsh part, but then it is unclear what you want.
> 
> I didn't say the users didn't get the scripts from the distribution, I
> said I didn't know of anyone that did. I just checked the installation
> instructions of Homebrew, and they do seem to install the zsh
> completion from contrib, whoever, by the time I see the bug reports,
> those users already downloaded the most recent version from GitHub
> [1].

I might not be the representative zsh user, but just as one data point: 
I have never downloaded the completion scripts from anywhere. I always 
use the one that comes with my "distro" (which is the Mac git installer, 
most of the time, which puts it in /usr/local/git/contrib/completion/). 
I symlink that to ~/.zfunc/_git.

> On the other hand my distribution (Arch Linux) does not enable the zsh
> script by default, it just lies dormant in /usr/share/git/completion,
> which nobody uses. So users in Arch Linux naturally would download the
> latest version from GitHub [1] as well.
> 
> So, which distributions package and enable the zsh script by default? Who knows.
> 
> I suggested you to graduate those scripts out of contrib so
> distributions would trust the scripts enough to enable them by
> default, but you refused.
> 
> What you do with the scripts is up to you, I only know what would
> happen depending on what you do. 1) If you leave them as is, some
> distributions would enable them, others don't, and people will keep
> downloading the scripts from git's GitHub [1]. 2) If you graduate
> them, more--if not all--distributions would enable them by default,
> and less people would download them. 3) If you remove them, people
> would look for another git repository to download those scripts from.

I don't think it makes a difference whether the scripts live in contrib 
or not. Bash completion is also in contrib, and yet it seems to be 
shipped and enabled by most distros, as far as I can tell.

I guess the reason why zsh completion is not enabled by default in 
distros is simply that there are far fewer zsh users than bash users, so 
packagers don't bother. That's just my unfounded guess, of course.

>> Are you saying that by adding the latest and greatest, these real
>> users who so far couldn't rely on distros now can start to do so,
>> and we'll make their life easier by updating the 29-patch series
>> (which I presume is the v2 of this 14-patch series)?
> 
> No. Many of them will keep downloading the completion from git's
> mirror in GitHub [1].
> 
> Unless you graduate the scripts out of contrib.
> 
>> In any case, some Zsh users, even though they are not active
>> developers for the completion script, may have something good to
>> say, now the 29-patch series has been posted to the list and queued.
>> I didn't look at the zsh part, but I didn't find anything glaringly
>> wrong in the changes to the bash completion.
> 
> That's great. Although I have v3 already, since I found a couple of issues.
> 
> I'll send those.
> 
> Cheers.
> 
> [1] https://github.com/git/git/blob/master/contrib/completion/git-completion.zsh
> 

^ permalink raw reply	[flat|nested] 58+ messages in thread

* Re: [PATCH 00/14] completion: a bunch of updates
  2020-10-28  9:09             ` Stefan Haller
@ 2020-10-28 16:31               ` Felipe Contreras
  2020-10-28 17:34                 ` Stefan Haller
  2020-10-29 17:16                 ` Junio C Hamano
  0 siblings, 2 replies; 58+ messages in thread
From: Felipe Contreras @ 2020-10-28 16:31 UTC (permalink / raw)
  To: Stefan Haller
  Cc: Junio C Hamano, Git, SZEDER Gábor,
	Nguyễn Thái Ngọc Duy

On Wed, Oct 28, 2020 at 3:09 AM Stefan Haller <lists@haller-berlin.de> wrote:
>
> On 28.10.20 1:06, Felipe Contreras wrote:

> > I didn't say the users didn't get the scripts from the distribution, I
> > said I didn't know of anyone that did. I just checked the installation
> > instructions of Homebrew, and they do seem to install the zsh
> > completion from contrib, whoever, by the time I see the bug reports,
> > those users already downloaded the most recent version from GitHub
> > [1].
>
> I might not be the representative zsh user, but just as one data point:
> I have never downloaded the completion scripts from anywhere. I always
> use the one that comes with my "distro" (which is the Mac git installer,
> most of the time, which puts it in /usr/local/git/contrib/completion/).
> I symlink that to ~/.zfunc/_git.

That's interesting. Where did you get the idea to do that?

> > On the other hand my distribution (Arch Linux) does not enable the zsh
> > script by default, it just lies dormant in /usr/share/git/completion,
> > which nobody uses. So users in Arch Linux naturally would download the
> > latest version from GitHub [1] as well.
> >
> > So, which distributions package and enable the zsh script by default? Who knows.
> >
> > I suggested you to graduate those scripts out of contrib so
> > distributions would trust the scripts enough to enable them by
> > default, but you refused.
> >
> > What you do with the scripts is up to you, I only know what would
> > happen depending on what you do. 1) If you leave them as is, some
> > distributions would enable them, others don't, and people will keep
> > downloading the scripts from git's GitHub [1]. 2) If you graduate
> > them, more--if not all--distributions would enable them by default,
> > and less people would download them. 3) If you remove them, people
> > would look for another git repository to download those scripts from.
>
> I don't think it makes a difference whether the scripts live in contrib
> or not. Bash completion is also in contrib, and yet it seems to be
> shipped and enabled by most distros, as far as I can tell.

Apples and oranges.

There is no default completion for git in bash, neither in bash, nor
in bash-completion, so if the distribution doesn't install the
completion in the right place
(/usr/share/bash-completion/completions/git), then the user would have
no git completion.

On zsh the situation is different; zsh by default has a git completion
(/usr/share/zsh/functions/Completion/Unix/_git), and some might argue
it's more complete than git's zsh completion, so why would
distribution maintainers chose the one in 'contrib' (an unofficial
contributed script) over the official one? Indeed they don't, at least
on Arch Linux.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 58+ messages in thread

* Re: [PATCH 00/14] completion: a bunch of updates
  2020-10-28 16:31               ` Felipe Contreras
@ 2020-10-28 17:34                 ` Stefan Haller
  2020-10-29 17:16                 ` Junio C Hamano
  1 sibling, 0 replies; 58+ messages in thread
From: Stefan Haller @ 2020-10-28 17:34 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Junio C Hamano, Git, SZEDER Gábor,
	Nguyễn Thái Ngọc Duy

On 28.10.20 17:31, Felipe Contreras wrote:
> On Wed, Oct 28, 2020 at 3:09 AM Stefan Haller <lists@haller-berlin.de> wrote:
>>
>> On 28.10.20 1:06, Felipe Contreras wrote:
> 
>>> I didn't say the users didn't get the scripts from the distribution, I
>>> said I didn't know of anyone that did. I just checked the installation
>>> instructions of Homebrew, and they do seem to install the zsh
>>> completion from contrib, whoever, by the time I see the bug reports,
>>> those users already downloaded the most recent version from GitHub
>>> [1].
>>
>> I might not be the representative zsh user, but just as one data point:
>> I have never downloaded the completion scripts from anywhere. I always
>> use the one that comes with my "distro" (which is the Mac git installer,
>> most of the time, which puts it in /usr/local/git/contrib/completion/).
>> I symlink that to ~/.zfunc/_git.
> 
> That's interesting. Where did you get the idea to do that?

 From the documentation in git-completion.zsh. I must have set this up 
at a time where the documentation was still correct (i.e. before it got 
broken in <https://github.com/gitster/git/commit/176f5adfdb01a>.

-Stefan

^ permalink raw reply	[flat|nested] 58+ messages in thread

* Re: [PATCH 00/14] completion: a bunch of updates
  2020-10-28 16:31               ` Felipe Contreras
  2020-10-28 17:34                 ` Stefan Haller
@ 2020-10-29 17:16                 ` Junio C Hamano
  2020-10-29 17:27                   ` Junio C Hamano
                                     ` (2 more replies)
  1 sibling, 3 replies; 58+ messages in thread
From: Junio C Hamano @ 2020-10-29 17:16 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Stefan Haller, Git, SZEDER Gábor,
	Nguyễn Thái Ngọc Duy

Felipe Contreras <felipe.contreras@gmail.com> writes:

> On Wed, Oct 28, 2020 at 3:09 AM Stefan Haller <lists@haller-berlin.de> wrote:
>>
>> I might not be the representative zsh user, but just as one data point:
>> I have never downloaded the completion scripts from anywhere. I always
>> use the one that comes with my "distro" (which is the Mac git installer,
>> most of the time, which puts it in /usr/local/git/contrib/completion/).
>> I symlink that to ~/.zfunc/_git.

That's one data point.

>> I don't think it makes a difference whether the scripts live in contrib
>> or not. Bash completion is also in contrib, and yet it seems to be
>> shipped and enabled by most distros, as far as I can tell.
>
> Apples and oranges.
>
> There is no default completion for git in bash, neither in bash, nor
> in bash-completion, so if the distribution doesn't install the
> completion in the right place
> (/usr/share/bash-completion/completions/git), then the user would have
> no git completion.

True, as far as I know.  https://github.com/scop/bash-completion does not
seem to have an entry for "git" (/usr/share/bash-completion/completions/git
however is locally there on my box---probably the corp IT folks installed
it there), so what we ship in contrib/ is the most used (if not the only)
implementation of bash completion script.

> On zsh the situation is different; zsh by default has a git completion
> (/usr/share/zsh/functions/Completion/Unix/_git), and some might argue
> it's more complete than git's zsh completion,

How is that completion script developed, maintained and distributed?

By "by default" I believe you mean that it gets installed when you
install zsh automatically.  Is the situation different on macOS land
(which I can believe, unfortunately)?

Stefan?  If at least some people argue what comes with zsh by
default is more complete than the one we have in contrib/, what led
you choose to "symlink" the less complete one to use it instead?

Another question (this is for Felipe).  Is

  https://github.com/ohmyzsh/ohmyzsh/blob/master/plugins/gitfast

the one that comes with zsh by default, or is it something else
(perhaps it is the go-to version for those who are not satisfied
with the version that comes with zsh by default???)?

Thanks.

^ permalink raw reply	[flat|nested] 58+ messages in thread

* Re: [PATCH 00/14] completion: a bunch of updates
  2020-10-29 17:16                 ` Junio C Hamano
@ 2020-10-29 17:27                   ` Junio C Hamano
  2020-11-02 20:18                     ` Felipe Contreras
  2020-10-30  8:01                   ` Stefan Haller
  2020-11-02 19:18                   ` Felipe Contreras
  2 siblings, 1 reply; 58+ messages in thread
From: Junio C Hamano @ 2020-10-29 17:27 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Stefan Haller, Git, SZEDER Gábor,
	Nguyễn Thái Ngọc Duy

Junio C Hamano <gitster@pobox.com> writes:

> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> On zsh the situation is different; zsh by default has a git completion
>> (/usr/share/zsh/functions/Completion/Unix/_git), and some might argue
>> it's more complete than git's zsh completion,
>
> How is that completion script developed, maintained and distributed?
>
> By "by default" I believe you mean that it gets installed when you
> install zsh automatically.  Is the situation different on macOS land
> (which I can believe, unfortunately)?
> ...

Web searching for "zsh git autocompletion" gave a few interesting insights.

 - https://medium.com/@oliverspryn/adding-git-completion-to-zsh-60f3b0e7ffbc
   was the first hit, which is about how to use what we ship in contrib/

 - https://stackoverflow.com/questions/24513873/ which was near the top
   had these gems.

      https://stackoverflow.com/a/58517668
      Actually, ZSH does know how to do git completion out of the box, but
      you need to turn on the completion feature itself (which from the
      steps you described I guess you haven't done)

   and

      https://stackoverflow.com/a/63894520
      Turns out the problem for me wass that when installing git via
      homebrew, git installs its own zsh shell extension which is
      considerably less complete/capable than the default that
      oh-my-szh installs. Find out what versions your git install is
      and then remove the zsh autocompletions. Mine were here and
      deleted thusly:

	rm -rf /usr/local/Cellar/git/2.28.0/share/zsh/

The "knows out of the box" in https://stackoverflow.com/a/58517668
is matches your "zsh by default has".

> so why would
> distribution maintainers chose the one in 'contrib' (an unofficial
> contributed script) over the official one? Indeed they don't, at least
> on Arch Linux.

You're right.  They would certainly not, and the situation is quite
different from bash completion where we seem to be the authoritative
implementation.

This leads me in a totally different direction.

We are making life worse for the zsh users by shipping our own
version, aren't we?  If we didn't ship our own completion script for
them, the user did not have to remove the one "which is considerably
less complete/capable".  Perhaps we are misleading users with our
version that has an implicit "came from those who know Git the best
in the world" label that gives it more authenticity than it
deserves.  A good zsh autocompletion would need to be written and
reviewed by those who know zsh completion well.  They also need to
know Git somewhat, but the expertise on the former would be a lot
more important, I would think.

But as you said in
<CAMP44s3wqxTmgQpMgk2cM33EvtwrvvXYv4_90GKGmHb8yJHAKg@mail.gmail.com>

    The answer is obvious: the set of zsh users and the set of git
    developers don't overlap.

this community is not equipped to give good reviews and improvement
suggestions on zsh matters to your patches.  And I do not have a
feeling that the situation would change soon.

Do your recent 29-patch improvements not just fill the "gap" but
surpass the one that comes by default with zsh?  I have this nagging
feeling that the effort to make the autocompletion better for Git
users who use zsh may be better made by you ("git blame" tells me that
you seem to be the only one who's invested heavily in the script,
unfortunately) joining forces with those who develop and maintain the
autocompletion that comes by default with zsh.  We may also want to
have a tombstone in contrib/completion/ to redirect the users to the
default version and away from our "less complete/capable" one.

Thanks.

^ permalink raw reply	[flat|nested] 58+ messages in thread

* Re: [PATCH 00/14] completion: a bunch of updates
  2020-10-29 17:16                 ` Junio C Hamano
  2020-10-29 17:27                   ` Junio C Hamano
@ 2020-10-30  8:01                   ` Stefan Haller
  2020-10-30 17:19                     ` Junio C Hamano
  2020-11-02 19:18                   ` Felipe Contreras
  2 siblings, 1 reply; 58+ messages in thread
From: Stefan Haller @ 2020-10-30  8:01 UTC (permalink / raw)
  To: Junio C Hamano, Felipe Contreras
  Cc: Git, SZEDER Gábor, Nguyễn Thái Ngọc Duy

On 29.10.20 18:16, Junio C Hamano wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
> 
>> On Wed, Oct 28, 2020 at 3:09 AM Stefan Haller <lists@haller-berlin.de> wrote:
>>>
>>> I might not be the representative zsh user, but just as one data point:
>>> I have never downloaded the completion scripts from anywhere. I always
>>> use the one that comes with my "distro" (which is the Mac git installer,
>>> most of the time, which puts it in /usr/local/git/contrib/completion/).
>>> I symlink that to ~/.zfunc/_git.
> 
> That's one data point.
> 
>>> I don't think it makes a difference whether the scripts live in contrib
>>> or not. Bash completion is also in contrib, and yet it seems to be
>>> shipped and enabled by most distros, as far as I can tell.
>>
>> Apples and oranges.
>>
>> There is no default completion for git in bash, neither in bash, nor
>> in bash-completion, so if the distribution doesn't install the
>> completion in the right place
>> (/usr/share/bash-completion/completions/git), then the user would have
>> no git completion.
> 
> True, as far as I know.  https://github.com/scop/bash-completion does not
> seem to have an entry for "git" (/usr/share/bash-completion/completions/git
> however is locally there on my box---probably the corp IT folks installed
> it there), so what we ship in contrib/ is the most used (if not the only)
> implementation of bash completion script.
> 
>> On zsh the situation is different; zsh by default has a git completion
>> (/usr/share/zsh/functions/Completion/Unix/_git), and some might argue
>> it's more complete than git's zsh completion,
> 
> How is that completion script developed, maintained and distributed?
> 
> By "by default" I believe you mean that it gets installed when you
> install zsh automatically.  Is the situation different on macOS land
> (which I can believe, unfortunately)?

It's the same on Mac; I get zsh's builtin git completion when I don't
install ours.

> Stefan?  If at least some people argue what comes with zsh by
> default is more complete than the one we have in contrib/, what led
> you choose to "symlink" the less complete one to use it instead?

I'm not sure I can answer the question which one is more complete. Ours
is certainly complete enough for my daily use, but this might not mean much.

The reason why I chose ours over the one that comes with zsh is that
ours is *way* faster. If I type "git log origin/mas<tab>", with zsh's
completion it takes between one and two seconds to auto-complete this to
"origin/master". With ours it's instantaneous.

> Another question (this is for Felipe).  Is
> 
>    https://github.com/ohmyzsh/ohmyzsh/blob/master/plugins/gitfast
> 
> the one that comes with zsh by default, or is it something else
> (perhaps it is the go-to version for those who are not satisfied
> with the version that comes with zsh by default???)?
> 
> Thanks.
> 

^ permalink raw reply	[flat|nested] 58+ messages in thread

* Re: [PATCH 00/14] completion: a bunch of updates
  2020-10-30  8:01                   ` Stefan Haller
@ 2020-10-30 17:19                     ` Junio C Hamano
  2020-11-02 20:29                       ` Felipe Contreras
  2020-11-03  9:59                       ` Stefan Haller
  0 siblings, 2 replies; 58+ messages in thread
From: Junio C Hamano @ 2020-10-30 17:19 UTC (permalink / raw)
  To: Stefan Haller
  Cc: Felipe Contreras, Git, SZEDER Gábor,
	Nguyễn Thái Ngọc Duy

Stefan Haller <lists@haller-berlin.de> writes:

>> How is that completion script developed, maintained and distributed?

I think we should ask this question to those in Zsh development
community.

>> By "by default" I believe you mean that it gets installed when you
>> install zsh automatically.  Is the situation different on macOS land
>> (which I can believe, unfortunately)?
>
> It's the same on Mac; I get zsh's builtin git completion when I don't
> install ours.

I see.  That makes sense.

>> Stefan?  If at least some people argue what comes with zsh by
>> default is more complete than the one we have in contrib/, what led
>> you choose to "symlink" the less complete one to use it instead?
>
> I'm not sure I can answer the question which one is more complete. Ours
> is certainly complete enough for my daily use, but this might not mean much.
>
> The reason why I chose ours over the one that comes with zsh is that
> ours is *way* faster. If I type "git log origin/mas<tab>", with zsh's
> completion it takes between one and two seconds to auto-complete this to
> "origin/master". With ours it's instantaneous.

That is a very good data point.  I re-read the blurb on the
"gitfast" thing (below) in ohmyzsh and learned that ...

>> Another question (this is for Felipe).  Is
>> 
>>    https://github.com/ohmyzsh/ohmyzsh/blob/master/plugins/gitfast
>> 
>> the one that comes with zsh by default, or is it something else
>> (perhaps it is the go-to version for those who are not satisfied
>> with the version that comes with zsh by default???)?

... it repackages what we have in contrib/ and promises a faster
completion (which aligns with the reason you stated why you chose
ours) than the Zsh default one and being always fresh (by frequent
updates from our contrib/).  In other words, my understanding is
that it is positioned as a competitor to the Zsh default.

After making a brief observation for my previous message in the
thread, I understand that oh-my-zsh is a very popular colleciton of
third-party stuff for Zsh users, so it seems to me that the real
useful choices, if I or somebody else were to become a new Zsh user,
are between sticking with the Zsh default or grabbing the improved
version from oh-my-zsh collection.  I could also use from our
contrib/ but I would have to ask myself twice why should I, as a
(hypothetical) new Zsh user, bother, especially when the latter
promises to be always fresh anyway.

Thanks.




^ permalink raw reply	[flat|nested] 58+ messages in thread

* Re: [PATCH 00/14] completion: a bunch of updates
  2020-10-29 17:16                 ` Junio C Hamano
  2020-10-29 17:27                   ` Junio C Hamano
  2020-10-30  8:01                   ` Stefan Haller
@ 2020-11-02 19:18                   ` Felipe Contreras
  2 siblings, 0 replies; 58+ messages in thread
From: Felipe Contreras @ 2020-11-02 19:18 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Stefan Haller, Git, SZEDER Gábor,
	Nguyễn Thái Ngọc Duy

On Thu, Oct 29, 2020 at 11:16 AM Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:

> > On zsh the situation is different; zsh by default has a git completion
> > (/usr/share/zsh/functions/Completion/Unix/_git), and some might argue
> > it's more complete than git's zsh completion,
>
> How is that completion script developed, maintained and distributed?

By the maintainers of Zsh.

> By "by default" I believe you mean that it gets installed when you
> install zsh automatically.  Is the situation different on macOS land
> (which I can believe, unfortunately)?

I don't believe it's different.

> Another question (this is for Felipe).  Is
>
>   https://github.com/ohmyzsh/ohmyzsh/blob/master/plugins/gitfast
>
> the one that comes with zsh by default, or is it something else
> (perhaps it is the go-to version for those who are not satisfied
> with the version that comes with zsh by default???)?

It's for the people who are not satisfied with the default Zsh
version, which are a lot.

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 58+ messages in thread

* Re: [PATCH 00/14] completion: a bunch of updates
  2020-10-29 17:27                   ` Junio C Hamano
@ 2020-11-02 20:18                     ` Felipe Contreras
  2020-11-03  1:49                       ` Junio C Hamano
  0 siblings, 1 reply; 58+ messages in thread
From: Felipe Contreras @ 2020-11-02 20:18 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Stefan Haller, Git, SZEDER Gábor,
	Nguyễn Thái Ngọc Duy

On Thu, Oct 29, 2020 at 11:27 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Junio C Hamano <gitster@pobox.com> writes:
>
> > Felipe Contreras <felipe.contreras@gmail.com> writes:
> >
> >> On zsh the situation is different; zsh by default has a git completion
> >> (/usr/share/zsh/functions/Completion/Unix/_git), and some might argue
> >> it's more complete than git's zsh completion,
> >
> > How is that completion script developed, maintained and distributed?
> >
> > By "by default" I believe you mean that it gets installed when you
> > install zsh automatically.  Is the situation different on macOS land
> > (which I can believe, unfortunately)?
> > ...
>
> Web searching for "zsh git autocompletion" gave a few interesting insights.
>
>  - https://medium.com/@oliverspryn/adding-git-completion-to-zsh-60f3b0e7ffbc
>    was the first hit, which is about how to use what we ship in contrib/

It's weird that he didn't have completion. He probably hadn't enabled
completion (in general).

>  - https://stackoverflow.com/questions/24513873/ which was near the top
>    had these gems.

> The "knows out of the box" in https://stackoverflow.com/a/58517668
> is matches your "zsh by default has".

This is just the tip of the iceberg.

In the past people that wanted to have the Zsh default could do `brew
install git --without-completions`, but the Homebrew team decided to
remove that option, so now everyone gets the completions overridden by
installing Git.

https://github.com/Homebrew/homebrew-core/commit/f710a1395f44224e4bcc3518ee9c13a0dc850be1

> > so why would
> > distribution maintainers chose the one in 'contrib' (an unofficial
> > contributed script) over the official one? Indeed they don't, at least
> > on Arch Linux.
>
> You're right.  They would certainly not, and the situation is quite
> different from bash completion where we seem to be the authoritative
> implementation.
>
> This leads me in a totally different direction.
>
> We are making life worse for the zsh users by shipping our own
> version, aren't we?  If we didn't ship our own completion script for
> them, the user did not have to remove the one "which is considerably
> less complete/capable".

No. You are assuming the opinion of one user in Stack Overflow is a fact.

There's a reason people prefer to use Git's official completion, and
there's a reason I wrote the wrapper.

The Zsh default completion is *incredibly* slow. Just as a point of
comparison when I do `git checkout <tab>` on the Linux kernel git
repository, it takes *three* seconds to complete. With the Git
official completion it's instantaneous, just like in Bash.

This defeats the whole purpose of completion. If it takes less time
for me to type the thing than it takes the completion to complete it,
then the completion is useless. I explained this to the Zsh
developers[1], and they didn't care.

They prioritize completeness over usability.

I even wrote a blog post about the issue:

https://felipec.wordpress.com/2013/07/31/how-i-fixed-git-zsh-completion/

> Perhaps we are misleading users with our
> version that has an implicit "came from those who know Git the best
> in the world" label that gives it more authenticity than it
> deserves.

And perhaps not.

> A good zsh autocompletion would need to be written and
> reviewed by those who know zsh completion well.

No. Those people don't care if their completion is unusable.

Zsh users want a completion that is usable, and achieves the purpose
of a completion; to make the user more productive. Not a completion
Zsh developers feel proud about.

> They also need to
> know Git somewhat, but the expertise on the former would be a lot
> more important, I would think.

I disagree. To make the Git completion fast, efficient, and consistent
to how Git is supposed to be used, it's much more important to know
Git.

For example, if you do `git <tab>` in Git's Zsh completion, you get
only porcelain commands, if you do the same in Zsh's default
completion, you get "check-attr" in the list, which I doubt any Git
developer would consider something the user should see by default.

You can do `git check-<tab>` and the Git's Zsh completion I wrote will
still complete it, even though it's not visible initially.

So in that sense *my* code is superior; 1) It shows only the common
commands by default, 2) all commands can be completed anyway, and 3)
can be configured to show aliases too, and the order can be configured
too.

Why didn't the Zsh default completion do this? I don't know.

> But as you said in
> <CAMP44s3wqxTmgQpMgk2cM33EvtwrvvXYv4_90GKGmHb8yJHAKg@mail.gmail.com>
>
>     The answer is obvious: the set of zsh users and the set of git
>     developers don't overlap.
>
> this community is not equipped to give good reviews and improvement
> suggestions on zsh matters to your patches.  And I do not have a
> feeling that the situation would change soon.

Neither does any other community.

But in this community at least some people try.

> Do your recent 29-patch improvements not just fill the "gap" but
> surpass the one that comes by default with zsh?  I have this nagging
> feeling that the effort to make the autocompletion better for Git
> users who use zsh may be better made by you ("git blame" tells me that
> you seem to be the only one who's invested heavily in the script,
> unfortunately) joining forces with those who develop and maintain the
> autocompletion that comes by default with zsh.

This is not possible, as the Zsh maintainers don't care about usability.

Plus, the whole point of my wrapper is to leverage the Bash
completion, which is actively maintained. The Zsh developers would
*never* agree to use the Bash completion in any capacity.

The current situation is far from ideal, but I don't see any obvious
way to improve it.

[1] https://www.zsh.org/mla/workers/2011/msg00506.html

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 58+ messages in thread

* Re: [PATCH 00/14] completion: a bunch of updates
  2020-10-30 17:19                     ` Junio C Hamano
@ 2020-11-02 20:29                       ` Felipe Contreras
  2020-11-02 23:05                         ` Aaron Schrab
  2020-11-03  9:59                       ` Stefan Haller
  1 sibling, 1 reply; 58+ messages in thread
From: Felipe Contreras @ 2020-11-02 20:29 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Stefan Haller, Git, SZEDER Gábor,
	Nguyễn Thái Ngọc Duy

On Fri, Oct 30, 2020 at 11:19 AM Junio C Hamano <gitster@pobox.com> wrote:

> After making a brief observation for my previous message in the
> thread, I understand that oh-my-zsh is a very popular colleciton of
> third-party stuff for Zsh users, so it seems to me that the real
> useful choices, if I or somebody else were to become a new Zsh user,
> are between sticking with the Zsh default or grabbing the improved
> version from oh-my-zsh collection.  I could also use from our
> contrib/ but I would have to ask myself twice why should I, as a
> (hypothetical) new Zsh user, bother, especially when the latter
> promises to be always fresh anyway.

More or less. Oh-My-Zsh is very popular, but not everyone uses it, and
it's certainly not cheap on resources (it makes the startup a bit
slower).

So the two options are:

1. Use the script in contrib/ (or git-completion)
2. Use Oh-My-Zsh and enable the "gitfast" plugin

In my opinion to use the Zsh default completion is not an option.

Which is why I think distributions should package the Git Zsh
completion by default (like Hombrew does), which will happen more
easily if Git graduated those scripts and installed them by default.

If this close-to-ideal solution was considered, I would investigate
again if there's a way to automate the testing of the Zsh wrapper, so
that it doesn't break when the Bash script is updated (which is the
usual way the Zsh wrapper breaks).

Cheers.

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 58+ messages in thread

* Re: [PATCH 00/14] completion: a bunch of updates
  2020-11-02 20:29                       ` Felipe Contreras
@ 2020-11-02 23:05                         ` Aaron Schrab
  2020-11-03  1:35                           ` Junio C Hamano
  2020-11-03 22:37                           ` Felipe Contreras
  0 siblings, 2 replies; 58+ messages in thread
From: Aaron Schrab @ 2020-11-02 23:05 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Junio C Hamano, Stefan Haller, Git, SZEDER Gábor,
	Nguyễn Thái Ngọc Duy

At 14:29 -0600 02 Nov 2020, Felipe Contreras <felipe.contreras@gmail.com> wrote:
>So the two options are:
>
>1. Use the script in contrib/ (or git-completion)
>2. Use Oh-My-Zsh and enable the "gitfast" plugin
>
>In my opinion to use the Zsh default completion is not an option.
>
>Which is why I think distributions should package the Git Zsh
>completion by default (like Hombrew does), which will happen more
>easily if Git graduated those scripts and installed them by default.

The option that you consider to be invalid is definitely *my* preferred 
option. That Homebrew's git package installs the completion in a way 
that overrides that from the zsh package was a source of continual 
annoyance for me until I put in a long-term workaround for that (I had 
just been removing the symlink every time I noticed that it had been 
created by a new install of the git package).

While the completion provided by git.git may be faster, the one from zsh 
is fast enough on the repositories that I generally work with.  At least 
with my configuration and the completion code currently shipped by 
Homobrew's git package there are at least a couple of things that seem 
to be quite broken.

The one that generally caused me to notice that the wrong completion 
code was being used is that it doesn't provide completion for creating 
local branches to based off of remote ones; I'd previously thought that 
was just a missing feature, but looking into it a bit more now it looks 
like that **should** happen as long as I don't set 
GIT_COMPLETION_CHECKOUT_NO_GUESS=1.

The other thing that seems to not work with the completion from git.git 
is completion of single-dash options. Although that may be more of a 
missing feature rather than something that's broken. Since there isn't 
any description for long options, I'd guess there wouldn't be for short 
options either; in which case there isn't really any point to trying to 
offer completion for those. But, having additional descriptions for 
options is one of my favorite features of zsh completion.

^ permalink raw reply	[flat|nested] 58+ messages in thread

* Re: [PATCH 00/14] completion: a bunch of updates
  2020-11-02 23:05                         ` Aaron Schrab
@ 2020-11-03  1:35                           ` Junio C Hamano
  2020-11-03 23:46                             ` Felipe Contreras
  2020-11-03 22:37                           ` Felipe Contreras
  1 sibling, 1 reply; 58+ messages in thread
From: Junio C Hamano @ 2020-11-03  1:35 UTC (permalink / raw)
  To: Aaron Schrab
  Cc: Felipe Contreras, Stefan Haller, Git, SZEDER Gábor,
	Nguyễn Thái Ngọc Duy

Aaron Schrab <aaron@schrab.com> writes:

> At 14:29 -0600 02 Nov 2020, Felipe Contreras <felipe.contreras@gmail.com> wrote:
>>So the two options are:
>>
>>1. Use the script in contrib/ (or git-completion)
>>2. Use Oh-My-Zsh and enable the "gitfast" plugin
>>
>>In my opinion to use the Zsh default completion is not an option.
>>
>>Which is why I think distributions should package the Git Zsh
>>completion by default (like Hombrew does), which will happen more
>>easily if Git graduated those scripts and installed them by default.
>
> The option that you consider to be invalid is definitely *my*
> preferred option. That Homebrew's git package installs the completion
> in a way that overrides that from the zsh package was a source of
> continual annoyance for me until I put in a long-term workaround for
> that (I had just been removing the symlink every time I noticed that
> it had been created by a new install of the git package).

Thanks for a data point.  

My understanding is that Felipe's 1 & 2 are essentially the same
thing in the larger picture but they come in different packaging.

If we talk about two choices, I think they are between the Zsh
default or gitfast from oh-my-zsh.

And your preference is the former.  It is not like I picked a wrong
SO thread and was swayed by an oddball opinion of somebody who
prefers the Zsh default.  Even though the preference is up to
individual users, the important point here is that Zsh default is
not universally unusable for everybody and it is one of the valid
choices.

^ permalink raw reply	[flat|nested] 58+ messages in thread

* Re: [PATCH 00/14] completion: a bunch of updates
  2020-11-02 20:18                     ` Felipe Contreras
@ 2020-11-03  1:49                       ` Junio C Hamano
  2020-11-04  0:09                         ` Felipe Contreras
  0 siblings, 1 reply; 58+ messages in thread
From: Junio C Hamano @ 2020-11-03  1:49 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Stefan Haller, Git, SZEDER Gábor,
	Nguyễn Thái Ngọc Duy

Felipe Contreras <felipe.contreras@gmail.com> writes:

>> We are making life worse for the zsh users by shipping our own
>> version, aren't we?  If we didn't ship our own completion script for
>> them, the user did not have to remove the one "which is considerably
>> less complete/capable".
>
> No. You are assuming the opinion of one user in Stack Overflow is a fact.
>
> There's a reason people prefer to use Git's official completion, and
> there's a reason I wrote the wrapper.

Do you mean by "the wrapper" the 'gitfast' one you offer in the
oh-my-zsh collection?  If so, yes, I agree that 'gitfast' as "maybe
less complete but usably faster auto-completer" is a good thing for
end-users to have as an alternative to the Zsh default.

But that was not the point I was raising.  I was saying that it was
not making life better for them that we are posing as a valid third
choice in that competition.

Your "wrapper" can still be updated regularly to the latest to grab
the bash completion part from "git.git".  To the Zsh audience,
however, it would be more straight-forward if the choices were
"there is Zsh default completion, but if you want to use a
different/better version, grab 'gitfast' from the oh-my-zsh
collection" than "you can use Zsh default, or you can install
'gitfast' in the way Zsh users are accustomed to from the oh-my-zsh
collection, or you can manually install from git.git".  After all,
the latter two would give them the same thing.

It may be a good idea to leave a message in contrib/completion that
nudges people toward 'gitfast' as an alternative for thowe who want
to use something other than the default Zsh autocompletion.

Doing so would remove one level of unnecessary middleman (that is
us) from the picture and make it simpler for end-users by reducing
the number of choices from three to two.

> They prioritize completeness over usability.
> ...
> No. Those people don't care if their completion is unusable.
> ...
> This is not possible, as the Zsh maintainers don't care about usability.

I think our code of conduct applies to derogatory statements made on
even those who do not regularly appear in this community.  If you
want to attack them, please don't do it here.  Thanks.

^ permalink raw reply	[flat|nested] 58+ messages in thread

* Re: [PATCH 00/14] completion: a bunch of updates
  2020-10-30 17:19                     ` Junio C Hamano
  2020-11-02 20:29                       ` Felipe Contreras
@ 2020-11-03  9:59                       ` Stefan Haller
  2020-11-03 17:12                         ` Junio C Hamano
  1 sibling, 1 reply; 58+ messages in thread
From: Stefan Haller @ 2020-11-03  9:59 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Felipe Contreras, Git, SZEDER Gábor,
	Nguyễn Thái Ngọc Duy

On 30.10.20 18:19, Junio C Hamano wrote:
> Stefan Haller <lists@haller-berlin.de> writes:
> 
>>> How is that completion script developed, maintained and distributed?
> 
> I think we should ask this question to those in Zsh development
> community.
> 
>>> By "by default" I believe you mean that it gets installed when you
>>> install zsh automatically.  Is the situation different on macOS land
>>> (which I can believe, unfortunately)?
>>
>> It's the same on Mac; I get zsh's builtin git completion when I don't
>> install ours.
> 
> I see.  That makes sense.
> 
>>> Stefan?  If at least some people argue what comes with zsh by
>>> default is more complete than the one we have in contrib/, what led
>>> you choose to "symlink" the less complete one to use it instead?
>>
>> I'm not sure I can answer the question which one is more complete. Ours
>> is certainly complete enough for my daily use, but this might not mean much.
>>
>> The reason why I chose ours over the one that comes with zsh is that
>> ours is *way* faster. If I type "git log origin/mas<tab>", with zsh's
>> completion it takes between one and two seconds to auto-complete this to
>> "origin/master". With ours it's instantaneous.
> 
> That is a very good data point.  I re-read the blurb on the
> "gitfast" thing (below) in ohmyzsh and learned that ...
> 
>>> Another question (this is for Felipe).  Is
>>>
>>>    https://github.com/ohmyzsh/ohmyzsh/blob/master/plugins/gitfast
>>>
>>> the one that comes with zsh by default, or is it something else
>>> (perhaps it is the go-to version for those who are not satisfied
>>> with the version that comes with zsh by default???)?
> 
> ... it repackages what we have in contrib/ and promises a faster
> completion (which aligns with the reason you stated why you chose
> ours) than the Zsh default one and being always fresh (by frequent
> updates from our contrib/).  In other words, my understanding is
> that it is positioned as a competitor to the Zsh default.
> 
> After making a brief observation for my previous message in the
> thread, I understand that oh-my-zsh is a very popular colleciton of
> third-party stuff for Zsh users, so it seems to me that the real
> useful choices, if I or somebody else were to become a new Zsh user,
> are between sticking with the Zsh default or grabbing the improved
> version from oh-my-zsh collection.  I could also use from our
> contrib/ but I would have to ask myself twice why should I, as a
> (hypothetical) new Zsh user, bother, especially when the latter
> promises to be always fresh anyway.

Using stuff from oh-my-zsh is not an option for me. I'm not using
oh-my-zsh, and I don't want to begin using it just for this one package.

I could use Felipe's version from
<https://github.com/felipec/git-completion> (and in fact, that's what
I'm doing right now, for testing. Works great. :-)  However, I'd have to
remember to manually update it every so often.

So yes, I prefer to use the one from git's distribution, because it is
automatically kept up to date whenever I update git (as long as I
symlink to it rather than copy it.)

-Stefan

^ permalink raw reply	[flat|nested] 58+ messages in thread

* Re: [PATCH 00/14] completion: a bunch of updates
  2020-11-03  9:59                       ` Stefan Haller
@ 2020-11-03 17:12                         ` Junio C Hamano
  2020-11-03 20:07                           ` Stefan Haller
  0 siblings, 1 reply; 58+ messages in thread
From: Junio C Hamano @ 2020-11-03 17:12 UTC (permalink / raw)
  To: Stefan Haller
  Cc: Felipe Contreras, Git, SZEDER Gábor,
	Nguyễn Thái Ngọc Duy

Stefan Haller <lists@haller-berlin.de> writes:

> Using stuff from oh-my-zsh is not an option for me. I'm not using
> oh-my-zsh, and I don't want to begin using it just for this one package.
>
> I could use Felipe's version from
> <https://github.com/felipec/git-completion> (and in fact, that's what
> I'm doing right now, for testing. Works great. :-)  However, I'd have to
> remember to manually update it every so often.
>
> So yes, I prefer to use the one from git's distribution, because it is
> automatically kept up to date whenever I update git (as long as I
> symlink to it rather than copy it.)

Thanks for another data point.  

You'd need to add to another "as long as", which is "as long as it
keeps up with felipec/git-completion".  If we fall bahind, you'd be
better off getting updates directly from there, not from us.  

And I suspect that not many Zsh users want to care about the
distinction between the two.  If it were as easy to grab the latest
version of Felipe's as an update of Git from your distro, wouldn't
most people rather choose to do so?

If we are not doing much reviews on Zsh completion on this list, due
to lack of interest and expertise, then we will either fall behind,
or blindly copy, Felipe's repository and republish as a small part
of our project, without adding much value ourselves.

Which is not a very happy place for us to be in.  I dunno.

^ permalink raw reply	[flat|nested] 58+ messages in thread

* Re: [PATCH 00/14] completion: a bunch of updates
  2020-11-03 17:12                         ` Junio C Hamano
@ 2020-11-03 20:07                           ` Stefan Haller
  0 siblings, 0 replies; 58+ messages in thread
From: Stefan Haller @ 2020-11-03 20:07 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Felipe Contreras, Git, SZEDER Gábor,
	Nguyễn Thái Ngọc Duy

On 03.11.20 18:12, Junio C Hamano wrote:
> Stefan Haller <lists@haller-berlin.de> writes:
> 
>> Using stuff from oh-my-zsh is not an option for me. I'm not using
>> oh-my-zsh, and I don't want to begin using it just for this one package.
>>
>> I could use Felipe's version from
>> <https://github.com/felipec/git-completion> (and in fact, that's what
>> I'm doing right now, for testing. Works great. :-)  However, I'd have to
>> remember to manually update it every so often.
>>
>> So yes, I prefer to use the one from git's distribution, because it is
>> automatically kept up to date whenever I update git (as long as I
>> symlink to it rather than copy it.)
> 
> Thanks for another data point.  
> 
> You'd need to add to another "as long as", which is "as long as it
> keeps up with felipec/git-completion".  If we fall bahind, you'd be
> better off getting updates directly from there, not from us.  

Yes, that's true of course. That's why I'm happy that Felipe contributes
his version here, and that you are accepting it quickly.

> And I suspect that not many Zsh users want to care about the
> distinction between the two.  If it were as easy to grab the latest
> version of Felipe's as an update of Git from your distro, wouldn't
> most people rather choose to do so?
> 
> If we are not doing much reviews on Zsh completion on this list, due
> to lack of interest and expertise, then we will either fall behind,
> or blindly copy, Felipe's repository and republish as a small part
> of our project, without adding much value ourselves.
> 
> Which is not a very happy place for us to be in.  I dunno.

I still think it's valuable to do this. And I think the fact that it's
in contrib makes it less likely that we fall behind, because you can
just choose to merge updates without very thorough reviews (like with
the current patch series), which you probably wouldn't feel comfortable
doing if it had "graduated" out of contrib, as Felipe requests.

-Stefan

^ permalink raw reply	[flat|nested] 58+ messages in thread

* Re: [PATCH 00/14] completion: a bunch of updates
  2020-11-02 23:05                         ` Aaron Schrab
  2020-11-03  1:35                           ` Junio C Hamano
@ 2020-11-03 22:37                           ` Felipe Contreras
  1 sibling, 0 replies; 58+ messages in thread
From: Felipe Contreras @ 2020-11-03 22:37 UTC (permalink / raw)
  To: Felipe Contreras, Junio C Hamano, Stefan Haller, Git,
	SZEDER Gábor, Nguyễn Thái Ngọc Duy

On Mon, Nov 2, 2020 at 5:05 PM Aaron Schrab <aaron@schrab.com> wrote:

> The option that you consider to be invalid is definitely *my* preferred
> option. That Homebrew's git package installs the completion in a way
> that overrides that from the zsh package was a source of continual
> annoyance for me until I put in a long-term workaround for that (I had
> just been removing the symlink every time I noticed that it had been
> created by a new install of the git package).

You are probably a minority.

Just put the location of your desired completion in front of your fpath

  fpath=(/usr/share/zsh/functions/Completion/Unix $fpath)

> While the completion provided by git.git may be faster, the one from zsh
> is fast enough on the repositories that I generally work with.

There's no such thing as "fast enough" for most Git developers.

> At least
> with my configuration and the completion code currently shipped by
> Homobrew's git package there are at least a couple of things that seem
> to be quite broken.

If they are still broken in this version:
https://github.com/felipec/git-completion

Feel free to open an issue report.

> The one that generally caused me to notice that the wrong completion
> code was being used is that it doesn't provide completion for creating
> local branches to based off of remote ones; I'd previously thought that
> was just a missing feature, but looking into it a bit more now it looks
> like that **should** happen as long as I don't set
> GIT_COMPLETION_CHECKOUT_NO_GUESS=1.

That was a bug because somebody added a new function to the Bash
script and forgot to add it to the Zsh script.

I've sent the fix:

https://lore.kernel.org/git/20201028020712.442623-14-felipe.contreras@gmail.com/

> The other thing that seems to not work with the completion from git.git
> is completion of single-dash options. Although that may be more of a
> missing feature rather than something that's broken. Since there isn't
> any description for long options, I'd guess there wouldn't be for short
> options either; in which case there isn't really any point to trying to
> offer completion for those. But, having additional descriptions for
> options is one of my favorite features of zsh completion.

That's a feature request.

It may work at some point, but it would require a lot of work. In the
meantime you can simply use the default Zsh completion.

The point of the Zsh completion is to offer everything the Bash
completion offers. Which is clearly fine for most people, since that's
the completion most people use.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 58+ messages in thread

* Re: [PATCH 00/14] completion: a bunch of updates
  2020-11-03  1:35                           ` Junio C Hamano
@ 2020-11-03 23:46                             ` Felipe Contreras
  0 siblings, 0 replies; 58+ messages in thread
From: Felipe Contreras @ 2020-11-03 23:46 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Aaron Schrab, Stefan Haller, Git, SZEDER Gábor,
	Nguyễn Thái Ngọc Duy

On Mon, Nov 2, 2020 at 7:35 PM Junio C Hamano <gitster@pobox.com> wrote:

> My understanding is that Felipe's 1 & 2 are essentially the same
> thing in the larger picture but they come in different packaging.

Yes, and no. That's like saying you get the same thing with Gentoo
than with Ubuntu.

Yes; you get the same thing, but no; Oh-My-Zsh comes with a lot more
you might not necessarily want.

> And your preference is the former.  It is not like I picked a wrong
> SO thread and was swayed by an oddball opinion of somebody who
> prefers the Zsh default.  Even though the preference is up to
> individual users, the important point here is that Zsh default is
> not universally unusable for everybody and it is one of the valid
> choices.

I think that's debatable. If I had to be it would be on a 50/50 split.
But that's considering that most people never change their default.

If my script were the default, my bet would be on a 90/10 split (at
least). Yes, there are some people who do prefer the Zsh default but
not many.

Seriously, submit yourself to one hour of the default Zsh completion,
I bet you will consider it torture.

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 58+ messages in thread

* Re: [PATCH 00/14] completion: a bunch of updates
  2020-11-03  1:49                       ` Junio C Hamano
@ 2020-11-04  0:09                         ` Felipe Contreras
  2020-11-04 18:08                           ` Junio C Hamano
  0 siblings, 1 reply; 58+ messages in thread
From: Felipe Contreras @ 2020-11-04  0:09 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Stefan Haller, Git, SZEDER Gábor,
	Nguyễn Thái Ngọc Duy

On Mon, Nov 2, 2020 at 7:49 PM Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:

> > There's a reason people prefer to use Git's official completion, and
> > there's a reason I wrote the wrapper.
>
> Do you mean by "the wrapper" the 'gitfast' one you offer in the
> oh-my-zsh collection?  If so, yes, I agree that 'gitfast' as "maybe
> less complete but usably faster auto-completer" is a good thing for
> end-users to have as an alternative to the Zsh default.

No, I mean the Git Zsh wrapper that I wrote.

It's in three locations right now:

1. https://git.kernel.org/pub/scm/git/git.git/tree/contrib/completion/git-completion.zsh
2. https://github.com/ohmyzsh/ohmyzsh/blob/master/plugins/gitfast/_git
3. https://github.com/felipec/git-completion/blob/master/git-completion.zsh

All these are the same thing.

If git was installing this wrapper by default, there would be no need
for the gitfast plugin, nor the git-completion project.

> But that was not the point I was raising.  I was saying that it was
> not making life better for them that we are posing as a valid third
> choice in that competition.
>
> Your "wrapper" can still be updated regularly to the latest to grab
> the bash completion part from "git.git".  To the Zsh audience,
> however, it would be more straight-forward if the choices were
> "there is Zsh default completion, but if you want to use a
> different/better version, grab 'gitfast' from the oh-my-zsh
> collection" than "you can use Zsh default, or you can install
> 'gitfast' in the way Zsh users are accustomed to from the oh-my-zsh
> collection, or you can manually install from git.git".  After all,
> the latter two would give them the same thing.

The default is by definition not a choice. You make a choice to move
away from the default.

And the gitfast plugin is not different from what was in contrib, in
fact I wrote a script to update it directly from there [1] (I changed
it recently to fetch from the git-completion project).

> It may be a good idea to leave a message in contrib/completion that
> nudges people toward 'gitfast' as an alternative for thowe who want
> to use something other than the default Zsh autocompletion.

If we are going to leave a message it would have to be for
git-completion, since not all zsh users use Oh-MyZsh (maybe most
don't), and gitfast is nothing but a copy of git-completion.

> Doing so would remove one level of unnecessary middleman (that is
> us) from the picture and make it simpler for end-users by reducing
> the number of choices from three to two.

This would break the experience of Homebrew users, and possibly Linux
distributions that do package this wrapper by default.

But if you are not going to consider graduating the scripts, we might
as well break the experience for everyone, so everyone is at the same
level.

> > They prioritize completeness over usability.
> > ...
> > No. Those people don't care if their completion is unusable.
> > ...
> > This is not possible, as the Zsh maintainers don't care about usability.
>
> I think our code of conduct applies to derogatory statements made on
> even those who do not regularly appear in this community.

To state the opinion of a person is not a derogatory statement. This
is literally what they said:

> > > Now, how about you make a compromise between "correctness" and
> > > usability?
> >
> > No.

I'm not doing anything but repeating their stated opinion. It is a
fact. If you don't like their stated opinion, feel free to talk to
them.

Cheers.

[1] https://github.com/ohmyzsh/ohmyzsh/blob/d69bad8eb4157e5fd5c1a4ce98f93cf522477a8c/plugins/gitfast/update

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 58+ messages in thread

* Re: [PATCH 00/14] completion: a bunch of updates
  2020-11-04  0:09                         ` Felipe Contreras
@ 2020-11-04 18:08                           ` Junio C Hamano
  2020-11-05  1:09                             ` Felipe Contreras
  0 siblings, 1 reply; 58+ messages in thread
From: Junio C Hamano @ 2020-11-04 18:08 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Stefan Haller, Git, SZEDER Gábor,
	Nguyễn Thái Ngọc Duy

Felipe Contreras <felipe.contreras@gmail.com> writes:

> To state the opinion of a person is not a derogatory statement. This
> is literally what they said:
>
>> > > Now, how about you make a compromise between "correctness" and
>> > > usability?
>> >
>> > No.
>
> I'm not doing anything but repeating their stated opinion. It is a
> fact. If you don't like their stated opinion, feel free to talk to
> them.

I had to read the exchange three times to be reasonably confident
that the party that was asking the question was you and Zsh folks
was who said "No.", as there were so little in the context to go by,
in order to tell what was being discussed (I initially even thought
they asked the question and you gave a short "no", before realizing
it probably is the other way around).

In the short quote given without enough context, I cannot see
anything more than a disagreement of the degree of "correctness" and
"usability" expected by the two parties in the discussion.

Even if I knew what exact "incorrectness" and "usability" were on
topic back when you two argued, I know people strike balance at
different place.  Even though I may agree with your argument in that
particular case, I can understand (if not accept) if Zsh folks
thought differently.  And it does not matter if I agree with you
that they are better off taking a small "incorrectness" to gain
"usability"---the Zsh show is run over there by Zsh folks, and I am
not a participant.

But the take-away I got from your short quote was that I see no
evidence that Zsh folks do not care about usability.

^ permalink raw reply	[flat|nested] 58+ messages in thread

* Re: [PATCH 00/14] completion: a bunch of updates
  2020-11-04 18:08                           ` Junio C Hamano
@ 2020-11-05  1:09                             ` Felipe Contreras
  2020-11-05 22:09                               ` Junio C Hamano
  0 siblings, 1 reply; 58+ messages in thread
From: Felipe Contreras @ 2020-11-05  1:09 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Stefan Haller, Git, SZEDER Gábor,
	Nguyễn Thái Ngọc Duy

On Wed, Nov 4, 2020 at 12:08 PM Junio C Hamano <gitster@pobox.com> wrote:

> But the take-away I got from your short quote was that I see no
> evidence that Zsh folks do not care about usability.

This exchange happened 9 years ago, so I wouldn't place too much of a
burden on what they actually meant by what they said.

Even if my interpretation of what they said at that point of time is
100% incorrect; it's not a *derogatory* statement; it would simply be
an unfactual statement.

I will contact them again with some fixes to their code, but not just yet.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 58+ messages in thread

* Re: [PATCH 00/14] completion: a bunch of updates
  2020-11-05  1:09                             ` Felipe Contreras
@ 2020-11-05 22:09                               ` Junio C Hamano
  0 siblings, 0 replies; 58+ messages in thread
From: Junio C Hamano @ 2020-11-05 22:09 UTC (permalink / raw)
  To: Felipe Contreras
  Cc: Stefan Haller, Git, SZEDER Gábor,
	Nguyễn Thái Ngọc Duy

Felipe Contreras <felipe.contreras@gmail.com> writes:

> On Wed, Nov 4, 2020 at 12:08 PM Junio C Hamano <gitster@pobox.com> wrote:
>
>> But the take-away I got from your short quote was that I see no
>> evidence that Zsh folks do not care about usability.
>
> This exchange happened 9 years ago, so I wouldn't place too much of a
> burden on what they actually meant by what they said.

In other words, you are now saying that it does not demonstrate your
earlier claim that they do not care about usability at all?

> Even if my interpretation of what they said at that point of time is
> 100% incorrect; it's not a *derogatory* statement; it would simply be
> an unfactual statement.

So, the short quote given without much context was an attempt to
mislead those who are reading this discussion?  That sounds even
worse to me.

In any case, so far you managed to convince me even less that it
would help the Zsh userbase to carry a "by default has to fall
behind" copy of what they can get, or their distro packagers can
package, the latest and greatest directly from your github
repository as part of the release of this project.

Thanks, and bye for now.

^ permalink raw reply	[flat|nested] 58+ messages in thread

end of thread, other threads:[~2020-11-05 22:09 UTC | newest]

Thread overview: 58+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-21 22:30 [PATCH 00/14] completion: a bunch of updates Felipe Contreras
2019-06-21 22:30 ` [PATCH 01/14] completion: zsh: fix __gitcomp_direct() Felipe Contreras
2019-06-22 15:03   ` Felipe Contreras
2019-06-21 22:30 ` [PATCH 02/14] completion: zsh: fix for directories with spaces Felipe Contreras
2019-06-21 22:30 ` [PATCH 03/14] completion: remove zsh hack Felipe Contreras
2019-06-21 22:30 ` [PATCH 04/14] completion: zsh: improve main function selection Felipe Contreras
2019-06-21 22:30 ` [PATCH 05/14] completion: prompt: fix color for Zsh Felipe Contreras
2019-06-21 22:30 ` [PATCH 06/14] completion: bash: cleanup cygwin check Felipe Contreras
2019-06-21 22:31 ` [PATCH 07/14] completion: zsh: update installation instructions Felipe Contreras
2019-06-21 22:31 ` [PATCH 08/14] completion: bash: remove old compat wrappers Felipe Contreras
2019-06-21 22:31 ` [PATCH 09/14] completion: bash: remove zsh wrapper Felipe Contreras
2019-06-21 22:31 ` [PATCH 10/14] completion: zsh: trivial cleanups Felipe Contreras
2019-06-21 22:31 ` [PATCH 11/14] test: completion: tests for __gitcomp regression Felipe Contreras
2019-07-03 17:38   ` Junio C Hamano
2019-07-03 17:49   ` SZEDER Gábor
2019-06-21 22:31 ` [PATCH 12/14] test: completion: use global config Felipe Contreras
2019-07-03 17:22   ` Junio C Hamano
2019-06-21 22:31 ` [PATCH 13/14] completion: add default options Felipe Contreras
2019-06-22  3:01   ` Duy Nguyen
2019-06-22  4:36     ` Felipe Contreras
2019-06-24 17:22     ` Junio C Hamano
2019-06-25  1:38       ` Felipe Contreras
2019-06-25  3:32         ` Duy Nguyen
2019-06-21 22:31 ` [PATCH 14/14] completion: add default merge strategies Felipe Contreras
2019-06-24 17:23   ` Junio C Hamano
2019-06-25  1:11     ` Felipe Contreras
2019-06-25  1:43       ` Junio C Hamano
2019-07-03 17:14       ` SZEDER Gábor
2019-07-03 17:50 ` [PATCH 00/14] completion: a bunch of updates Junio C Hamano
2019-07-03 19:06   ` SZEDER Gábor
2020-10-25  3:51     ` Felipe Contreras
2020-10-25  3:46   ` Felipe Contreras
2020-10-27 20:23     ` Junio C Hamano
2020-10-27 22:19       ` Felipe Contreras
2020-10-27 23:32         ` Junio C Hamano
2020-10-28  0:06           ` Felipe Contreras
2020-10-28  9:09             ` Stefan Haller
2020-10-28 16:31               ` Felipe Contreras
2020-10-28 17:34                 ` Stefan Haller
2020-10-29 17:16                 ` Junio C Hamano
2020-10-29 17:27                   ` Junio C Hamano
2020-11-02 20:18                     ` Felipe Contreras
2020-11-03  1:49                       ` Junio C Hamano
2020-11-04  0:09                         ` Felipe Contreras
2020-11-04 18:08                           ` Junio C Hamano
2020-11-05  1:09                             ` Felipe Contreras
2020-11-05 22:09                               ` Junio C Hamano
2020-10-30  8:01                   ` Stefan Haller
2020-10-30 17:19                     ` Junio C Hamano
2020-11-02 20:29                       ` Felipe Contreras
2020-11-02 23:05                         ` Aaron Schrab
2020-11-03  1:35                           ` Junio C Hamano
2020-11-03 23:46                             ` Felipe Contreras
2020-11-03 22:37                           ` Felipe Contreras
2020-11-03  9:59                       ` Stefan Haller
2020-11-03 17:12                         ` Junio C Hamano
2020-11-03 20:07                           ` Stefan Haller
2020-11-02 19:18                   ` Felipe Contreras

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).