Re: [PATCH v2 1/6] CodingGuidelines: add shell piping guidelines

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

 



On Wed, Sep 19, 2018 at 5:36 AM Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:
>
> On Tue, Sep 18, 2018 at 10:11 PM Matthew DeVore <matvore@xxxxxxxxxx> wrote:
> > Yes, it's probably better to add a point about that. Here is the new
> > documentation after applying your suggestions:
> >
> >  - If a piped sequence which spans multiple lines, put each statement
>
> s/which//
Done.

On Wed, Sep 19, 2018 at 7:24 AM Junio C Hamano <gitster@xxxxxxxxx> wrote:
> The formatting advice to place '|' at the end applies equally to
> '&&' and '||' because these three syntactic elements share exactly
> the same trait: the shell knows you haven't finished speaking when
> it sees them at the end of the line and keeps listening, and humans
> would know that too, so there is no need for explicitly continuing
> the line with backslash.
>
I've reworded the text to indicate the advice applies to && and || as well.

> Organizationally speaking, I wonder if the above about formatting
> would better appear separate from the latter two points that are
> about semantics.
>
I moved the formatting point to right under the point about formatting
if statements, which does seem like a more natural progression.

Here is the new patch to summarize the changes (warning: tabs are mangled):

--------------------------------------------------------------------------------

    CodingGuidelines: add shell piping guidelines

    Add two guidelines:

     - pipe characters should appear at the end of lines, and not cause
       indentation
     - pipes should be avoided when they swallow exit codes that can
       potentially fail

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index 48aa4edfb..6d265327c 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -118,6 +118,24 @@ For shell scripts specifically (not exhaustive):
                 do this
         fi

+ - If a command sequence joined with && or || or | spans multiple
+   lines, put each command on a separate line and put && and || and |
+   operators at the end of each line, rather than the start. This
+   means you don't need to use \ to join lines, since the above
+   operators imply the sequence isn't finished.
+
+        (incorrect)
+        grep blob verify_pack_result \
+        | awk -f print_1.awk \
+        | sort >actual &&
+        ...
+
+        (correct)
+        grep blob verify_pack_result |
+        awk -f print_1.awk |
+        sort >actual &&
+        ...
+
  - We prefer "test" over "[ ... ]".

  - We do not write the noiseword "function" in front of shell
@@ -163,6 +181,15 @@ For shell scripts specifically (not exhaustive):

    does not have such a problem.

+ - In a piped chain such as "grep blob objects | sort", the exit codes
+   returned by processes besides the last are ignored. This means that
+   if git crashes at the beginning or middle of a chain, it may go
+   undetected. Prefer writing the output of that command to a
+   temporary file with '>' rather than pipe it.
+
+ - The $(git ...) construct also discards git's exit code, so if the
+   goal is to test that particular command, redirect its output to a
+   temporary file rather than wrap it with $( ).

 For C programs:



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux