* [PATCH] Fix a logic bug in _zle
@ 2021-05-31 8:50 Marlon Richert
2021-05-31 17:08 ` Bart Schaefer
0 siblings, 1 reply; 6+ messages in thread
From: Marlon Richert @ 2021-05-31 8:50 UTC (permalink / raw)
To: Zsh hackers list
[-- Attachment #1: Type: text/plain, Size: 16 bytes --]
See attachment.
[-- Attachment #2: 0001-Fix-a-logic-bug-in-_zle.txt --]
[-- Type: text/plain, Size: 1033 bytes --]
From 1b0bf4e25de818894e4374a7bad4e07f2f2a5e48 Mon Sep 17 00:00:00 2001
From: Marlon Richert <marlon.richert@gmail.com>
Date: Mon, 31 May 2021 11:46:21 +0300
Subject: [PATCH] Fix a logic bug in _zle
---
Completion/Zsh/Command/_zle | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/Completion/Zsh/Command/_zle b/Completion/Zsh/Command/_zle
index e01d0a943..0b8ef7a15 100644
--- a/Completion/Zsh/Command/_zle
+++ b/Completion/Zsh/Command/_zle
@@ -51,11 +51,11 @@ case "$state[1]" in
;;
(widget*)
_wanted -C "$context[1]" widgets expl "${state_descr[1]:-widget}" _widgets && ret=0
- ;&
- (function)
- [[ $state[1] != *function ]] || # Handle fall-through
+ ;|
+ (*function)
_wanted -C "$context[1]" functions expl 'widget shell function' \
- compadd -M 'r:|-=* r:|=*' -k functions && ret=0
+ compadd -M 'r:|-=* r:|=*' -k functions &&
+ ret=0
;;
(comp-widget)
_wanted -C "$context[1]" widgets expl 'completion widget' \
--
2.30.1 (Apple Git-130)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix a logic bug in _zle
2021-05-31 8:50 [PATCH] Fix a logic bug in _zle Marlon Richert
@ 2021-05-31 17:08 ` Bart Schaefer
2021-05-31 17:48 ` Marlon Richert
0 siblings, 1 reply; 6+ messages in thread
From: Bart Schaefer @ 2021-05-31 17:08 UTC (permalink / raw)
To: Marlon Richert; +Cc: Zsh hackers list
For purposes of the commit log, we should be told what this means in
terms of a behavior change.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix a logic bug in _zle
2021-05-31 17:08 ` Bart Schaefer
@ 2021-05-31 17:48 ` Marlon Richert
2021-05-31 21:56 ` Bart Schaefer
0 siblings, 1 reply; 6+ messages in thread
From: Marlon Richert @ 2021-05-31 17:48 UTC (permalink / raw)
To: Bart Schaefer; +Cc: Zsh hackers list
[-- Attachment #1: Type: text/plain, Size: 213 bytes --]
On Mon, May 31, 2021 at 8:08 PM Bart Schaefer <schaefer@brasslantern.com> wrote:
>
> For purposes of the commit log, we should be told what this means in
> terms of a behavior change.
Is it good enough this way?
[-- Attachment #2: 0001-Fix-a-bug-where-if-state-1-widget-function-_zle-woul.txt --]
[-- Type: text/plain, Size: 1087 bytes --]
From f3be8c5cefd5251ad9317fe5ca11a400f6e9ffe1 Mon Sep 17 00:00:00 2001
From: Marlon Richert <marlon.richert@gmail.com>
Date: Mon, 31 May 2021 20:45:58 +0300
Subject: [PATCH] Fix a bug where, if $state[1] == widget-function, _zle would
always return 0
---
Completion/Zsh/Command/_zle | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/Completion/Zsh/Command/_zle b/Completion/Zsh/Command/_zle
index e01d0a943..0b8ef7a15 100644
--- a/Completion/Zsh/Command/_zle
+++ b/Completion/Zsh/Command/_zle
@@ -51,11 +51,11 @@ case "$state[1]" in
;;
(widget*)
_wanted -C "$context[1]" widgets expl "${state_descr[1]:-widget}" _widgets && ret=0
- ;&
- (function)
- [[ $state[1] != *function ]] || # Handle fall-through
+ ;|
+ (*function)
_wanted -C "$context[1]" functions expl 'widget shell function' \
- compadd -M 'r:|-=* r:|=*' -k functions && ret=0
+ compadd -M 'r:|-=* r:|=*' -k functions &&
+ ret=0
;;
(comp-widget)
_wanted -C "$context[1]" widgets expl 'completion widget' \
--
2.30.1 (Apple Git-130)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix a logic bug in _zle
2021-05-31 17:48 ` Marlon Richert
@ 2021-05-31 21:56 ` Bart Schaefer
2021-06-01 8:53 ` Marlon Richert
0 siblings, 1 reply; 6+ messages in thread
From: Bart Schaefer @ 2021-05-31 21:56 UTC (permalink / raw)
To: Marlon Richert; +Cc: Zsh hackers list
On Mon, May 31, 2021 at 10:49 AM Marlon Richert
<marlon.richert@gmail.com> wrote:
>
> On Mon, May 31, 2021 at 8:08 PM Bart Schaefer <schaefer@brasslantern.com> wrote:
> >
> > For purposes of the commit log, we should be told what this means in
> > terms of a behavior change.
>
> Is it good enough this way?
Better, but you could have just said that in the email text rather
than embed it in the patch file.
However, I can't figure out when $state[1] == widget-function is ever
true. Did you possibly mean widget-or-function ? If not, can you
give an example?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix a logic bug in _zle
2021-05-31 21:56 ` Bart Schaefer
@ 2021-06-01 8:53 ` Marlon Richert
2021-07-18 22:31 ` Lawrence Velázquez
0 siblings, 1 reply; 6+ messages in thread
From: Marlon Richert @ 2021-06-01 8:53 UTC (permalink / raw)
To: Bart Schaefer; +Cc: Zsh hackers list
On Tue, Jun 1, 2021 at 12:56 AM Bart Schaefer <schaefer@brasslantern.com> wrote:
>
> On Mon, May 31, 2021 at 10:49 AM Marlon Richert
> <marlon.richert@gmail.com> wrote:
> >
> > On Mon, May 31, 2021 at 8:08 PM Bart Schaefer <schaefer@brasslantern.com> wrote:
> > >
> > > For purposes of the commit log, we should be told what this means in
> > > terms of a behavior change.
> >
> > Is it good enough this way?
>
> Better, but you could have just said that in the email text rather
> than embed it in the patch file.
Ah, OK. I thought you meant I should include it in the patch itself.
> However, I can't figure out when $state[1] == widget-function is ever
> true. Did you possibly mean widget-or-function ? If not, can you
> give an example?
Sorry, typo. Yes, I meant widget-or-function. But actually, I realize
now that I made further mistakes in my explanation yesterday evening.
I'll try to explain better.
What happens in the unpatched _zle code is this:
* On line 54, `;&` causes the (widget*) case to always proceed to the
(function) case.
* On line 56, `[[ $state[1] != *function ]] ||` checks to see whether
the flow should actually continue from (widget*) to (function).
* However,
* when `[[ $state[1] != *function ]]` is true,
* then we correctly skip the _wanted call on lines 57-58,
* but because the _wanted call is followed by `&& ret=0`
* and because || and && have equal precedence and are left associative,
* then we incorrectly do `ret=0`,
So, to correctly summarize the bug: When $state[1] == widget, _zle
always returns 0, instead of returning 0 for success and non-zero for
failure. The patch fixes this. If you could update the commit message
accordingly, that would be great.
Apologies for the confusion.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix a logic bug in _zle
2021-06-01 8:53 ` Marlon Richert
@ 2021-07-18 22:31 ` Lawrence Velázquez
0 siblings, 0 replies; 6+ messages in thread
From: Lawrence Velázquez @ 2021-07-18 22:31 UTC (permalink / raw)
To: zsh-workers; +Cc: Marlon Richert
On Tue, Jun 1, 2021, at 4:53 AM, Marlon Richert wrote:
> On Tue, Jun 1, 2021 at 12:56 AM Bart Schaefer <schaefer@brasslantern.com> wrote:
> >
> > On Mon, May 31, 2021 at 10:49 AM Marlon Richert
> > <marlon.richert@gmail.com> wrote:
> > >
> > > On Mon, May 31, 2021 at 8:08 PM Bart Schaefer <schaefer@brasslantern.com> wrote:
> > > >
> > > > For purposes of the commit log, we should be told what this means in
> > > > terms of a behavior change.
> > >
> > > Is it good enough this way?
> >
> > Better, but you could have just said that in the email text rather
> > than embed it in the patch file.
>
> Ah, OK. I thought you meant I should include it in the patch itself.
>
> > However, I can't figure out when $state[1] == widget-function is ever
> > true. Did you possibly mean widget-or-function ? If not, can you
> > give an example?
>
> Sorry, typo. Yes, I meant widget-or-function. But actually, I realize
> now that I made further mistakes in my explanation yesterday evening.
> I'll try to explain better.
>
> What happens in the unpatched _zle code is this:
> * On line 54, `;&` causes the (widget*) case to always proceed to the
> (function) case.
> * On line 56, `[[ $state[1] != *function ]] ||` checks to see whether
> the flow should actually continue from (widget*) to (function).
> * However,
> * when `[[ $state[1] != *function ]]` is true,
> * then we correctly skip the _wanted call on lines 57-58,
> * but because the _wanted call is followed by `&& ret=0`
> * and because || and && have equal precedence and are left associative,
> * then we incorrectly do `ret=0`,
>
> So, to correctly summarize the bug: When $state[1] == widget, _zle
> always returns 0, instead of returning 0 for success and non-zero for
> failure. The patch fixes this. If you could update the commit message
> accordingly, that would be great.
ping for further feedback on or committing of 48969
--
vq
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-07-18 22:32 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-31 8:50 [PATCH] Fix a logic bug in _zle Marlon Richert
2021-05-31 17:08 ` Bart Schaefer
2021-05-31 17:48 ` Marlon Richert
2021-05-31 21:56 ` Bart Schaefer
2021-06-01 8:53 ` Marlon Richert
2021-07-18 22:31 ` Lawrence Velázquez
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).