Proposed patch for options.yo (was: Re: bug in single_command)

[Richard Hartmann <richih.mailinglist@gmail.com>]

First of all, sorry for the subject of the email. I wanted to change it
after I found out what the actual issue was but forgot. The body of the
mail changed quite substantially. Duh..

On Wed, Feb 11, 2009 at 19:13, Bart Schaefer <schaefer@brasslantern.com> wrote:

> I suggest changing "the command line" here to something like "in the
> flags supplied at invocation of the shell" to match terminology that
> is used elsewhere, e.g., the manual section "Invocation".

Very good point. I'll check against the docs that my terminology is
correct.


> It's true that "the command line" used elsewhere in the manual often
> refers to the equivalent of the ZLE input buffer.  On the other hand,
> it seems so obvious that "setopt single_command" would cause the shell
> to immediately exit (after all, it has now executed a single command!)
> that I hardly believe thsi is a source of significant confusion.

Hardly significant, agreed. But when reading docs, I go into pedantic
prick mode, looking for stuff to improve :p
Liberal in what you require, strict in what you provide and all that.


> A better question might be why these options are not mentioned in the
> "Invocation" section, since that's the only context in which they make
> sense, rather than being relegated to the "Single Letter Options".

I'll see where that fits and work it into the patch.


> Without looking, I'd suspect that the reason is that dosetopt() may at
> times be called from a context where emitting an error message is not
> appropriate.

No idea about appropriate (did not dig too deeply), but it _is_ used
without any checking of return values etc. Namely in

Src/builtin.c 540 & 541:

    dosetopt(VERBOSE, 0, 0);
    dosetopt(XTRACE, 0, 0);

All occurences in Src/init.c

Src/options.c 506:

static void
setoption(HashNode hn, int value)
{
    dosetopt(((Optname) hn)->optno, value, 0);
}


> In general it's bad programming to spew to the standard
> error stream at too low a level because it takes control away from the
> caller.

Agreed, but I didn't know it's too low when I wrote this (and I don't
think it's my place to decide those matters, anyway). That said, I
should have checked all places it is used in before suggesting direct
printing. Return values? Or passing back an actual string?


Richard