blame: enable funcname blaming with userdiff driver

This series fixes a bug in git blame where any userdiff driver configured via the attributes mechanism is ignored, which more often than not means thatgit blame -L :<funcname> dies for paths that have a configured userdiff driver. That's patch 6/6.

Patches 1-5 are documentation improvements around the line-log functionality.

Notes (I can't add in-patch commentaries using GitGitGadget (yet, I hope 1
[https://github.com/gitgitgadget/gitgitgadget/issues/173]), so I'm providing what would have been an in-patch commentary on patch 6 below):

* In patch 6, I considered fixing that bug in a different way by initializing sb.path inside blame.c::setup_scoreboard. This function already receives path as its second argument, so changing that would not impact the API. Probably Thomas thought that sb.path was already initialized in sb when he modified builtin/blame.c::prepare_blame_range to also send sb.path to line-range.c::parse_range_arg in 13b8f68c1f (log -L: :pattern:file syntax to find by funcname, 2013-03-28).

Initializing the path in setup_scoreboard would mean we could also simplify the API of blame.c::setup_blame_bloom_data since it would not need to receive path separately and so its second argument could be removed. I went for the simpler alternative of just sending path to parse_range_arg instead of sb.path since it felt simpler, but if we feel it would be better to actually initialize sb.path in setup_scoreboard, I'll gladly tweak that for v2.

* The fact that this bug has been present ever since git blame learned -L :<funcname> makes me a little sad. I think that the fact that the diff attribute (either with a builtin userdiff pattern or with diff.*.xfuncname ) influences way more than just the hunk header, but also features like git log -L, git blame -L, and the different options that I mention in patch 4/6, is not well-known enough. I'm not saying the code or the doc is wrong, but I think it's a visibility issue. Maybe more blog posts about that would help.... I noticed that the "Git Attributes" chapter in the ProGit book 2
[https://git-scm.com/book/en/v2/Customizing-Git-Git-Attributes] does not even mention the builtin userdiff patterns, so I'll probably do a PR there to mention it. Any other ideas for improving discoverability of this very cool feature ?

Philippe Blain (6):
doc: log, gitk: move '-L' description to 'line-range-options.txt'
doc: line-range: improve formatting
blame-options.txt: also mention 'funcname' in '-L' description
doc: add more pointers to gitattributes(5) for userdiff
line-log: mention both modes in 'blame' and 'log' short help
blame: enable funcname blaming with userdiff driver

 Documentation/blame-options.txt      |  9 +++++----
 Documentation/diff-options.txt       |  5 ++++-
 Documentation/git-grep.txt           |  6 ++++--
 Documentation/git-log.txt            | 16 ++--------------
 Documentation/gitk.txt               | 21 ++-------------------
 Documentation/line-range-format.txt  | 28 +++++++++++++++-------------
 Documentation/line-range-options.txt | 27 +++++++++++++++++++++++++++
 builtin/blame.c                      |  5 +++--
 builtin/log.c                        |  4 ++--
 t/annotate-tests.sh                  | 18 ++++++++++++++++++
 10 files changed, 82 insertions(+), 57 deletions(-)
 create mode 100644 Documentation/line-range-options.txt

base-commit: 1d1c4a875900d69c7f0a31e44c3e370dc80ab1ce
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-774%2Fphil-blain%2Fblame-funcname-userdiff-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-774/phil-blain/blame-funcname-userdiff-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/774

gitgitgadget

From: Philippe Blain <[email protected]>

The description of the '-L' option for git log and gitk is the same, but is repeated in both 'git-log.txt' and 'gitk.txt'.

Remove the duplication by creating a new file, 'line-range-options.txt', and include it in both files.

---
 Documentation/git-log.txt            | 16 ++--------------
 Documentation/gitk.txt               | 21 ++-------------------
 Documentation/line-range-options.txt | 27 +++++++++++++++++++++++++++
 3 files changed, 31 insertions(+), 33 deletions(-)
 create mode 100644 Documentation/line-range-options.txt

index 2b8ac5ff88..631dfff77c 100644
@@ -77,20 +77,8 @@ produced by `--stat`, etc.
 	Intended to speed up tools that read log messages from `git log`
 	output by allowing them to allocate space in advance.
 
--L <start>,<end>:<file>::
--L :<funcname>:<file>::
-	Trace the evolution of the line range given by "<start>,<end>"
-	(or the function name regex <funcname>) within the <file>.  You may
-	not give any pathspec limiters.  This is currently limited to
-	a walk starting from a single revision, i.e., you may only
-	give zero or one positive revision arguments, and
-	<start> and <end> (or <funcname>) must exist in the starting revision.
-	You can specify this option more than once. Implies `--patch`.
-	Patch output can be suppressed using `--no-patch`, but other diff formats
-	(namely `--raw`, `--numstat`, `--shortstat`, `--dirstat`, `--summary`,
-	`--name-only`, `--name-status`, `--check`) are not currently implemented.
-+
-include::line-range-format.txt[]
+:git-log: 1
+include::line-range-options.txt[]
 
 <revision range>::
 	Show only commits in the specified revision range.  When no

index c653ebb6a8..761b764c18 100644
@@ -98,25 +98,8 @@ linkgit:git-rev-list[1] for a complete list.
 	(See "History simplification" in linkgit:git-log[1] for a more
 	detailed explanation.)
 
--L<start>,<end>:<file>::
--L:<funcname>:<file>::
-
-	Trace the evolution of the line range given by "<start>,<end>"
-	(or the function name regex <funcname>) within the <file>.  You may
-	not give any pathspec limiters.  This is currently limited to
-	a walk starting from a single revision, i.e., you may only
-	give zero or one positive revision arguments, and
-	<start> and <end> (or <funcname>) must exist in the starting revision.
-	You can specify this option more than once. Implies `--patch`.
-	Patch output can be suppressed using `--no-patch`, but other diff formats
-	(namely `--raw`, `--numstat`, `--shortstat`, `--dirstat`, `--summary`,
-	`--name-only`, `--name-status`, `--check`) are not currently implemented.
-+
-*Note:* gitk (unlike linkgit:git-log[1]) currently only understands
-this option if you specify it "glued together" with its argument.  Do
-*not* put a space after `-L`.
-+
-include::line-range-format.txt[]
+:gitk: 1
+include::line-range-options.txt[]
 
 <revision range>::
 

new file mode 100644
index 0000000000..9e3d98d44f
@@ -0,0 +1,27 @@
+ifdef::git-log[]
+-L <start>,<end>:<file>::
+-L :<funcname>:<file>::
+endif::git-log[]
+ifdef::gitk[]
+-L<start>,<end>:<file>::
+-L:<funcname>:<file>::
+endif::gitk[]
+
+	Trace the evolution of the line range given by "<start>,<end>"
+	(or the function name regex <funcname>) within the <file>.  You may
+	not give any pathspec limiters.  This is currently limited to
+	a walk starting from a single revision, i.e., you may only
+	give zero or one positive revision arguments, and
+	<start> and <end> (or <funcname>) must exist in the starting revision.
+	You can specify this option more than once. Implies `--patch`.
+	Patch output can be suppressed using `--no-patch`, but other diff formats
+	(namely `--raw`, `--numstat`, `--shortstat`, `--dirstat`, `--summary`,
+	`--name-only`, `--name-status`, `--check`) are not currently implemented.
++
+ifdef::gitk[]
+*Note:* gitk (unlike linkgit:git-log[1]) currently only understands
+this option if you specify it "glued together" with its argument.  Do
+*not* put a space after `-L`.
+endif::gitk[]
++
+include::line-range-format.txt[]

gitgitgadget

They are moral equivalents but differ slightly, both in an insignificant way (i.e. SP after -L and its parameters) and a significant way (i.e. Note about gitk usage), so "the same, but is repeated" is not sufficient to explain why the contents of the new file looks the way it is.

Makes sense. As to the conditional, I actually think the version with SP after -L do not have to be listed and instead to show just the "stuck" form as the standardised way. If the option takes an optional value, you must have to use the "stuck" form anyway, and showing that you _could_ have SP there unnecessarily throws extra bytes at the reader with no real gain.

Hi Junio,

OK. I'll expand on that.

I'm not sure I understand here... The <range> argument is not optional here (git log -L by itself is meaningless)... I know gitcli(7) recommends the stuck form, but this option has been listed as -L <range> in the git-log documentation ever since it appeared... So I did not think it wise to change it, that's why I introduce the conditional.

Thanks,

Philippe.

I do not think showing alternative ways distracts without adding much value. We should remember that most users are learning Git to only to do things they want to do and not necessarily learning Git to master all the slightly different ways to do the same thing. "You can write option and its argument separately most of the time but you must write option and its argument in the stuck form some other times---you must think if the argument is optional or not, whenyou want to write your argument separately" crams more things to learn than the simple "Write option and its arguments in the stuck form and it always works".

From: Philippe Blain <[email protected]>

Improve the formatting of the description of the line-range option '-L' for git log, gitk and git blame:

- Use bold for <start>, <end> and <funcname>
- Use backticks for literals

---
 Documentation/blame-options.txt      |  4 ++--
 Documentation/line-range-format.txt  | 24 ++++++++++++------------
 Documentation/line-range-options.txt |  6 +++---
 3 files changed, 17 insertions(+), 17 deletions(-)

index 88750af7ae..48bf0eeec5 100644
@@ -14,8 +14,8 @@
 	Annotate only the given line range. May be specified multiple times.
 	Overlapping ranges are allowed.
 +
-<start> and <end> are optional. ``-L <start>'' or ``-L <start>,'' spans from
-<start> to end of file. ``-L ,<end>'' spans from start of file to <end>.
+'<start>' and '<end>' are optional. `-L <start>` or `-L <start>,` spans from
+'<start>' to end of file. `-L ,<end>` spans from start of file to '<end>'.
 +
 include::line-range-format.txt[]
 

index 829676ff98..6ee159b683 100644
@@ -1,30 +1,30 @@
-<start> and <end> can take one of these forms:
+'<start>' and '<end>' can take one of these forms:
 
 - number
 +
-If <start> or <end> is a number, it specifies an
+If '<start>' or '<end>' is a number, it specifies an
 absolute line number (lines count from 1).
 +
 
-- /regex/
+- `/regex/`
 +
 This form will use the first line matching the given
-POSIX regex. If <start> is a regex, it will search from the end of
+POSIX regex. If '<start>' is a regex, it will search from the end of
 the previous `-L` range, if any, otherwise from the start of file.
-If <start> is ``^/regex/'', it will search from the start of file.
-If <end> is a regex, it will search
-starting at the line given by <start>.
+If '<start>' is `^/regex/`, it will search from the start of file.
+If '<end>' is a regex, it will search
+starting at the line given by '<start>'.
 +
 
 - +offset or -offset
 +
-This is only valid for <end> and will specify a number
-of lines before or after the line given by <start>.
+This is only valid for '<end>' and will specify a number
+of lines before or after the line given by '<start>'.
 
 +
-If ``:<funcname>'' is given in place of <start> and <end>, it is a
+If "`:<funcname>`" is given in place of '<start>' and '<end>', it is a
 regular expression that denotes the range from the first funcname line
-that matches <funcname>, up to the next funcname line. ``:<funcname>''
+that matches '<funcname>', up to the next funcname line. `:<funcname>`
 searches from the end of the previous `-L` range, if any, otherwise
-from the start of file. ``^:<funcname>'' searches from the start of
+from the start of file. `^:<funcname>` searches from the start of
 file.

index 9e3d98d44f..d2ef1a6d67 100644
@@ -7,12 +7,12 @@ ifdef::gitk[]
 -L:<funcname>:<file>::
 endif::gitk[]
 
-	Trace the evolution of the line range given by "<start>,<end>"
-	(or the function name regex <funcname>) within the <file>.  You may
+	Trace the evolution of the line range given by "'<start>,<end>'"
+	(or the function name regex '<funcname>') within the '<file>'. You may
 	not give any pathspec limiters.  This is currently limited to
 	a walk starting from a single revision, i.e., you may only
 	give zero or one positive revision arguments, and
-	<start> and <end> (or <funcname>) must exist in the starting revision.
+	'<start>' and '<end>' (or '<funcname>') must exist in the starting revision.
 	You can specify this option more than once. Implies `--patch`.
 	Patch output can be suppressed using `--no-patch`, but other diff formats
 	(namely `--raw`, `--numstat`, `--shortstat`, `--dirstat`, `--summary`,

gitgitgadget

My impression is that it is more common in Git documentation for these placeholders to be formatted with backticks rather than as bold (or, if not more common currently, at least is trending that way). That's not to say that my impression is necessarily accurate.

I think the impression is fairly in line with the reality. That is not to say that it is a good future to aim for.

My personal preference is

- use as-is for things that must be typed as-is (e.g. -L)

- use 'emph' for things that are placeholder (e.g. '<start>'').

Yes, I compared using

git grep '`<' | wc -l

and

git grep "'<" | wc -l

and the same for their closing counterparts and the backticks win.

Thanks for clarifying. It is indeed what I tried to do.

From: Philippe Blain <[email protected]>

Make it clearer that a function can be blamed by feeding git blame '-L :<funcname>' by mentioning it at the beginnning of the description of the '-L' option.

---
 Documentation/blame-options.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

index 48bf0eeec5..8b4a90e349 100644
@@ -11,8 +11,9 @@
 
 -L <start>,<end>::
 -L :<funcname>::
-	Annotate only the given line range. May be specified multiple times.
-	Overlapping ranges are allowed.
+	Annotate only the line range given by '<start>,<end>'
+	(or by the function name regex '<funcname>').
+	May be specified multiple times. Overlapping ranges are allowed.
 +
 '<start>' and '<end>' are optional. `-L <start>` or `-L <start>,` spans from
 '<start>' to end of file. `-L ,<end>` spans from start of file to '<end>'.

gitgitgadget

I think it makes tons of sense to mention both <start>,<end> range and <funcname> range in the description.

It is not a new issue you are introducing with this change, but I think the latter does not deserve to be treated as if it were an afterthought by being a mere parenthetical mention.

I agree. I'll address that for the next version.

From: Philippe Blain <[email protected]>

Several Git commands can make use of the builtin userdiff patterns, but it's not obvious in the documentation. Add pointers to the 'Defining a custom hunk header' part of gitattributes(5) in the description of the following options:

- the '--function-context' option of git diff and friends
- the '--function-context' option of git grep
- the '-L :<funcname>' option of git log, gitk and git blame

In 'git-grep.txt', take the opportunity to use backticks in the description of '--show-function', and improve the wording of the desription of '--function-context'.

---
 Documentation/diff-options.txt      | 5 ++++-
 Documentation/git-grep.txt          | 6 ++++--
 Documentation/line-range-format.txt | 4 +++-
 3 files changed, 11 insertions(+), 4 deletions(-)

index 573fb9bb71..39b511bb6b 100644
@@ -695,7 +695,10 @@ endif::git-format-patch[]
 
 -W::
 --function-context::
-	Show whole surrounding functions of changes.
+	Show whole functions as context lines for each changes.
+	The function names are determined in the same way as
+	`git diff` works out patch hunk headers (see 'Defining a
+	custom hunk-header' in linkgit:gitattributes[5]).
 
 ifndef::git-format-patch[]
 ifndef::git-log[]

index 6077ff01a4..4e0ba8234a 100644
@@ -241,7 +241,7 @@ providing this option will cause it to die.
 --show-function::
 	Show the preceding line that contains the function name of
 	the match, unless the matching line is a function name itself.
-	The name is determined in the same way as 'git diff' works out
+	The name is determined in the same way as `git diff` works out
 	patch hunk headers (see 'Defining a custom hunk-header' in
 	linkgit:gitattributes[5]).
 

@@ -266,7 +266,9 @@ providing this option will cause it to die.
 	Show the surrounding text from the previous line containing a
 	function name up to the one before the next function name,
 	effectively showing the whole function in which the match was
-	found.
+	found. The function names are determined in the same way as
+	`git diff` works out patch hunk headers (see 'Defining a
+	custom hunk-header' in linkgit:gitattributes[5]).
 
 --threads <num>::
 	Number of grep worker threads to use.

index 6ee159b683..ab3c70a1d5 100644
@@ -27,4 +27,6 @@ regular expression that denotes the range from the first funcname line
 that matches '<funcname>', up to the next funcname line. `:<funcname>`
 searches from the end of the previous `-L` range, if any, otherwise
 from the start of file. `^:<funcname>` searches from the start of
-file.
+file. The function names are determined in the same way as `git diff`
+works out patch hunk headers (see 'Defining a custom hunk-header'
+in linkgit:gitattributes[5]).

gitgitgadget

From: Philippe Blain <[email protected]>

'git blame -h' and 'git log -h' both show '-L <n,m>' and describe this option as "Process only line range n,m, counting from 1". No hint is given that a function name regex can also be used.

Use <range> instead, and expand the description of the option to mention both modes. Remove "counting from 1" as it's uneeded; it's uncommon to refer to the first line of a file as "line 0".

Also, for 'git log', improve the wording to better reflect the long help.

---
 builtin/blame.c | 3 ++-
 builtin/log.c   | 4 ++--
 2 files changed, 4 insertions(+), 3 deletions(-)

index bb0f29300e..05f69211dd 100644
@@ -889,7 +889,8 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 		OPT_STRING(0, "contents", &contents_from, N_("file"), N_("Use <file>'s contents as the final image")),
 		OPT_CALLBACK_F('C', NULL, &opt, N_("score"), N_("Find line copies within and across files"), PARSE_OPT_OPTARG, blame_copy_callback),
 		OPT_CALLBACK_F('M', NULL, &opt, N_("score"), N_("Find line movements within and across files"), PARSE_OPT_OPTARG, blame_move_callback),
-		OPT_STRING_LIST('L', NULL, &range_list, N_("n,m"), N_("Process only line range n,m, counting from 1")),
+		OPT_STRING_LIST('L', NULL, &range_list, N_("range"),
+				N_("Process only the given line range (<range>=<start>,<end>) or function (<range>=:<funcname>)")),
 		OPT__ABBREV(&abbrev),
 		OPT_END()
 	};

index 0a7ed4bef9..e6a3bb7803 100644
@@ -183,8 +183,8 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
 				N_("pattern"), N_("do not decorate refs that match <pattern>")),
 		OPT_CALLBACK_F(0, "decorate", NULL, NULL, N_("decorate options"),
 			       PARSE_OPT_OPTARG, decorate_callback),
-		OPT_CALLBACK('L', NULL, &line_cb, "n,m:file",
-			     N_("Process line range n,m in file, counting from 1"),
+		OPT_CALLBACK('L', NULL, &line_cb, "range:file",
+			     N_("Trace the evolution of the given line range (<range>=<start>,<end>) or function (<range>=:<funcname>) in <file>"),
 			     log_line_range_callback),
 		OPT_END()
 	};

gitgitgadget

Hi Eric,

Yes, I agree. I've shortened it for v2 (and also made the changes you suggested for 2/6 and 4/6.

Thanks for taking a look,

Philippe.

From: Philippe Blain <[email protected]>

In blame.c::cmd_blame, we send the 'path' field of the 'sb' 'struct blame_scoreboard' as the 'path' argument to 'line-range.c::parse_range_arg', but 'sb.path' is not set yet; it's set to the local variable 'path' a few lines later at line 1137.

This 'path' argument is only used in 'parse_range_arg' if we are blaming a funcname, i.e. git blame -L :<funcname> <path>, and in that case it is sent to 'parse_range_funcname', where it is used to determine if a userdiff driver should be used for said <path> to match the given funcname.

Since 'path' is yet unset, the userdiff driver is never used, so we fall back to the default funcname regex, which is usually not appropriate for paths that are set to use a specific userdiff driver, and thus either we match some unrelated lines, or we die with

fatal: -L parameter '<funcname>' starting at line 1: no match

This has been the case ever since git blame learned to blame a funcname in 13b8f68c1f (log -L: :pattern:file syntax to find by funcname, 2013-03-28).

Enable funcname blaming for paths using specific userdiff drivers by sending the local variable 'path' to 'parse_range_arg' instead of the yet unset 'sb.path'.

Add a regression test in 'annotate-tests.sh', which is sourced in t8001-annotate.sh and t8002-blame.sh, leveraging an existing file used to test the userdiff patterns in t4018-diff-funcname.

---
 builtin/blame.c     |  2 +-
 t/annotate-tests.sh | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

index 05f69211dd..917fedc635 100644
@@ -1104,7 +1104,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 		long bottom, top;
 		if (parse_range_arg(range_list.items[range_i].string,
 				    nth_line_cb, &sb, lno, anchor,
-				    &bottom, &top, sb.path,
+				    &bottom, &top, path,
 				    the_repository->index))
 			usage(blame_usage);
 		if ((!lno && (top || bottom)) || lno < bottom)

index d933af5714..3aee61d2cc 100644
@@ -479,6 +479,24 @@ test_expect_success 'blame -L ^:RE (absolute: end-of-file)' '
 	check_count -f hello.c -L$n -L^:ma.. F 4 G 1 H 1
 '
 
+test_expect_success 'setup -L :funcname with userdiff driver' '
+	echo "fortran-* diff=fortran" >.gitattributes &&
+	fortran_file=fortran-external-function &&
+	orig_file="$TEST_DIRECTORY/t4018/$fortran_file" &&
+	cp $orig_file . &&
+	git add $fortran_file &&
+	GIT_AUTHOR_NAME="A" GIT_AUTHOR_EMAIL="[email protected]" \
+	git commit -m "add fortran file" &&
+	sed -e "s/ChangeMe/IWasChanged/" <"$orig_file" >$fortran_file &&
+	git add $fortran_file &&
+	GIT_AUTHOR_NAME="B" GIT_AUTHOR_EMAIL="[email protected]" \
+	git commit -m "change fortran file"
+'
+
+test_expect_success 'blame -L :funcname with userdiff driver' '
+	check_count -f fortran-external-function -L:RIGHT A 7 B 1
+'
+
 test_expect_success 'setup incremental' '
 	(
 	GIT_AUTHOR_NAME=I &&

gitgitgadget

Good find.

Hmph. The reason why sb.path is not set to path at this point but later gets set is? I am wondering if this is leaving an unneeded room for sb.path and path to diverge by mistake. IOW, I wonder if it is better to set sb.path as early as we figure out which path we are interested in, forget we have local variable "path" and use sb.path consistently throughout the code.

But that is merely a potential future clean-up. The local variable path is still used one more time in the error message given when this parse_range_arg() fails, so at least this change makes the use of path more consistent. I like the simplicity of this fix.

Thanks, that change also makes sense.

I asked myself the same question, and could not come up with a good answer. I don't know if you read what I wrote in the cover letter (which I would have included in the in-patch commentary for this here patch if GitGitGadget had this feature), but I'm copying it here just in case you did not:

* In patch 6, I considered fixing that bug in a different way by initializing sb.path inside blame.c::setup_scoreboard. This function already receives 'path' as its second argument, so changing that would not impact the API. Probably Thomas thought that sb.path was already initialized in sb when he modified builtin/blame.c::prepare_blame_range to also send sb.path to line-range.c::parse_range_arg in 13b8f68c1f (log -L: :pattern:file syntax to find by funcname, 2013-03-28).

Initializing the path in setup_scoreboard would mean we could also simplify the API of blame.c::setup_blame_bloom_data since it would not need to receive path separately and so its second argument could be removed. I went for the simpler alternative of just sending 'path' to parse_range_arg instead of sb.path since it felt simpler, but if we feel it would be better to actually initialize sb.path in setup_scoreboard, I'll gladly tweak that for v2.

I also like its simplicity, and that's why I chose to submit this as v1. But I completely agree with you that it is "dangerous" in the sense that some further modifications to the code could then make the same mistake and use 'sb.path' thinking it is defined when it is not.

So I'm thinking of instead initializing it in setup_scoreboard for v2.

Cheers,

Philippe.

That does sound like a sensible way to clean it up. Thanks.

Changes since v1:

* patch 1: tweaked the message and took Junio's suggestion to only list the stuck form if the option in 'git-log' and 'gitk'.
* patch 2: very minor formatting changes (removed double quotes)
* patch 3: removed the parentheses as suggested by Junio, and did the same for git-log/gitk.
* patch 4: grammar fix pointed out by Eric
* patch 5: shortened the help text as suggested by Eric
* patch 6: changed the approach by initializing 'sb.path' earlier in 'cmd_blame', along with some of sb's other fields.
* patches 7-8: new cleanup patches following the new patch 6 to simplify the interface of two functions in 'blame.c'

----------------------------------------------------------------------------

This series fixes a bug in git blame where any userdiff driver configured via the attributes mechanism is ignored, which more often than not means thatgit blame -L :<funcname> dies for paths that have a configured userdiff driver. That's patch 6/6.

Patches 1-5 are documentation improvements around the line-log functionality.

Patches 7-8 are code clean-up patches following the changes in patch 6.

The fact that this bug has been present ever since git blame learned -L :<funcname> makes me a little sad. I think that the fact that the diff attribute (either with a builtin userdiff pattern or with diff.*.xfuncname ) influences way more than just the hunk header, but also features like git log -L, git blame -L, and the different options that I mention in patch 4/6, is not well-known enough. I'm not saying the code or the doc is wrong, but I think it's a visibility issue. Maybe more blog posts about that would help.... I noticed that the "Git Attributes" chapter in the ProGit book [1] does not even mention the builtin userdiff patterns, so I'll probably do a PR there to mention it. Any other ideas for improving discoverability of this very cool feature ?

[1] https://git-scm.com/book/en/v2/Customizing-Git-Git-Attributes

CC: Thomas Rast [email protected] [[email protected]], Eric Sunshine [email protected] [[email protected]], René Scharfe [email protected] [[email protected]]

Philippe Blain (8):
doc: log, gitk: move '-L' description to 'line-range-options.txt'
doc: line-range: improve formatting
blame-options.txt: also mention 'funcname' in '-L' description
doc: add more pointers to gitattributes(5) for userdiff
line-log: mention both modes in 'blame' and 'log' short help
blame: enable funcname blaming with userdiff driver
blame: simplify 'setup_scoreboard' interface
blame: simplify 'setup_blame_bloom_data' interface

 Documentation/blame-options.txt      |  9 +++++----
 Documentation/diff-options.txt       |  5 ++++-
 Documentation/git-grep.txt           |  6 ++++--
 Documentation/git-log.txt            | 15 +--------------
 Documentation/gitk.txt               | 20 +-------------------
 Documentation/line-range-format.txt  | 28 +++++++++++++++-------------
 Documentation/line-range-options.txt | 15 +++++++++++++++
 blame.c                              | 16 +++++++---------
 blame.h                              |  4 +---
 builtin/blame.c                      | 11 ++++++-----
 builtin/log.c                        |  4 ++--
 t/annotate-tests.sh                  | 18 ++++++++++++++++++
 12 files changed, 79 insertions(+), 72 deletions(-)
 create mode 100644 Documentation/line-range-options.txt

base-commit: 1d1c4a875900d69c7f0a31e44c3e370dc80ab1ce
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-774%2Fphil-blain%2Fblame-funcname-userdiff-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-774/phil-blain/blame-funcname-userdiff-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/774

Range-diff vs v1:

1: 96f6f95abc ! 1: b4fa5fb5ae doc: log, gitk: move '-L' description to 'line-range-options.txt'

     @@ Metadata
       ## Commit message ##
          doc: log, gitk: move '-L' description to 'line-range-options.txt'
      
     -    The description of the '-L' option for `git log` and `gitk` is the
     -    same, but is repeated in both 'git-log.txt' and 'gitk.txt'.
     +    The description of the '-L' option for `git log` and `gitk` is almost
     +    the same, but is repeated in both 'git-log.txt' and 'gitk.txt' (the
     +    difference being that 'git-log.txt' lists the option with a space
     +    after '-L', while 'gitk.txt' lists it as stuck and notes that `gitk`
     +    only understands the stuck form).
      
     -    Remove the duplication by creating a new file, 'line-range-options.txt',
     +    Reduce duplication by creating a new file, 'line-range-options.txt',
          and include it in both files.
      
     +    To simplify the presentation, only list the stuck form for both
     +    commands, and remove the note about `gitk` only understanding the stuck
     +    form.
     +
          Signed-off-by: Philippe Blain <[email protected]>
      
       ## Documentation/git-log.txt ##
     @@ Documentation/git-log.txt: produced by `--stat`, etc.
      -	`--name-only`, `--name-status`, `--check`) are not currently implemented.
      -+
      -include::line-range-format.txt[]
     -+:git-log: 1
      +include::line-range-options.txt[]
       
       <revision range>::
     @@ Documentation/gitk.txt: linkgit:git-rev-list[1] for a complete list.
      -*not* put a space after `-L`.
      -+
      -include::line-range-format.txt[]
     -+:gitk: 1
      +include::line-range-options.txt[]
       
       <revision range>::
     @@ Documentation/gitk.txt: linkgit:git-rev-list[1] for a complete list.
      
       ## Documentation/line-range-options.txt (new) ##
      @@
     -+ifdef::git-log[]
     -+-L <start>,<end>:<file>::
     -+-L :<funcname>:<file>::
     -+endif::git-log[]
     -+ifdef::gitk[]
      +-L<start>,<end>:<file>::
      +-L:<funcname>:<file>::
     -+endif::gitk[]
      +
      +	Trace the evolution of the line range given by "<start>,<end>"
      +	(or the function name regex <funcname>) within the <file>.  You may
     @@ Documentation/line-range-options.txt (new)
      +	(namely `--raw`, `--numstat`, `--shortstat`, `--dirstat`, `--summary`,
      +	`--name-only`, `--name-status`, `--check`) are not currently implemented.
      ++
     -+ifdef::gitk[]
     -+*Note:* gitk (unlike linkgit:git-log[1]) currently only understands
     -+this option if you specify it "glued together" with its argument.  Do
     -+*not* put a space after `-L`.
     -+endif::gitk[]
     -++
      +include::line-range-format.txt[]

2: 7d3fc0a503 ! 2: 6c44b1c576 doc: line-range: improve formatting
@@ Documentation/line-range-format.txt

+
-If ``:<funcname>'' is given in place of <start> and <end>, it is a
-+If ":<funcname>" is given in place of '<start>' and '<end>', it is a ++If :<funcname> is given in place of '<start>' and '<end>', it is a
regular expression that denotes the range from the first funcname line
-that matches <funcname>, up to the next funcname line. ``:<funcname>'' +that matches '<funcname>', up to the next funcname line. :<funcname>
@@ Documentation/line-range-format.txt
file.

## Documentation/line-range-options.txt ##
-@@ Documentation/line-range-options.txt: ifdef::gitk[] +@@
+ -L<start>,<end>:<file>::
-L:<funcname>:<file>::
- endif::gitk[]

- Trace the evolution of the line range given by "<start>,<end>"
- (or the function name regex <funcname>) within the <file>. You may
-+ Trace the evolution of the line range given by "'<start>,<end>'"
++ Trace the evolution of the line range given by '<start>,<end>'
+ (or the function name regex '<funcname>') within the '<file>'. You may
not give any pathspec limiters. This is currently limited to
a walk starting from a single revision, i.e., you may only
3: f7b64bf330 ! 3: d38f0f7e26 blame-options.txt: also mention 'funcname' in '-L' description
@@ Commit message
'-L :<funcname>' by mentioning it at the beginnning of the description of the '-L' option.

+ Also, in 'line-range-options.txt', which is used for git-log(1) and
+ gitk(1), do not parenthesize the mention of the ':<funcname>' mode, to
+ place it on equal footing with the '<start>,<end>' mode.
+

## Documentation/blame-options.txt ##
@@ Documentation/blame-options.txt
-L :<funcname>::
- Annotate only the given line range. May be specified multiple times.
- Overlapping ranges are allowed.
-+ Annotate only the line range given by '<start>,<end>'
-+ (or by the function name regex '<funcname>').
++ Annotate only the line range given by '<start>,<end>',
++ or by the function name regex '<funcname>'.
+ May be specified multiple times. Overlapping ranges are allowed.
+
'<start>' and '<end>' are optional. -L <start> or -L <start>, spans from '<start>' to end of file. -L ,<end> spans from start of file to '<end>'.
+
+ ## Documentation/line-range-options.txt ##
+@@
+ -L<start>,<end>:<file>::
+ -L:<funcname>:<file>::
+
+- Trace the evolution of the line range given by '<start>,<end>'
+- (or the function name regex '<funcname>') within the '<file>'. You may
++ Trace the evolution of the line range given by '<start>,<end>',
++ or by the function name regex '<funcname>', within the '<file>'. You may
+ not give any pathspec limiters. This is currently limited to
+ a walk starting from a single revision, i.e., you may only
+ give zero or one positive revision arguments, and
4: da76a10717 ! 4: 9443a681e3 doc: add more pointers to gitattributes(5) for userdiff
@@ Documentation/diff-options.txt: endif::git-format-patch[]
-W::
--function-context::
- Show whole surrounding functions of changes.
-+ Show whole functions as context lines for each changes.
++ Show whole function as context lines for each change.
+ The function names are determined in the same way as
+ git diff works out patch hunk headers (see 'Defining a
+ custom hunk-header' in linkgit:gitattributes[5]).
5: 27ef94e9cc ! 5: ae61d27a38 line-log: mention both modes in 'blame' and 'log' short help

     @@ builtin/blame.c: int cmd_blame(int argc, const char **argv, const char *prefix)
       		OPT_CALLBACK_F('M', NULL, &opt, N_("score"), N_("Find line movements within and across files"), PARSE_OPT_OPTARG, blame_move_callback),
      -		OPT_STRING_LIST('L', NULL, &range_list, N_("n,m"), N_("Process only line range n,m, counting from 1")),
      +		OPT_STRING_LIST('L', NULL, &range_list, N_("range"),
     -+				N_("Process only the given line range (<range>=<start>,<end>) or function (<range>=:<funcname>)")),
     ++				N_("Process only line range <start>,<end> or function :<funcname>")),
       		OPT__ABBREV(&abbrev),
       		OPT_END()
       	};
     @@ builtin/log.c: static void cmd_log_init_finish(int argc, const char **argv, cons
      -		OPT_CALLBACK('L', NULL, &line_cb, "n,m:file",
      -			     N_("Process line range n,m in file, counting from 1"),
      +		OPT_CALLBACK('L', NULL, &line_cb, "range:file",
     -+			     N_("Trace the evolution of the given line range (<range>=<start>,<end>) or function (<range>=:<funcname>) in <file>"),
     ++			     N_("Trace the evolution of line range <start>,<end> or function :<funcname> in <file>"),
       			     log_line_range_callback),
       		OPT_END()
       	};

6: a1e1c977d0 ! 6: c77108e864 blame: enable funcname blaming with userdiff driver
@@ Commit message
funcname, 2013-03-28).

Enable funcname blaming for paths using specific userdiff drivers by
- sending the local variable 'path' to 'parse_range_arg' instead of the
- yet unset 'sb.path'.
+ initializing 'sb.path' earlier in 'cmd_blame', when some of its other
+ fields are initialized, so that it is set when passed to
+ 'parse_range_arg'.

Add a regression test in 'annotate-tests.sh', which is sourced in t8001-annotate.sh and t8002-blame.sh, leveraging an existing file used to test the userdiff patterns in t4018-diff-funcname.

+ Also, use 'sb.path' instead of 'path' when constructing the error
+ message at line 1114, for consistency.
+

## builtin/blame.c ##
@@ builtin/blame.c: int cmd_blame(int argc, const char **argv, const char *prefix)
- long bottom, top;
- if (parse_range_arg(range_list.items[range_i].string,
- nth_line_cb, &sb, lno, anchor,
-- &bottom, &top, sb.path,
-+ &bottom, &top, path,
- the_repository->index))
- usage(blame_usage);
+ sb.contents_from = contents_from;
+ sb.reverse = reverse;
+ sb.repo = the_repository;
++ sb.path = path;
+ build_ignorelist(&sb, &ignore_revs_file_list, &ignore_rev_list);
+ string_list_clear(&ignore_revs_file_list, 0);
+ string_list_clear(&ignore_rev_list, 0);
+@@ builtin/blame.c: int cmd_blame(int argc, const char **argv, const char *prefix)
if ((!lno && (top || bottom)) || lno < bottom)
+ die(Q_("file %s has only %lu line",
+ "file %s has only %lu lines",
+- lno), path, lno);
++ lno), sb.path, lno);
+ if (bottom < 1)
+ bottom = 1;
+ if (top < 1 || lno < top)
+@@ builtin/blame.c: int cmd_blame(int argc, const char **argv, const char *prefix)
+ string_list_clear(&range_list, 0);
+
+ sb.ent = NULL;
+- sb.path = path;
+
+ if (blame_move_score)
+ sb.move_score = blame_move_score;

## t/annotate-tests.sh ##
@@ t/annotate-tests.sh: test_expect_success 'blame -L ^:RE (absolute: end-of-file)' '

 -:  ---------- > 7:  3b28696e51 blame: simplify 'setup_scoreboard' interface
 -:  ---------- > 8:  7aca3274d2 blame: simplify 'setup_blame_bloom_data' interface

gitgitgadget

From: Philippe Blain <[email protected]>

The description of the '-L' option for git log and gitk is almost the same, but is repeated in both 'git-log.txt' and 'gitk.txt' (the difference being that 'git-log.txt' lists the option with a space after '-L', while 'gitk.txt' lists it as stuck and notes that gitk only understands the stuck form).

Reduce duplication by creating a new file, 'line-range-options.txt', and include it in both files.

To simplify the presentation, only list the stuck form for both commands, and remove the note about gitk only understanding the stuck form.

---
 Documentation/git-log.txt            | 15 +--------------
 Documentation/gitk.txt               | 20 +-------------------
 Documentation/line-range-options.txt | 15 +++++++++++++++
 3 files changed, 17 insertions(+), 33 deletions(-)
 create mode 100644 Documentation/line-range-options.txt

index 2b8ac5ff88..dd189a353a 100644
@@ -77,20 +77,7 @@ produced by `--stat`, etc.
 	Intended to speed up tools that read log messages from `git log`
 	output by allowing them to allocate space in advance.
 
--L <start>,<end>:<file>::
--L :<funcname>:<file>::
-	Trace the evolution of the line range given by "<start>,<end>"
-	(or the function name regex <funcname>) within the <file>.  You may
-	not give any pathspec limiters.  This is currently limited to
-	a walk starting from a single revision, i.e., you may only
-	give zero or one positive revision arguments, and
-	<start> and <end> (or <funcname>) must exist in the starting revision.
-	You can specify this option more than once. Implies `--patch`.
-	Patch output can be suppressed using `--no-patch`, but other diff formats
-	(namely `--raw`, `--numstat`, `--shortstat`, `--dirstat`, `--summary`,
-	`--name-only`, `--name-status`, `--check`) are not currently implemented.
-+
-include::line-range-format.txt[]
+include::line-range-options.txt[]
 
 <revision range>::
 	Show only commits in the specified revision range.  When no

index c653ebb6a8..d50e9ed10e 100644
@@ -98,25 +98,7 @@ linkgit:git-rev-list[1] for a complete list.
 	(See "History simplification" in linkgit:git-log[1] for a more
 	detailed explanation.)
 
--L<start>,<end>:<file>::
--L:<funcname>:<file>::
-
-	Trace the evolution of the line range given by "<start>,<end>"
-	(or the function name regex <funcname>) within the <file>.  You may
-	not give any pathspec limiters.  This is currently limited to
-	a walk starting from a single revision, i.e., you may only
-	give zero or one positive revision arguments, and
-	<start> and <end> (or <funcname>) must exist in the starting revision.
-	You can specify this option more than once. Implies `--patch`.
-	Patch output can be suppressed using `--no-patch`, but other diff formats
-	(namely `--raw`, `--numstat`, `--shortstat`, `--dirstat`, `--summary`,
-	`--name-only`, `--name-status`, `--check`) are not currently implemented.
-+
-*Note:* gitk (unlike linkgit:git-log[1]) currently only understands
-this option if you specify it "glued together" with its argument.  Do
-*not* put a space after `-L`.
-+
-include::line-range-format.txt[]
+include::line-range-options.txt[]
 
 <revision range>::
 

new file mode 100644
index 0000000000..266263f6b4
@@ -0,0 +1,15 @@
+-L<start>,<end>:<file>::
+-L:<funcname>:<file>::
+
+	Trace the evolution of the line range given by "<start>,<end>"
+	(or the function name regex <funcname>) within the <file>.  You may
+	not give any pathspec limiters.  This is currently limited to
+	a walk starting from a single revision, i.e., you may only
+	give zero or one positive revision arguments, and
+	<start> and <end> (or <funcname>) must exist in the starting revision.
+	You can specify this option more than once. Implies `--patch`.
+	Patch output can be suppressed using `--no-patch`, but other diff formats
+	(namely `--raw`, `--numstat`, `--shortstat`, `--dirstat`, `--summary`,
+	`--name-only`, `--name-status`, `--check`) are not currently implemented.
++
+include::line-range-format.txt[]

gitgitgadget

From: Philippe Blain <[email protected]>

Improve the formatting of the description of the line-range option '-L' for git log, gitk and git blame:

- Use bold for <start>, <end> and <funcname>
- Use backticks for literals

---
 Documentation/blame-options.txt      |  4 ++--
 Documentation/line-range-format.txt  | 24 ++++++++++++------------
 Documentation/line-range-options.txt |  6 +++---
 3 files changed, 17 insertions(+), 17 deletions(-)

index 88750af7ae..48bf0eeec5 100644
@@ -14,8 +14,8 @@
 	Annotate only the given line range. May be specified multiple times.
 	Overlapping ranges are allowed.
 +
-<start> and <end> are optional. ``-L <start>'' or ``-L <start>,'' spans from
-<start> to end of file. ``-L ,<end>'' spans from start of file to <end>.
+'<start>' and '<end>' are optional. `-L <start>` or `-L <start>,` spans from
+'<start>' to end of file. `-L ,<end>` spans from start of file to '<end>'.
 +
 include::line-range-format.txt[]
 

index 829676ff98..43759eef89 100644
@@ -1,30 +1,30 @@
-<start> and <end> can take one of these forms:
+'<start>' and '<end>' can take one of these forms:
 
 - number
 +
-If <start> or <end> is a number, it specifies an
+If '<start>' or '<end>' is a number, it specifies an
 absolute line number (lines count from 1).
 +
 
-- /regex/
+- `/regex/`
 +
 This form will use the first line matching the given
-POSIX regex. If <start> is a regex, it will search from the end of
+POSIX regex. If '<start>' is a regex, it will search from the end of
 the previous `-L` range, if any, otherwise from the start of file.
-If <start> is ``^/regex/'', it will search from the start of file.
-If <end> is a regex, it will search
-starting at the line given by <start>.
+If '<start>' is `^/regex/`, it will search from the start of file.
+If '<end>' is a regex, it will search
+starting at the line given by '<start>'.
 +
 
 - +offset or -offset
 +
-This is only valid for <end> and will specify a number
-of lines before or after the line given by <start>.
+This is only valid for '<end>' and will specify a number
+of lines before or after the line given by '<start>'.
 
 +
-If ``:<funcname>'' is given in place of <start> and <end>, it is a
+If `:<funcname>` is given in place of '<start>' and '<end>', it is a
 regular expression that denotes the range from the first funcname line
-that matches <funcname>, up to the next funcname line. ``:<funcname>''
+that matches '<funcname>', up to the next funcname line. `:<funcname>`
 searches from the end of the previous `-L` range, if any, otherwise
-from the start of file. ``^:<funcname>'' searches from the start of
+from the start of file. `^:<funcname>` searches from the start of
 file.

index 266263f6b4..1c90127300 100644
@@ -1,12 +1,12 @@
 -L<start>,<end>:<file>::
 -L:<funcname>:<file>::
 
-	Trace the evolution of the line range given by "<start>,<end>"
-	(or the function name regex <funcname>) within the <file>.  You may
+	Trace the evolution of the line range given by '<start>,<end>'
+	(or the function name regex '<funcname>') within the '<file>'. You may
 	not give any pathspec limiters.  This is currently limited to
 	a walk starting from a single revision, i.e., you may only
 	give zero or one positive revision arguments, and
-	<start> and <end> (or <funcname>) must exist in the starting revision.
+	'<start>' and '<end>' (or '<funcname>') must exist in the starting revision.
 	You can specify this option more than once. Implies `--patch`.
 	Patch output can be suppressed using `--no-patch`, but other diff formats
 	(namely `--raw`, `--numstat`, `--shortstat`, `--dirstat`, `--summary`,

gitgitgadget

From: Philippe Blain <[email protected]>

Make it clearer that a function can be blamed by feeding git blame '-L :<funcname>' by mentioning it at the beginnning of the description of the '-L' option.

Also, in 'line-range-options.txt', which is used for git-log(1) and gitk(1), do not parenthesize the mention of the ':<funcname>' mode, to place it on equal footing with the '<start>,<end>' mode.

---
 Documentation/blame-options.txt      | 5 +++--
 Documentation/line-range-options.txt | 4 ++--
 2 files changed, 5 insertions(+), 4 deletions(-)

index 48bf0eeec5..dc3bceb6d1 100644
@@ -11,8 +11,9 @@
 
 -L <start>,<end>::
 -L :<funcname>::
-	Annotate only the given line range. May be specified multiple times.
-	Overlapping ranges are allowed.
+	Annotate only the line range given by '<start>,<end>',
+	or by the function name regex '<funcname>'.
+	May be specified multiple times. Overlapping ranges are allowed.
 +
 '<start>' and '<end>' are optional. `-L <start>` or `-L <start>,` spans from
 '<start>' to end of file. `-L ,<end>` spans from start of file to '<end>'.

index 1c90127300..8e295a62b8 100644
@@ -1,8 +1,8 @@
 -L<start>,<end>:<file>::
 -L:<funcname>:<file>::
 
-	Trace the evolution of the line range given by '<start>,<end>'
-	(or the function name regex '<funcname>') within the '<file>'. You may
+	Trace the evolution of the line range given by '<start>,<end>',
+	or by the function name regex '<funcname>', within the '<file>'. You may
 	not give any pathspec limiters.  This is currently limited to
 	a walk starting from a single revision, i.e., you may only
 	give zero or one positive revision arguments, and

gitgitgadget

From: Philippe Blain <[email protected]>

Several Git commands can make use of the builtin userdiff patterns, but it's not obvious in the documentation. Add pointers to the 'Defining a custom hunk header' part of gitattributes(5) in the description of the following options:

- the '--function-context' option of git diff and friends
- the '--function-context' option of git grep
- the '-L :<funcname>' option of git log, gitk and git blame

In 'git-grep.txt', take the opportunity to use backticks in the description of '--show-function', and improve the wording of the desription of '--function-context'.

---
 Documentation/diff-options.txt      | 5 ++++-
 Documentation/git-grep.txt          | 6 ++++--
 Documentation/line-range-format.txt | 4 +++-
 3 files changed, 11 insertions(+), 4 deletions(-)

index 573fb9bb71..72b558ce21 100644
@@ -695,7 +695,10 @@ endif::git-format-patch[]
 
 -W::
 --function-context::
-	Show whole surrounding functions of changes.
+	Show whole function as context lines for each change.
+	The function names are determined in the same way as
+	`git diff` works out patch hunk headers (see 'Defining a
+	custom hunk-header' in linkgit:gitattributes[5]).
 
 ifndef::git-format-patch[]
 ifndef::git-log[]

index 6077ff01a4..4e0ba8234a 100644
@@ -241,7 +241,7 @@ providing this option will cause it to die.
 --show-function::
 	Show the preceding line that contains the function name of
 	the match, unless the matching line is a function name itself.
-	The name is determined in the same way as 'git diff' works out
+	The name is determined in the same way as `git diff` works out
 	patch hunk headers (see 'Defining a custom hunk-header' in
 	linkgit:gitattributes[5]).
 

@@ -266,7 +266,9 @@ providing this option will cause it to die.
 	Show the surrounding text from the previous line containing a
 	function name up to the one before the next function name,
 	effectively showing the whole function in which the match was
-	found.
+	found. The function names are determined in the same way as
+	`git diff` works out patch hunk headers (see 'Defining a
+	custom hunk-header' in linkgit:gitattributes[5]).
 
 --threads <num>::
 	Number of grep worker threads to use.

index 43759eef89..9b51e9fb66 100644
@@ -27,4 +27,6 @@ regular expression that denotes the range from the first funcname line
 that matches '<funcname>', up to the next funcname line. `:<funcname>`
 searches from the end of the previous `-L` range, if any, otherwise
 from the start of file. `^:<funcname>` searches from the start of
-file.
+file. The function names are determined in the same way as `git diff`
+works out patch hunk headers (see 'Defining a custom hunk-header'
+in linkgit:gitattributes[5]).

gitgitgadget

From: Philippe Blain <[email protected]>

'git blame -h' and 'git log -h' both show '-L <n,m>' and describe this option as "Process only line range n,m, counting from 1". No hint is given that a function name regex can also be used.

Use <range> instead, and expand the description of the option to mention both modes. Remove "counting from 1" as it's uneeded; it's uncommon to refer to the first line of a file as "line 0".

Also, for 'git log', improve the wording to better reflect the long help.

---
 builtin/blame.c | 3 ++-
 builtin/log.c   | 4 ++--
 2 files changed, 4 insertions(+), 3 deletions(-)

index bb0f29300e..224b6f18d4 100644
@@ -889,7 +889,8 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 		OPT_STRING(0, "contents", &contents_from, N_("file"), N_("Use <file>'s contents as the final image")),
 		OPT_CALLBACK_F('C', NULL, &opt, N_("score"), N_("Find line copies within and across files"), PARSE_OPT_OPTARG, blame_copy_callback),
 		OPT_CALLBACK_F('M', NULL, &opt, N_("score"), N_("Find line movements within and across files"), PARSE_OPT_OPTARG, blame_move_callback),
-		OPT_STRING_LIST('L', NULL, &range_list, N_("n,m"), N_("Process only line range n,m, counting from 1")),
+		OPT_STRING_LIST('L', NULL, &range_list, N_("range"),
+				N_("Process only line range <start>,<end> or function :<funcname>")),
 		OPT__ABBREV(&abbrev),
 		OPT_END()
 	};

index 0a7ed4bef9..c87ce09325 100644
@@ -183,8 +183,8 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
 				N_("pattern"), N_("do not decorate refs that match <pattern>")),
 		OPT_CALLBACK_F(0, "decorate", NULL, NULL, N_("decorate options"),
 			       PARSE_OPT_OPTARG, decorate_callback),
-		OPT_CALLBACK('L', NULL, &line_cb, "n,m:file",
-			     N_("Process line range n,m in file, counting from 1"),
+		OPT_CALLBACK('L', NULL, &line_cb, "range:file",
+			     N_("Trace the evolution of line range <start>,<end> or function :<funcname> in <file>"),
 			     log_line_range_callback),
 		OPT_END()
 	};

gitgitgadget

From: Philippe Blain <[email protected]>

In blame.c::cmd_blame, we send the 'path' field of the 'sb' 'struct blame_scoreboard' as the 'path' argument to 'line-range.c::parse_range_arg', but 'sb.path' is not set yet; it's set to the local variable 'path' a few lines later at line 1137.

This 'path' argument is only used in 'parse_range_arg' if we are blaming a funcname, i.e. git blame -L :<funcname> <path>, and in that case it is sent to 'parse_range_funcname', where it is used to determine if a userdiff driver should be used for said <path> to match the given funcname.

Since 'path' is yet unset, the userdiff driver is never used, so we fall back to the default funcname regex, which is usually not appropriate for paths that are set to use a specific userdiff driver, and thus either we match some unrelated lines, or we die with

fatal: -L parameter '<funcname>' starting at line 1: no match

This has been the case ever since git blame learned to blame a funcname in 13b8f68c1f (log -L: :pattern:file syntax to find by funcname, 2013-03-28).

Enable funcname blaming for paths using specific userdiff drivers by initializing 'sb.path' earlier in 'cmd_blame', when some of its other fields are initialized, so that it is set when passed to 'parse_range_arg'.

Add a regression test in 'annotate-tests.sh', which is sourced in t8001-annotate.sh and t8002-blame.sh, leveraging an existing file used to test the userdiff patterns in t4018-diff-funcname.

Also, use 'sb.path' instead of 'path' when constructing the error message at line 1114, for consistency.

---
 builtin/blame.c     |  4 ++--
 t/annotate-tests.sh | 18 ++++++++++++++++++
 2 files changed, 20 insertions(+), 2 deletions(-)

index 224b6f18d4..f2e528fcbc 100644
@@ -1081,6 +1081,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	sb.contents_from = contents_from;
 	sb.reverse = reverse;
 	sb.repo = the_repository;
+	sb.path = path;
 	build_ignorelist(&sb, &ignore_revs_file_list, &ignore_rev_list);
 	string_list_clear(&ignore_revs_file_list, 0);
 	string_list_clear(&ignore_rev_list, 0);

@@ -1110,7 +1111,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 		if ((!lno && (top || bottom)) || lno < bottom)
 			die(Q_("file %s has only %lu line",
 			       "file %s has only %lu lines",
-			       lno), path, lno);
+			       lno), sb.path, lno);
 		if (bottom < 1)
 			bottom = 1;
 		if (top < 1 || lno < top)

@@ -1135,7 +1136,6 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	string_list_clear(&range_list, 0);
 
 	sb.ent = NULL;
-	sb.path = path;
 
 	if (blame_move_score)
 		sb.move_score = blame_move_score;

index d933af5714..3aee61d2cc 100644
@@ -479,6 +479,24 @@ test_expect_success 'blame -L ^:RE (absolute: end-of-file)' '
 	check_count -f hello.c -L$n -L^:ma.. F 4 G 1 H 1
 '
 
+test_expect_success 'setup -L :funcname with userdiff driver' '
+	echo "fortran-* diff=fortran" >.gitattributes &&
+	fortran_file=fortran-external-function &&
+	orig_file="$TEST_DIRECTORY/t4018/$fortran_file" &&
+	cp $orig_file . &&
+	git add $fortran_file &&
+	GIT_AUTHOR_NAME="A" GIT_AUTHOR_EMAIL="[email protected]" \
+	git commit -m "add fortran file" &&
+	sed -e "s/ChangeMe/IWasChanged/" <"$orig_file" >$fortran_file &&
+	git add $fortran_file &&
+	GIT_AUTHOR_NAME="B" GIT_AUTHOR_EMAIL="[email protected]" \
+	git commit -m "change fortran file"
+'
+
+test_expect_success 'blame -L :funcname with userdiff driver' '
+	check_count -f fortran-external-function -L:RIGHT A 7 B 1
+'
+
 test_expect_success 'setup incremental' '
 	(
 	GIT_AUTHOR_NAME=I &&

gitgitgadget

From: Philippe Blain <[email protected]>

The previous commit moved the initialization of 'sb.path' in 'builtin/blame.c::cmd_blame' before the call to 'blame.c::setup_scoreboard'. Since 'cmd_blame' is the only caller of 'setup_scoreboard', it is now unnecessary for 'setup_scoreboard' to receive 'path' as a separate argument, as 'sb.path' is already initialized.

Remove this argument from setup_scoreboard's interface and use the 'path' field of the 'sb' 'struct blame_scoreboard' instead.

---
 blame.c         | 11 +++++------
 blame.h         |  1 -
 builtin/blame.c |  2 +-
 3 files changed, 6 insertions(+), 8 deletions(-)

index 686845b2b4..128cb7ba55 100644
@@ -2764,7 +2764,6 @@ void init_scoreboard(struct blame_scoreboard *sb)
 }
 
 void setup_scoreboard(struct blame_scoreboard *sb,
-		      const char *path,
 		      struct blame_origin **orig)
 {
 	const char *final_commit_name = NULL;

@@ -2803,7 +2802,7 @@ void setup_scoreboard(struct blame_scoreboard *sb,
 		setup_work_tree();
 		sb->final = fake_working_tree_commit(sb->repo,
 						     &sb->revs->diffopt,
-						     path, sb->contents_from);
+						     sb->path, sb->contents_from);
 		add_pending_object(sb->revs, &(sb->final->object), ":");
 	}
 

@@ -2846,12 +2845,12 @@ void setup_scoreboard(struct blame_scoreboard *sb,
 		sb->final_buf_size = o->file.size;
 	}
 	else {
-		o = get_origin(sb->final, path);
+		o = get_origin(sb->final, sb->path);
 		if (fill_blob_sha1_and_mode(sb->repo, o))
-			die(_("no such path %s in %s"), path, final_commit_name);
+			die(_("no such path %s in %s"), sb->path, final_commit_name);
 
 		if (sb->revs->diffopt.flags.allow_textconv &&
-		    textconv_object(sb->repo, path, o->mode, &o->blob_oid, 1, (char **) &sb->final_buf,
+		    textconv_object(sb->repo, sb->path, o->mode, &o->blob_oid, 1, (char **) &sb->final_buf,
 				    &sb->final_buf_size))
 			;
 		else

@@ -2861,7 +2860,7 @@ void setup_scoreboard(struct blame_scoreboard *sb,
 		if (!sb->final_buf)
 			die(_("cannot read blob %s for path %s"),
 			    oid_to_hex(&o->blob_oid),
-			    path);
+			    sb->path);
 	}
 	sb->num_read_blob++;
 	prepare_lines(sb);

index b6bbee4147..e8c185c8ef 100644
@@ -181,7 +181,6 @@ const char *blame_nth_line(struct blame_scoreboard *sb, long lno);
 
 void init_scoreboard(struct blame_scoreboard *sb);
 void setup_scoreboard(struct blame_scoreboard *sb,
-		      const char *path,
 		      struct blame_origin **orig);
 void setup_blame_bloom_data(struct blame_scoreboard *sb,
 			    const char *path);

index f2e528fcbc..5d8f4c4599 100644
@@ -1085,7 +1085,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	build_ignorelist(&sb, &ignore_revs_file_list, &ignore_rev_list);
 	string_list_clear(&ignore_revs_file_list, 0);
 	string_list_clear(&ignore_rev_list, 0);
-	setup_scoreboard(&sb, path, &o);
+	setup_scoreboard(&sb, &o);
 
 	/*
 	 * Changed-path Bloom filters are disabled when looking

gitgitgadget

From: Philippe Blain <[email protected]>

The penultimate commit moved the initialization of 'sb.path' in 'builtin/blame.c::cmd_blame' before the call to 'blame.c::setup_blame_bloom_data'. Since 'cmd_blame' is the only caller of 'setup_blame_bloom_data', it is now unnecessary for 'setup_blame_bloom_data' to receive 'path' as a separate argument, as 'sb.path' is already initialized.

Remove this argument from setup_blame_bloom_data's interface and use the 'path' field of the 'sb' 'struct blame_scoreboard' instead.

---
 blame.c         | 5 ++---
 blame.h         | 3 +--
 builtin/blame.c | 2 +-
 3 files changed, 4 insertions(+), 6 deletions(-)

index 128cb7ba55..9042aa3f2b 100644
@@ -2887,8 +2887,7 @@ struct blame_entry *blame_entry_prepend(struct blame_entry *head,
 	return new_head;
 }
 
-void setup_blame_bloom_data(struct blame_scoreboard *sb,
-			    const char *path)
+void setup_blame_bloom_data(struct blame_scoreboard *sb)
 {
 	struct blame_bloom_data *bd;
 	struct bloom_filter_settings *bs;

@@ -2908,7 +2907,7 @@ void setup_blame_bloom_data(struct blame_scoreboard *sb,
 	bd->nr = 0;
 	ALLOC_ARRAY(bd->keys, bd->alloc);
 
-	add_bloom_key(bd, path);
+	add_bloom_key(bd, sb->path);
 
 	sb->bloom_data = bd;
 }

index e8c185c8ef..38bde535b3 100644
@@ -182,8 +182,7 @@ const char *blame_nth_line(struct blame_scoreboard *sb, long lno);
 void init_scoreboard(struct blame_scoreboard *sb);
 void setup_scoreboard(struct blame_scoreboard *sb,
 		      struct blame_origin **orig);
-void setup_blame_bloom_data(struct blame_scoreboard *sb,
-			    const char *path);
+void setup_blame_bloom_data(struct blame_scoreboard *sb);
 void cleanup_scoreboard(struct blame_scoreboard *sb);
 
 struct blame_entry *blame_entry_prepend(struct blame_entry *head,

index 5d8f4c4599..2f8b06e589 100644
@@ -1092,7 +1092,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	 * for copies.
 	 */
 	if (!(opt & PICKAXE_BLAME_COPY))
-		setup_blame_bloom_data(&sb, path);
+		setup_blame_bloom_data(&sb);
 
 	lno = sb.num_lines;
 

gitgitgadget