* Useless assignment in _rm
@ 2020-06-21 12:09 zsugabubus
2020-06-21 13:32 ` Daniel Shahaf
0 siblings, 1 reply; 5+ messages in thread
From: zsugabubus @ 2020-06-21 12:09 UTC (permalink / raw)
To: zsh-workers
Hi!
As much as I understand, the assignment is not needed because in the
next line the whole array will be reassigned.
It caused issues with `rm -r<TAB> anything`:
_rm:72: line: assignment to invalid subscript range
_rm:72: line: assignment to invalid subscript range
--
zsugabubus
diff --git a/Completion/Unix/Command/_rm b/Completion/Unix/Command/_rm
index ea9190d..82b382b 100644
--- a/Completion/Unix/Command/_rm
+++ b/Completion/Unix/Command/_rm
@@ -69,7 +69,6 @@ _arguments -C -s -S $opts \
case $state in
(file)
- line[CURRENT]=()
line=( ${line//(#m)[\[\]()\\*?#<>~\^\|]/\\$MATCH} )
_files -F line && ret=0
;;
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Useless assignment in _rm
2020-06-21 12:09 Useless assignment in _rm zsugabubus
@ 2020-06-21 13:32 ` Daniel Shahaf
2020-06-22 10:27 ` zsugabubus
0 siblings, 1 reply; 5+ messages in thread
From: Daniel Shahaf @ 2020-06-21 13:32 UTC (permalink / raw)
To: zsugabubus; +Cc: zsh-workers
zsugabubus wrote on Sun, 21 Jun 2020 14:09 +0200:
> It caused issues with `rm -r<TAB> anything`:
> _rm:72: line: assignment to invalid subscript range
> _rm:72: line: assignment to invalid subscript range
>
CURRENT is 0 at that point which causes the error. In «rm -r<TAB> -f
foo», CURRENT gets set to -1.
I suppose anything that uses «*::…» or «*:::…» should verify that
CURRENT is >=1 before using it?
> As much as I understand, the assignment is not needed because in the
> next line the whole array will be reassigned.
That's not quite right: the assignment uses $line. The patch causes
«rm foo<TAB>» when foo and foobar both exist to complete foobar;
without the patch the completion is considered ambiguous.
> diff --git a/Completion/Unix/Command/_rm b/Completion/Unix/Command/_rm
> index ea9190d..82b382b 100644
> --- a/Completion/Unix/Command/_rm
> +++ b/Completion/Unix/Command/_rm
> @@ -69,7 +69,6 @@ _arguments -C -s -S $opts \
>
> case $state in
> (file)
> - line[CURRENT]=()
> line=( ${line//(#m)[\[\]()\\*?#<>~\^\|]/\\$MATCH} )
> _files -F line && ret=0
> ;;
Possible further improvements here: use ${(b)} in the assignment and
use the «zstyle … ignore-line other» functionality rather than reinvent it.
However, as above, I think the fix to the bug is just to check CURRENT.
Cheers,
Daniel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Useless assignment in _rm
2020-06-21 13:32 ` Daniel Shahaf
@ 2020-06-22 10:27 ` zsugabubus
2020-06-23 12:40 ` Daniel Shahaf
0 siblings, 1 reply; 5+ messages in thread
From: zsugabubus @ 2020-06-22 10:27 UTC (permalink / raw)
To: Daniel Shahaf; +Cc: zsh-workers
On Sun, Jun 21, 2020 at 01:32:57PM +0000, Daniel Shahaf wrote:
> I suppose anything that uses «*::…» or «*:::…» should verify that
> CURRENT is >=1 before using it?
I searched for similar assignments and queries but I could not find
other place where an extra check would be needed.
(“$words[CURRENT]” is safe no?, because a negative number will just
index backwards so it gives the correct word; and out-of-bounds or zero
just returns nothing.)
> > As much as I understand, the assignment is not needed because in the
> > next line the whole array will be reassigned.
>
> That's not quite right: the assignment uses $line. The patch causes
> «rm foo<TAB>» when foo and foobar both exist to complete foobar;
> without the patch the completion is considered ambiguous.
You are absolutely right!!
--
zsugabubus
diff --git a/Completion/Unix/Command/_rm b/Completion/Unix/Command/_rm
index ea9190d..e66b77f 100644
--- a/Completion/Unix/Command/_rm
+++ b/Completion/Unix/Command/_rm
@@ -69,7 +69,7 @@ _arguments -C -s -S $opts \
case $state in
(file)
- line[CURRENT]=()
+ (( CURRENT > 0 )) && line[CURRENT]=()
line=( ${line//(#m)[\[\]()\\*?#<>~\^\|]/\\$MATCH} )
_files -F line && ret=0
;;
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Useless assignment in _rm
2020-06-22 10:27 ` zsugabubus
@ 2020-06-23 12:40 ` Daniel Shahaf
2020-06-28 13:13 ` Daniel Shahaf
0 siblings, 1 reply; 5+ messages in thread
From: Daniel Shahaf @ 2020-06-23 12:40 UTC (permalink / raw)
To: zsugabubus; +Cc: zsh-workers
zsugabubus wrote on Mon, 22 Jun 2020 12:27 +0200:
> diff --git a/Completion/Unix/Command/_rm b/Completion/Unix/Command/_rm
> index ea9190d..e66b77f 100644
> --- a/Completion/Unix/Command/_rm
> +++ b/Completion/Unix/Command/_rm
> @@ -69,7 +69,7 @@ _arguments -C -s -S $opts \
>
> case $state in
> (file)
> - line[CURRENT]=()
> + (( CURRENT > 0 )) && line[CURRENT]=()
> line=( ${line//(#m)[\[\]()\\*?#<>~\^\|]/\\$MATCH} )
> _files -F line && ret=0
> ;;
Thanks; I'll commit this unless someone sees a reason not to.
In the future, could you put patches _above_ the dash-dash-space line?
Many MUAs trim by default everything after that line.
Thanks,
Daniel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Useless assignment in _rm
2020-06-23 12:40 ` Daniel Shahaf
@ 2020-06-28 13:13 ` Daniel Shahaf
0 siblings, 0 replies; 5+ messages in thread
From: Daniel Shahaf @ 2020-06-28 13:13 UTC (permalink / raw)
To: zsugabubus; +Cc: zsh-workers
Daniel Shahaf wrote on Tue, 23 Jun 2020 12:40 +0000:
> zsugabubus wrote on Mon, 22 Jun 2020 12:27 +0200:
> > diff --git a/Completion/Unix/Command/_rm b/Completion/Unix/Command/_rm
> > index ea9190d..e66b77f 100644
> > --- a/Completion/Unix/Command/_rm
> > +++ b/Completion/Unix/Command/_rm
> > @@ -69,7 +69,7 @@ _arguments -C -s -S $opts \
> >
> > case $state in
> > (file)
> > - line[CURRENT]=()
> > + (( CURRENT > 0 )) && line[CURRENT]=()
> > line=( ${line//(#m)[\[\]()\\*?#<>~\^\|]/\\$MATCH} )
> > _files -F line && ret=0
> > ;;
>
> Thanks; I'll commit this unless someone sees a reason not to.
Applied, thanks.
I edited your vanity plate of an email address out. I'm afraid it
amounts to name-calling and as such has no place in official
documentation, which ChangeLog is.
Also, I forgot to say it before, thanks for auditing other uses of *::
and *:::.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-06-28 13:14 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-21 12:09 Useless assignment in _rm zsugabubus
2020-06-21 13:32 ` Daniel Shahaf
2020-06-22 10:27 ` zsugabubus
2020-06-23 12:40 ` Daniel Shahaf
2020-06-28 13:13 ` 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).