* 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).