[PATCH] Fix a logic bug in _zle

[PATCH] Fix a logic bug in _zle

[Marlon Richert]

[-- 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)

Re: [PATCH] Fix a logic bug in _zle

[Bart Schaefer]

For purposes of the commit log, we should be told what this means in
terms of a behavior change.

Re: [PATCH] Fix a logic bug in _zle

[Marlon Richert]

[-- 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)

Re: [PATCH] Fix a logic bug in _zle

[Bart Schaefer]

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?

Re: [PATCH] Fix a logic bug in _zle

[Marlon Richert]

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.

Re: [PATCH] Fix a logic bug in _zle

[Lawrence Velázquez]

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