On 9/17/2017 12:47 AM, Junio C Hamano wrote:
Ben Peart <benpeart@xxxxxxxxxxxxx> 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.
