All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Denton Liu <liu.denton@gmail.com>
Cc: Emily Shaffer <emilyshaffer@google.com>,
	git@vger.kernel.org,
	Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
Subject: Re: [PATCH v2] grep: support the --pathspec-from-file option
Date: Wed, 04 Dec 2019 13:24:55 -0800	[thread overview]
Message-ID: <xmqqy2vr3hso.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <20191204210514.GA89300@generichostname> (Denton Liu's message of "Wed, 4 Dec 2019 13:05:14 -0800")

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

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

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

  reply	other threads:[~2019-12-04 21:25 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-22  1:16 [PATCH] grep: provide pathspecs/patterns via file or stdin Emily Shaffer
2019-11-22  2:14 ` Denton Liu
2019-11-22  2:34   ` Junio C Hamano
2019-11-22  3:56     ` Junio C Hamano
2019-11-22 18:52     ` Denton Liu
2019-11-22 22:02     ` Emily Shaffer
2019-11-22 22:06       ` Emily Shaffer
2019-11-23  0:28       ` Junio C Hamano
2019-11-22  2:24 ` Junio C Hamano
2019-12-04 20:39 ` [PATCH v2] grep: support the --pathspec-from-file option Emily Shaffer
2019-12-04 21:05   ` Denton Liu
2019-12-04 21:24     ` Junio C Hamano [this message]
2019-12-04 22:24   ` Junio C Hamano
2019-12-13  3:07     ` Emily Shaffer
2019-12-05 11:58   ` Alexandr Miloslavskiy
2019-12-13  4:00     ` Emily Shaffer
2019-12-06 11:22   ` Johannes Schindelin
2019-12-06 11:34   ` SZEDER Gábor
2019-12-13  4:12   ` [PATCH v3] " Emily Shaffer
2019-12-13 13:04     ` Alexandr Miloslavskiy
2019-12-13 18:26     ` Junio C Hamano
2019-12-13 20:13       ` Alexandr Miloslavskiy
2019-12-17  0:33         ` Emily Shaffer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqy2vr3hso.fsf@gitster-ct.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=alexandr.miloslavskiy@syntevo.com \
    --cc=emilyshaffer@google.com \
    --cc=git@vger.kernel.org \
    --cc=liu.denton@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.