Re: [PATCH v6 10/12] fsmonitor: add test cases for fsmonitor extension

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 17, 2017, 04:47 UTC
Message-ID: <xmqqy3peszho.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170915192043.4516-11-benpeart@microsoft.com>

Ben Peart <benpeart@microsoft.com> writes:

Show 10 quoted lines

> +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:

 - 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

Show 25 quoted lines

> 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?

> +}
> \ No newline at end of file

Oops.

Thanks.