{"thread":{"id":"53545","subject":"[PATCH 0/4] Selectively show only blamed limes","startedAt":"2020-05-25T21:58:03Z","lastAt":"2020-05-31T16:28:37Z","messageCount":14,"participants":["Philip Oakley","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"398557","messageId":"20200525215751.1735-1-philipoakley@iee.email","threadId":"53545","inReplyTo":null,"subject":"[PATCH 0/4] Selectively show only blamed limes","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2020-05-25T21:57:47Z","receivedAt":"2020-05-25T21:58:03Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"While `git blame` is able to select interesting line regions of a\nfile, it's not easy to just display blame lines since a recent date,\nespecially for large files.\n\nThe recent discussion on CMake[1] notes the issue of ensuring any CMake\nscheme kept up with changes to the fairly long Makefile. This short\nseries first clarifies the marking of 'boundary' commits outwith the\nrevision range or time range, then adds a `--blame-only` option to\nsupress non-blame (boundary) commit lines, with tests, and finally\nadds tests for the long standing `-b` option that simply blanks out\nthe oid, but still displays the whole file.\n\nPhilip\n\n\ncc: Sibi Siddharthan <sibisiddharthan.github@gmail.com>\n[1] https://lore.kernel.org/git/pull.614.v2.git.1589302254.gitgitgadget@gmail.com/\n\nPhilip Oakley (4):\n  doc: blame: show the boundary commit '^' caret mark\n  blame: add option to show only blamed commits `--blame-only`\n  blame: do not show boundary commits, only those blamed\n  blame: test the -b option, use blank oid for boundary commits.\n\n Documentation/blame-options.txt |  4 ++++\n Documentation/git-blame.txt     |  8 ++++----\n builtin/blame.c                 |  3 +++\n t/t8002-blame.sh                | 19 +++++++++++++++++++\n 4 files changed, 30 insertions(+), 4 deletions(-)\n\n-- \n2.26.2.windows.1.13.g9dddff6983\n\n"},{"id":"398558","messageId":"20200525215751.1735-2-philipoakley@iee.email","threadId":"53545","inReplyTo":"20200525215751.1735-1-philipoakley@iee.email","subject":"[PATCH 1/4] doc: blame: show the boundary commit '^' caret mark","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2020-05-25T21:57:48Z","receivedAt":"2020-05-25T21:58:03Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"Make the boundary commits' ^ caret findable in the documentation.\n\nSigned-off-by: Philip Oakley <philipoakley@iee.email>\n---\n Documentation/git-blame.txt | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt\nindex 7e81541996..02f9ad6fe9 100644\n--- a/Documentation/git-blame.txt\n+++ b/Documentation/git-blame.txt\n@@ -87,8 +87,8 @@ include::blame-options.txt[]\n \n --abbrev=<n>::\n \tInstead of using the default 7+1 hexadecimal digits as the\n-\tabbreviated object name, use <n>+1 digits. Note that 1 column\n-\tis used for a caret to mark the boundary commit.\n+\tabbreviated commit object name, use <n>+1 digits. Note that 1 column\n+\tis used for a caret (`^`) to mark boundary commits.\n \n \n THE PORCELAIN FORMAT\n@@ -164,7 +164,7 @@ When revision range specifiers are used to limit the annotation,\n lines that have not changed since the range boundary (either the\n commit v2.6.18 or the most recent commit that is more than 3\n weeks old in the above example) are blamed for that range\n-boundary commit.\n+boundary commit (preceded by `^` if shown).\n \n A particularly useful way is to see if an added file has lines\n created by copy-and-paste from existing files.  Sometimes this\n-- \n2.26.2.windows.1.13.g9dddff6983\n\n"},{"id":"398559","messageId":"20200525215751.1735-3-philipoakley@iee.email","threadId":"53545","inReplyTo":"20200525215751.1735-1-philipoakley@iee.email","subject":"[PATCH 2/4] blame: add option to show only blamed commits `--blame-only`","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2020-05-25T21:57:49Z","receivedAt":"2020-05-25T21:58:04Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"For large files the will be many lines which are not blamed with the\nrevision range or date period which clutter the output.\n\nAdd an option to suppress the display of boundary commits lines, not\njust the object id (`-b` option).\n\nThis is non-functional at this stage. The next commit will add\nfunctionality, tests, and documenation.\n\nSigned-off-by: Philip Oakley <philipoakley@iee.email>\n---\n Documentation/blame-options.txt | 4 ++++\n Documentation/git-blame.txt     | 2 +-\n builtin/blame.c                 | 2 ++\n 3 files changed, 7 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt\nindex 5d122db6e9..b4b27033a6 100644\n--- a/Documentation/blame-options.txt\n+++ b/Documentation/blame-options.txt\n@@ -2,6 +2,10 @@\n \tShow blank SHA-1 for boundary commits.  This can also\n \tbe controlled via the `blame.blankboundary` config option.\n \n+--blame-only::\n+\tDo not show any boundary commit lines created by revision range\n+\tspecifiers. Only show blamed lines.\n+\n --root::\n \tDo not treat root commits as boundaries.  This can also be\n \tcontrolled via the `blame.showRoot` config option.\ndiff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt\nindex 02f9ad6fe9..74b6344c7b 100644\n--- a/Documentation/git-blame.txt\n+++ b/Documentation/git-blame.txt\n@@ -9,7 +9,7 @@ SYNOPSIS\n --------\n [verse]\n 'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w] [--incremental]\n-\t    [-L <range>] [-S <revs-file>] [-M] [-C] [-C] [-C] [--since=<date>]\n+\t    [-L <range>] [-S <revs-file>] [-M] [-C] [-C] [-C] [--since=<date>] [--blame-only]\n \t    [--ignore-rev <rev>] [--ignore-revs-file <file>]\n \t    [--progress] [--abbrev=<n>] [<rev> | --contents <file> | --reverse <rev>..<rev>]\n \t    [--] <file>\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex bf1cecdf3f..b699c777c4 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -45,6 +45,7 @@ static int max_score_digits;\n static int show_root;\n static int reverse;\n static int blank_boundary;\n+static int blame_only;\n static int incremental;\n static int xdl_opts;\n static int abbrev = -1;\n@@ -843,6 +844,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n \tconst struct option options[] = {\n \t\tOPT_BOOL(0, \"incremental\", &incremental, N_(\"Show blame entries as we find them, incrementally\")),\n \t\tOPT_BOOL('b', NULL, &blank_boundary, N_(\"Show blank SHA-1 for boundary commits (Default: off)\")),\n+\t\tOPT_BOOL(0, \"blame-only\", &blame_only, N_(\"Only show blamed commits (Default: off)\")),\n \t\tOPT_BOOL(0, \"root\", &show_root, N_(\"Do not treat root commits as boundaries (Default: off)\")),\n \t\tOPT_BOOL(0, \"show-stats\", &show_stats, N_(\"Show work cost statistics\")),\n \t\tOPT_BOOL(0, \"progress\", &show_progress, N_(\"Force progress reporting\")),\n-- \n2.26.2.windows.1.13.g9dddff6983\n\n"},{"id":"398560","messageId":"20200525215751.1735-4-philipoakley@iee.email","threadId":"53545","inReplyTo":"20200525215751.1735-1-philipoakley@iee.email","subject":"[PATCH 3/4] blame: do not show boundary commits, only those blamed","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2020-05-25T21:57:50Z","receivedAt":"2020-05-25T21:58:06Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"Make the option functional and add tests\n\nSigned-off-by: Philip Oakley <philipoakley@iee.email>\n---\n builtin/blame.c  | 1 +\n t/t8002-blame.sh | 7 +++++++\n 2 files changed, 8 insertions(+)\n\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex b699c777c4..61e5a7ff44 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -475,6 +475,7 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int\n \t\t\tfputs(color, stdout);\n \n \t\tif (suspect->commit->object.flags & UNINTERESTING) {\n+\t\t\tif (blame_only) continue;\n \t\t\tif (blank_boundary)\n \t\t\t\tmemset(hex, ' ', length);\n \t\t\telse if (!(opt & OUTPUT_ANNOTATE_COMPAT)) {\ndiff --git a/t/t8002-blame.sh b/t/t8002-blame.sh\nindex eea048e52c..d4877c7c54 100755\n--- a/t/t8002-blame.sh\n+++ b/t/t8002-blame.sh\n@@ -122,4 +122,11 @@ test_expect_success '--exclude-promisor-objects does not BUG-crash' '\n \ttest_must_fail git blame --exclude-promisor-objects one\n '\n \n+test_expect_success 'test --blame-only, exclude boundary commits' '\n+\tgit blame --blame-only branch1.. -- file >actual &&\n+\tgit blame branch1.. -- file >full &&\n+\tsed -e \"/^\\^/d\" full >expected &&\n+\ttest_cmp expected actual\n+'\n+\n test_done\n-- \n2.26.2.windows.1.13.g9dddff6983\n\n"},{"id":"398561","messageId":"20200525215751.1735-5-philipoakley@iee.email","threadId":"53545","inReplyTo":"20200525215751.1735-1-philipoakley@iee.email","subject":"[PATCH 4/4] blame: test the -b option, use blank oid for boundary commits.","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2020-05-25T21:57:51Z","receivedAt":"2020-05-25T21:58:07Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"4c10a5caa7 (blame: -b (blame.blankboundary) and --root (blame.showroot),\n2006-12-18) introduced the -b option. Add a test.\n\nThe sed script removes the last hex digit from boundary commit oids\n'^hexx msg' -> '^hex  msg' until all leading hex's are gone, finally\nremoving the boundary commit marker.\n\nSigned-off-by: Philip Oakley <philipoakley@iee.email>\n---\n t/t8002-blame.sh | 12 ++++++++++++\n 1 file changed, 12 insertions(+)\n\ndiff --git a/t/t8002-blame.sh b/t/t8002-blame.sh\nindex d4877c7c54..de0f81abe9 100755\n--- a/t/t8002-blame.sh\n+++ b/t/t8002-blame.sh\n@@ -129,4 +129,16 @@ test_expect_success 'test --blame-only, exclude boundary commits' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'test -b option, blank oid for boundary commits' '\n+\tgit blame -b branch1.. -- file >actual &&\n+\tgit blame branch1.. -- file >full &&\n+\tsed -e \"/^\\^/{\n+\t\t:loop;\n+\t\ts/^\\(\\^[0-9a-f]*\\)[0-9a-f] \\(.*\\)/\\1  \\2/g;\n+\t\ttloop;\n+\t\ts/^\\^/ /;\n+\t}\" full >expected &&\n+\ttest_cmp expected actual\n+'\n+\n test_done\n-- \n2.26.2.windows.1.13.g9dddff6983\n\n"},{"id":"398614","messageId":"20200527072318.GA4006199@coredump.intra.peff.net","threadId":"53545","inReplyTo":"20200525215751.1735-1-philipoakley@iee.email","subject":"Re: [PATCH 0/4] Selectively show only blamed limes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-05-27T07:23:18Z","receivedAt":"2020-05-27T07:23:22Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, May 25, 2020 at 10:57:47PM +0100, Philip Oakley wrote:\n\n> While `git blame` is able to select interesting line regions of a\n> file, it's not easy to just display blame lines since a recent date,\n> especially for large files.\n\nI'm not convinced this is a useful thing to do in general. Lines don't\nalways stand on their own, and you'd lack context for deciphering them.\nSo a real example from \"git blame -b --since=1.year.ago Makefile\", for\nexample (pardon the long lines):\n\n  3a94cb31d52 (Johannes Schindelin        2019-07-29 13:08:16 -0700 2734)              -e 's|@@PROG@@|$(patsubst test-%,t/helper/test-%$(X),$(@F))$(patsubst git%,$(X),$(filter $(@F),$(BINDIR_PROGRAMS_NEED_X)))|' < $< > $@ && \\\n\nbut in context it looks like:\n\n              (Junio C Hamano             2019-05-19 16:46:42 +0900 2730) bin-wrappers/%: wrap-for-bin.sh\n              (Junio C Hamano             2019-05-19 16:46:42 +0900 2731)         @mkdir -p bin-wrappers\n              (Junio C Hamano             2019-05-19 16:46:42 +0900 2732)         $(QUIET_GEN)sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n              (Junio C Hamano             2019-05-19 16:46:42 +0900 2733)              -e 's|@@BUILD_DIR@@|$(shell pwd)|' \\\n  3a94cb31d52 (Johannes Schindelin        2019-07-29 13:08:16 -0700 2734)              -e 's|@@PROG@@|$(patsubst test-%,t/helper/test-%$(X),$(@F))$(patsubst git%,$(X),$(filter $(@F),$(BINDIR_PROGRAMS_NEED_X)))|' < $< > $@ && \\\n              (Junio C Hamano             2019-05-19 16:46:42 +0900 2735)         chmod +x $@\n\nOf course there are cases where it might be useful. But you can already\ndo:\n\n  git blame --since=1.year.ago Makefile | grep -v ^^\n\nI'm not totally opposed to the feature. I'm just really struggling to\nsee how it would be generally useful. For special cases where you're\njust counting up lines, you'd be more likely to post-process the result\nanyway, at which point --line-porcelain is often easier to work with.\n\n> Philip Oakley (4):\n>   doc: blame: show the boundary commit '^' caret mark\n\nThis doc fix seems worthwhile on its own, though.\n\n>   blame: add option to show only blamed commits `--blame-only`\n>   blame: do not show boundary commits, only those blamed\n\nIf we do go this direction, these really ought to be a single commit.\n\n>   blame: test the -b option, use blank oid for boundary commits.\n\nThis one might be worth doing independently, too.\n\n-Peff\n"},{"id":"398615","messageId":"20200527073020.GB4006199@coredump.intra.peff.net","threadId":"53545","inReplyTo":"20200525215751.1735-5-philipoakley@iee.email","subject":"Re: [PATCH 4/4] blame: test the -b option, use blank oid for boundary commits.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-05-27T07:30:20Z","receivedAt":"2020-05-27T07:30:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, May 25, 2020 at 10:57:51PM +0100, Philip Oakley wrote:\n\n> The sed script removes the last hex digit from boundary commit oids\n> '^hexx msg' -> '^hex  msg' until all leading hex's are gone, finally\n> removing the boundary commit marker.\n\nThanks for documenting this, as the sed was rather hard to read:\n\n> +test_expect_success 'test -b option, blank oid for boundary commits' '\n> +\tgit blame -b branch1.. -- file >actual &&\n> +\tgit blame branch1.. -- file >full &&\n> +\tsed -e \"/^\\^/{\n> +\t\t:loop;\n> +\t\ts/^\\(\\^[0-9a-f]*\\)[0-9a-f] \\(.*\\)/\\1  \\2/g;\n> +\t\ttloop;\n> +\t\ts/^\\^/ /;\n> +\t}\" full >expected &&\n\nI wonder if we can make it simpler.\n\nIn perl I'd probably just replace the whole string with the equivalent\nnumber of spaces, like:\n\n  perl -pe 's/^\\^\\S+/\" \" x length($&)/e'\n\nbut I suppose some would consider that pretty magical, too. It might be\nsimpler still to just avoid testing leading whitespace:\n\n   sed 's/^\\^[0-9a-f]* *//' <full >expected &&\n   sed 's/^ *//' <actual >actual.stripped &&\n   test_cmp expected actual.stripped\n\nbut perhaps the indentation is a useful part of what we're testing.\n\n-Peff\n"},{"id":"398622","messageId":"0da84168-3d81-fb14-32ad-010ef2436958@iee.email","threadId":"53545","inReplyTo":"20200527072318.GA4006199@coredump.intra.peff.net","subject":"Re: [PATCH 0/4] Selectively show only blamed limes","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2020-05-27T10:24:54Z","receivedAt":"2020-05-27T10:24:57Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"Hi Peff\n\nOn 27/05/2020 08:23, Jeff King wrote:\n> On Mon, May 25, 2020 at 10:57:47PM +0100, Philip Oakley wrote:\n>\n>> While `git blame` is able to select interesting line regions of a\n>> file, it's not easy to just display blame lines since a recent date,\n>> especially for large files.\n> I'm not convinced this is a useful thing to do in general. Lines don't\n> always stand on their own, and you'd lack context for deciphering them.\n> So a real example from \"git blame -b --since=1.year.ago Makefile\", for\n> example (pardon the long lines):\n\nMy contention is that there is a lot of effort needed _beforehand_ to\nreach that point. We've already had to page down 2700 line to get there,\nand then had to carefully select the differing before and after context\nlines.\n\nThe use of the --blame-only (especially with broad date range --since)\nis to quickly narrow focus with the rather large file to the few areas\nof concern (probably related why the -b was introduced initially?)\n\n>\n>   3a94cb31d52 (Johannes Schindelin        2019-07-29 13:08:16 -0700 2734)              -e 's|@@PROG@@|$(patsubst test-%,t/helper/test-%$(X),$(@F))$(patsubst git%,$(X),$(filter $(@F),$(BINDIR_PROGRAMS_NEED_X)))|' < $< > $@ && \\\n>\n> but in context it looks like:\n>\n>               (Junio C Hamano             2019-05-19 16:46:42 +0900 2730) bin-wrappers/%: wrap-for-bin.sh\n>               (Junio C Hamano             2019-05-19 16:46:42 +0900 2731)         @mkdir -p bin-wrappers\n>               (Junio C Hamano             2019-05-19 16:46:42 +0900 2732)         $(QUIET_GEN)sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \\\n>               (Junio C Hamano             2019-05-19 16:46:42 +0900 2733)              -e 's|@@BUILD_DIR@@|$(shell pwd)|' \\\n>   3a94cb31d52 (Johannes Schindelin        2019-07-29 13:08:16 -0700 2734)              -e 's|@@PROG@@|$(patsubst test-%,t/helper/test-%$(X),$(@F))$(patsubst git%,$(X),$(filter $(@F),$(BINDIR_PROGRAMS_NEED_X)))|' < $< > $@ && \\\n>               (Junio C Hamano             2019-05-19 16:46:42 +0900 2735)         chmod +x $@\n>\n> Of course there are cases where it might be useful. But you can already\n> do:\n>\n>   git blame --since=1.year.ago Makefile | grep -v ^^\n>\n> I'm not totally opposed to the feature. I'm just really struggling to\n> see how it would be generally useful. For special cases where you're\n> just counting up lines, you'd be more likely to post-process the result\n> anyway, at which point --line-porcelain is often easier to work with.\nI'm also thinking that use of Git has expanded well beyond its core\nLinux VCS roots, so not all users are ready for the grep regex [1].\nHence the expansion of the `-b` to truly filter only the blamed commits.\n\n\n>\n>> Philip Oakley (4):\n>>   doc: blame: show the boundary commit '^' caret mark\n> This doc fix seems worthwhile on its own, though.\n>\n>>   blame: add option to show only blamed commits `--blame-only`\n>>   blame: do not show boundary commits, only those blamed\n> If we do go this direction, these really ought to be a single commit.\n>\n>>   blame: test the -b option, use blank oid for boundary commits.\n> This one might be worth doing independently, too.\nOK. The final test look a while to come up with, so it ended up last.\n\n>\n> -Peff\nThanks\nPhilip\n\n[1] https://stackoverflow.com/a/42540014/717355 \"Git Blame see changes\nafter a certain date\" (longer regex for the same effect)\n"},{"id":"398624","messageId":"aabb67b8-b342-aab9-6213-f7c2d5758719@iee.email","threadId":"53545","inReplyTo":"20200527073020.GB4006199@coredump.intra.peff.net","subject":"Re: [PATCH 4/4] blame: test the -b option, use blank oid for boundary commits.","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2020-05-27T10:52:19Z","receivedAt":"2020-05-27T10:52:22Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"On 27/05/2020 08:30, Jeff King wrote:\n> On Mon, May 25, 2020 at 10:57:51PM +0100, Philip Oakley wrote:\n>\n>> The sed script removes the last hex digit from boundary commit oids\n>> '^hexx msg' -> '^hex  msg' until all leading hex's are gone, finally\n>> removing the boundary commit marker.\n> Thanks for documenting this, as the sed was rather hard to read:\nIt was hard to write as well;-) lots of dead ends in the sed language.\n>\n>> +test_expect_success 'test -b option, blank oid for boundary commits' '\n>> +\tgit blame -b branch1.. -- file >actual &&\n>> +\tgit blame branch1.. -- file >full &&\n>> +\tsed -e \"/^\\^/{\n>> +\t\t:loop;\n>> +\t\ts/^\\(\\^[0-9a-f]*\\)[0-9a-f] \\(.*\\)/\\1  \\2/g;\n>> +\t\ttloop;\n>> +\t\ts/^\\^/ /;\n>> +\t}\" full >expected &&\n> I wonder if we can make it simpler.\n>\n> In perl I'd probably just replace the whole string with the equivalent\n> number of spaces, like:\n>\n>   perl -pe 's/^\\^\\S+/\" \" x length($&)/e'\nI'm not a perl person, so I thought sed might be an easy target but ...\nit wasn't.\n>\n> but I suppose some would consider that pretty magical, too. \nI'd need to look it up!\n\n> It might be\n> simpler still to just avoid testing leading whitespace:\n>\n>    sed 's/^\\^[0-9a-f]* *//' <full >expected &&\n>    sed 's/^ *//' <actual >actual.stripped &&\n>    test_cmp expected actual.stripped\n>\n> but perhaps the indentation is a useful part of what we're testing.\n\nThe code does take care to exactly match indentation, so I felt that the\nindentation should be tested.\n\nPhilip\n\n"},{"id":"398751","messageId":"20200528164533.GA1223132@coredump.intra.peff.net","threadId":"53545","inReplyTo":"0da84168-3d81-fb14-32ad-010ef2436958@iee.email","subject":"Re: [PATCH 0/4] Selectively show only blamed limes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-05-28T16:45:33Z","receivedAt":"2020-05-28T16:45:36Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, May 27, 2020 at 11:24:54AM +0100, Philip Oakley wrote:\n\n> > I'm not convinced this is a useful thing to do in general. Lines don't\n> > always stand on their own, and you'd lack context for deciphering them.\n> > So a real example from \"git blame -b --since=1.year.ago Makefile\", for\n> > example (pardon the long lines):\n> \n> My contention is that there is a lot of effort needed _beforehand_ to\n> reach that point. We've already had to page down 2700 line to get there,\n> and then had to carefully select the differing before and after context\n> lines.\n\nI think:\n\n  git blame -b --since=1.year.ago ... | less \"+/^[^ ]\"\n\nworks pretty well to get you to the right spot, with context (and then\n\"/<enter>\" within less takes you to the next line).\n\nBut that's also pretty magical.\n\n> The use of the --blame-only (especially with broad date range --since)\n> is to quickly narrow focus with the rather large file to the few areas\n> of concern (probably related why the -b was introduced initially?)\n\nPerhaps. I think when you get an out-of-context chunk, though, it's\ntricky to take the next step.\n\nI'm still skeptical this will turn out to be all that useful in\npractice. But it really isn't much code to carry, so I don't have any\nreal objection.\n\n-Peff\n"},{"id":"398773","messageId":"xmqq4krz52vb.fsf@gitster.c.googlers.com","threadId":"53545","inReplyTo":"20200528164533.GA1223132@coredump.intra.peff.net","subject":"Re: [PATCH 0/4] Selectively show only blamed limes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-05-28T18:30:16Z","receivedAt":"2020-05-28T18:30:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Wed, May 27, 2020 at 11:24:54AM +0100, Philip Oakley wrote:\n>\n>> > I'm not convinced this is a useful thing to do in general. Lines don't\n>> > always stand on their own, and you'd lack context for deciphering them.\n>> > So a real example from \"git blame -b --since=1.year.ago Makefile\", for\n>> > example (pardon the long lines):\n>> \n>> My contention is that there is a lot of effort needed _beforehand_ to\n>> reach that point. We've already had to page down 2700 line to get there,\n>> and then had to carefully select the differing before and after context\n>> lines.\n>\n> I think:\n>\n>   git blame -b --since=1.year.ago ... | less \"+/^[^ ]\"\n>\n> works pretty well to get you to the right spot, with context (and then\n> \"/<enter>\" within less takes you to the next line).\n\nYup.  I think this is mostly about knowing how to use the tools that\nalready exist.  I am not interested in this new \"feature\", either.\n\nThanks.\n"},{"id":"398933","messageId":"58ce3749-d3ef-a9fe-be94-d5373cf70f15@iee.email","threadId":"53545","inReplyTo":"xmqq4krz52vb.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 0/4] Selectively show only blamed limes","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2020-05-30T16:18:18Z","receivedAt":"2020-05-30T16:18:23Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"On 28/05/2020 19:30, Junio C Hamano wrote:\n> Jeff King <peff@peff.net> writes:\n>\n>> On Wed, May 27, 2020 at 11:24:54AM +0100, Philip Oakley wrote:\n>>\n>>>> I'm not convinced this is a useful thing to do in general. Lines don't\n>>>> always stand on their own, and you'd lack context for deciphering them.\n>>>> So a real example from \"git blame -b --since=1.year.ago Makefile\", for\n>>>> example (pardon the long lines):\n>>> My contention is that there is a lot of effort needed _beforehand_ to\n>>> reach that point. We've already had to page down 2700 line to get there,\n>>> and then had to carefully select the differing before and after context\n>>> lines.\n>> I think:\n>>\n>>   git blame -b --since=1.year.ago ... | less \"+/^[^ ]\"\n>>\n>> works pretty well to get you to the right spot, with context (and then\n>> \"/<enter>\" within less takes you to the next line).\n> Yup.  I think this is mostly about knowing how to use the tools that\n> already exist.  I am not interested in this new \"feature\", either.\n>\n> Thanks.\nIs the presumption of knowing the *nix tools something that should\ndocumented, or is that an over simplification?\n\nI'll trim this series to just the '^' documentation change and the '-b'\noption test (after v2.27.0)\n\nPhilip\n"},{"id":"398951","messageId":"xmqqtuzww0si.fsf@gitster.c.googlers.com","threadId":"53545","inReplyTo":"20200528164533.GA1223132@coredump.intra.peff.net","subject":"Re: [PATCH 0/4] Selectively show only blamed limes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-05-31T16:02:05Z","receivedAt":"2020-05-31T16:02:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> I think:\n>\n>   git blame -b --since=1.year.ago ... | less \"+/^[^ ]\"\n>\n> works pretty well to get you to the right spot, with context (and then\n> \"/<enter>\" within less takes you to the next line).\n>\n> But that's also pretty magical.\n\nInstead of teaching each and every tool (like \"git blame\", \"diff\",\n\"grep\", etc.) a special trick to remove the lines from its output,\nit may make a lot more sense to add a feature to pagers that lets\nthe users selectively \"hide\" the matching lines given a pattern.\nThen when viewing \"git blame -b\" output, you can first declare\n\"lines that begin with a whitespace are sometimes uninteresting\" and\nthen tell your pager \"now unshow uninteresting lines\", \"the same, but\nshow 3 lines at the boundary of runs of uninteresting lines\", \"now\nshow everything\", etc.\n\nThat's another reason why I do not particularly find the proposed\nfeature interesting.\n\n"},{"id":"398953","messageId":"xmqqlfl8vzko.fsf@gitster.c.googlers.com","threadId":"53545","inReplyTo":"xmqqtuzww0si.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 0/4] Selectively show only blamed limes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-05-31T16:28:23Z","receivedAt":"2020-05-31T16:28:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Instead of teaching each and every tool (like \"git blame\", \"diff\",\n> \"grep\", etc.) a special trick to remove the lines from its output,\n> it may make a lot more sense to add a feature to pagers that lets\n> the users selectively \"hide\" the matching lines given a pattern.\n> Then when viewing \"git blame -b\" output, you can first declare\n> \"lines that begin with a whitespace are sometimes uninteresting\" and\n> then tell your pager \"now unshow uninteresting lines\", \"the same, but\n> show 3 lines at the boundary of runs of uninteresting lines\", \"now\n> show everything\", etc.\n\nI forgot to say what I wanted to touch a bit more.  Such a feature\nin pager is readily reusable for other types of payload.  \"Lines\nthat begin with a whitespace are sometimes uninteresting\" can also\nbe used for output from \"diff -u -U<largenumber>\", and \"lines that\ndo not match '^[^:]*:[0-9]*:' are not interesting\" can be used for\noutput from \"grep n -C\".  That would be a lot more sensible than\ntweaking each and every tool that generates output that sometimes\nneeds to be read in full and sometimes needs to be shown with less\ninteresting parts ellided.\n\n"}]}