Re: [PATCH v4 5/7] builtin/grep.c: add '--column' option to 'git-grep(1)'

[Phillip Wood <phillip.wood@talktalk.net>]

Hi Taylor

On 05/05/18 03:43, Taylor Blau wrote:
> 
> Teach 'git-grep(1)' a new option, '--column', to show the column
> number of the first match on a non-context line.
> 
> For example:
> 
>   $ git grep -n --column foo | head -n3
>   .clang-format:51:14:# myFunction(foo, bar, baz);
>   .clang-format:64:7:# int foo();
>   .clang-format:75:8:# void foo()
> 
> Signed-off-by: Taylor Blau <me@ttaylorr.com>
> ---
>  Documentation/git-grep.txt |  5 ++++-
>  builtin/grep.c             |  1 +
>  t/t7810-grep.sh            | 22 ++++++++++++++++++++++
>  3 files changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
> index 18b494731f..5409a24399 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]
> +	   [-F | --fixed-strings] [-n | --line-number] [--column]
>  	   [-l | --files-with-matches] [-L | --files-without-match]
>  	   [(-O | --open-files-in-pager) [<pager>]]
>  	   [-z | --null]
> @@ -169,6 +169,9 @@ providing this option will cause it to die.
>  --line-number::
>  	Prefix the line number to matching lines.
>  
> +--column::
> +	Prefix the 1-indexed column number of the first match on non-context lines.
> +

I think it would be useful to explain what the column number actually is
so that users know how to consume it. Is it a count of bytes, multi-byte
characters or graphemes? It would probably be worth testing with a file
that contains multi-byte characters to check for future regressions.

Best Wishes

Phillip

>  -l::
>  --files-with-matches::
>  --name-only::
> diff --git a/builtin/grep.c b/builtin/grep.c
> index 5f32d2ce84..5c83f17759 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -829,6 +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(0, "column", &opt.columnnum, N_("show column number of first match")),
>  		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/t/t7810-grep.sh b/t/t7810-grep.sh
> index 1797f632a3..a03c3416e7 100755
> --- a/t/t7810-grep.sh
> +++ b/t/t7810-grep.sh
> @@ -99,6 +99,28 @@ do
>  		test_cmp expected actual
>  	'
>  
> +	test_expect_success "grep -w $L (with --column)" '
> +		{
> +			echo ${HC}file:5:foo mmap bar
> +			echo ${HC}file:14:foo_mmap bar mmap
> +			echo ${HC}file:5:foo mmap bar_mmap
> +			echo ${HC}file:14:foo_mmap bar mmap baz
> +		} >expected &&
> +		git grep --column -w -e mmap $H >actual &&
> +		test_cmp expected actual
> +	'
> +
> +	test_expect_success "grep -w $L (with --{line,column}-number)" '
> +		{
> +			echo ${HC}file:1:5:foo mmap bar
> +			echo ${HC}file:3:14:foo_mmap bar mmap
> +			echo ${HC}file:4:5:foo mmap bar_mmap
> +			echo ${HC}file:5:14:foo_mmap bar mmap baz
> +		} >expected &&
> +		git grep -n --column -w -e mmap $H >actual &&
> +		test_cmp expected actual
> +	'
> +
>  	test_expect_success "grep -w $L" '
>  		{
>  			echo ${HC}file:1:foo mmap bar
>