Re: quoting of strings in errors

[Pádraig Brady]

On 02/11/15 15:16, Jim Meyering wrote:
> On Mon, Nov 2, 2015 at 6:33 AM, Pádraig Brady <address@hidden> wrote:
>> On 28/10/15 18:19, Jim Meyering wrote:
>>> On Wed, Oct 28, 2015 at 10:30 AM, Pádraig Brady <address@hidden> wrote:
>>>> On 28/10/15 17:01, Jim Meyering wrote:
>>>>> On Wed, Oct 28, 2015 at 6:18 AM, Pádraig Brady <address@hidden> wrote:
>>>>>> seq 10 | shuf --random-source="blah"$'\r'
>>>>>
>>>>> Thank you for pursuing this.
>>>>> Properly quoting unusual names like those is definitely welcome,
>>>>
>>>> Cool. At least with this patch, the quoting is consistent across all utils.
>>>> I.E. we were already using quote() in most places.
>>>>
>>>>> however, in the remaining 99% of use cases, I find the added quotes
>>>>> to be most unwelcome: at least two extra bytes per line, in addition to
>>>>> the common hassles with multi-byte rendering.
>>>>>
>>>>> What do you think about a mode that quotes only when necessary?
>>
>> Note one caveat with not using quotes by default, is it
>> might result in errors that are harder to understand.
>> For example the problematic item is not immediately
>> obvious in the following message:
>>
>>   $ fname="file"
>>   $ fmt "$fname"
>>   fmt: cannot open file for reading: No such file or directory
>>
>> By using shell_escape_always rather than shell_escape, we
>> still get the non multi-byte quoting and copy/paste benefits?
>>
>>   $ fname="file"
>>   $ fmt "$fname"
>>   fmt: cannot open 'file' for reading: No such file or directory
>>
>> What do you think?
> 
> In a context like that, where there is no delimiter already,
> I agree completely that the quotes are welcome.
> The cases where I prefer to omit the quotes are where the
> name is already delimited by ": " on one side and ":" on the other:
> 
>   $ touch k; chmod 0 k; cat k
>   cat: k: Permission denied
> 
> Hmm... not trivial, I'm sure, but I'll bet one could automate the
> conversion and add a syntax-check rule to keep things consistent.

Done within this set of 8 related patches, available at:
http://www.pixelbeat.org/patches/coreutils/coreutils-filename-escaping.patch


commit 09f795be42120e4047b438d36d3771e5e810e9f9
Author: Pádraig Brady <address@hidden>
Date:   Wed Oct 28 13:02:31 2015 +0000

    all: replace most uses of quotearg_colon() with quote()

    Related to commit v8.24-61-g6796698 this provides
    more consistent quoting, as quotearg_colon() defaults
    to "literal" quoting by default, while quote()
    provides appropriate quoting for diagnostics by default.

    * gl/modules/randread: Depend on quote module rather than quotearg.
    * gl/lib/randread.c: Used quote() not quotearg_colon().
    * src/: Likewise.
    * src/shred.c: Likewise. Also avoid unnecessary quoting
    introducing overhead when wiping names.
    * cfg.mk: Relax the matching expression to allow
    "qname" variables as used in shred.c to satisfy the check.
    * tests/: Adjust accordingly.

commit dc7514324fddd54945b8f900466f19206ec961be
Author: Pádraig Brady <address@hidden>
Date:   Sun Nov 1 18:48:22 2015 +0000

    md5sum: ensure a single status line per file

    * src/md5sum.c: Use the same file name escaping method used
    when generating and checking checksums.  I.E. ensure a single line
    per file by starting the line with '\' for any file name containing '\n'
    and replacing those with "\\n".
    * NEWS: Move the item from changes in behavior to improvements,
    since this is no longer a backwards incompat change when
    processing stdout status messages.
    * tests/misc/md5sum.pl: Remove quotes from expected status output.
    * tests/misc/sha1sum.pl: Likewise.

commit 6b1c4b6a729bdfaf406b470390a39b2f4e9ae7c5
Author: Pádraig Brady <address@hidden>
Date:   Mon Nov 2 12:43:33 2015 +0000

    build: update gnulib submodule to latest

    Includes support for "shell-escape" from quotearg

commit 2dd90a80dd882de49191af38e61d105ae9dc3f60
Author: Pádraig Brady <address@hidden>
Date:   Sun Nov 1 18:53:26 2015 +0000

    all: avoid quoting file names when possible

    Quote file names using the "shell-escape" or "shell-escape-always"
    methods, which quote as appropriate for most shells,
    and better support copy and paste of presented names.
    The "always" variant is used when the file name is
    embedded in an error message with surrounding spaces.

    * cfg.mk (sc_error_shell_quotes): A new syntax check rule
    to suggest quotef() where appropriate.
    (sc_error_shell_always_quotes): Likewise for quoteaf().
    * src/system.h (quotef): A new define to apply shell quoting
    when needed.  I.E. when shell character or ':' is present.
    (quoteaf): Likewise, but always quote.
    * src/*.c: Use quotef() and quoteaf() rather than quote()
    where appropriate.
    * tests/: Adjust accordingly.

commit 880184def22fd789cfdba692cde3270cdf6c9a88
Author: Pádraig Brady <address@hidden>
Date:   Tue Nov 3 09:49:50 2015 +0000

    test: use consistent quoting

    * src/test.c (test_syntax_error): Reuse verror() rather than
    open coding the error output format.
    (term): Don't hardcode '' quoting.
    (main): Likewise.

commit 32fd2e129af4dddf146ca15328de4798cde9c5c8
Author: Pádraig Brady <address@hidden>
Date:   Tue Nov 3 10:13:11 2015 +0000

    ls: document and test new shell-escape quoting

    * doc/coreutils.texi (ls invocation): Describe the new
    'shell-escape' and 'shell-escape-always' quoting options.
    * src/ls.c (usage): Mention the new quoting options.
    * tests/misc/ls-misc.pl: Add a test for 'shell-escape'

commit a7f82a2b3fb2cd3c8a80dd61816d601068c2f70f
Author: Pádraig Brady <address@hidden>
Date:   Tue Nov 3 11:07:06 2015 +0000

    ls: avoid redundant processing when already escaping

    This is mainly noticeable when the multi-byte code
    within ls.c is triggered by multi-byte quotes.

    $ seq 200000 | xargs touch
    $ time ls-old -U --quoting=locale --hide-control-chars >/dev/null
    real    0m0.483s
    $ time ls-new -U --quoting=locale --hide-control-chars >/dev/null
    real    0m0.430s

    * src/ls.c (quote_name): Avoid rescanning the output looking for
    unprintable chars when we know the quoting mode already escapes them.
    * tests/misc/ls-misc.pl: Add tests for all quoting modes, with and
    without -q, to verify this assumption.

commit b446faf8288b7ebc1868d4f5ff8d8df001a01d60
Author: Pádraig Brady <address@hidden>
Date:   Tue Nov 3 12:56:22 2015 +0000

    printf: support the %q format to quote for shell

    * src/printf.c (usage): Mention the new format.
    (print_formatted): Handle the quoting by calling
    out to the quotearg module with "shell-escape" mode.
    * doc/coreutils.texi (printf invocation): Document %q.
    * tests/misc/printf-quote.sh: New test.
    * tests/local.mk: Reference new test.
    * NEWS: Mention the new feature.