Hi Emily, On Wed, Dec 04, 2019 at 12:39:11PM -0800, Emily Shaffer wrote: > @@ -289,6 +293,20 @@ In future versions we may learn to support patterns containing \0 for > more search backends, until then we'll die when the pattern type in > question doesn't support them. > > +--pathspec-from-file <file>:: > + Read pathspec from <file> instead of the command line. If `<file>` is > + exactly `-` then standard input is used; standard input cannot be used > + for both --patterns-from-file and --pathspec-from-file. Pathspec elements > + are separated by LF or CR/LF. Pathspec elements can be quoted as > + explained for the configuration variable `core.quotePath` (see > + linkgit:git-config[1]). See also `--pathspec-file-nul` and global > + `--literal-pathspecs`. > + > +--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? As in, is it possible for patterns to contain inline newlines? If it's not possible, then that option probably isn't necessary. > + > -e:: > The next parameter is the pattern. This option has to be > used for patterns starting with `-` and should be used in > @@ -1125,6 +1129,44 @@ test_expect_success 'grep --no-index descends into repos, but not .git' ' > ) > ' > > +test_expect_success 'setup pathspecs-file tests' ' > +cat >excluded-file <<EOF && > +bar > +EOF > +cat >pathspec-file <<EOF && > +foo > +bar > +baz > +EOF > +cat >unrelated-file <<EOF && > +xyz > +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? > + > +cat >pathspecs <<EOF > +pathspec-file > +unrelated-file > +EOF > + > +cat >expected <<EOF > +pathspec-file:bar > +EOF In an earlier email, I was wondering aloud why these two blocks were outside of the test case above. I think the answer to that is that you're following the existing style of the test case. In that case, could I pester you to do some test clean up while you're at it? I think it'd be nice to move the cats into their respective test cases (with <<-\EOF) and also rename the files from 'expected' to 'expect'. But otherwise, I think it's fine as is as well. Thanks, Denton > + > +test_expect_success 'grep --pathspec-from-file with file' ' > + git grep --pathspec-from-file pathspecs "bar" >actual && > + test_cmp expected actual > +' > + > +test_expect_success 'grep --pathspec-file with stdin' ' > + git grep --pathspec-from-file - "bar" <pathspecs >actual && > + test_cmp expected actual > +' > + > +test_expect_success 'grep with two stdin inputs fails' ' > + test_must_fail git grep --pathspec-from-file - --patterns-from-file - <pathspecs > +' > + > test_expect_success 'setup double-dash tests' ' > cat >double-dash <<EOF && > -- > -- > 2.24.0.393.g34dc348eaf-goog >