Re: [PATCH 01/19] check_bindir: convert test -a/-o to && and ||
Junio C Hamano writes: > As I already said, I think "better known" is much less of an issue > than that "-a/-o" is "more error prone", and that is the reason why > we may want to do this rewrite. > > I do not know offhand how busy the tree would be when we can apply > these patches post-release without them getting rebased, but the > zero-th step before this series may want to be a patch like this. ... and this time with a proposed log message. -- >8 -- Subject: [PATCH] CodingGuidelines: avoid "test -a/-o " The construct is error-prone; "test" being built-in in most modern shells, the reason to avoid "test && test " spawning one extra process by using a single "test -a " no longer exists. Signed-off-by: Junio C Hamano --- Documentation/CodingGuidelines | 13 + 1 file changed, 13 insertions(+) diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines index 3d08671..4d90c77 100644 --- a/Documentation/CodingGuidelines +++ b/Documentation/CodingGuidelines @@ -151,6 +151,19 @@ For shell scripts specifically (not exhaustive): interface translatable. See "Marking strings for translation" in po/README. + - We do not write our "test" command with "-a" and "-o" and use "&&" + or "||" to concatenate multiple "test" commands instead, because + the use of "-a/-o" is often error-prone. E.g. + + test -n "$x" -a "$a" = "$b" + + is buggy and breaks when $x is "=", but + + test -n "$x" && test "$a" = "$b" + + does not have such a problem. + + For C programs: - We use tabs to indent, and interpret tabs as taking up to -- 2.0.0-rc3-438-g36dae77 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/19] check_bindir: convert test -a/-o to && and ||
Elia Pinto writes: > The interaction with unary operators and operator precedence > for && and || are better known than -a and -o, and for that > reason we prefer them. Replace all existing instances > of -a and -o to save readers from the burden of thinking > about such things. > > Signed-off-by: Elia Pinto > --- Thanks. As I already said, I think "better known" is much less of an issue than that "-a/-o" is "more error prone", and that is the reason why we may want to do this rewrite. I do not know offhand how busy the tree would be when we can apply these patches post-release without them getting rebased, but the zero-th step before this series may want to be a patch like this. Documentation/CodingGuidelines | 13 + 1 file changed, 13 insertions(+) diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines index ef67b53..7864c5b 100644 --- a/Documentation/CodingGuidelines +++ b/Documentation/CodingGuidelines @@ -106,6 +106,19 @@ For shell scripts specifically (not exhaustive): interface translatable. See "Marking strings for translation" in po/README. + - We do not write our "test" command with "-a" and "-o" and use "&&" + or "||" to concatenate multiple "test" commands instead, because + the use of "-a/-o" is often error-prone. E.g. + + test -n "$x" -a "$a" = "$b" + + is buggy and breaks when $x is "=", but + + test -n "$x" && test "$a" = "$b" + + does not have such a problem. + + For C programs: - We use tabs to indent, and interpret tabs as taking up to -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 01/19] check_bindir: convert test -a/-o to && and ||
The interaction with unary operators and operator precedence for && and || are better known than -a and -o, and for that reason we prefer them. Replace all existing instances of -a and -o to save readers from the burden of thinking about such things. Signed-off-by: Elia Pinto --- check_bindir |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/check_bindir b/check_bindir index a1c4c3e..623eadc 100755 --- a/check_bindir +++ b/check_bindir @@ -2,7 +2,7 @@ bindir="$1" gitexecdir="$2" gitcmd="$3" -if test "$bindir" != "$gitexecdir" -a -x "$gitcmd" +if test "$bindir" != "$gitexecdir" && test -x "$gitcmd" then echo echo "!! You have installed git-* commands to new gitexecdir." -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
