Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm
Precedence: bulk
Received-SPF: pass (ns1.primenet.com.au: SPF record at _spf.mail.yahoo.com designates 212.82.96.203 as permitted sender)
In-reply-to: <20160720065421.GE28939@tarsus.local2>
From: Oliver Kiddle <okiddle@yahoo.co.uk>
References: <160716185103.ZM5258@torch.brasslantern.com> <1468767614-9635-1-git-send-email-danielsh@tarsus.local2> <91351.1468835277@hydra.kiddle.eu> <20160720065421.GE28939@tarsus.local2>
To: Daniel Shahaf <d.s@daniel.shahaf.name>
cc: zsh-workers@zsh.org
Subject: Re: [PATCH v2] Re: [PATCH] _add-zle-hook-widget: New completion.
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <22467.1469114906.1@hydra.kiddle.eu>
Date: Thu, 21 Jul 2016 17:28:26 +0200
Message-ID: <22468.1469114906@hydra.kiddle.eu>

Daniel Shahaf wrote:
> This is just a coding style question; there are arguments for both
> sides.  What's the house style for completion functions, to have
> explicit 'return' statements or not to have them?
>
> I'll make the v3 iteration use the house style, whatever it is.

I don't claim to be the arbiter on whatever the house style is.
Most early completion code was written by Sven and I've tried to
be consistent with that. Looking over some examples now, he didn't
appear to use superfluous return statements.

In the end, it is more important to get the return status right,
however.

In the process of looking, I noticed that _normal appears to use a
ret variable without declaring it local. _main_complete does set
ret to 1 so this never actually matters. Furthermore, while checking
that this wasn't intentional I noticed a couple of functions that
use _normal where _default was meant. If you haven't got a compset
-q, -n or similar changes to words/CURRENT, _normal ends up being
a never ending recursive loop.

Oliver

diff --git a/Completion/Base/Core/_normal b/Completion/Base/Core/_normal
index 539b378..dd607d2 100644
--- a/Completion/Base/Core/_normal
+++ b/Completion/Base/Core/_normal
@@ -30,9 +30,9 @@ if [[ CURRENT -eq 1 ]]; then
   curcontext="${curcontext%:*:*}:-command-:"
 
   comp="$_comps[-command-]"
-  [[ -n "$comp" ]] && eval "$comp" && ret=0
+  [[ -n "$comp" ]] && eval "$comp" && return
 
-  return ret
+  return 1
 fi
 
 _set_command
diff --git a/Completion/Unix/Command/_iostat b/Completion/Unix/Command/_iostat
index 7dc33a1..6653a5d 100644
--- a/Completion/Unix/Command/_iostat
+++ b/Completion/Unix/Command/_iostat
@@ -129,4 +129,4 @@ if (( $#args )); then
   return
 fi
 
-_normal
+_default
diff --git a/Completion/Unix/Command/_top b/Completion/Unix/Command/_top
index 10c0e34..0259c23 100644
--- a/Completion/Unix/Command/_top
+++ b/Completion/Unix/Command/_top
@@ -98,4 +98,4 @@ if (( $#specs )); then
   return
 fi
 
-_normal
+_default