dash.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Martijn Dekker <martijn@inlv.org>
To: Harald van Dijk <harald@gigawatt.nl>, dash@vger.kernel.org
Subject: Re: [PATCH] quote arguments in xtrace output
Date: Mon, 27 Feb 2017 21:08:10 +0100	[thread overview]
Message-ID: <6e8434e8-87e3-db2a-9d80-147f064c4793@inlv.org> (raw)
In-Reply-To: <f9c5d0a5-69bc-c543-7de8-59e5e9cb105b@gigawatt.nl>

Op 27-02-17 om 20:24 schreef Harald van Dijk:
> The general approach looks nice. I think this misses a few cases though
> if you're aiming to make it suitable for re-entry:
>   $ src/dash -c 'set -x; \!'
>   + !
>   src/dash: 1: !: not found
>   $ src/dash -c 'set -x; \a=b'
>   + a=b
>   src/dash: 1: a=b: not found
>   $ src/dash -c 'set -x; \if'
>   + if
>   src/dash: 1: if: not found
> I could create binaries by those names to get rid of the errors.
> Regardless, in none these cases would the original command be
> re-executed if the set -x output were entered into the shell.

Good point.

The first and second cases would be fixed by simply removing "!" and "="
from SHELLSAFECHARS. Not a big loss.

Shell reserved words (a.k.a. shell keywords) such as "if" are harder to
fix. The single_quote() routine, if told to quote conditionally, would
have to iterate through the internal list of shell reserved words and
quote the string if it matches one of those. I will see if I can make
that happen.

Actually, after testing bash, yash, zsh, ksh93 and mksh as well as dash,
it looks like no current shell manages to quote strings that would
otherwise be shell keywords in xtrace output, with the exception of the
"!" keyword. So fixing this would make dash's xtrace output better than
any other shell's. :)

Since shell reserved words are part of the grammar, they never show up
in xtrace output, so at least the output is unambiguous: you know
anything that looks like a reserved word isn't.

> It may be okay to leave this as it is and acknowledge that it doesn't
> handle all cases, as long as it handles enough of them to be an
> improvement.

Yes. I would still remove "!" and "=" from SHELLSAFECHARS though; at
least that way you won't get output that looks like a negation or
assignment but isn't.

> If the modification to single_quote can handle all cases (perhaps
> quoting unnecessarily if it's unclear), then the conditional
> parameter could be removed and applied unconditionally: as far as I
> can tell, there are no existing calls that require the single quotes
> for simple words.

True, but this would change the output of something like:

$ dash -c 'alias stuff=xyz; trap true EXIT; alias; trap'


trap -- 'true' EXIT


trap -- true EXIT

While there is technically nothing wrong with this, I think it's nicer
to leave those quoted unconditionally, because that facilitates editing
the output of those commands to feed it back to the shell.

- M.

  reply	other threads:[~2017-02-27 22:05 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-27  6:22 Martijn Dekker
2017-02-27 19:24 ` Harald van Dijk
2017-02-27 20:08   ` Martijn Dekker [this message]
2017-02-27 21:34     ` Harald van Dijk
2017-02-27 23:17     ` Martijn Dekker
2017-02-28  3:39       ` Martijn Dekker
2017-03-18  4:31         ` Martijn Dekker

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:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6e8434e8-87e3-db2a-9d80-147f064c4793@inlv.org \
    --to=martijn@inlv.org \
    --cc=dash@vger.kernel.org \
    --cc=harald@gigawatt.nl \
    --subject='Re: [PATCH] quote arguments in xtrace output' \


* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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