From: Daniel Shahaf <d.s@daniel.shahaf.name>
To: Martijn Dekker <martijn@inlv.org>
Cc: Zsh hackers list <zsh-workers@zsh.org>
Subject: Re: [PATCH] don't exit shell on [[ -o invalid@option ]]
Date: Tue, 14 Nov 2017 23:52:49 +0000 [thread overview]
Message-ID: <20171114235249.egcynklamldfcogv@tarpaulin.shahaf.local2> (raw)
In-Reply-To: <d039a540-6850-86cf-8808-2daf8f85182e@inlv.org>
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
next prev parent reply other threads:[~2017-11-14 23:52 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-11-10 18:24 Martijn Dekker
2017-11-10 18:53 ` Eric Cook
2017-11-10 19:03 ` Martijn Dekker
2017-11-10 22:37 ` Bart Schaefer
2017-11-11 12:45 ` Peter Stephenson
2017-11-11 19:01 ` Martijn Dekker
2017-11-11 23:19 ` Bart Schaefer
2017-11-12 19:56 ` Peter Stephenson
2017-11-14 12:26 ` Daniel Shahaf
2017-11-14 13:22 ` Martijn Dekker
2017-11-14 23:52 ` Daniel Shahaf [this message]
2017-11-16 0:52 ` Martijn Dekker
2017-11-18 18:22 ` Daniel Shahaf
2017-11-19 14:46 ` Martijn Dekker
2017-11-19 19:41 ` Bart Schaefer
2017-11-19 19:53 ` Peter Stephenson
2017-11-20 1:22 ` Daniel Shahaf
2017-11-24 21:50 ` Daniel Shahaf
2017-12-03 7:28 ` Mikael Magnusson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20171114235249.egcynklamldfcogv@tarpaulin.shahaf.local2 \
--to=d.s@daniel.shahaf.name \
--cc=martijn@inlv.org \
--cc=zsh-workers@zsh.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).