Re: [PATCH v2] grep: support the --pathspec-from-file option

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 4, 2019, 21:24 UTC
Message-ID: <xmqqy2vr3hso.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20191204210514.GA89300@generichostname>

Denton Liu <liu.denton@gmail.com> writes:

Show 6 quoted lines

>> +--pathspec-file-nul::
>> +	Only meaningful with `--pathspec-from-file`. Pathspec elements are
>> +	separated with NUL character and all other characters are taken
>> +	literally (including newlines and quotes).
>
> Does it make sense to have a corresponding --patterns-file-nul option?

I do not think so. "grep" is always record oriented and the record separter is the LF, so patterns file can safely be delimited with LF.

Show 10 quoted lines

>> +test_expect_success 'setup pathspecs-file tests' '
>> +cat >excluded-file <<EOF &&
>> +bar
>> +EOF
>> ...
>> +git add excluded-file pathspec-file unrelated-file
>> +'
>
> Could you please change these here-docs to be <<-\EOF and then indent
> the test case?

Good suggestion.

	test_expect_success 'setup ...' '
		cat >excluded-file <<-\EOF &&
		bar
		EOF
		...
		git add ...
	'

If each line in these files consists of a short single token (which seems to be the case), perhaps consider using test_write_lines?

	test_write_lines >excluded-file bar &&
	test_write_lines >pathspec-file foo bar baz &&
	test_write_lines >unrelated-file xyz &&
	test_write_lines >pathspecs pathspec-file unrelated-file &&