On Fri, Sep 4, 2015 at 2:34 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Gábor Bernát <bernat@xxxxxxxxxxxxxx> writes:
>> +echo $(date +%s) | grep -q '^[0-9]+$'; 2>/dev/null && show_seconds=t
>
> That is very strange construct. I think you meant to say something
> like
>
> if date '+%s' 2>/dev/null | grep -q '^[0-9][0-9]*$'
> then
> show_seconds=t
> else
> show_seconds=
> fi
The final format suggested[1] for this test was:
{ echo $(date +%s) | grep -q '^[0-9][0-9]*$'; } 2>/dev/null &&
show_eta=t
> A handful of points:
>
> * "echo $(any-command)" is suspect, unless you are trying to let
> the shell munge output from any-command, which is not the case.
Primarily my fault. I don't know what I was thinking when suggesting that.
> * "grep" without -E (or "egrep") takes BRE, which "+" (one or more)
> is not part of.
This seems to have mutated from the suggested form.
> * That semicolon is a syntax error. I think whoever suggested you
> to use it meant to squelch possible errors from "date" that does
> not understand the "%s" format.
This also mutated. The suggested form wanted to suppress errors from
'date' if it complained about "%s", and from 'grep'. In retrospect,
applying it to 'grep' is questionable. I was recalling this warning
from the Autoconf manual[2]:
Some of the options required by Posix are not portable in
practice. Don't use ‘grep -q’ to suppress output, because many
grep implementations (e.g., Solaris) do not support -q. Don't use
‘grep -s’ to suppress output either, because Posix says -s does
not suppress output, only some error messages; also, the -s
option of traditional grep behaved like -q does in most modern
implementations. Instead, redirect the standard output and
standard error (in case the file doesn't exist) of grep to
/dev/null. Check the exit status of grep to determine whether it
found a match.
however, Git tests use 'grep -q' heavily, so perhaps we don't worry about that.
> * I do not think you are clearing show_seconds to empty anywhere,
> so an environment variable the user may have when s/he starts
> filter-branch will seep through and confuse you.
The empty assignment was implied in my example, but I should have been
more explicit and shown a more complete snippet:
show_eta=
...
{ echo $(date +%s) | grep -q '^[0-9][0-9]*$'; } 2>/dev/null &&
show_eta=t
The suggested 'if' form has the attribute of being clearer.
[1]: http://thread.gmane.org/gmane.comp.version-control.git/276531/focus=276837
[2]: https://www.gnu.org/software/autoconf/manual/autoconf.html#grep
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html