On Sun, Apr 22, 2018 at 08:32:28PM -0400, Eric Sunshine wrote: > On Sun, Apr 22, 2018 at 4:47 PM, Taylor Blau <me@xxxxxxxxxxxx> wrote: > > This commit teaches 'git-grep(1)' a new option, '--column-number'. This > > option builds upon previous commits to show the column number of the > > first match on a non-context line. > > Imperative mood (and dropping unnecessary "builds upon previous"): > > Teach 'git-grep(1)' a new option '--column-number' which shows the > column number of the first match on a non-context line. Thanks. I am not used to writing in this mood, but have amended my patches locally to conform to your proposed layout and have reworded each to be in the imperative mood. > > For example: > > > > $ ./git-grep -n --column-number foo | head -n3 > > .clang-format:51:14:# myFunction(foo, bar, baz); > > .clang-format:64:7:# int foo(); > > .clang-format:75:8:# void foo() > > > > Now that configuration variables such as grep.columnNumber and > > color.grep.columnNumber have a visible effect, we document them in this > > patch as well. > > As mentioned in my review of patch 2, document the configuration > variables in the patch which introduces them. Thanks again, I have moved this introduction to the relevant patch. > > While we're at it, change color.grep.linenumber to color.grep.lineNumber > > to match the casing of nearby variables. > > > > Signed-off-by: Taylor Blau <me@xxxxxxxxxxxx> > > --- > > diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh > > @@ -99,6 +99,28 @@ do > > + test_expect_success "grep -w $L" ' > > + ... > > + ' > > + > > + test_expect_success "grep -w $L" ' > > + ... > > + ' > > + > > test_expect_success "grep -w $L" ' > > I realize that several existing tests in this script are already > guilty of this sin, but please give each new test a distinct title > reflective of what it is actually testing in order to make it easier > to correlate failed test output with the actual test code. :-). I have changed this locally to indicate which is which in the hopes that it will provide more clarity should these tests fail at any point. Thanks, Taylor
