coreutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: env: add -S option (split string for shebang lines in scripts)


From: Pádraig Brady
Subject: Re: env: add -S option (split string for shebang lines in scripts)
Date: Mon, 23 Apr 2018 20:38:27 -0700
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0

On 22/04/18 03:25, Assaf Gordon wrote:
> Hello,
> 
> Following recent discussion about shebang lines [1][2],
> attached is a patch to add "env -S", with the same syntax as freebsd's 
> env(1).
> 
> [1] https://lists.gnu.org/r/coreutils/2017-05/msg00020.html
> [2] https://lists.gnu.org/r/bug-coreutils/2018-04/msg00031.html
> 
> Also included a small patch adding "--debug/-v" option.
> 
> It includes tests with close to %100 coverage, but no documentation yet.
> If you think this is a good addition, I'll update the patch with 
> documentation.

This is quite useful functionality to have in a standard tool.
Thanks a lot for implementing it.

I'd rather be consistent with other utils and just use the long --debug form.
Oh I see, the FreeBSD version supports -v. So in that case I agree with
supporting the short option also.

Please add an extra space after "--split-string=S " so that the man page
will be formatted appropriately.

There is no need to specify "inline" on functions generally
(unless defining in headers). Best let the compiler sort it out.

The error() for --debug may need translations or use
a debug() wrapper to avoid the syntax check

In scan_varname(), I think we should we restrict to:
s/isalpha/c_isalpha/
s/isalnum/c_isalnum/

I see the test script identifies the case where the OS does split
and handles spaces in the src dir. Excellent.

I noticed some tweaks for the test which are attached.

------ some general notes -----

I see that if one wants to use other options in combo with -S
they need to be part of the -S option. I.E. after -S.
It would be good to illustrate that in the docs,
or perhaps issue a warning if -S comes after other options?

For example it looks buggy that -u TERM
is just ignored in this example:

  src/env -v -u TERM -S sh -c "echo \$TERM"

We should at least warn about this case, but maybe error out.

Another gotcha is that portably specifying -v
with -S (i.e. with only one option) requires:
  src/env '-vS sh -c "echo \$TERM"'

So I would document two forms in the man page/--help like:

  env        [OPTION]... [-] [NAME=VALUE]... [COMMAND [ARG]...]
  env -[v]S '[OPTION]... [-] [NAME=VALUE]... [COMMAND [ARG]...]'

and make it clear usage() that -S is only needed with shebang lines.
The quotes above are important to document too,
as otherwise consistent processing (like interpolation etc.) wouldn't
be done on operating systems that did split shebangs
(or when used in a standard command invocation).

thanks!
Pádraig

Attachment: env-S-test-tweaks.diff
Description: Text Data


reply via email to

[Prev in Thread] Current Thread [Next in Thread]