On 9/17/2017 12:47 AM, Junio C Hamano wrote:
Ben Peart <benpe...@microsoft.com> writes:

+write_integration_script() {
+       write_script .git/hooks/fsmonitor-test<<-\EOF
+       if [ "$#" -ne 2 ]; then
+               echo "$0: exactly 2 arguments expected"
+               exit 2
+       fi
+       if [ "$1" != 1 ]; then
+               echo -e "Unsupported core.fsmonitor hook version.\n" >&2
+               exit 1
+       fi

In addition to "echo -e" thing pointed out earlier, these look
somewhat unusual in our shell scripts, relative to what
Documentation/CodingGuidelines tells us to do:

I'm happy to make these changes. I understand the difficulty of creating a consistent coding style especially after the fact.

<soapbox>

Copy/paste is usually a developers best friend as it allows you to avoid a lot of errors by reusing existing tested code. One of the times it backfires is when that code doesn't match the current desired coding style.

I only point these out to help lend some additional impetus to the effort to formalize the coding style and to provide tooling to handle what should mostly be a mechanical process. IMO, the goal should be to save the maintainer and contributors the cost of having to write up and respond to formatting feedback. :)

Some stats on these same coding style errors in the current bash scripts:

298 instances of "[a-z]\(\).*\{" ie "function_name() {" (no space)
140 instances of "if \[ .* \]" (not using the preferred "test")
293 instances of "if .*; then"

Wouldn't it be great not to have to write up style feedback for when these all get copy/pasted into new scripts? :)

</soapbox>


  - We prefer a space between the function name and the parentheses,
    and no space inside the parentheses. The opening "{" should also
    be on the same line.

        (incorrect)
        my_function(){
                ...

        (correct)
        my_function () {
                ...

  - We prefer "test" over "[ ... ]".

  - Do not write control structures on a single line with semicolon.
    "then" should be on the next line for if statements, and "do"
    should be on the next line for "while" and "for".

        (incorrect)
        if test -f hello; then
                do this
        fi

        (correct)
        if test -f hello
        then
                do this
        fi

diff --git a/t/t7519/fsmonitor-watchman b/t/t7519/fsmonitor-watchman
new file mode 100755
index 0000000000..aaee5d1fe3
--- /dev/null
+++ b/t/t7519/fsmonitor-watchman
@@ -0,0 +1,128 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+use IPC::Open2;
+ ...
+       open (my $fh, ">", ".git/watchman-query.json");
+       print $fh "[\"query\", \"$git_work_tree\", { \
+       \"since\": $time, \
+       \"fields\": [\"name\"], \
+       \"expression\": [\"not\", [\"allof\", [\"since\", $time, \"cclock\"], [\"not\", 
\"exists\"]]] \
+       }]";
+       close $fh;
+
+       print CHLD_IN "[\"query\", \"$git_work_tree\", { \
+       \"since\": $time, \
+       \"fields\": [\"name\"], \
+       \"expression\": [\"not\", [\"allof\", [\"since\", $time, \"cclock\"], [\"not\", 
\"exists\"]]] \
+       }]";

This look painful to read, write and maintain.  IIRC, Perl supports
the <<HERE document syntax quite similar to shell; would it make
these "print" we see above easier?


I agree! I'm definitely *not* a perl developer so was unaware of this construct. A few minutes with stack overflow and now I can clean this up.

+}
\ No newline at end of file

Oops.

Thanks.

Reply via email to