* PATCH: following -C option in make completion [not found] <2024-1573077069.922525.ref@CD2K.zG8N.q383> @ 2019-11-06 21:51 ` Oliver Kiddle 2019-12-22 4:31 ` Daniel Shahaf 0 siblings, 1 reply; 4+ messages in thread From: Oliver Kiddle @ 2019-11-06 21:51 UTC (permalink / raw) To: Zsh workers In the make completion, there's some very old logic for picking out the argument to -C. _arguments makes this rather easier if we just look in $opt_args and has the advantage that we can handle -Cdir and --directory forms (not just -C dir). This patch also uses the computed $basedir value for our view of the GNU make $(CURDIR) macro so it will handle include files referenced relative to $(CURDIR). Previously $basedir was forced into absolute form which I don't think gains us anything - _files -W doesn't care. Neither does finding files to include. But perhaps this breaks things for some form or another. I'm also not sure what the (q) modifier achieved. I've used ${(Q)~opt_args... so it'll expand usernames and remove a level of quoting. opt_args also does some extra quoting for colons which could also be removed. We could do with some sort of safe-eval for where completion functions make use of bits of the command line - expanding variables and named directories is useful but command-substitutions not so. Oliver diff --git a/Completion/Unix/Command/_make b/Completion/Unix/Command/_make index 3dcf479c3..06971f07a 100644 --- a/Completion/Unix/Command/_make +++ b/Completion/Unix/Command/_make @@ -118,35 +118,9 @@ _make-parseMakefile () { done } -_make-findBasedir () { - local file index basedir - basedir=$PWD - for (( index=0; index < $#@; index++ )) - do - if [[ $@[index] == -C ]] - then - file=${~@[index+1]} 2>/dev/null - if [[ -z $file ]] - then - # make returns with an error if an empty arg is given - # even if the concatenated path is a valid directory - return - elif [[ $file == /* ]] - then - # Absolute path, replace base directory - basedir=$file - else - # Relative, concatenate path - basedir=$basedir/$file - fi - fi - done - print -- $basedir -} - _make() { - local prev="$words[CURRENT-1]" file expl tmp is_gnu incl match + local prev="$words[CURRENT-1]" file expl tmp is_gnu incl match basedir local context state state_descr line local -a option_specs local -A VARIABLES VAR_ARGS opt_args @@ -214,15 +188,18 @@ _make() { _arguments -s $option_specs \ '*:make target:->target' && ret=0 + basedir=${(Q)~opt_args[-C]:-${opt_args[--directory]}} + VAR_ARGS[CURDIR]="${basedir:=$PWD}" + case $state in (dir) _description directories expl "$state_descr" - _files "$expl[@]" -W ${(q)$(_make-findBasedir ${words[1,CURRENT-1]})} -/ && ret=0 + _files "$expl[@]" -W $basedir -/ && ret=0 ;; (file) _description files expl "$state_descr" - _files "$expl[@]" -W ${(q)$(_make-findBasedir $words)} && ret=0 + _files "$expl[@]" -W $basedir && ret=0 ;; (debug) @@ -239,11 +216,9 @@ _make() { file=${(v)opt_args[(I)(-f|--file|--makefile)]} if [[ -n $file ]] then - [[ $file == [^/]* ]] && file=${(q)$(_make-findBasedir $words)}/$file + [[ $file == [^/]* ]] && file=$basedir/$file [[ -r $file ]] || file= else - local basedir - basedir=${$(_make-findBasedir $words)} if [[ $is_gnu == gnu && -r $basedir/GNUmakefile ]] then file=$basedir/GNUmakefile @@ -287,7 +262,7 @@ _make() { _alternative \ 'targets:make target:compadd -Q -a TARGETS' \ 'variables:make variable:compadd -S = -F keys -k VARIABLES' \ - '*:file:_files' && ret=0 + '*:file:_files -W $basedir' && ret=0 fi esac ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: PATCH: following -C option in make completion 2019-11-06 21:51 ` PATCH: following -C option in make completion Oliver Kiddle @ 2019-12-22 4:31 ` Daniel Shahaf 2020-01-02 18:00 ` Oliver Kiddle 0 siblings, 1 reply; 4+ messages in thread From: Daniel Shahaf @ 2019-12-22 4:31 UTC (permalink / raw) To: Oliver Kiddle; +Cc: Zsh workers Good morning Oliver, Oliver Kiddle wrote on Wed, Nov 06, 2019 at 22:51:09 +0100: > In the make completion, there's some very old logic for picking out the > argument to -C. _arguments makes this rather easier if we just look in > $opt_args and has the advantage that we can handle -Cdir and --directory > forms (not just -C dir). > > This patch also uses the computed $basedir value for our view of the GNU > make $(CURDIR) macro so it will handle include files referenced relative > to $(CURDIR). > > Previously $basedir was forced into absolute form which I don't think > gains us anything - _files -W doesn't care. Neither does finding files > to include. But perhaps this breaks things for some form or another. > > I'm also not sure what the (q) modifier achieved. I've used > ${(Q)~opt_args... so it'll expand usernames and remove a level of > quoting. opt_args also does some extra quoting for colons which could > also be removed. We could do with some sort of safe-eval for where > completion functions make use of bits of the command line - expanding > variables and named directories is useful but command-substitutions not > so. I've ran into a regression, and reverting this patch fixes it. Using current master, and the following setup: % cd "$(mktemp -d)" % mkdir foo The following two work: % make -C <TAB> [offers foo] % make -C ./fo<TAB> [expands to ./foo] But the following does not: % make -C fo<TAB> [no matches] It does work if I revert this patch. WDYT? Cheers, Daniel ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: PATCH: following -C option in make completion 2019-12-22 4:31 ` Daniel Shahaf @ 2020-01-02 18:00 ` Oliver Kiddle 2020-01-03 19:51 ` Daniel Shahaf 0 siblings, 1 reply; 4+ messages in thread From: Oliver Kiddle @ 2020-01-02 18:00 UTC (permalink / raw) To: Daniel Shahaf; +Cc: Zsh workers Daniel Shahaf wrote: > % make -C fo<TAB> > [no matches] > > It does work if I revert this patch. The easy quick fix would be to use _directories for -C. That's only wrong if multiple -C options are used which is obscure enough that it didn't occur to me when I wrote the patch. The state is used for -C's argument itself because of that GNU specific feature of allowing -C to be repeatable. But after make -C <tab>, $opt_args[-C] is set to the empty string for the current -C option. Hence the breakage you observed. I think it is preferable to use $opt_args over scanning $words as in the original code. That'll only be wrong if the user mixes -C and --directory (the old code ignored --directory completely so I don't feel bad about that). The following patch changes the basedir assignment. The new expansion is especially gnarly so I'd appreciate if you could give it some testing. It has to: - (temporarily) convert quoted colons to nulls - remove one level of shell quoting - prepend $PWD: so the current directory is the default and to workaround problems with splitting giving a string not an array when there's no separator at all - split on colons and use (@) and double quotes to avoid losing a final empty element - expand named directories for each element - throw away initial elements if a later one is absolute - drop the last element for -C (but not -I) - turn nulls back into colons - join elements with / Oliver diff --git a/Completion/Unix/Command/_make b/Completion/Unix/Command/_make index 06971f07a..21ed56184 100644 --- a/Completion/Unix/Command/_make +++ b/Completion/Unix/Command/_make @@ -120,12 +120,12 @@ _make-parseMakefile () { _make() { - local prev="$words[CURRENT-1]" file expl tmp is_gnu incl match basedir + local prev="$words[CURRENT-1]" file expl tmp is_gnu incl match basedir nul=$'\0' local context state state_descr line local -a option_specs local -A VARIABLES VAR_ARGS opt_args local -aU TARGETS keys - local ret=1 + local -i cdir=-1 ret=1 # VAR=VAL on the current command line for tmp in $words; do @@ -142,7 +142,7 @@ _make() { incl="(-|)include" option_specs=( '(-B --always-make)'{-B,--always-make}'[unconditionally make all targets]' - '*'{-C,--directory=}'[change directory first]:change to directory:->dir' + '*'{-C,--directory=}'[change directory first]:change to directory:->cdir' '-d[print lots of debug information]' '--debug=-[print various types of debug information]:debug options:->debug' '(-e --environment-overrides)'{-e,--environment-overrides}'[environment variables override makefiles]' @@ -177,7 +177,7 @@ _make() { # Basic make options only. incl=.include option_specs=( - '-C[change directory first]:directory:->dir' + '-C[change directory first]:directory:->cdir' '-I[include directory for makefiles]:directory:->dir' '-f[specify makefile]:makefile:->file' '-o[specify file not to remake]:file not to remake:->file' @@ -188,11 +188,12 @@ _make() { _arguments -s $option_specs \ '*:make target:->target' && ret=0 - basedir=${(Q)~opt_args[-C]:-${opt_args[--directory]}} - VAR_ARGS[CURDIR]="${basedir:=$PWD}" + [[ $state = cdir ]] && cdir=-2 + basedir=${(j./.)${${~"${(@s.:.):-$PWD:${(Q)${opt_args[-C]:-$opt_args[--directory]}//\\:/$nul}}"}[(R)/*,cdir]}//$nul/:} + VAR_ARGS[CURDIR]="${basedir}" case $state in - (dir) + (*dir) _description directories expl "$state_descr" _files "$expl[@]" -W $basedir -/ && ret=0 ;; ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: PATCH: following -C option in make completion 2020-01-02 18:00 ` Oliver Kiddle @ 2020-01-03 19:51 ` Daniel Shahaf 0 siblings, 0 replies; 4+ messages in thread From: Daniel Shahaf @ 2020-01-03 19:51 UTC (permalink / raw) To: zsh-workers Oliver Kiddle wrote on Thu, Jan 02, 2020 at 19:00:41 +0100: > The following patch changes the basedir assignment. The new expansion is > especially gnarly so I'd appreciate if you could give it some testing. I've tested it and couldn't break it. Thanks for the patch. ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-01-03 19:53 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <2024-1573077069.922525.ref@CD2K.zG8N.q383> 2019-11-06 21:51 ` PATCH: following -C option in make completion Oliver Kiddle 2019-12-22 4:31 ` Daniel Shahaf 2020-01-02 18:00 ` Oliver Kiddle 2020-01-03 19:51 ` Daniel Shahaf
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/zsh/ 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 NNTP newsgroup(s).