{"thread":{"id":"54521","subject":"Re: [PATCH 5/6] line-log: mention both modes in 'blame' and 'log' short help","startedAt":"2020-10-28T21:44:59Z","lastAt":"2020-11-01T17:29:03Z","messageCount":30,"participants":["Eric Sunshine","Philippe Blain via GitGitGadget","Philippe Blain","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"408554","messageId":"CAPig+cRFaT9ww3K368b58m-xgW843jhZWjXDspVrfdyYY-_wRA@mail.gmail.com","threadId":"54521","inReplyTo":"27ef94e9cc4189c3d74e984437dcce24e1f29678.1603889270.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 5/6] line-log: mention both modes in 'blame' and 'log' short help","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-10-28T17:33:09Z","receivedAt":"2020-10-28T21:44:59Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Oct 28, 2020 at 8:48 AM Philippe Blain via GitGitGadget\n wrote:\n> 'git blame -h' and 'git log -h' both show '-L ' and describe this\n> option as \"Process only line range n,m, counting from 1\". No hint is\n> given that a function name regex can also be used.\n>\n> Use instead, and expand the description of the option to mention\n> both modes. Remove \"counting from 1\" as it's uneeded; it's uncommon to\n> refer to the first line of a file as \"line 0\".\n>\n> Signed-off-by: Philippe Blain \n> ---\n> diff --git a/builtin/blame.c b/builtin/blame.c\n> @@ -889,7 +889,8 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n> - OPT_STRING_LIST('L', NULL, &range_list, N_(\"n,m\"), N_(\"Process only line range n,m, counting from 1\")),\n> + OPT_STRING_LIST('L', NULL, &range_list, N_(\"range\"),\n> + N_(\"Process only the given line range (=,) or function (=:)\")),\n\nThe \"=\" bit is redundant and confusing (and ugly). Considering\nthat the description already says \"Process only the given line\n_range_\", it should be fine to drop the \"=\" lead-in. Perhaps:\n\n Process only the given line range , or :\"\n\nThis might feel too succinct, but that's often true of short -h help.\nSuch succinctness is generally acceptable as long as more detailed\ndocumentation can be discovered easily (such as in the 'man' page).\n\nSame comment regarding the rest of the changes in this patch.\n"},{"id":"408562","messageId":"pull.774.git.1603889270.gitgitgadget@gmail.com","threadId":"54521","inReplyTo":null,"subject":"[PATCH 0/6] blame: enable funcname blaming with userdiff driver","fromName":"Philippe Blain via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-10-28T12:47:44Z","receivedAt":"2020-10-28T22:05:22Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"This series fixes a bug in git blame where any userdiff driver configured\nvia the attributes mechanism is ignored, which more often than not means\nthatgit blame -L : dies for paths that have a configured userdiff\ndriver. That's patch 6/6. \n\nPatches 1-5 are documentation improvements around the line-log\nfunctionality.\n\nNotes (I can't add in-patch commentaries using GitGitGadget (yet, I hope 1\n[https://github.com/gitgitgadget/gitgitgadget/issues/173]), so I'm providing\nwhat would have been an in-patch commentary on patch 6 below):\n\n * In patch 6, I considered fixing that bug in a different way by\n initializing sb.path inside blame.c::setup_scoreboard. This function\n already receives path as its second argument, so changing that would not\n impact the API. Probably Thomas thought that sb.path was already\n initialized in sb when he modified builtin/blame.c::prepare_blame_range \n to also send sb.path to line-range.c::parse_range_arg in 13b8f68c1f (log\n -L: :pattern:file syntax to find by funcname, 2013-03-28). \n \n Initializing the path in setup_scoreboard would mean we could also\n simplify the API of blame.c::setup_blame_bloom_data since it would not\n need to receive path separately and so its second argument could be\n removed. I went for the simpler alternative of just sending path to \n parse_range_arg instead of sb.path since it felt simpler, but if we feel\n it would be better to actually initialize sb.path in setup_scoreboard,\n I'll gladly tweak that for v2.\n \n \n * The fact that this bug has been present ever since git blame learned -L\n : makes me a little sad. I think that the fact that the diff \n attribute (either with a builtin userdiff pattern or with \n diff.*.xfuncname ) influences way more than just the hunk header, but\n also features like git log -L, git blame -L, and the different options\n that I mention in patch 4/6, is not well-known enough. I'm not saying the\n code or the doc is wrong, but I think it's a visibility issue. Maybe more\n blog posts about that would help.... I noticed that the \"Git Attributes\"\n chapter in the ProGit book 2\n [https://git-scm.com/book/en/v2/Customizing-Git-Git-Attributes] does not\n even mention the builtin userdiff patterns, so I'll probably do a PR\n there to mention it. Any other ideas for improving discoverability of\n this very cool feature ?\n\nPhilippe Blain (6):\n doc: log, gitk: move '-L' description to 'line-range-options.txt'\n doc: line-range: improve formatting\n blame-options.txt: also mention 'funcname' in '-L' description\n doc: add more pointers to gitattributes(5) for userdiff\n line-log: mention both modes in 'blame' and 'log' short help\n blame: enable funcname blaming with userdiff driver\n\n Documentation/blame-options.txt | 9 +++++----\n Documentation/diff-options.txt | 5 ++++-\n Documentation/git-grep.txt | 6 ++++--\n Documentation/git-log.txt | 16 ++--------------\n Documentation/gitk.txt | 21 ++-------------------\n Documentation/line-range-format.txt | 28 +++++++++++++++-------------\n Documentation/line-range-options.txt | 27 +++++++++++++++++++++++++++\n builtin/blame.c | 5 +++--\n builtin/log.c | 4 ++--\n t/annotate-tests.sh | 18 ++++++++++++++++++\n 10 files changed, 82 insertions(+), 57 deletions(-)\n create mode 100644 Documentation/line-range-options.txt\n\n\nbase-commit: 1d1c4a875900d69c7f0a31e44c3e370dc80ab1ce\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-774%2Fphil-blain%2Fblame-funcname-userdiff-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-774/phil-blain/blame-funcname-userdiff-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/774\n-- \ngitgitgadget\n"},{"id":"408571","messageId":"f7b64bf3302f934c44cf9364e4f008bb9ce26def.1603889270.git.gitgitgadget@gmail.com","threadId":"54521","inReplyTo":"pull.774.git.1603889270.gitgitgadget@gmail.com","subject":"[PATCH 3/6] blame-options.txt: also mention 'funcname' in '-L' description","fromName":"Philippe Blain via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-10-28T12:47:47Z","receivedAt":"2020-10-28T22:41:29Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"From: Philippe Blain \n\nMake it clearer that a function can be blamed by feeding `git blame`\n'-L :' by mentioning it at the beginnning of the description\nof the '-L' option.\n\nSigned-off-by: Philippe Blain \n---\n Documentation/blame-options.txt | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt\nindex 48bf0eeec5..8b4a90e349 100644\n--- a/Documentation/blame-options.txt\n+++ b/Documentation/blame-options.txt\n@@ -11,8 +11,9 @@\n \n -L ,::\n -L :::\n-\tAnnotate only the given line range. May be specified multiple times.\n-\tOverlapping ranges are allowed.\n+\tAnnotate only the line range given by ','\n+\t(or by the function name regex '').\n+\tMay be specified multiple times. Overlapping ranges are allowed.\n +\n '' and '' are optional. `-L ` or `-L ,` spans from\n '' to end of file. `-L ,` spans from start of file to ''.\n-- \ngitgitgadget\n\n"},{"id":"408574","messageId":"CAPig+cQcjjj3x_iTDX=yBUai6xHrShsGO5J3sdpX-2UhM7_OJQ@mail.gmail.com","threadId":"54521","inReplyTo":"7d3fc0a503b160ea15363a4465af4647bce6a30b.1603889270.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/6] doc: line-range: improve formatting","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-10-28T17:23:02Z","receivedAt":"2020-10-28T22:44:38Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Oct 28, 2020 at 8:48 AM Philippe Blain via GitGitGadget\n wrote:\n> Improve the formatting of the description of the line-range option '-L'\n> for `git log`, `gitk` and `git blame`:\n>\n> - Use bold for , and \n\nMy impression is that it is more common in Git documentation for these\nplaceholders to be formatted with backticks rather than as bold (or,\nif not more common currently, at least is trending that way). That's\nnot to say that my impression is necessarily accurate.\n"},{"id":"408578","messageId":"CAPig+cTA7akd7oB_14yBRhxXrH-mVX3yVtaPDdFDT+OTZdQuQQ@mail.gmail.com","threadId":"54521","inReplyTo":"da76a10717c917abab9e1bd5ab78db2a4f7ca7c0.1603889270.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 4/6] doc: add more pointers to gitattributes(5) for userdiff","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-10-28T17:26:34Z","receivedAt":"2020-10-28T23:02:57Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Oct 28, 2020 at 8:48 AM Philippe Blain via GitGitGadget\n wrote:\n> Several Git commands can make use of the builtin userdiff patterns, but\n> it's not obvious in the documentation. Add pointers to the 'Defining a\n> custom hunk header' part of gitattributes(5) in the description of the\n> following options:\n> [...]\n> Signed-off-by: Philippe Blain \n> ---\n> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\n> @@ -695,7 +695,10 @@ endif::git-format-patch[]\n> --function-context::\n> - Show whole surrounding functions of changes.\n> + Show whole functions as context lines for each changes.\n\ns/changes/change/\n\n> + The function names are determined in the same way as\n> + `git diff` works out patch hunk headers (see 'Defining a\n> + custom hunk-header' in linkgit:gitattributes[5]).\n"},{"id":"408581","messageId":"a1e1c977d0978424fb07c97be0479f43a325cbea.1603889270.git.gitgitgadget@gmail.com","threadId":"54521","inReplyTo":"pull.774.git.1603889270.gitgitgadget@gmail.com","subject":"[PATCH 6/6] blame: enable funcname blaming with userdiff driver","fromName":"Philippe Blain via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-10-28T12:47:50Z","receivedAt":"2020-10-28T23:20:57Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"From: Philippe Blain \n\nIn blame.c::cmd_blame, we send the 'path' field of the 'sb' 'struct\nblame_scoreboard' as the 'path' argument to\n'line-range.c::parse_range_arg', but 'sb.path' is not set yet; it's set\nto the local variable 'path' a few lines later at line 1137.\n\nThis 'path' argument is only used in 'parse_range_arg' if we are blaming\na funcname, i.e. `git blame -L : `, and in that case it\nis sent to 'parse_range_funcname', where it is used to determine if a\nuserdiff driver should be used for said to match the given\nfuncname.\n\nSince 'path' is yet unset, the userdiff driver is never used, so we fall\nback to the default funcname regex, which is usually not appropriate for\npaths that are set to use a specific userdiff driver, and thus either we\nmatch some unrelated lines, or we die with\n\n fatal: -L parameter '' starting at line 1: no match\n\nThis has been the case ever since `git blame` learned to blame a\nfuncname in 13b8f68c1f (log -L: :pattern:file syntax to find by\nfuncname, 2013-03-28).\n\nEnable funcname blaming for paths using specific userdiff drivers by\nsending the local variable 'path' to 'parse_range_arg' instead of the\nyet unset 'sb.path'.\n\nAdd a regression test in 'annotate-tests.sh', which is sourced in\nt8001-annotate.sh and t8002-blame.sh, leveraging an existing file used\nto test the userdiff patterns in t4018-diff-funcname.\n\nSigned-off-by: Philippe Blain \n---\n builtin/blame.c | 2 +-\n t/annotate-tests.sh | 18 ++++++++++++++++++\n 2 files changed, 19 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 05f69211dd..917fedc635 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -1104,7 +1104,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n \t\tlong bottom, top;\n \t\tif (parse_range_arg(range_list.items[range_i].string,\n \t\t\t\t nth_line_cb, &sb, lno, anchor,\n-\t\t\t\t &bottom, &top, sb.path,\n+\t\t\t\t &bottom, &top, path,\n \t\t\t\t the_repository->index))\n \t\t\tusage(blame_usage);\n \t\tif ((!lno && (top || bottom)) || lno < bottom)\ndiff --git a/t/annotate-tests.sh b/t/annotate-tests.sh\nindex d933af5714..3aee61d2cc 100644\n--- a/t/annotate-tests.sh\n+++ b/t/annotate-tests.sh\n@@ -479,6 +479,24 @@ test_expect_success 'blame -L ^:RE (absolute: end-of-file)' '\n \tcheck_count -f hello.c -L$n -L^:ma.. F 4 G 1 H 1\n '\n \n+test_expect_success 'setup -L :funcname with userdiff driver' '\n+\techo \"fortran-* diff=fortran\" >.gitattributes &&\n+\tfortran_file=fortran-external-function &&\n+\torig_file=\"$TEST_DIRECTORY/t4018/$fortran_file\" &&\n+\tcp $orig_file . &&\n+\tgit add $fortran_file &&\n+\tGIT_AUTHOR_NAME=\"A\" GIT_AUTHOR_EMAIL=\"A@test.git\" \\\n+\tgit commit -m \"add fortran file\" &&\n+\tsed -e \"s/ChangeMe/IWasChanged/\" <\"$orig_file\" >$fortran_file &&\n+\tgit add $fortran_file &&\n+\tGIT_AUTHOR_NAME=\"B\" GIT_AUTHOR_EMAIL=\"B@test.git\" \\\n+\tgit commit -m \"change fortran file\"\n+'\n+\n+test_expect_success 'blame -L :funcname with userdiff driver' '\n+\tcheck_count -f fortran-external-function -L:RIGHT A 7 B 1\n+'\n+\n test_expect_success 'setup incremental' '\n \t(\n \tGIT_AUTHOR_NAME=I &&\n-- \ngitgitgadget\n"},{"id":"408582","messageId":"da76a10717c917abab9e1bd5ab78db2a4f7ca7c0.1603889270.git.gitgitgadget@gmail.com","threadId":"54521","inReplyTo":"pull.774.git.1603889270.gitgitgadget@gmail.com","subject":"[PATCH 4/6] doc: add more pointers to gitattributes(5) for userdiff","fromName":"Philippe Blain via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-10-28T12:47:48Z","receivedAt":"2020-10-28T23:23:30Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"From: Philippe Blain \n\nSeveral Git commands can make use of the builtin userdiff patterns, but\nit's not obvious in the documentation. Add pointers to the 'Defining a\ncustom hunk header' part of gitattributes(5) in the description of the\nfollowing options:\n\n- the '--function-context' option of `git diff` and friends\n- the '--function-context' option of `git grep`\n- the '-L :' option of `git log`, `gitk` and `git blame`\n\nIn 'git-grep.txt', take the opportunity to use backticks in the\ndescription of '--show-function', and improve the wording of the\ndesription of '--function-context'.\n\nSigned-off-by: Philippe Blain \n---\n Documentation/diff-options.txt | 5 ++++-\n Documentation/git-grep.txt | 6 ++++--\n Documentation/line-range-format.txt | 4 +++-\n 3 files changed, 11 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex 573fb9bb71..39b511bb6b 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -695,7 +695,10 @@ endif::git-format-patch[]\n \n -W::\n --function-context::\n-\tShow whole surrounding functions of changes.\n+\tShow whole functions as context lines for each changes.\n+\tThe function names are determined in the same way as\n+\t`git diff` works out patch hunk headers (see 'Defining a\n+\tcustom hunk-header' in linkgit:gitattributes[5]).\n \n ifndef::git-format-patch[]\n ifndef::git-log[]\ndiff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt\nindex 6077ff01a4..4e0ba8234a 100644\n--- a/Documentation/git-grep.txt\n+++ b/Documentation/git-grep.txt\n@@ -241,7 +241,7 @@ providing this option will cause it to die.\n --show-function::\n \tShow the preceding line that contains the function name of\n \tthe match, unless the matching line is a function name itself.\n-\tThe name is determined in the same way as 'git diff' works out\n+\tThe name is determined in the same way as `git diff` works out\n \tpatch hunk headers (see 'Defining a custom hunk-header' in\n \tlinkgit:gitattributes[5]).\n \n@@ -266,7 +266,9 @@ providing this option will cause it to die.\n \tShow the surrounding text from the previous line containing a\n \tfunction name up to the one before the next function name,\n \teffectively showing the whole function in which the match was\n-\tfound.\n+\tfound. The function names are determined in the same way as\n+\t`git diff` works out patch hunk headers (see 'Defining a\n+\tcustom hunk-header' in linkgit:gitattributes[5]).\n \n --threads ::\n \tNumber of grep worker threads to use.\ndiff --git a/Documentation/line-range-format.txt b/Documentation/line-range-format.txt\nindex 6ee159b683..ab3c70a1d5 100644\n--- a/Documentation/line-range-format.txt\n+++ b/Documentation/line-range-format.txt\n@@ -27,4 +27,6 @@ regular expression that denotes the range from the first funcname line\n that matches '', up to the next funcname line. `:`\n searches from the end of the previous `-L` range, if any, otherwise\n from the start of file. `^:` searches from the start of\n-file.\n+file. The function names are determined in the same way as `git diff`\n+works out patch hunk headers (see 'Defining a custom hunk-header'\n+in linkgit:gitattributes[5]).\n-- \ngitgitgadget\n\n"},{"id":"408584","messageId":"27ef94e9cc4189c3d74e984437dcce24e1f29678.1603889270.git.gitgitgadget@gmail.com","threadId":"54521","inReplyTo":"pull.774.git.1603889270.gitgitgadget@gmail.com","subject":"[PATCH 5/6] line-log: mention both modes in 'blame' and 'log' short help","fromName":"Philippe Blain via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-10-28T12:47:49Z","receivedAt":"2020-10-28T23:26:41Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"From: Philippe Blain \n\n'git blame -h' and 'git log -h' both show '-L ' and describe this\noption as \"Process only line range n,m, counting from 1\". No hint is\ngiven that a function name regex can also be used.\n\nUse instead, and expand the description of the option to mention\nboth modes. Remove \"counting from 1\" as it's uneeded; it's uncommon to\nrefer to the first line of a file as \"line 0\".\n\nAlso, for 'git log', improve the wording to better reflect the long help.\n\nSigned-off-by: Philippe Blain \n---\n builtin/blame.c | 3 ++-\n builtin/log.c | 4 ++--\n 2 files changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex bb0f29300e..05f69211dd 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -889,7 +889,8 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n \t\tOPT_STRING(0, \"contents\", &contents_from, N_(\"file\"), N_(\"Use 's contents as the final image\")),\n \t\tOPT_CALLBACK_F('C', NULL, &opt, N_(\"score\"), N_(\"Find line copies within and across files\"), PARSE_OPT_OPTARG, blame_copy_callback),\n \t\tOPT_CALLBACK_F('M', NULL, &opt, N_(\"score\"), N_(\"Find line movements within and across files\"), PARSE_OPT_OPTARG, blame_move_callback),\n-\t\tOPT_STRING_LIST('L', NULL, &range_list, N_(\"n,m\"), N_(\"Process only line range n,m, counting from 1\")),\n+\t\tOPT_STRING_LIST('L', NULL, &range_list, N_(\"range\"),\n+\t\t\t\tN_(\"Process only the given line range (=,) or function (=:)\")),\n \t\tOPT__ABBREV(&abbrev),\n \t\tOPT_END()\n \t};\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 0a7ed4bef9..e6a3bb7803 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -183,8 +183,8 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n \t\t\t\tN_(\"pattern\"), N_(\"do not decorate refs that match \")),\n \t\tOPT_CALLBACK_F(0, \"decorate\", NULL, NULL, N_(\"decorate options\"),\n \t\t\t PARSE_OPT_OPTARG, decorate_callback),\n-\t\tOPT_CALLBACK('L', NULL, &line_cb, \"n,m:file\",\n-\t\t\t N_(\"Process line range n,m in file, counting from 1\"),\n+\t\tOPT_CALLBACK('L', NULL, &line_cb, \"range:file\",\n+\t\t\t N_(\"Trace the evolution of the given line range (=,) or function (=:) in \"),\n \t\t\t log_line_range_callback),\n \t\tOPT_END()\n \t};\n-- \ngitgitgadget\n\n"},{"id":"408615","messageId":"7d3fc0a503b160ea15363a4465af4647bce6a30b.1603889270.git.gitgitgadget@gmail.com","threadId":"54521","inReplyTo":"pull.774.git.1603889270.gitgitgadget@gmail.com","subject":"[PATCH 2/6] doc: line-range: improve formatting","fromName":"Philippe Blain via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-10-28T12:47:46Z","receivedAt":"2020-10-29T02:05:10Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"From: Philippe Blain \n\nImprove the formatting of the description of the line-range option '-L'\nfor `git log`, `gitk` and `git blame`:\n\n- Use bold for , and \n- Use backticks for literals\n\nSigned-off-by: Philippe Blain \n---\n Documentation/blame-options.txt | 4 ++--\n Documentation/line-range-format.txt | 24 ++++++++++++------------\n Documentation/line-range-options.txt | 6 +++---\n 3 files changed, 17 insertions(+), 17 deletions(-)\n\ndiff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt\nindex 88750af7ae..48bf0eeec5 100644\n--- a/Documentation/blame-options.txt\n+++ b/Documentation/blame-options.txt\n@@ -14,8 +14,8 @@\n \tAnnotate only the given line range. May be specified multiple times.\n \tOverlapping ranges are allowed.\n +\n- and are optional. ``-L '' or ``-L ,'' spans from\n- to end of file. ``-L ,'' spans from start of file to .\n+'' and '' are optional. `-L ` or `-L ,` spans from\n+'' to end of file. `-L ,` spans from start of file to ''.\n +\n include::line-range-format.txt[]\n \ndiff --git a/Documentation/line-range-format.txt b/Documentation/line-range-format.txt\nindex 829676ff98..6ee159b683 100644\n--- a/Documentation/line-range-format.txt\n+++ b/Documentation/line-range-format.txt\n@@ -1,30 +1,30 @@\n- and can take one of these forms:\n+'' and '' can take one of these forms:\n \n - number\n +\n-If or is a number, it specifies an\n+If '' or '' is a number, it specifies an\n absolute line number (lines count from 1).\n +\n \n-- /regex/\n+- `/regex/`\n +\n This form will use the first line matching the given\n-POSIX regex. If is a regex, it will search from the end of\n+POSIX regex. If '' is a regex, it will search from the end of\n the previous `-L` range, if any, otherwise from the start of file.\n-If is ``^/regex/'', it will search from the start of file.\n-If is a regex, it will search\n-starting at the line given by .\n+If '' is `^/regex/`, it will search from the start of file.\n+If '' is a regex, it will search\n+starting at the line given by ''.\n +\n \n - +offset or -offset\n +\n-This is only valid for and will specify a number\n-of lines before or after the line given by .\n+This is only valid for '' and will specify a number\n+of lines before or after the line given by ''.\n \n +\n-If ``:'' is given in place of and , it is a\n+If \"`:`\" is given in place of '' and '', it is a\n regular expression that denotes the range from the first funcname line\n-that matches , up to the next funcname line. ``:''\n+that matches '', up to the next funcname line. `:`\n searches from the end of the previous `-L` range, if any, otherwise\n-from the start of file. ``^:'' searches from the start of\n+from the start of file. `^:` searches from the start of\n file.\ndiff --git a/Documentation/line-range-options.txt b/Documentation/line-range-options.txt\nindex 9e3d98d44f..d2ef1a6d67 100644\n--- a/Documentation/line-range-options.txt\n+++ b/Documentation/line-range-options.txt\n@@ -7,12 +7,12 @@ ifdef::gitk[]\n -L::::\n endif::gitk[]\n \n-\tTrace the evolution of the line range given by \",\"\n-\t(or the function name regex ) within the . You may\n+\tTrace the evolution of the line range given by \"','\"\n+\t(or the function name regex '') within the ''. You may\n \tnot give any pathspec limiters. This is currently limited to\n \ta walk starting from a single revision, i.e., you may only\n \tgive zero or one positive revision arguments, and\n-\t and (or ) must exist in the starting revision.\n+\t'' and '' (or '') must exist in the starting revision.\n \tYou can specify this option more than once. Implies `--patch`.\n \tPatch output can be suppressed using `--no-patch`, but other diff formats\n \t(namely `--raw`, `--numstat`, `--shortstat`, `--dirstat`, `--summary`,\n-- \ngitgitgadget\n\n"},{"id":"408638","messageId":"96f6f95abcbd79d432073cb294ba12b71300580f.1603889270.git.gitgitgadget@gmail.com","threadId":"54521","inReplyTo":"pull.774.git.1603889270.gitgitgadget@gmail.com","subject":"[PATCH 1/6] doc: log, gitk: move '-L' description to 'line-range-options.txt'","fromName":"Philippe Blain via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-10-28T12:47:45Z","receivedAt":"2020-10-29T02:45:53Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"From: Philippe Blain \n\nThe description of the '-L' option for `git log` and `gitk` is the\nsame, but is repeated in both 'git-log.txt' and 'gitk.txt'.\n\nRemove the duplication by creating a new file, 'line-range-options.txt',\nand include it in both files.\n\nSigned-off-by: Philippe Blain \n---\n Documentation/git-log.txt | 16 ++--------------\n Documentation/gitk.txt | 21 ++-------------------\n Documentation/line-range-options.txt | 27 +++++++++++++++++++++++++++\n 3 files changed, 31 insertions(+), 33 deletions(-)\n create mode 100644 Documentation/line-range-options.txt\n\ndiff --git a/Documentation/git-log.txt b/Documentation/git-log.txt\nindex 2b8ac5ff88..631dfff77c 100644\n--- a/Documentation/git-log.txt\n+++ b/Documentation/git-log.txt\n@@ -77,20 +77,8 @@ produced by `--stat`, etc.\n \tIntended to speed up tools that read log messages from `git log`\n \toutput by allowing them to allocate space in advance.\n \n--L ,:::\n--L ::::\n-\tTrace the evolution of the line range given by \",\"\n-\t(or the function name regex ) within the . You may\n-\tnot give any pathspec limiters. This is currently limited to\n-\ta walk starting from a single revision, i.e., you may only\n-\tgive zero or one positive revision arguments, and\n-\t and (or ) must exist in the starting revision.\n-\tYou can specify this option more than once. Implies `--patch`.\n-\tPatch output can be suppressed using `--no-patch`, but other diff formats\n-\t(namely `--raw`, `--numstat`, `--shortstat`, `--dirstat`, `--summary`,\n-\t`--name-only`, `--name-status`, `--check`) are not currently implemented.\n-+\n-include::line-range-format.txt[]\n+:git-log: 1\n+include::line-range-options.txt[]\n \n ::\n \tShow only commits in the specified revision range. When no\ndiff --git a/Documentation/gitk.txt b/Documentation/gitk.txt\nindex c653ebb6a8..761b764c18 100644\n--- a/Documentation/gitk.txt\n+++ b/Documentation/gitk.txt\n@@ -98,25 +98,8 @@ linkgit:git-rev-list[1] for a complete list.\n \t(See \"History simplification\" in linkgit:git-log[1] for a more\n \tdetailed explanation.)\n \n--L,:::\n--L::::\n-\n-\tTrace the evolution of the line range given by \",\"\n-\t(or the function name regex ) within the . You may\n-\tnot give any pathspec limiters. This is currently limited to\n-\ta walk starting from a single revision, i.e., you may only\n-\tgive zero or one positive revision arguments, and\n-\t and (or ) must exist in the starting revision.\n-\tYou can specify this option more than once. Implies `--patch`.\n-\tPatch output can be suppressed using `--no-patch`, but other diff formats\n-\t(namely `--raw`, `--numstat`, `--shortstat`, `--dirstat`, `--summary`,\n-\t`--name-only`, `--name-status`, `--check`) are not currently implemented.\n-+\n-*Note:* gitk (unlike linkgit:git-log[1]) currently only understands\n-this option if you specify it \"glued together\" with its argument. Do\n-*not* put a space after `-L`.\n-+\n-include::line-range-format.txt[]\n+:gitk: 1\n+include::line-range-options.txt[]\n \n ::\n \ndiff --git a/Documentation/line-range-options.txt b/Documentation/line-range-options.txt\nnew file mode 100644\nindex 0000000000..9e3d98d44f\n--- /dev/null\n+++ b/Documentation/line-range-options.txt\n@@ -0,0 +1,27 @@\n+ifdef::git-log[]\n+-L ,:::\n+-L ::::\n+endif::git-log[]\n+ifdef::gitk[]\n+-L,:::\n+-L::::\n+endif::gitk[]\n+\n+\tTrace the evolution of the line range given by \",\"\n+\t(or the function name regex ) within the . You may\n+\tnot give any pathspec limiters. This is currently limited to\n+\ta walk starting from a single revision, i.e., you may only\n+\tgive zero or one positive revision arguments, and\n+\t and (or ) must exist in the starting revision.\n+\tYou can specify this option more than once. Implies `--patch`.\n+\tPatch output can be suppressed using `--no-patch`, but other diff formats\n+\t(namely `--raw`, `--numstat`, `--shortstat`, `--dirstat`, `--summary`,\n+\t`--name-only`, `--name-status`, `--check`) are not currently implemented.\n++\n+ifdef::gitk[]\n+*Note:* gitk (unlike linkgit:git-log[1]) currently only understands\n+this option if you specify it \"glued together\" with its argument. Do\n+*not* put a space after `-L`.\n+endif::gitk[]\n++\n+include::line-range-format.txt[]\n-- \ngitgitgadget\n\n"},{"id":"408642","messageId":"3D7C25D5-E8DE-4414-ADAC-A0E0FA28CC5D@gmail.com","threadId":"54521","inReplyTo":"CAPig+cRFaT9ww3K368b58m-xgW843jhZWjXDspVrfdyYY-_wRA@mail.gmail.com","subject":"Re: [PATCH 5/6] line-log: mention both modes in 'blame' and 'log' short help","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2020-10-29T12:43:53Z","receivedAt":"2020-10-29T12:44:05Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Hi Eric, \n\n\n> Le 28 oct. 2020 à 13:33, Eric Sunshine a écrit :\n> \n> On Wed, Oct 28, 2020 at 8:48 AM Philippe Blain via GitGitGadget\n> wrote:\n>> 'git blame -h' and 'git log -h' both show '-L ' and describe this\n>> option as \"Process only line range n,m, counting from 1\". No hint is\n>> given that a function name regex can also be used.\n>> \n>> Use instead, and expand the description of the option to mention\n>> both modes. Remove \"counting from 1\" as it's uneeded; it's uncommon to\n>> refer to the first line of a file as \"line 0\".\n>> \n>> Signed-off-by: Philippe Blain \n>> ---\n>> diff --git a/builtin/blame.c b/builtin/blame.c\n>> @@ -889,7 +889,8 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n>> - OPT_STRING_LIST('L', NULL, &range_list, N_(\"n,m\"), N_(\"Process only line range n,m, counting from 1\")),\n>> + OPT_STRING_LIST('L', NULL, &range_list, N_(\"range\"),\n>> + N_(\"Process only the given line range (=,) or function (=:)\")),\n> \n> The \"=\" bit is redundant and confusing (and ugly). Considering\n> that the description already says \"Process only the given line\n> _range_\", it should be fine to drop the \"=\" lead-in. Perhaps:\n> \n> Process only the given line range , or :\"\n> \n> This might feel too succinct, but that's often true of short -h help.\n> Such succinctness is generally acceptable as long as more detailed\n> documentation can be discovered easily (such as in the 'man' page).\n> \n> Same comment regarding the rest of the changes in this patch.\n\nYes, I agree. I've shortened it for v2 (and also made the changes you suggested\nfor 2/6 and 4/6.\n\nThanks for taking a look,\n\nPhilippe.\n\n\n"},{"id":"408657","messageId":"xmqqft5w7poo.fsf@gitster.c.googlers.com","threadId":"54521","inReplyTo":"96f6f95abcbd79d432073cb294ba12b71300580f.1603889270.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/6] doc: log, gitk: move '-L' description to 'line-range-options.txt'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-10-29T20:16:55Z","receivedAt":"2020-10-29T20:17:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Philippe Blain via GitGitGadget\" writes:\n\n> From: Philippe Blain \n>\n> The description of the '-L' option for `git log` and `gitk` is the\n> same, but is repeated in both 'git-log.txt' and 'gitk.txt'.\n\nThey are moral equivalents but differ slightly, both in\nan insignificant way (i.e. SP after -L and its parameters) and a\nsignificant way (i.e. Note about gitk usage), so \"the same, but is\nrepeated\" is not sufficient to explain why the contents of the new\nfile looks the way it is.\n\n> Remove the duplication by creating a new file, 'line-range-options.txt',\n> and include it in both files.\n\nMakes sense. As to the conditional, I actually think the version\nwith SP after -L do not have to be listed and instead to show just\nthe \"stuck\" form as the standardised way. If the option takes an\noptional value, you must have to use the \"stuck\" form anyway, and\nshowing that you _could_ have SP there unnecessarily throws extra\nbytes at the reader with no real gain.\n\n> diff --git a/Documentation/gitk.txt b/Documentation/gitk.txt\n> index c653ebb6a8..761b764c18 100644\n> --- a/Documentation/gitk.txt\n> +++ b/Documentation/gitk.txt\n> @@ -98,25 +98,8 @@ linkgit:git-rev-list[1] for a complete list.\n> \t(See \"History simplification\" in linkgit:git-log[1] for a more\n> \tdetailed explanation.)\n> \n> --L,:::\n> --L::::\n> ...\n> -\t`--name-only`, `--name-status`, `--check`) are not currently implemented.\n> -+\n> -*Note:* gitk (unlike linkgit:git-log[1]) currently only understands\n> -this option if you specify it \"glued together\" with its argument. Do\n> -*not* put a space after `-L`.\n> -+\n> -include::line-range-format.txt[]\n> +:gitk: 1\n> +include::line-range-options.txt[]\n\nIt looked puzzling to see the inclusion of line-range-format is lost\nfrom here, but it turns out to be OK after all when we look at the\nnew line-range-options file.\n\n> diff --git a/Documentation/line-range-options.txt b/Documentation/line-range-options.txt\n> new file mode 100644\n> index 0000000000..9e3d98d44f\n> --- /dev/null\n> +++ b/Documentation/line-range-options.txt\n> @@ -0,0 +1,27 @@\n> +ifdef::git-log[]\n> +-L ,:::\n> +-L ::::\n> +endif::git-log[]\n> +ifdef::gitk[]\n> +-L,:::\n> +-L::::\n> +endif::gitk[]\n\nAs I said, my vote goes to reducing the variations and using only\nthe latter form for simplicity.\n\n> +\tTrace the evolution of the line range given by \",\"\n> +\t(or the function name regex ) within the . You may\n> +\tnot give any pathspec limiters. This is currently limited to\n> +\ta walk starting from a single revision, i.e., you may only\n> +\tgive zero or one positive revision arguments, and\n> +\t and (or ) must exist in the starting revision.\n> +\tYou can specify this option more than once. Implies `--patch`.\n> +\tPatch output can be suppressed using `--no-patch`, but other diff formats\n> +\t(namely `--raw`, `--numstat`, `--shortstat`, `--dirstat`, `--summary`,\n> +\t`--name-only`, `--name-status`, `--check`) are not currently implemented.\n\nThis text is the same as both originals, which is good.\n\n> ++\n> +ifdef::gitk[]\n> +*Note:* gitk (unlike linkgit:git-log[1]) currently only understands\n> +this option if you specify it \"glued together\" with its argument. Do\n> +*not* put a space after `-L`.\n> +endif::gitk[]\n\nI think it may make sense to keep this part, mostly from \"keep the\nresult as close to the original as possible while refactoring\"\nprinciple, but if we only teach the \"stuck\" form in \"git log\", it\nbecomes irrelevant---one fewer thing the users need to learn and\nremember, it becomes easier for them to concentrate on what really\nmatters.\n\n\n> +include::line-range-format.txt[]\n\nAnd the answer to my earlier puzzlement is here. I actually would\nhave expected that it would be the matter of moving the common text\nyou moved from gitk/log documentation to this file instead to the\ntop of the line-range-format.txt file, but the latter is included\nalso by blame-options.txt, and the description of the option needs\nto be somewhat different between blame and log anyway, so I think\nthis is the best we can do at this step in the series.\n\nLooking good so far...\n"},{"id":"408658","messageId":"xmqqblgk7pjv.fsf@gitster.c.googlers.com","threadId":"54521","inReplyTo":"CAPig+cQcjjj3x_iTDX=yBUai6xHrShsGO5J3sdpX-2UhM7_OJQ@mail.gmail.com","subject":"Re: [PATCH 2/6] doc: line-range: improve formatting","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-10-29T20:19:48Z","receivedAt":"2020-10-29T20:19:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> On Wed, Oct 28, 2020 at 8:48 AM Philippe Blain via GitGitGadget\n> wrote:\n>> Improve the formatting of the description of the line-range option '-L'\n>> for `git log`, `gitk` and `git blame`:\n>>\n>> - Use bold for , and \n>\n> My impression is that it is more common in Git documentation for these\n> placeholders to be formatted with backticks rather than as bold (or,\n> if not more common currently, at least is trending that way). That's\n> not to say that my impression is necessarily accurate.\n\nI think the impression is fairly in line with the reality. That is\nnot to say that it is a good future to aim for.\n\nMy personal preference is\n\n - use `as-is` for things that must be typed as-is (e.g. `-L`)\n\n - use 'emph' for things that are placeholder (e.g. ''').\n\n"},{"id":"408659","messageId":"xmqq5z6s7pa6.fsf@gitster.c.googlers.com","threadId":"54521","inReplyTo":"f7b64bf3302f934c44cf9364e4f008bb9ce26def.1603889270.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 3/6] blame-options.txt: also mention 'funcname' in '-L' description","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-10-29T20:25:37Z","receivedAt":"2020-10-29T20:25:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Philippe Blain via GitGitGadget\" writes:\n\n> From: Philippe Blain \n>\n> Make it clearer that a function can be blamed by feeding `git blame`\n> '-L :' by mentioning it at the beginnning of the description\n> of the '-L' option.\n\nI think it makes tons of sense to mention both , range\nand range in the description.\n\nIt is not a new issue you are introducing with this change, but I\nthink the latter does not deserve to be treated as if it were an\nafterthought by being a mere parenthetical mention.\n\n> -L ,::\n> -L :::\n> -\tAnnotate only the given line range. May be specified multiple times.\n> -\tOverlapping ranges are allowed.\n> +\tAnnotate only the line range given by ','\n> +\t(or by the function name regex '').\n> +\tMay be specified multiple times. Overlapping ranges are allowed.\n> +\n> '' and '' are optional. `-L ` or `-L ,` spans from\n> '' to end of file. `-L ,` spans from start of file to ''.\n"},{"id":"408665","messageId":"xmqqy2jo6a0z.fsf@gitster.c.googlers.com","threadId":"54521","inReplyTo":"a1e1c977d0978424fb07c97be0479f43a325cbea.1603889270.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 6/6] blame: enable funcname blaming with userdiff driver","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-10-29T20:40:28Z","receivedAt":"2020-10-29T20:41:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Philippe Blain via GitGitGadget\" writes:\n\n> From: Philippe Blain \n>\n> In blame.c::cmd_blame, we send the 'path' field of the 'sb' 'struct\n> blame_scoreboard' as the 'path' argument to\n> 'line-range.c::parse_range_arg', but 'sb.path' is not set yet; it's set\n> to the local variable 'path' a few lines later at line 1137.\n>\n> This 'path' argument is only used in 'parse_range_arg' if we are blaming\n> a funcname, i.e. `git blame -L : `, and in that case it\n> is sent to 'parse_range_funcname', where it is used to determine if a\n> userdiff driver should be used for said to match the given\n> funcname.\n>\n> Since 'path' is yet unset, the userdiff driver is never used, so we fall\n> back to the default funcname regex, which is usually not appropriate for\n> paths that are set to use a specific userdiff driver, and thus either we\n> match some unrelated lines, or we die with\n>\n> fatal: -L parameter '' starting at line 1: no match\n>\n> This has been the case ever since `git blame` learned to blame a\n> funcname in 13b8f68c1f (log -L: :pattern:file syntax to find by\n> funcname, 2013-03-28).\n\nGood find.\n\n> Enable funcname blaming for paths using specific userdiff drivers by\n> sending the local variable 'path' to 'parse_range_arg' instead of the\n> yet unset 'sb.path'.\n\nHmph. The reason why sb.path is not set to path at this point but\nlater gets set is? I am wondering if this is leaving an unneeded\nroom for sb.path and path to diverge by mistake. IOW, I wonder if\nit is better to set sb.path as early as we figure out which path we\nare interested in, forget we have local variable \"path\" and use\nsb.path consistently throughout the code.\n\nBut that is merely a potential future clean-up. The local variable\npath is still used one more time in the error message given when\nthis parse_range_arg() fails, so at least this change makes the use\nof path more consistent. I like the simplicity of this fix.\n\n> Add a regression test in 'annotate-tests.sh', which is sourced in\n> t8001-annotate.sh and t8002-blame.sh, leveraging an existing file used\n> to test the userdiff patterns in t4018-diff-funcname.\n\nThanks, that change also makes sense.\n\n"},{"id":"408788","messageId":"C8539F4F-D51D-463B-825F-385AD743723E@gmail.com","threadId":"54521","inReplyTo":"xmqqft5w7poo.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 1/6] doc: log, gitk: move '-L' description to 'line-range-options.txt'","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2020-10-31T17:18:01Z","receivedAt":"2020-10-31T17:18:08Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Hi Junio,\n\n> Le 29 oct. 2020 à 16:16, Junio C Hamano a écrit :\n> \n> \"Philippe Blain via GitGitGadget\" writes:\n> \n>> From: Philippe Blain \n>> \n>> The description of the '-L' option for `git log` and `gitk` is the\n>> same, but is repeated in both 'git-log.txt' and 'gitk.txt'.\n> \n> They are moral equivalents but differ slightly, both in\n> an insignificant way (i.e. SP after -L and its parameters) and a\n> significant way (i.e. Note about gitk usage), so \"the same, but is\n> repeated\" is not sufficient to explain why the contents of the new\n> file looks the way it is.\n\nOK. I'll expand on that.\n\n>> Remove the duplication by creating a new file, 'line-range-options.txt',\n>> and include it in both files.\n> \n> Makes sense. As to the conditional, I actually think the version\n> with SP after -L do not have to be listed and instead to show just\n> the \"stuck\" form as the standardised way. If the option takes an\n> optional value, you must have to use the \"stuck\" form anyway, and\n> showing that you _could_ have SP there unnecessarily throws extra\n> bytes at the reader with no real gain.\n\nI'm not sure I understand here... The argument is not optional\nhere (`git log -L` by itself is meaningless)... I know gitcli(7) recommends the\nstuck form, but this option has been listed as `-L ` in the git-log\ndocumentation ever since it appeared... So I did not think it wise to change it, \nthat's why I introduce the conditional.\n\nThanks,\n\nPhilippe.\n\n"},{"id":"408789","messageId":"1EAEFFE7-F690-43F7-83C7-BD07A42C91D7@gmail.com","threadId":"54521","inReplyTo":"xmqqblgk7pjv.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 2/6] doc: line-range: improve formatting","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2020-10-31T17:20:44Z","receivedAt":"2020-10-31T17:20:49Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"\n> Le 29 oct. 2020 à 16:19, Junio C Hamano a écrit :\n> \n> Eric Sunshine writes:\n> \n>> On Wed, Oct 28, 2020 at 8:48 AM Philippe Blain via GitGitGadget\n>> wrote:\n>>> Improve the formatting of the description of the line-range option '-L'\n>>> for `git log`, `gitk` and `git blame`:\n>>> \n>>> - Use bold for , and \n>> \n>> My impression is that it is more common in Git documentation for these\n>> placeholders to be formatted with backticks rather than as bold (or,\n>> if not more common currently, at least is trending that way). That's\n>> not to say that my impression is necessarily accurate.\n> \n> I think the impression is fairly in line with the reality. \n\nYes, I compared using \n\n git grep '`<' | wc -l \n\nand \n\n git grep \"'<\" | wc -l\n\nand the same for their closing counterparts and the backticks win.\n\n> That is\n> not to say that it is a good future to aim for.\n> \n> My personal preference is\n> \n> - use `as-is` for things that must be typed as-is (e.g. `-L`)\n> \n> - use 'emph' for things that are placeholder (e.g. ''').\n\nThanks for clarifying. It is indeed what I tried to do.\n\n"},{"id":"408790","messageId":"45C117E5-1855-4BE5-A021-D1438B770A21@gmail.com","threadId":"54521","inReplyTo":"xmqq5z6s7pa6.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 3/6] blame-options.txt: also mention 'funcname' in '-L' description","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2020-10-31T17:22:07Z","receivedAt":"2020-10-31T17:22:12Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"\n> Le 29 oct. 2020 à 16:25, Junio C Hamano a écrit :\n> \n> \"Philippe Blain via GitGitGadget\" writes:\n> \n>> From: Philippe Blain \n>> \n>> Make it clearer that a function can be blamed by feeding `git blame`\n>> '-L :' by mentioning it at the beginnning of the description\n>> of the '-L' option.\n> \n> I think it makes tons of sense to mention both , range\n> and range in the description.\n> \n> It is not a new issue you are introducing with this change, but I\n> think the latter does not deserve to be treated as if it were an\n> afterthought by being a mere parenthetical mention.\n\nI agree. I'll address that for the next version.\n\n"},{"id":"408791","messageId":"xmqqmu02483u.fsf@gitster.c.googlers.com","threadId":"54521","inReplyTo":"C8539F4F-D51D-463B-825F-385AD743723E@gmail.com","subject":"Re: [PATCH 1/6] doc: log, gitk: move '-L' description to 'line-range-options.txt'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-10-31T17:29:25Z","receivedAt":"2020-10-31T17:29:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Philippe Blain writes:\n\n>> Makes sense. As to the conditional, I actually think the version\n>> with SP after -L do not have to be listed and instead to show just\n>> the \"stuck\" form as the standardised way. If the option takes an\n>> optional value, you must have to use the \"stuck\" form anyway, and\n>> showing that you _could_ have SP there unnecessarily throws extra\n>> bytes at the reader with no real gain.\n>\n> I'm not sure I understand here... The argument is not optional\n> here (`git log -L` by itself is meaningless)... I know gitcli(7) recommends the\n> stuck form, ...\n\nI do not think showing alternative ways distracts without adding\nmuch value. We should remember that most users are learning Git to\nonly to do things they want to do and not necessarily learning Git\nto master all the slightly different ways to do the same thing.\n\"You can write option and its argument separately most of the time\nbut you must write option and its argument in the stuck form some\nother times---you must think if the argument is optional or not,\nwhenyou want to write your argument separately\" crams more things to\nlearn than the simple \"Write option and its arguments in the stuck\nform and it always works\".\n"},{"id":"408792","messageId":"81143637-F77C-49C5-B55A-57E92AC45881@gmail.com","threadId":"54521","inReplyTo":"xmqqy2jo6a0z.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 6/6] blame: enable funcname blaming with userdiff driver","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2020-10-31T18:02:16Z","receivedAt":"2020-10-31T18:02:22Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"\n> Le 29 oct. 2020 à 16:40, Junio C Hamano a écrit :\n> \n> \"Philippe Blain via GitGitGadget\" writes:\n> \n>> From: Philippe Blain \n>> \n>> In blame.c::cmd_blame, we send the 'path' field of the 'sb' 'struct\n>> blame_scoreboard' as the 'path' argument to\n>> 'line-range.c::parse_range_arg', but 'sb.path' is not set yet; it's set\n>> to the local variable 'path' a few lines later at line 1137.\n>> \n>> This 'path' argument is only used in 'parse_range_arg' if we are blaming\n>> a funcname, i.e. `git blame -L : `, and in that case it\n>> is sent to 'parse_range_funcname', where it is used to determine if a\n>> userdiff driver should be used for said to match the given\n>> funcname.\n>> \n>> Since 'path' is yet unset, the userdiff driver is never used, so we fall\n>> back to the default funcname regex, which is usually not appropriate for\n>> paths that are set to use a specific userdiff driver, and thus either we\n>> match some unrelated lines, or we die with\n>> \n>> fatal: -L parameter '' starting at line 1: no match\n>> \n>> This has been the case ever since `git blame` learned to blame a\n>> funcname in 13b8f68c1f (log -L: :pattern:file syntax to find by\n>> funcname, 2013-03-28).\n> \n> Good find.\n> \n>> Enable funcname blaming for paths using specific userdiff drivers by\n>> sending the local variable 'path' to 'parse_range_arg' instead of the\n>> yet unset 'sb.path'.\n> \n> Hmph. The reason why sb.path is not set to path at this point but\n> later gets set is? I am wondering if this is leaving an unneeded\n> room for sb.path and path to diverge by mistake. IOW, I wonder if\n> it is better to set sb.path as early as we figure out which path we\n> are interested in, forget we have local variable \"path\" and use\n> sb.path consistently throughout the code.\n\nI asked myself the same question, and could not come up with a \ngood answer. I don't know if you read what I wrote in the cover letter\n(which I would have included in the in-patch commentary for this here\npatch if GitGitGadget had this feature), but I'm copying it here just \nin case you did not: \n\n* In patch 6, I considered fixing that bug in a different way by\n initializing sb.path inside blame.c::setup_scoreboard. This function\n already receives 'path' as its second argument, so changing that would not\n impact the API. Probably Thomas thought that sb.path was already\n initialized in sb when he modified builtin/blame.c::prepare_blame_range \n to also send sb.path to line-range.c::parse_range_arg in 13b8f68c1f (log\n -L: :pattern:file syntax to find by funcname, 2013-03-28). \n\n Initializing the path in setup_scoreboard would mean we could also\n simplify the API of blame.c::setup_blame_bloom_data since it would not\n need to receive path separately and so its second argument could be\n removed. I went for the simpler alternative of just sending 'path' to \n parse_range_arg instead of sb.path since it felt simpler, but if we feel\n it would be better to actually initialize sb.path in setup_scoreboard,\n I'll gladly tweak that for v2.\n\n> But that is merely a potential future clean-up. The local variable\n> path is still used one more time in the error message given when\n> this parse_range_arg() fails, so at least this change makes the use\n> of path more consistent. I like the simplicity of this fix.\n\nI also like its simplicity, and that's why I chose to submit this as v1.\nBut I completely agree with you that it is \"dangerous\" in the sense\nthat some further modifications to the code could then make the same mistake\nand use 'sb.path' thinking it is defined when it is not.\n\nSo I'm thinking of instead initializing it in setup_scoreboard for v2.\n\nCheers,\n\nPhilippe."},{"id":"408793","messageId":"xmqqimaq43za.fsf@gitster.c.googlers.com","threadId":"54521","inReplyTo":"81143637-F77C-49C5-B55A-57E92AC45881@gmail.com","subject":"Re: [PATCH 6/6] blame: enable funcname blaming with userdiff driver","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-10-31T18:58:33Z","receivedAt":"2020-10-31T18:58:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Philippe Blain writes:\n\n> * In patch 6, I considered fixing that bug in a different way by\n> initializing sb.path inside blame.c::setup_scoreboard. This function\n> already receives 'path' as its second argument, so changing that would not\n> impact the API. Probably Thomas thought that sb.path was already\n> initialized in sb when he modified builtin/blame.c::prepare_blame_range \n> to also send sb.path to line-range.c::parse_range_arg in 13b8f68c1f (log\n> -L: :pattern:file syntax to find by funcname, 2013-03-28). \n>\n> Initializing the path in setup_scoreboard would mean we could also\n> simplify the API of blame.c::setup_blame_bloom_data since it would not\n> need to receive path separately and so its second argument could be\n> removed. I went for the simpler alternative of just sending 'path' to \n> parse_range_arg instead of sb.path since it felt simpler, but if we feel\n> it would be better to actually initialize sb.path in setup_scoreboard,\n> I'll gladly tweak that for v2.\n>\n>> But that is merely a potential future clean-up. The local variable\n>> path is still used one more time in the error message given when\n>> this parse_range_arg() fails, so at least this change makes the use\n>> of path more consistent. I like the simplicity of this fix.\n>\n> I also like its simplicity, and that's why I chose to submit this as v1.\n> But I completely agree with you that it is \"dangerous\" in the sense\n> that some further modifications to the code could then make the same mistake\n> and use 'sb.path' thinking it is defined when it is not.\n>\n> So I'm thinking of instead initializing it in setup_scoreboard for v2.\n\nThat does sound like a sensible way to clean it up. Thanks.\n"},{"id":"408814","messageId":"b4fa5fb5ae5cb1d23bde1481a73b088826aa3f44.1604251728.git.gitgitgadget@gmail.com","threadId":"54521","inReplyTo":"pull.774.v2.git.1604251727.gitgitgadget@gmail.com","subject":"[PATCH v2 1/8] doc: log, gitk: move '-L' description to 'line-range-options.txt'","fromName":"Philippe Blain via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-11-01T17:28:40Z","receivedAt":"2020-11-01T17:28:53Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"From: Philippe Blain \n\nThe description of the '-L' option for `git log` and `gitk` is almost\nthe same, but is repeated in both 'git-log.txt' and 'gitk.txt' (the\ndifference being that 'git-log.txt' lists the option with a space\nafter '-L', while 'gitk.txt' lists it as stuck and notes that `gitk`\nonly understands the stuck form).\n\nReduce duplication by creating a new file, 'line-range-options.txt',\nand include it in both files.\n\nTo simplify the presentation, only list the stuck form for both\ncommands, and remove the note about `gitk` only understanding the stuck\nform.\n\nSigned-off-by: Philippe Blain \n---\n Documentation/git-log.txt | 15 +--------------\n Documentation/gitk.txt | 20 +-------------------\n Documentation/line-range-options.txt | 15 +++++++++++++++\n 3 files changed, 17 insertions(+), 33 deletions(-)\n create mode 100644 Documentation/line-range-options.txt\n\ndiff --git a/Documentation/git-log.txt b/Documentation/git-log.txt\nindex 2b8ac5ff88..dd189a353a 100644\n--- a/Documentation/git-log.txt\n+++ b/Documentation/git-log.txt\n@@ -77,20 +77,7 @@ produced by `--stat`, etc.\n \tIntended to speed up tools that read log messages from `git log`\n \toutput by allowing them to allocate space in advance.\n \n--L ,:::\n--L ::::\n-\tTrace the evolution of the line range given by \",\"\n-\t(or the function name regex ) within the . You may\n-\tnot give any pathspec limiters. This is currently limited to\n-\ta walk starting from a single revision, i.e., you may only\n-\tgive zero or one positive revision arguments, and\n-\t and (or ) must exist in the starting revision.\n-\tYou can specify this option more than once. Implies `--patch`.\n-\tPatch output can be suppressed using `--no-patch`, but other diff formats\n-\t(namely `--raw`, `--numstat`, `--shortstat`, `--dirstat`, `--summary`,\n-\t`--name-only`, `--name-status`, `--check`) are not currently implemented.\n-+\n-include::line-range-format.txt[]\n+include::line-range-options.txt[]\n \n ::\n \tShow only commits in the specified revision range. When no\ndiff --git a/Documentation/gitk.txt b/Documentation/gitk.txt\nindex c653ebb6a8..d50e9ed10e 100644\n--- a/Documentation/gitk.txt\n+++ b/Documentation/gitk.txt\n@@ -98,25 +98,7 @@ linkgit:git-rev-list[1] for a complete list.\n \t(See \"History simplification\" in linkgit:git-log[1] for a more\n \tdetailed explanation.)\n \n--L,:::\n--L::::\n-\n-\tTrace the evolution of the line range given by \",\"\n-\t(or the function name regex ) within the . You may\n-\tnot give any pathspec limiters. This is currently limited to\n-\ta walk starting from a single revision, i.e., you may only\n-\tgive zero or one positive revision arguments, and\n-\t and (or ) must exist in the starting revision.\n-\tYou can specify this option more than once. Implies `--patch`.\n-\tPatch output can be suppressed using `--no-patch`, but other diff formats\n-\t(namely `--raw`, `--numstat`, `--shortstat`, `--dirstat`, `--summary`,\n-\t`--name-only`, `--name-status`, `--check`) are not currently implemented.\n-+\n-*Note:* gitk (unlike linkgit:git-log[1]) currently only understands\n-this option if you specify it \"glued together\" with its argument. Do\n-*not* put a space after `-L`.\n-+\n-include::line-range-format.txt[]\n+include::line-range-options.txt[]\n \n ::\n \ndiff --git a/Documentation/line-range-options.txt b/Documentation/line-range-options.txt\nnew file mode 100644\nindex 0000000000..266263f6b4\n--- /dev/null\n+++ b/Documentation/line-range-options.txt\n@@ -0,0 +1,15 @@\n+-L,:::\n+-L::::\n+\n+\tTrace the evolution of the line range given by \",\"\n+\t(or the function name regex ) within the . You may\n+\tnot give any pathspec limiters. This is currently limited to\n+\ta walk starting from a single revision, i.e., you may only\n+\tgive zero or one positive revision arguments, and\n+\t and (or ) must exist in the starting revision.\n+\tYou can specify this option more than once. Implies `--patch`.\n+\tPatch output can be suppressed using `--no-patch`, but other diff formats\n+\t(namely `--raw`, `--numstat`, `--shortstat`, `--dirstat`, `--summary`,\n+\t`--name-only`, `--name-status`, `--check`) are not currently implemented.\n++\n+include::line-range-format.txt[]\n-- \ngitgitgadget\n\n"},{"id":"408818","messageId":"pull.774.v2.git.1604251727.gitgitgadget@gmail.com","threadId":"54521","inReplyTo":"pull.774.git.1603889270.gitgitgadget@gmail.com","subject":"[PATCH v2 0/8] blame: enable funcname blaming with userdiff driver","fromName":"Philippe Blain via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-11-01T17:28:39Z","receivedAt":"2020-11-01T17:28:53Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Changes since v1:\n\n * patch 1: tweaked the message and took Junio's suggestion to only list the\n stuck form if the option in 'git-log' and 'gitk'.\n * patch 2: very minor formatting changes (removed double quotes)\n * patch 3: removed the parentheses as suggested by Junio, and did the same\n for git-log/gitk.\n * patch 4: grammar fix pointed out by Eric\n * patch 5: shortened the help text as suggested by Eric\n * patch 6: changed the approach by initializing 'sb.path' earlier in\n 'cmd_blame', along with some of sb's other fields.\n * patches 7-8: new cleanup patches following the new patch 6 to simplify\n the interface of two functions in 'blame.c'\n\n\n----------------------------------------------------------------------------\n\nThis series fixes a bug in git blame where any userdiff driver configured\nvia the attributes mechanism is ignored, which more often than not means\nthatgit blame -L : dies for paths that have a configured userdiff\ndriver. That's patch 6/6. \n\nPatches 1-5 are documentation improvements around the line-log\nfunctionality.\n\nPatches 7-8 are code clean-up patches following the changes in patch 6.\n\nThe fact that this bug has been present ever since git blame learned -L\n: makes me a little sad. I think that the fact that the diff \nattribute (either with a builtin userdiff pattern or with diff.*.xfuncname )\ninfluences way more than just the hunk header, but also features like git\nlog -L, git blame -L, and the different options that I mention in patch 4/6,\nis not well-known enough. I'm not saying the code or the doc is wrong, but I\nthink it's a visibility issue. Maybe more blog posts about that would\nhelp.... I noticed that the \"Git Attributes\" chapter in the ProGit book [1]\ndoes not even mention the builtin userdiff patterns, so I'll probably do a\nPR there to mention it. Any other ideas for improving discoverability of\nthis very cool feature ?\n\n[1] https://git-scm.com/book/en/v2/Customizing-Git-Git-Attributes\n\nCC: Thomas Rast tr@thomasrast.ch [tr@thomasrast.ch], Eric Sunshine \nsunshine@sunshineco.com [sunshine@sunshineco.com], René Scharfe l.s.r@web.de\n[l.s.r@web.de]\n\nPhilippe Blain (8):\n doc: log, gitk: move '-L' description to 'line-range-options.txt'\n doc: line-range: improve formatting\n blame-options.txt: also mention 'funcname' in '-L' description\n doc: add more pointers to gitattributes(5) for userdiff\n line-log: mention both modes in 'blame' and 'log' short help\n blame: enable funcname blaming with userdiff driver\n blame: simplify 'setup_scoreboard' interface\n blame: simplify 'setup_blame_bloom_data' interface\n\n Documentation/blame-options.txt | 9 +++++----\n Documentation/diff-options.txt | 5 ++++-\n Documentation/git-grep.txt | 6 ++++--\n Documentation/git-log.txt | 15 +--------------\n Documentation/gitk.txt | 20 +-------------------\n Documentation/line-range-format.txt | 28 +++++++++++++++-------------\n Documentation/line-range-options.txt | 15 +++++++++++++++\n blame.c | 16 +++++++---------\n blame.h | 4 +---\n builtin/blame.c | 11 ++++++-----\n builtin/log.c | 4 ++--\n t/annotate-tests.sh | 18 ++++++++++++++++++\n 12 files changed, 79 insertions(+), 72 deletions(-)\n create mode 100644 Documentation/line-range-options.txt\n\n\nbase-commit: 1d1c4a875900d69c7f0a31e44c3e370dc80ab1ce\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-774%2Fphil-blain%2Fblame-funcname-userdiff-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-774/phil-blain/blame-funcname-userdiff-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/774\n\nRange-diff vs v1:\n\n 1: 96f6f95abc ! 1: b4fa5fb5ae doc: log, gitk: move '-L' description to 'line-range-options.txt'\n @@ Metadata\n ## Commit message ##\n doc: log, gitk: move '-L' description to 'line-range-options.txt'\n \n - The description of the '-L' option for `git log` and `gitk` is the\n - same, but is repeated in both 'git-log.txt' and 'gitk.txt'.\n + The description of the '-L' option for `git log` and `gitk` is almost\n + the same, but is repeated in both 'git-log.txt' and 'gitk.txt' (the\n + difference being that 'git-log.txt' lists the option with a space\n + after '-L', while 'gitk.txt' lists it as stuck and notes that `gitk`\n + only understands the stuck form).\n \n - Remove the duplication by creating a new file, 'line-range-options.txt',\n + Reduce duplication by creating a new file, 'line-range-options.txt',\n and include it in both files.\n \n + To simplify the presentation, only list the stuck form for both\n + commands, and remove the note about `gitk` only understanding the stuck\n + form.\n +\n Signed-off-by: Philippe Blain \n \n ## Documentation/git-log.txt ##\n @@ Documentation/git-log.txt: produced by `--stat`, etc.\n -\t`--name-only`, `--name-status`, `--check`) are not currently implemented.\n -+\n -include::line-range-format.txt[]\n -+:git-log: 1\n +include::line-range-options.txt[]\n \n ::\n @@ Documentation/gitk.txt: linkgit:git-rev-list[1] for a complete list.\n -*not* put a space after `-L`.\n -+\n -include::line-range-format.txt[]\n -+:gitk: 1\n +include::line-range-options.txt[]\n \n ::\n @@ Documentation/gitk.txt: linkgit:git-rev-list[1] for a complete list.\n \n ## Documentation/line-range-options.txt (new) ##\n @@\n -+ifdef::git-log[]\n -+-L ,:::\n -+-L ::::\n -+endif::git-log[]\n -+ifdef::gitk[]\n +-L,:::\n +-L::::\n -+endif::gitk[]\n +\n +\tTrace the evolution of the line range given by \",\"\n +\t(or the function name regex ) within the . You may\n @@ Documentation/line-range-options.txt (new)\n +\t(namely `--raw`, `--numstat`, `--shortstat`, `--dirstat`, `--summary`,\n +\t`--name-only`, `--name-status`, `--check`) are not currently implemented.\n ++\n -+ifdef::gitk[]\n -+*Note:* gitk (unlike linkgit:git-log[1]) currently only understands\n -+this option if you specify it \"glued together\" with its argument. Do\n -+*not* put a space after `-L`.\n -+endif::gitk[]\n -++\n +include::line-range-format.txt[]\n 2: 7d3fc0a503 ! 2: 6c44b1c576 doc: line-range: improve formatting\n @@ Documentation/line-range-format.txt\n \n +\n -If ``:'' is given in place of and , it is a\n -+If \"`:`\" is given in place of '' and '', it is a\n ++If `:` is given in place of '' and '', it is a\n regular expression that denotes the range from the first funcname line\n -that matches , up to the next funcname line. ``:''\n +that matches '', up to the next funcname line. `:`\n @@ Documentation/line-range-format.txt\n file.\n \n ## Documentation/line-range-options.txt ##\n -@@ Documentation/line-range-options.txt: ifdef::gitk[]\n +@@\n + -L,:::\n -L::::\n - endif::gitk[]\n \n -\tTrace the evolution of the line range given by \",\"\n -\t(or the function name regex ) within the . You may\n -+\tTrace the evolution of the line range given by \"','\"\n ++\tTrace the evolution of the line range given by ','\n +\t(or the function name regex '') within the ''. You may\n \tnot give any pathspec limiters. This is currently limited to\n \ta walk starting from a single revision, i.e., you may only\n 3: f7b64bf330 ! 3: d38f0f7e26 blame-options.txt: also mention 'funcname' in '-L' description\n @@ Commit message\n '-L :' by mentioning it at the beginnning of the description\n of the '-L' option.\n \n + Also, in 'line-range-options.txt', which is used for git-log(1) and\n + gitk(1), do not parenthesize the mention of the ':' mode, to\n + place it on equal footing with the ',' mode.\n +\n Signed-off-by: Philippe Blain \n \n ## Documentation/blame-options.txt ##\n @@ Documentation/blame-options.txt\n -L :::\n -\tAnnotate only the given line range. May be specified multiple times.\n -\tOverlapping ranges are allowed.\n -+\tAnnotate only the line range given by ','\n -+\t(or by the function name regex '').\n ++\tAnnotate only the line range given by ',',\n ++\tor by the function name regex ''.\n +\tMay be specified multiple times. Overlapping ranges are allowed.\n +\n '' and '' are optional. `-L ` or `-L ,` spans from\n '' to end of file. `-L ,` spans from start of file to ''.\n +\n + ## Documentation/line-range-options.txt ##\n +@@\n + -L,:::\n + -L::::\n + \n +-\tTrace the evolution of the line range given by ','\n +-\t(or the function name regex '') within the ''. You may\n ++\tTrace the evolution of the line range given by ',',\n ++\tor by the function name regex '', within the ''. You may\n + \tnot give any pathspec limiters. This is currently limited to\n + \ta walk starting from a single revision, i.e., you may only\n + \tgive zero or one positive revision arguments, and\n 4: da76a10717 ! 4: 9443a681e3 doc: add more pointers to gitattributes(5) for userdiff\n @@ Documentation/diff-options.txt: endif::git-format-patch[]\n -W::\n --function-context::\n -\tShow whole surrounding functions of changes.\n -+\tShow whole functions as context lines for each changes.\n ++\tShow whole function as context lines for each change.\n +\tThe function names are determined in the same way as\n +\t`git diff` works out patch hunk headers (see 'Defining a\n +\tcustom hunk-header' in linkgit:gitattributes[5]).\n 5: 27ef94e9cc ! 5: ae61d27a38 line-log: mention both modes in 'blame' and 'log' short help\n @@ builtin/blame.c: int cmd_blame(int argc, const char **argv, const char *prefix)\n \t\tOPT_CALLBACK_F('M', NULL, &opt, N_(\"score\"), N_(\"Find line movements within and across files\"), PARSE_OPT_OPTARG, blame_move_callback),\n -\t\tOPT_STRING_LIST('L', NULL, &range_list, N_(\"n,m\"), N_(\"Process only line range n,m, counting from 1\")),\n +\t\tOPT_STRING_LIST('L', NULL, &range_list, N_(\"range\"),\n -+\t\t\t\tN_(\"Process only the given line range (=,) or function (=:)\")),\n ++\t\t\t\tN_(\"Process only line range , or function :\")),\n \t\tOPT__ABBREV(&abbrev),\n \t\tOPT_END()\n \t};\n @@ builtin/log.c: static void cmd_log_init_finish(int argc, const char **argv, cons\n -\t\tOPT_CALLBACK('L', NULL, &line_cb, \"n,m:file\",\n -\t\t\t N_(\"Process line range n,m in file, counting from 1\"),\n +\t\tOPT_CALLBACK('L', NULL, &line_cb, \"range:file\",\n -+\t\t\t N_(\"Trace the evolution of the given line range (=,) or function (=:) in \"),\n ++\t\t\t N_(\"Trace the evolution of line range , or function : in \"),\n \t\t\t log_line_range_callback),\n \t\tOPT_END()\n \t};\n 6: a1e1c977d0 ! 6: c77108e864 blame: enable funcname blaming with userdiff driver\n @@ Commit message\n funcname, 2013-03-28).\n \n Enable funcname blaming for paths using specific userdiff drivers by\n - sending the local variable 'path' to 'parse_range_arg' instead of the\n - yet unset 'sb.path'.\n + initializing 'sb.path' earlier in 'cmd_blame', when some of its other\n + fields are initialized, so that it is set when passed to\n + 'parse_range_arg'.\n \n Add a regression test in 'annotate-tests.sh', which is sourced in\n t8001-annotate.sh and t8002-blame.sh, leveraging an existing file used\n to test the userdiff patterns in t4018-diff-funcname.\n \n + Also, use 'sb.path' instead of 'path' when constructing the error\n + message at line 1114, for consistency.\n +\n Signed-off-by: Philippe Blain \n \n ## builtin/blame.c ##\n @@ builtin/blame.c: int cmd_blame(int argc, const char **argv, const char *prefix)\n - \t\tlong bottom, top;\n - \t\tif (parse_range_arg(range_list.items[range_i].string,\n - \t\t\t\t nth_line_cb, &sb, lno, anchor,\n --\t\t\t\t &bottom, &top, sb.path,\n -+\t\t\t\t &bottom, &top, path,\n - \t\t\t\t the_repository->index))\n - \t\t\tusage(blame_usage);\n + \tsb.contents_from = contents_from;\n + \tsb.reverse = reverse;\n + \tsb.repo = the_repository;\n ++\tsb.path = path;\n + \tbuild_ignorelist(&sb, &ignore_revs_file_list, &ignore_rev_list);\n + \tstring_list_clear(&ignore_revs_file_list, 0);\n + \tstring_list_clear(&ignore_rev_list, 0);\n +@@ builtin/blame.c: int cmd_blame(int argc, const char **argv, const char *prefix)\n \t\tif ((!lno && (top || bottom)) || lno < bottom)\n + \t\t\tdie(Q_(\"file %s has only %lu line\",\n + \t\t\t \"file %s has only %lu lines\",\n +-\t\t\t lno), path, lno);\n ++\t\t\t lno), sb.path, lno);\n + \t\tif (bottom < 1)\n + \t\t\tbottom = 1;\n + \t\tif (top < 1 || lno < top)\n +@@ builtin/blame.c: int cmd_blame(int argc, const char **argv, const char *prefix)\n + \tstring_list_clear(&range_list, 0);\n + \n + \tsb.ent = NULL;\n +-\tsb.path = path;\n + \n + \tif (blame_move_score)\n + \t\tsb.move_score = blame_move_score;\n \n ## t/annotate-tests.sh ##\n @@ t/annotate-tests.sh: test_expect_success 'blame -L ^:RE (absolute: end-of-file)' '\n -: ---------- > 7: 3b28696e51 blame: simplify 'setup_scoreboard' interface\n -: ---------- > 8: 7aca3274d2 blame: simplify 'setup_blame_bloom_data' interface\n\n-- \ngitgitgadget\n"},{"id":"408815","messageId":"d38f0f7e268897effa200a4c61c03d5cf082eb13.1604251728.git.gitgitgadget@gmail.com","threadId":"54521","inReplyTo":"pull.774.v2.git.1604251727.gitgitgadget@gmail.com","subject":"[PATCH v2 3/8] blame-options.txt: also mention 'funcname' in '-L' description","fromName":"Philippe Blain via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-11-01T17:28:42Z","receivedAt":"2020-11-01T17:28:55Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"From: Philippe Blain \n\nMake it clearer that a function can be blamed by feeding `git blame`\n'-L :' by mentioning it at the beginnning of the description\nof the '-L' option.\n\nAlso, in 'line-range-options.txt', which is used for git-log(1) and\ngitk(1), do not parenthesize the mention of the ':' mode, to\nplace it on equal footing with the ',' mode.\n\nSigned-off-by: Philippe Blain \n---\n Documentation/blame-options.txt | 5 +++--\n Documentation/line-range-options.txt | 4 ++--\n 2 files changed, 5 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt\nindex 48bf0eeec5..dc3bceb6d1 100644\n--- a/Documentation/blame-options.txt\n+++ b/Documentation/blame-options.txt\n@@ -11,8 +11,9 @@\n \n -L ,::\n -L :::\n-\tAnnotate only the given line range. May be specified multiple times.\n-\tOverlapping ranges are allowed.\n+\tAnnotate only the line range given by ',',\n+\tor by the function name regex ''.\n+\tMay be specified multiple times. Overlapping ranges are allowed.\n +\n '' and '' are optional. `-L ` or `-L ,` spans from\n '' to end of file. `-L ,` spans from start of file to ''.\ndiff --git a/Documentation/line-range-options.txt b/Documentation/line-range-options.txt\nindex 1c90127300..8e295a62b8 100644\n--- a/Documentation/line-range-options.txt\n+++ b/Documentation/line-range-options.txt\n@@ -1,8 +1,8 @@\n -L,:::\n -L::::\n \n-\tTrace the evolution of the line range given by ','\n-\t(or the function name regex '') within the ''. You may\n+\tTrace the evolution of the line range given by ',',\n+\tor by the function name regex '', within the ''. You may\n \tnot give any pathspec limiters. This is currently limited to\n \ta walk starting from a single revision, i.e., you may only\n \tgive zero or one positive revision arguments, and\n-- \ngitgitgadget\n\n"},{"id":"408816","messageId":"9443a681e3c26410d0c262ac52fc604b39a89421.1604251728.git.gitgitgadget@gmail.com","threadId":"54521","inReplyTo":"pull.774.v2.git.1604251727.gitgitgadget@gmail.com","subject":"[PATCH v2 4/8] doc: add more pointers to gitattributes(5) for userdiff","fromName":"Philippe Blain via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-11-01T17:28:43Z","receivedAt":"2020-11-01T17:28:56Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"From: Philippe Blain \n\nSeveral Git commands can make use of the builtin userdiff patterns, but\nit's not obvious in the documentation. Add pointers to the 'Defining a\ncustom hunk header' part of gitattributes(5) in the description of the\nfollowing options:\n\n- the '--function-context' option of `git diff` and friends\n- the '--function-context' option of `git grep`\n- the '-L :' option of `git log`, `gitk` and `git blame`\n\nIn 'git-grep.txt', take the opportunity to use backticks in the\ndescription of '--show-function', and improve the wording of the\ndesription of '--function-context'.\n\nSigned-off-by: Philippe Blain \n---\n Documentation/diff-options.txt | 5 ++++-\n Documentation/git-grep.txt | 6 ++++--\n Documentation/line-range-format.txt | 4 +++-\n 3 files changed, 11 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex 573fb9bb71..72b558ce21 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -695,7 +695,10 @@ endif::git-format-patch[]\n \n -W::\n --function-context::\n-\tShow whole surrounding functions of changes.\n+\tShow whole function as context lines for each change.\n+\tThe function names are determined in the same way as\n+\t`git diff` works out patch hunk headers (see 'Defining a\n+\tcustom hunk-header' in linkgit:gitattributes[5]).\n \n ifndef::git-format-patch[]\n ifndef::git-log[]\ndiff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt\nindex 6077ff01a4..4e0ba8234a 100644\n--- a/Documentation/git-grep.txt\n+++ b/Documentation/git-grep.txt\n@@ -241,7 +241,7 @@ providing this option will cause it to die.\n --show-function::\n \tShow the preceding line that contains the function name of\n \tthe match, unless the matching line is a function name itself.\n-\tThe name is determined in the same way as 'git diff' works out\n+\tThe name is determined in the same way as `git diff` works out\n \tpatch hunk headers (see 'Defining a custom hunk-header' in\n \tlinkgit:gitattributes[5]).\n \n@@ -266,7 +266,9 @@ providing this option will cause it to die.\n \tShow the surrounding text from the previous line containing a\n \tfunction name up to the one before the next function name,\n \teffectively showing the whole function in which the match was\n-\tfound.\n+\tfound. The function names are determined in the same way as\n+\t`git diff` works out patch hunk headers (see 'Defining a\n+\tcustom hunk-header' in linkgit:gitattributes[5]).\n \n --threads ::\n \tNumber of grep worker threads to use.\ndiff --git a/Documentation/line-range-format.txt b/Documentation/line-range-format.txt\nindex 43759eef89..9b51e9fb66 100644\n--- a/Documentation/line-range-format.txt\n+++ b/Documentation/line-range-format.txt\n@@ -27,4 +27,6 @@ regular expression that denotes the range from the first funcname line\n that matches '', up to the next funcname line. `:`\n searches from the end of the previous `-L` range, if any, otherwise\n from the start of file. `^:` searches from the start of\n-file.\n+file. The function names are determined in the same way as `git diff`\n+works out patch hunk headers (see 'Defining a custom hunk-header'\n+in linkgit:gitattributes[5]).\n-- \ngitgitgadget\n\n"},{"id":"408817","messageId":"ae61d27a3869aefa5eb568b0d9e1e462aceacc93.1604251728.git.gitgitgadget@gmail.com","threadId":"54521","inReplyTo":"pull.774.v2.git.1604251727.gitgitgadget@gmail.com","subject":"[PATCH v2 5/8] line-log: mention both modes in 'blame' and 'log' short help","fromName":"Philippe Blain via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-11-01T17:28:44Z","receivedAt":"2020-11-01T17:28:58Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"From: Philippe Blain \n\n'git blame -h' and 'git log -h' both show '-L ' and describe this\noption as \"Process only line range n,m, counting from 1\". No hint is\ngiven that a function name regex can also be used.\n\nUse instead, and expand the description of the option to mention\nboth modes. Remove \"counting from 1\" as it's uneeded; it's uncommon to\nrefer to the first line of a file as \"line 0\".\n\nAlso, for 'git log', improve the wording to better reflect the long help.\n\nSigned-off-by: Philippe Blain \n---\n builtin/blame.c | 3 ++-\n builtin/log.c | 4 ++--\n 2 files changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex bb0f29300e..224b6f18d4 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -889,7 +889,8 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n \t\tOPT_STRING(0, \"contents\", &contents_from, N_(\"file\"), N_(\"Use 's contents as the final image\")),\n \t\tOPT_CALLBACK_F('C', NULL, &opt, N_(\"score\"), N_(\"Find line copies within and across files\"), PARSE_OPT_OPTARG, blame_copy_callback),\n \t\tOPT_CALLBACK_F('M', NULL, &opt, N_(\"score\"), N_(\"Find line movements within and across files\"), PARSE_OPT_OPTARG, blame_move_callback),\n-\t\tOPT_STRING_LIST('L', NULL, &range_list, N_(\"n,m\"), N_(\"Process only line range n,m, counting from 1\")),\n+\t\tOPT_STRING_LIST('L', NULL, &range_list, N_(\"range\"),\n+\t\t\t\tN_(\"Process only line range , or function :\")),\n \t\tOPT__ABBREV(&abbrev),\n \t\tOPT_END()\n \t};\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 0a7ed4bef9..c87ce09325 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -183,8 +183,8 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,\n \t\t\t\tN_(\"pattern\"), N_(\"do not decorate refs that match \")),\n \t\tOPT_CALLBACK_F(0, \"decorate\", NULL, NULL, N_(\"decorate options\"),\n \t\t\t PARSE_OPT_OPTARG, decorate_callback),\n-\t\tOPT_CALLBACK('L', NULL, &line_cb, \"n,m:file\",\n-\t\t\t N_(\"Process line range n,m in file, counting from 1\"),\n+\t\tOPT_CALLBACK('L', NULL, &line_cb, \"range:file\",\n+\t\t\t N_(\"Trace the evolution of line range , or function : in \"),\n \t\t\t log_line_range_callback),\n \t\tOPT_END()\n \t};\n-- \ngitgitgadget\n\n"},{"id":"408822","messageId":"6c44b1c576150212cfb23c6f3d0ca3563ddf4556.1604251728.git.gitgitgadget@gmail.com","threadId":"54521","inReplyTo":"pull.774.v2.git.1604251727.gitgitgadget@gmail.com","subject":"[PATCH v2 2/8] doc: line-range: improve formatting","fromName":"Philippe Blain via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-11-01T17:28:41Z","receivedAt":"2020-11-01T17:28:58Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"From: Philippe Blain \n\nImprove the formatting of the description of the line-range option '-L'\nfor `git log`, `gitk` and `git blame`:\n\n- Use bold for , and \n- Use backticks for literals\n\nSigned-off-by: Philippe Blain \n---\n Documentation/blame-options.txt | 4 ++--\n Documentation/line-range-format.txt | 24 ++++++++++++------------\n Documentation/line-range-options.txt | 6 +++---\n 3 files changed, 17 insertions(+), 17 deletions(-)\n\ndiff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt\nindex 88750af7ae..48bf0eeec5 100644\n--- a/Documentation/blame-options.txt\n+++ b/Documentation/blame-options.txt\n@@ -14,8 +14,8 @@\n \tAnnotate only the given line range. May be specified multiple times.\n \tOverlapping ranges are allowed.\n +\n- and are optional. ``-L '' or ``-L ,'' spans from\n- to end of file. ``-L ,'' spans from start of file to .\n+'' and '' are optional. `-L ` or `-L ,` spans from\n+'' to end of file. `-L ,` spans from start of file to ''.\n +\n include::line-range-format.txt[]\n \ndiff --git a/Documentation/line-range-format.txt b/Documentation/line-range-format.txt\nindex 829676ff98..43759eef89 100644\n--- a/Documentation/line-range-format.txt\n+++ b/Documentation/line-range-format.txt\n@@ -1,30 +1,30 @@\n- and can take one of these forms:\n+'' and '' can take one of these forms:\n \n - number\n +\n-If or is a number, it specifies an\n+If '' or '' is a number, it specifies an\n absolute line number (lines count from 1).\n +\n \n-- /regex/\n+- `/regex/`\n +\n This form will use the first line matching the given\n-POSIX regex. If is a regex, it will search from the end of\n+POSIX regex. If '' is a regex, it will search from the end of\n the previous `-L` range, if any, otherwise from the start of file.\n-If is ``^/regex/'', it will search from the start of file.\n-If is a regex, it will search\n-starting at the line given by .\n+If '' is `^/regex/`, it will search from the start of file.\n+If '' is a regex, it will search\n+starting at the line given by ''.\n +\n \n - +offset or -offset\n +\n-This is only valid for and will specify a number\n-of lines before or after the line given by .\n+This is only valid for '' and will specify a number\n+of lines before or after the line given by ''.\n \n +\n-If ``:'' is given in place of and , it is a\n+If `:` is given in place of '' and '', it is a\n regular expression that denotes the range from the first funcname line\n-that matches , up to the next funcname line. ``:''\n+that matches '', up to the next funcname line. `:`\n searches from the end of the previous `-L` range, if any, otherwise\n-from the start of file. ``^:'' searches from the start of\n+from the start of file. `^:` searches from the start of\n file.\ndiff --git a/Documentation/line-range-options.txt b/Documentation/line-range-options.txt\nindex 266263f6b4..1c90127300 100644\n--- a/Documentation/line-range-options.txt\n+++ b/Documentation/line-range-options.txt\n@@ -1,12 +1,12 @@\n -L,:::\n -L::::\n \n-\tTrace the evolution of the line range given by \",\"\n-\t(or the function name regex ) within the . You may\n+\tTrace the evolution of the line range given by ','\n+\t(or the function name regex '') within the ''. You may\n \tnot give any pathspec limiters. This is currently limited to\n \ta walk starting from a single revision, i.e., you may only\n \tgive zero or one positive revision arguments, and\n-\t and (or ) must exist in the starting revision.\n+\t'' and '' (or '') must exist in the starting revision.\n \tYou can specify this option more than once. Implies `--patch`.\n \tPatch output can be suppressed using `--no-patch`, but other diff formats\n \t(namely `--raw`, `--numstat`, `--shortstat`, `--dirstat`, `--summary`,\n-- \ngitgitgadget\n\n"},{"id":"408819","messageId":"c77108e864afe8c60018248923ff75cdef05efb4.1604251728.git.gitgitgadget@gmail.com","threadId":"54521","inReplyTo":"pull.774.v2.git.1604251727.gitgitgadget@gmail.com","subject":"[PATCH v2 6/8] blame: enable funcname blaming with userdiff driver","fromName":"Philippe Blain via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-11-01T17:28:45Z","receivedAt":"2020-11-01T17:28:59Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"From: Philippe Blain \n\nIn blame.c::cmd_blame, we send the 'path' field of the 'sb' 'struct\nblame_scoreboard' as the 'path' argument to\n'line-range.c::parse_range_arg', but 'sb.path' is not set yet; it's set\nto the local variable 'path' a few lines later at line 1137.\n\nThis 'path' argument is only used in 'parse_range_arg' if we are blaming\na funcname, i.e. `git blame -L : `, and in that case it\nis sent to 'parse_range_funcname', where it is used to determine if a\nuserdiff driver should be used for said to match the given\nfuncname.\n\nSince 'path' is yet unset, the userdiff driver is never used, so we fall\nback to the default funcname regex, which is usually not appropriate for\npaths that are set to use a specific userdiff driver, and thus either we\nmatch some unrelated lines, or we die with\n\n fatal: -L parameter '' starting at line 1: no match\n\nThis has been the case ever since `git blame` learned to blame a\nfuncname in 13b8f68c1f (log -L: :pattern:file syntax to find by\nfuncname, 2013-03-28).\n\nEnable funcname blaming for paths using specific userdiff drivers by\ninitializing 'sb.path' earlier in 'cmd_blame', when some of its other\nfields are initialized, so that it is set when passed to\n'parse_range_arg'.\n\nAdd a regression test in 'annotate-tests.sh', which is sourced in\nt8001-annotate.sh and t8002-blame.sh, leveraging an existing file used\nto test the userdiff patterns in t4018-diff-funcname.\n\nAlso, use 'sb.path' instead of 'path' when constructing the error\nmessage at line 1114, for consistency.\n\nSigned-off-by: Philippe Blain \n---\n builtin/blame.c | 4 ++--\n t/annotate-tests.sh | 18 ++++++++++++++++++\n 2 files changed, 20 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 224b6f18d4..f2e528fcbc 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -1081,6 +1081,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n \tsb.contents_from = contents_from;\n \tsb.reverse = reverse;\n \tsb.repo = the_repository;\n+\tsb.path = path;\n \tbuild_ignorelist(&sb, &ignore_revs_file_list, &ignore_rev_list);\n \tstring_list_clear(&ignore_revs_file_list, 0);\n \tstring_list_clear(&ignore_rev_list, 0);\n@@ -1110,7 +1111,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n \t\tif ((!lno && (top || bottom)) || lno < bottom)\n \t\t\tdie(Q_(\"file %s has only %lu line\",\n \t\t\t \"file %s has only %lu lines\",\n-\t\t\t lno), path, lno);\n+\t\t\t lno), sb.path, lno);\n \t\tif (bottom < 1)\n \t\t\tbottom = 1;\n \t\tif (top < 1 || lno < top)\n@@ -1135,7 +1136,6 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n \tstring_list_clear(&range_list, 0);\n \n \tsb.ent = NULL;\n-\tsb.path = path;\n \n \tif (blame_move_score)\n \t\tsb.move_score = blame_move_score;\ndiff --git a/t/annotate-tests.sh b/t/annotate-tests.sh\nindex d933af5714..3aee61d2cc 100644\n--- a/t/annotate-tests.sh\n+++ b/t/annotate-tests.sh\n@@ -479,6 +479,24 @@ test_expect_success 'blame -L ^:RE (absolute: end-of-file)' '\n \tcheck_count -f hello.c -L$n -L^:ma.. F 4 G 1 H 1\n '\n \n+test_expect_success 'setup -L :funcname with userdiff driver' '\n+\techo \"fortran-* diff=fortran\" >.gitattributes &&\n+\tfortran_file=fortran-external-function &&\n+\torig_file=\"$TEST_DIRECTORY/t4018/$fortran_file\" &&\n+\tcp $orig_file . &&\n+\tgit add $fortran_file &&\n+\tGIT_AUTHOR_NAME=\"A\" GIT_AUTHOR_EMAIL=\"A@test.git\" \\\n+\tgit commit -m \"add fortran file\" &&\n+\tsed -e \"s/ChangeMe/IWasChanged/\" <\"$orig_file\" >$fortran_file &&\n+\tgit add $fortran_file &&\n+\tGIT_AUTHOR_NAME=\"B\" GIT_AUTHOR_EMAIL=\"B@test.git\" \\\n+\tgit commit -m \"change fortran file\"\n+'\n+\n+test_expect_success 'blame -L :funcname with userdiff driver' '\n+\tcheck_count -f fortran-external-function -L:RIGHT A 7 B 1\n+'\n+\n test_expect_success 'setup incremental' '\n \t(\n \tGIT_AUTHOR_NAME=I &&\n-- \ngitgitgadget\n\n"},{"id":"408821","messageId":"7aca3274d2d25994984cd98f94ed8c3a8411749e.1604251728.git.gitgitgadget@gmail.com","threadId":"54521","inReplyTo":"pull.774.v2.git.1604251727.gitgitgadget@gmail.com","subject":"[PATCH v2 8/8] blame: simplify 'setup_blame_bloom_data' interface","fromName":"Philippe Blain via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-11-01T17:28:47Z","receivedAt":"2020-11-01T17:29:02Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"From: Philippe Blain \n\nThe penultimate commit moved the initialization of 'sb.path' in\n'builtin/blame.c::cmd_blame' before the call to\n'blame.c::setup_blame_bloom_data'. Since 'cmd_blame' is the only caller\nof 'setup_blame_bloom_data', it is now unnecessary for\n'setup_blame_bloom_data' to receive 'path' as a separate argument, as\n'sb.path' is already initialized.\n\nRemove this argument from setup_blame_bloom_data's interface and use the\n'path' field of the 'sb' 'struct blame_scoreboard' instead.\n\nSigned-off-by: Philippe Blain \n---\n blame.c | 5 ++---\n blame.h | 3 +--\n builtin/blame.c | 2 +-\n 3 files changed, 4 insertions(+), 6 deletions(-)\n\ndiff --git a/blame.c b/blame.c\nindex 128cb7ba55..9042aa3f2b 100644\n--- a/blame.c\n+++ b/blame.c\n@@ -2887,8 +2887,7 @@ struct blame_entry *blame_entry_prepend(struct blame_entry *head,\n \treturn new_head;\n }\n \n-void setup_blame_bloom_data(struct blame_scoreboard *sb,\n-\t\t\t const char *path)\n+void setup_blame_bloom_data(struct blame_scoreboard *sb)\n {\n \tstruct blame_bloom_data *bd;\n \tstruct bloom_filter_settings *bs;\n@@ -2908,7 +2907,7 @@ void setup_blame_bloom_data(struct blame_scoreboard *sb,\n \tbd->nr = 0;\n \tALLOC_ARRAY(bd->keys, bd->alloc);\n \n-\tadd_bloom_key(bd, path);\n+\tadd_bloom_key(bd, sb->path);\n \n \tsb->bloom_data = bd;\n }\ndiff --git a/blame.h b/blame.h\nindex e8c185c8ef..38bde535b3 100644\n--- a/blame.h\n+++ b/blame.h\n@@ -182,8 +182,7 @@ const char *blame_nth_line(struct blame_scoreboard *sb, long lno);\n void init_scoreboard(struct blame_scoreboard *sb);\n void setup_scoreboard(struct blame_scoreboard *sb,\n \t\t struct blame_origin **orig);\n-void setup_blame_bloom_data(struct blame_scoreboard *sb,\n-\t\t\t const char *path);\n+void setup_blame_bloom_data(struct blame_scoreboard *sb);\n void cleanup_scoreboard(struct blame_scoreboard *sb);\n \n struct blame_entry *blame_entry_prepend(struct blame_entry *head,\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 5d8f4c4599..2f8b06e589 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -1092,7 +1092,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n \t * for copies.\n \t */\n \tif (!(opt & PICKAXE_BLAME_COPY))\n-\t\tsetup_blame_bloom_data(&sb, path);\n+\t\tsetup_blame_bloom_data(&sb);\n \n \tlno = sb.num_lines;\n \n-- \ngitgitgadget\n"},{"id":"408820","messageId":"3b28696e51a9ba370d25b70844b9fb57a4bfaab2.1604251728.git.gitgitgadget@gmail.com","threadId":"54521","inReplyTo":"pull.774.v2.git.1604251727.gitgitgadget@gmail.com","subject":"[PATCH v2 7/8] blame: simplify 'setup_scoreboard' interface","fromName":"Philippe Blain via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-11-01T17:28:46Z","receivedAt":"2020-11-01T17:29:03Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"From: Philippe Blain \n\nThe previous commit moved the initialization of 'sb.path' in\n'builtin/blame.c::cmd_blame' before the call to\n'blame.c::setup_scoreboard'. Since 'cmd_blame' is the only caller of\n'setup_scoreboard', it is now unnecessary for 'setup_scoreboard' to\nreceive 'path' as a separate argument, as 'sb.path' is already\ninitialized.\n\nRemove this argument from setup_scoreboard's interface and use the\n'path' field of the 'sb' 'struct blame_scoreboard' instead.\n\nSigned-off-by: Philippe Blain \n---\n blame.c | 11 +++++------\n blame.h | 1 -\n builtin/blame.c | 2 +-\n 3 files changed, 6 insertions(+), 8 deletions(-)\n\ndiff --git a/blame.c b/blame.c\nindex 686845b2b4..128cb7ba55 100644\n--- a/blame.c\n+++ b/blame.c\n@@ -2764,7 +2764,6 @@ void init_scoreboard(struct blame_scoreboard *sb)\n }\n \n void setup_scoreboard(struct blame_scoreboard *sb,\n-\t\t const char *path,\n \t\t struct blame_origin **orig)\n {\n \tconst char *final_commit_name = NULL;\n@@ -2803,7 +2802,7 @@ void setup_scoreboard(struct blame_scoreboard *sb,\n \t\tsetup_work_tree();\n \t\tsb->final = fake_working_tree_commit(sb->repo,\n \t\t\t\t\t\t &sb->revs->diffopt,\n-\t\t\t\t\t\t path, sb->contents_from);\n+\t\t\t\t\t\t sb->path, sb->contents_from);\n \t\tadd_pending_object(sb->revs, &(sb->final->object), \":\");\n \t}\n \n@@ -2846,12 +2845,12 @@ void setup_scoreboard(struct blame_scoreboard *sb,\n \t\tsb->final_buf_size = o->file.size;\n \t}\n \telse {\n-\t\to = get_origin(sb->final, path);\n+\t\to = get_origin(sb->final, sb->path);\n \t\tif (fill_blob_sha1_and_mode(sb->repo, o))\n-\t\t\tdie(_(\"no such path %s in %s\"), path, final_commit_name);\n+\t\t\tdie(_(\"no such path %s in %s\"), sb->path, final_commit_name);\n \n \t\tif (sb->revs->diffopt.flags.allow_textconv &&\n-\t\t textconv_object(sb->repo, path, o->mode, &o->blob_oid, 1, (char **) &sb->final_buf,\n+\t\t textconv_object(sb->repo, sb->path, o->mode, &o->blob_oid, 1, (char **) &sb->final_buf,\n \t\t\t\t &sb->final_buf_size))\n \t\t\t;\n \t\telse\n@@ -2861,7 +2860,7 @@ void setup_scoreboard(struct blame_scoreboard *sb,\n \t\tif (!sb->final_buf)\n \t\t\tdie(_(\"cannot read blob %s for path %s\"),\n \t\t\t oid_to_hex(&o->blob_oid),\n-\t\t\t path);\n+\t\t\t sb->path);\n \t}\n \tsb->num_read_blob++;\n \tprepare_lines(sb);\ndiff --git a/blame.h b/blame.h\nindex b6bbee4147..e8c185c8ef 100644\n--- a/blame.h\n+++ b/blame.h\n@@ -181,7 +181,6 @@ const char *blame_nth_line(struct blame_scoreboard *sb, long lno);\n \n void init_scoreboard(struct blame_scoreboard *sb);\n void setup_scoreboard(struct blame_scoreboard *sb,\n-\t\t const char *path,\n \t\t struct blame_origin **orig);\n void setup_blame_bloom_data(struct blame_scoreboard *sb,\n \t\t\t const char *path);\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex f2e528fcbc..5d8f4c4599 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -1085,7 +1085,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n \tbuild_ignorelist(&sb, &ignore_revs_file_list, &ignore_rev_list);\n \tstring_list_clear(&ignore_revs_file_list, 0);\n \tstring_list_clear(&ignore_rev_list, 0);\n-\tsetup_scoreboard(&sb, path, &o);\n+\tsetup_scoreboard(&sb, &o);\n \n \t/*\n \t * Changed-path Bloom filters are disabled when looking\n-- \ngitgitgadget\n\n"}]}