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

[Martijn Dekker <martijn@xxxxxxxx>]

Op 15-11-17 om 01:52 schreef Daniel Shahaf:
> Martijn Dekker wrote on Tue, Nov 14, 2017 at 15:22:35 +0200:
>> 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.)

I would note additionally that '[[' is a feature originally copied from
ksh, so to me it does not seem like a good idea to differ from ksh
behaviour without strong reasons.

> (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.

I don't follow that reasoning. By definition, a non-existent option
cannot be on, so the condition "true if the named option var(option) is
on" is always false for a non-existent option. Had it said "false if the
named option var(option) is off", your point would have made sense to me.

In either case, not documenting that the shell aborts on a non-existent
option is definitely an omission. In fact that omission makes me wonder
if that behaviour was intended in the first place.

> (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 ]]?

Nothing, except that there is already a well-known way of checking if an
option exists, so it's not really necessary for [[ -o ... ]] to have
that functionality, particularly if it comes at the expense of not being
compatible with ksh and bash.

>>> 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.

Fair point. However, zsh makes this easy to work around with its dynamic
'no-' prefix for all options.

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

[...]
> 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
>     % 

I think that would be fine, as well as returning status 2 along with it.

I do think the warning (and the status 2) should be suppressed if
POSIX_IDENTIFIERS or some other emulation-relevant shell option is
active, so that 'emulate ksh' and 'emulate sh' reproduce ksh behaviour.

Thanks,

- Martijn