Re: [PATCH v2 0/6] Teach '--column-number' to 'git-grep(1)'
Taylor Blau writes: >> It seems that these two used to be "even when it is configured not >> to show linenumber, with -n it is shown and without -n it is not, >> when the new --column-number feature forces the command to show the >> "filename plus colon plus location info plus coon" header. I'm >> guessing that the reason why these got changed this round is because >> the old way was found too defensive (perhaps nobody sets >> grep.linenumber in .git/config in the tests that come before these)? > ... > Do you think that it is worth testing --column-number with and without > grep.columnnumber as above? I view the "git -c var.val=foo cmd" as a reasonable way to make sure that the test is not affected by any stale state in .git/config left behind by an earlier test that did "git config var.val bar" and then failed to clean itself up, but I do not think it is all that intereseting to test the inter-changeability between config and command line option "-n" while checking its interaction with another option e.g. "--column".
Re: [PATCH v2 0/6] Teach '--column-number' to 'git-grep(1)'
On Mon, Apr 23, 2018 at 08:28:14AM +0900, Junio C Hamano wrote:
> Taylor Blau writes:
>
> > diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
> > index 0cf654824d..7349c7fadc 100755
> > --- a/t/t7810-grep.sh
> > +++ b/t/t7810-grep.sh
> > @@ -106,7 +106,7 @@ do
> > echo ${HC}file:5:foo mmap bar_mmap
> > echo ${HC}file:14:foo_mmap bar mmap baz
> > } >expected &&
> > - git -c grep.linenumber=false grep -m -w -e mmap $H >actual &&
> > + git grep --column-number -w -e mmap $H >actual &&
> > test_cmp expected actual
> > '
> >
> > @@ -117,7 +117,7 @@ do
> > echo ${HC}file:4:5:foo mmap bar_mmap
> > echo ${HC}file:5:14:foo_mmap bar mmap baz
> > } >expected &&
> > - git -c grep.linenumber=false grep -n -m -w -e mmap $H >actual &&
> > + git grep -n --column-number -w -e mmap $H >actual &&
> > test_cmp expected actual
> > '
>
> It seems that these two used to be "even when it is configured not
> to show linenumber, with -n it is shown and without -n it is not,
> when the new --column-number feature forces the command to show the
> "filename plus colon plus location info plus coon" header. I'm
> guessing that the reason why these got changed this round is because
> the old way was found too defensive (perhaps nobody sets
> grep.linenumber in .git/config in the tests that come before these)?
Sort of. My aim with these new tests is to ensure that git-grep(1) works
with:
- '--line-number'
- '--column-number'
- '--line-number' and '--column-number' together
It seemed unrelated to be testing the various permutations of
grep.linenumber and --line-number along with the above three so I
removed these in order to focus only on the above three.
Do you think that it is worth testing --column-number with and without
grep.columnnumber as above?
Thanks,
Taylor
Re: [PATCH v2 0/6] Teach '--column-number' to 'git-grep(1)'
Taylor Blau writes:
> * Removed '-m' as an alias for '--column-number', per René's
> suggestion [1].
>
> * Fix some incorrect spelling of 'columnnumber'.
>
> * Change casing of 'color.grep.linenumber' to 'color.grep.lineNumber'
> to be consistent with 'color.grep.columnNumber'. This is an
> unrelated change, and one which I am happy to drop from this series.
> It was suggested by Martin in [2].
All sounds like good updates.
> diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
> index 0cf654824d..7349c7fadc 100755
> --- a/t/t7810-grep.sh
> +++ b/t/t7810-grep.sh
> @@ -106,7 +106,7 @@ do
> echo ${HC}file:5:foo mmap bar_mmap
> echo ${HC}file:14:foo_mmap bar mmap baz
> } >expected &&
> - git -c grep.linenumber=false grep -m -w -e mmap $H >actual &&
> + git grep --column-number -w -e mmap $H >actual &&
> test_cmp expected actual
> '
>
> @@ -117,7 +117,7 @@ do
> echo ${HC}file:4:5:foo mmap bar_mmap
> echo ${HC}file:5:14:foo_mmap bar mmap baz
> } >expected &&
> - git -c grep.linenumber=false grep -n -m -w -e mmap $H >actual &&
> + git grep -n --column-number -w -e mmap $H >actual &&
> test_cmp expected actual
> '
It seems that these two used to be "even when it is configured not
to show linenumber, with -n it is shown and without -n it is not,
when the new --column-number feature forces the command to show the
"filename plus colon plus location info plus coon" header. I'm
guessing that the reason why these got changed this round is because
the old way was found too defensive (perhaps nobody sets
grep.linenumber in .git/config in the tests that come before these)?
[PATCH v2 0/6] Teach '--column-number' to 'git-grep(1)'
Hi,
Attached is a re-roll of the series to add --column-number to
'git-grep(1)'.
Since last time, I have changed the following (an inter-diff is
available below for easier consumption):
* Removed '-m' as an alias for '--column-number', per René's
suggestion [1].
* Fix some incorrect spelling of 'columnnumber'.
* Change casing of 'color.grep.linenumber' to 'color.grep.lineNumber'
to be consistent with 'color.grep.columnNumber'. This is an
unrelated change, and one which I am happy to drop from this series.
It was suggested by Martin in [2].
Thanks in advance for your second round of review :-).
Thanks,
Taylor
[1]: https://public-inbox.org/git/cef29224-718f-21e9-0242-8bcd8e9c2...@web.de/
[2]:
https://public-inbox.org/git/can0hesp_bgqkf26g4tdow6wpsvr2cew6eqf3ajtkcv5pou_...@mail.gmail.com/
Taylor Blau (6):
grep.c: take regmatch_t as argument in match_line()
grep.c: take column number as argument to show_line()
grep.[ch]: teach columnnum, color_columnno to grep_opt
grep.c: display column number of first match
builtin/grep.c: show column numbers via --column-number
contrib/git-jump/git-jump: use column number when grep-ing
Documentation/config.txt | 7 ++-
Documentation/git-grep.txt | 8 +++-
builtin/grep.c | 1 +
contrib/git-jump/git-jump | 2 +-
grep.c | 33 -
grep.h | 2 ++
t/t7810-grep.sh| 22 ++
7 files changed, 63 insertions(+), 12 deletions(-)
Inter-diff (since v1):
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 02fd4b662b..1645fcf2ae 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1157,10 +1157,10 @@ color.grep.::
filename prefix (when not using `-h`)
`function`;;
function name lines (when using `-p`)
-`linenumber`;;
+`lineNumber`;;
line number prefix (when using `-n`)
-`columnnumber`;;
- column number prefix (when using `-m`)
+`columnNumber`;;
+ column number prefix (when using `--column-number`)
`match`;;
matching text (same as setting `matchContext` and `matchSelected`)
`matchContext`;;
diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index dd90f74ded..b75a039768 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -13,7 +13,7 @@ SYNOPSIS
[-v | --invert-match] [-h|-H] [--full-name]
[-E | --extended-regexp] [-G | --basic-regexp]
[-P | --perl-regexp]
- [-F | --fixed-strings] [-n | --line-number] [-m | --column-number]
+ [-F | --fixed-strings] [-n | --line-number] [--column-number]
[-l | --files-with-matches] [-L | --files-without-match]
[(-O | --open-files-in-pager) []]
[-z | --null]
@@ -172,7 +172,6 @@ providing this option will cause it to die.
--line-number::
Prefix the line number to matching lines.
--m::
--column-number::
Prefix the 1-indexed column number of the first match on non-context
lines.
diff --git a/builtin/grep.c b/builtin/grep.c
index faa65abab5..23ce97f998 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -829,7 +829,7 @@ int cmd_grep(int argc, const char **argv, const char
*prefix)
GREP_PATTERN_TYPE_PCRE),
OPT_GROUP(""),
OPT_BOOL('n', "line-number", &opt.linenum, N_("show line
numbers")),
- OPT_BOOL('m', "column-number", &opt.columnnum, N_("show column
numbers")),
+ OPT_BOOL(0, "column-number", &opt.columnnum, N_("show column
numbers")),
OPT_NEGBIT('h', NULL, &opt.pathname, N_("don't show
filenames"), 1),
OPT_BIT('H', NULL, &opt.pathname, N_("show filenames"), 1),
OPT_NEGBIT(0, "full-name", &opt.relative,
diff --git a/grep.c b/grep.c
index 5aeb893263..23250e60d0 100644
--- a/grep.c
+++ b/grep.c
@@ -115,7 +115,7 @@ int grep_config(const char *var, const char *value, void
*cb)
color = opt->color_function;
else if (!strcmp(var, "color.grep.linenumber"))
color = opt->color_lineno;
- else if (!strcmp(var, "color.grep.columnumber"))
+ else if (!strcmp(var, "color.grep.columnnumber"))
color = opt->color_columnno;
else if (!strcmp(var, "color.grep.matchcontext"))
color = opt->color_match_context;
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index 0cf654824d..7349c7fadc 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -106,7 +106,7 @@ do
echo ${HC}file:5:foo mmap bar_mmap
echo ${HC}file:14:foo_mmap bar mmap baz
} >expected &&
- git -c grep.linenumber=false grep -m -w -e mmap $H >actual &&
+ git grep --column-number -w -e mmap $H >actual &&
test_cmp expected actual
'
@@ -117,7 +117,7 @@ do
echo ${HC}f
