Re: [PATCH v3 2/4] completion: introduce __gitcomp_nl_append ()

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH v3 2/4] completion: introduce __gitcomp_nl_append ()

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:59:32

Ramkumar Ramachandra [off-list ref] writes:
quoted hunk
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 51c2dd4..bf358d6 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -233,6 +233,19 @@ __gitcomp_nl ()
 	__gitcompadd "$1" "${2-}" "${3-$cur}" "${4- }"
 }
 
+# Variation of __gitcomp_nl () that appends to the existing list of
+# completion candidates, COMPREPLY.
+__gitcomp_nl_append ()
+{
+	local IFS=$'\n'
+	local i=${#COMPREPLY[@]}
+	for x in $1; do
+		if [[ "$x" == "$3"* ]]; then
+			COMPREPLY[i++]="$2$x$4"
+		fi
+	done
+}
Hmph. Why so much duplication with __gitcompadd, though.

I would have expected that this "append" behaviour to be done at the
lower level by introducing __gitcompappend that does not forcibly
truncate by starting from a hard-coded i=0, i.e. a collection of
small helper functions plus a single implementation of the logic to
push elements into COMPREPLY[] in __gitcompappend, perhaps like
these:

	__gitcompappend () {
		local i=${#COMPREPLY[@]}
		for x in $1; do
                        if [[ "$x" == "$3"* ]]; then
                                COMPREPLY[i++]="$2$x$4"
                        fi
		done
        }

	__gitcompadd () {
		COMPREPLY=()
		__gitcompappend "$@"
	}

	__gitcomp_nl_append () {
		local IFS=$'\n'
                __gitcompappend "$1" "${2-}" "${3-$cur}" "${4- }"
        }

	__gitcomp_nl () {
		COMPREPLY=()
                __gitcomp_nl_append "$@"		
        }
        
Is it because going this route and doing it at such a low level
would make zsh completion (which I have no clue about ;-)
unnecessarily complex?
quoted hunk
+
 # Generates completion reply with compgen from newline-separated possible
 # completion filenames.
 # It accepts 1 to 3 arguments:
diff --git a/contrib/completion/git-completion.zsh b/contrib/completion/git-completion.zsh
index 6fca145..6b77968 100644
--- a/contrib/completion/git-completion.zsh
+++ b/contrib/completion/git-completion.zsh
@@ -76,6 +76,14 @@ __gitcomp_nl ()
 	compadd -Q -S "${4- }" -p "${2-}" -- ${=1} && _ret=0
 }
 
+__gitcomp_nl_append ()
+{
+	emulate -L zsh
+
+	local IFS=$'\n'
+	compadd -Q -S "${4- }" -p "${2-}" -- ${=1} && _ret=0
+}
+
 __gitcomp_file ()
 {
 	emulate -L zsh

Re: [PATCH v3 2/4] completion: introduce __gitcomp_nl_append ()

From: Ramkumar Ramachandra <hidden>
Date: 2016-06-15 22:59:33

Junio C Hamano wrote:
Is it because going this route and doing it at such a low level
would make zsh completion (which I have no clue about ;-)
unnecessarily complex?
The zsh completion only cares to override __gitcomp_nl () and
__gitcomp_nl_append (), without bothering to re-implement the
lower-level functions; so it's no problem at all. I wrote mine out by
thinking of a non-intrusive direct translation, while your version
focuses on eliminating duplication. I don't have a strong preference
either way, so I will resubmit with your version.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help