Re: [PATCH] don't exit shell on [[ -o invalid@option ]]

[Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx>]

Martijn Dekker wrote on Tue, Nov 14, 2017 at 15:22:35 +0200:
> Op 14-11-17 om 14:26 schreef Daniel Shahaf:
> > As Bart hinted, the proposed change is backwards *incompatible*: it
> > makes [[ -o invalid@option ]] return 1 where currently it returns 2.
> 
> Actually, it exits/aborts the shell with status 2. This is an essential
> difference from simply returning status 2.
> 

Good point, I'd overlooked that.

> > Moreover, if cross-version compatibility is the goal, why is it a good
> > thing to lump "This shell does not have INVALID_OPTION" and "This shell
> > has INVALID_OPTION and it's unset"?
> 
> Because
> (a) that's how all other [[ -o ... ]] implementations work,
> (b) that's what the current documentation in zshmisc.1 says that [[ -o
> ...] does, and
> (c) the canonical way to check if an option exists
>    if (set +o someoption) 2>/dev/null; then ...
> works fine on all shells.
> 
> Re (b), zshmisc.1 simply says that [[ -o ... ]] checks if the option is
> on. It says nothing about checking if the option exists, much less
> aborting the shell if it doesn't.

(a) That's a fair point, but I'm not convinced it swings the balance.
(I agree that being compatible with bash/ksh is a plus.)

(b) zshmisc.1 says "true if the named option var(option) is on".  It
does not say that it's valid for var(option) not to be an option name at
all.  Therefore, «[[ -o not_an_option ]]» is undefined behaviour; the
documentation does NOT promise that it would behave identically to [[ -o
an_unset_option ]].  In other words, the current behaviour is consistent
with the documentation.

(c) I don't follow your argument.  Okay, so «(set +o option)» works,
what does that have to do with the behaviour of [[ -o not_an_option ]]?

> > It's easy to imagine a situation in
> > which that'd be a bug: if INVALID_OPTION was added in zsh version N, is
> > set by default, and a plugin that was developed against version N is
> > installed by a user running version N-1.
> 
> I don't see how that would introduce a bug. If a new option NEW_OPTION
> is introduced in zsh version N, default on, then [[ -o NEW_OPTION ]] can
> be used to run code dependent on that new option only if that new option
> is on. That code will then never be run on version N-1, which is what is
> expected.
> 

You're considering the "if" branch, I'm considering the "else" branch:

    if [[ -o NEW_OPTION ]]; then lorem; else ipsum; fi

"lorem" will only run on zsh vN when the option is set, but "ipsum" will
run under vN with the option explicitly unset by the user AND under
vN-1.  Since the option is on by default, vN-1 should take the "then"
branch.

> > With the current code that
> > situation would result in a (proper) warning.
> 
> A mere warning would be fine, but what actually happens is that the
> shell aborts with an error message.
> 

Okay, so how about if we demoted the fatal error to a warning?  Like
this:

    % [[ -o not_an_option ]] || echo This gets run
    zsh: [[: no such option: not_an_option
    This gets run
    % 

> > Devil's advocate, but why can't people just do, today,
> > 
> >     if [[ -o INVALID_OPTION ]] 2>/dev/null; then
> 
> Because, on zsh (unlike bash and *ksh) that will cause the shell to
> exit, as in, the program aborts. Not only that, adding 2>/dev/null will
> cause the program to abort silently, because the error message is
> suppressed.
> 

If the fatal error were a warning, the 2> redirection would hide [['s
warning but the program would continue.

> You can of course do
> 
>     if ([[ -o INVALID_OPTION ]]) 2>/dev/null; then
> 
> but that comes at the cost of forking a subshell, so that had better not
> be within a loop with many iterations.
> 
> >     if [[ ${options[invalidoption]:-off} == off ]]; then
> 
> That's fine if the script needs to work on zsh only. Not very intuitive,
> though.

The «:-off» is just there in case 'set -u' is in effect.  If that's not
a concern, the code would be «[[ ${options[invalidoption]} == on ]]»,
which may be somewhat more bearable.

Cheers,

Daniel