{"thread":{"id":"57658","subject":"Re: [PATCH v2] blame: report correct number of lines in progress when using ranges","startedAt":"2022-04-04T21:23:13Z","lastAt":"2022-04-08T18:16:19Z","messageCount":11,"participants":["Edmundo Carmona Antoranz","Junio C Hamano","Bagas Sanjaya","Ævar Arnfjörð Bjarmason","Philip Oakley"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"453048","messageId":"CAOc6etaY8enMF0nmhaqA2+Oi6JmYFY-bPqsUFTXE=K8aFoXDnQ@mail.gmail.com","threadId":"57658","inReplyTo":"20220404182129.33992-1-eantoranz@gmail.com","subject":"Re: [PATCH v2] blame: report correct number of lines in progress when using ranges","fromName":"Edmundo Carmona Antoranz","fromEmail":"eantoranz@gmail.com","sentAt":"2022-04-04T18:25:59Z","receivedAt":"2022-04-04T21:23:13Z","isPatch":true,"sender":{"key":"eantoranz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1491018?v=4"},"body":"On Mon, Apr 4, 2022 at 8:21 PM Edmundo Carmona Antoranz\n<eantoranz@gmail.com> wrote:\n>\n> When using ranges, use their sizes as the limit for progress\n> instead of the size of the full file.\n>\n>  '\n>\n> +test_expect_success 'blame progress on a full file' '\n> +       cat >progress.txt <<-\\EOF &&\n> +       a simple test file\n> +\n> +       no relevant content is expected here\n> +\n> +       If the file is too short, we cannot test ranges\n> +\n> +       EOF\n> +       git add progress.txt &&\n> +       git commit -m \"add a file for testing progress\" &&\n\nI wonder if the preceding section should be kept in a\nseparate 'setup test'?\n\n> +       GIT_PROGRESS_DELAY=0 \\\n> +       git blame --progress progress.txt > /dev/null 2> full_progress.txt &&\n> +       grep \"Blaming lines: 100% (6/6), done.\" full_progress.txt\n> +'\n"},{"id":"453051","messageId":"20220404182129.33992-1-eantoranz@gmail.com","threadId":"57658","inReplyTo":null,"subject":"[PATCH v2] blame: report correct number of lines in progress when using ranges","fromName":"Edmundo Carmona Antoranz","fromEmail":"eantoranz@gmail.com","sentAt":"2022-04-04T18:21:28Z","receivedAt":"2022-04-04T21:23:13Z","isPatch":true,"sender":{"key":"eantoranz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1491018?v=4"},"body":"When using ranges, use their sizes as the limit for progress\ninstead of the size of the full file.\n\nBefore:\n$ git blame --progress builtin/blame.c > /dev/null\nBlaming lines: 100% (1210/1210), done.\n$ git blame --progress -L 100,120 -L 200,300 builtin/blame.c > /dev/null\nBlaming lines:  10% (122/1210), done.\n$\n\nAfter:\n$ ./git blame --progress builtin/blame.c > /dev/null\nBlaming lines: 100% (1210/1210), done.\n$ ./git blame --progress -L 100,120 -L 200,300 builtin/blame.c > /dev/null\nBlaming lines: 100% (122/122), done.\n$\n\nSigned-off-by: Edmundo Carmona Antoranz <eantoranz@gmail.com>\n---\n builtin/blame.c  |  6 +++++-\n t/t8002-blame.sh | 28 ++++++++++++++++++++++++++++\n 2 files changed, 33 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 8d15b68afc..e33372c56b 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -898,6 +898,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n \tunsigned int range_i;\n \tlong anchor;\n \tconst int hexsz = the_hash_algo->hexsz;\n+\tlong num_lines = 0;\n \n \tsetup_default_color_by_age();\n \tgit_config(git_blame_config, &output_option);\n@@ -1129,7 +1130,10 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n \tfor (range_i = ranges.nr; range_i > 0; --range_i) {\n \t\tconst struct range *r = &ranges.ranges[range_i - 1];\n \t\tent = blame_entry_prepend(ent, r->start, r->end, o);\n+\t\tnum_lines += (r->end - r->start);\n \t}\n+\tif (!num_lines)\n+\t\tnum_lines = sb.num_lines;\n \n \to->suspects = ent;\n \tprio_queue_put(&sb.commits, o->commit);\n@@ -1158,7 +1162,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n \tsb.found_guilty_entry = &found_guilty_entry;\n \tsb.found_guilty_entry_data = &pi;\n \tif (show_progress)\n-\t\tpi.progress = start_delayed_progress(_(\"Blaming lines\"), sb.num_lines);\n+\t\tpi.progress = start_delayed_progress(_(\"Blaming lines\"), num_lines);\n \n \tassign_blame(&sb, opt);\n \ndiff --git a/t/t8002-blame.sh b/t/t8002-blame.sh\nindex ee4fdd8f18..151a6fddfd 100755\n--- a/t/t8002-blame.sh\n+++ b/t/t8002-blame.sh\n@@ -129,6 +129,34 @@ 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 'blame progress on a full file' '\n+\tcat >progress.txt <<-\\EOF &&\n+\ta simple test file\n+\n+\tno relevant content is expected here\n+\n+\tIf the file is too short, we cannot test ranges\n+\n+\tEOF\n+\tgit add progress.txt &&\n+\tgit commit -m \"add a file for testing progress\" &&\n+\tGIT_PROGRESS_DELAY=0 \\\n+\tgit blame --progress progress.txt > /dev/null 2> full_progress.txt &&\n+\tgrep \"Blaming lines: 100% (6/6), done.\" full_progress.txt\n+'\n+\n+test_expect_success 'blame progress on a single range' '\n+\tGIT_PROGRESS_DELAY=0 \\\n+\tgit blame --progress -L 2,5 progress.txt > /dev/null 2> range_progress.txt &&\n+\tgrep \"Blaming lines: 100% (4/4), done.\" range_progress.txt\n+'\n+\n+test_expect_success 'blame progress on multiple ranges' '\n+\tGIT_PROGRESS_DELAY=0 \\\n+\tgit blame --progress -L 1,2 -L 4,6 progress.txt > /dev/null 2> range_progress.txt &&\n+\tgrep \"Blaming lines: 100% (5/5), done.\" range_progress.txt\n+'\n+\n test_expect_success 'blame with uncommitted edits in partial clone does not crash' '\n \tgit init server &&\n \techo foo >server/file.txt &&\n-- \n2.35.1\n\n"},{"id":"453052","messageId":"xmqqy20kprth.fsf@gitster.g","threadId":"57658","inReplyTo":"CAOc6etaY8enMF0nmhaqA2+Oi6JmYFY-bPqsUFTXE=K8aFoXDnQ@mail.gmail.com","subject":"Re: [PATCH v2] blame: report correct number of lines in progress when using ranges","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-04-04T19:32:58Z","receivedAt":"2022-04-04T21:23:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Edmundo Carmona Antoranz <eantoranz@gmail.com> writes:\n\n> On Mon, Apr 4, 2022 at 8:21 PM Edmundo Carmona Antoranz\n> <eantoranz@gmail.com> wrote:\n>>\n>> When using ranges, use their sizes as the limit for progress\n>> instead of the size of the full file.\n>>\n>>  '\n>>\n>> +test_expect_success 'blame progress on a full file' '\n>> +       cat >progress.txt <<-\\EOF &&\n>> +       a simple test file\n>> +\n>> +       no relevant content is expected here\n>> +\n>> +       If the file is too short, we cannot test ranges\n>> +\n>> +       EOF\n>> +       git add progress.txt &&\n>> +       git commit -m \"add a file for testing progress\" &&\n>\n> I wonder if the preceding section should be kept in a\n> separate 'setup test'?\n\nI actually wonder why we need a *new* test file to run this test,\ninstead of reusing what we already use in the existing test.\n\n>\n>> +       GIT_PROGRESS_DELAY=0 \\\n>> +       git blame --progress progress.txt > /dev/null 2> full_progress.txt &&\n\nStyle:\n\n\tgit blame --progress progress.txt >/dev/null 2>full_progress.txt &&\n"},{"id":"453099","messageId":"b2f5d6af-8da1-3a3a-cc21-848c14a8fb98@gmail.com","threadId":"57658","inReplyTo":"20220404182129.33992-1-eantoranz@gmail.com","subject":"Re: [PATCH v2] blame: report correct number of lines in progress when using ranges","fromName":"Bagas Sanjaya","fromEmail":"bagasdotme@gmail.com","sentAt":"2022-04-05T07:34:05Z","receivedAt":"2022-04-05T07:34:27Z","isPatch":true,"sender":{"key":"bagasdotme@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40219486?v=4"},"body":"On 05/04/22 01.21, Edmundo Carmona Antoranz wrote:\n> When using ranges, use their sizes as the limit for progress\n> instead of the size of the full file.\n> \n\nThe progress limit is defined by number of affected lines, right?\n\n> +test_expect_success 'blame progress on a full file' '\n> +\tcat >progress.txt <<-\\EOF &&\n> +\ta simple test file\n> +\n> +\tno relevant content is expected here\n> +\n> +\tIf the file is too short, we cannot test ranges\n> +\n> +\tEOF\n> +\tgit add progress.txt &&\n> +\tgit commit -m \"add a file for testing progress\" &&\n> +\tGIT_PROGRESS_DELAY=0 \\\n> +\tgit blame --progress progress.txt > /dev/null 2> full_progress.txt &&\n> +\tgrep \"Blaming lines: 100% (6/6), done.\" full_progress.txt\n> +'\n> +\n> +test_expect_success 'blame progress on a single range' '\n> +\tGIT_PROGRESS_DELAY=0 \\\n> +\tgit blame --progress -L 2,5 progress.txt > /dev/null 2> range_progress.txt &&\n> +\tgrep \"Blaming lines: 100% (4/4), done.\" range_progress.txt\n> +'\n> +\n> +test_expect_success 'blame progress on multiple ranges' '\n> +\tGIT_PROGRESS_DELAY=0 \\\n> +\tgit blame --progress -L 1,2 -L 4,6 progress.txt > /dev/null 2> range_progress.txt &&\n> +\tgrep \"Blaming lines: 100% (5/5), done.\" range_progress.txt\n> +'\n> +\n\nWhy not using test_i18ngrep?\n\n-- \nAn old man doll... just what I always wanted! - Clara\n"},{"id":"453100","messageId":"220405.86wng4km5c.gmgdl@evledraar.gmail.com","threadId":"57658","inReplyTo":"b2f5d6af-8da1-3a3a-cc21-848c14a8fb98@gmail.com","subject":"Re: [PATCH v2] blame: report correct number of lines in progress when using ranges","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-04-05T07:46:10Z","receivedAt":"2022-04-05T07:47:34Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Apr 05 2022, Bagas Sanjaya wrote:\n\n> On 05/04/22 01.21, Edmundo Carmona Antoranz wrote:\n>> When using ranges, use their sizes as the limit for progress\n>> instead of the size of the full file.\n>> \n>\n> The progress limit is defined by number of affected lines, right?\n>\n>> +test_expect_success 'blame progress on a full file' '\n>> +\tcat >progress.txt <<-\\EOF &&\n>> +\ta simple test file\n>> +\n>> +\tno relevant content is expected here\n>> +\n>> +\tIf the file is too short, we cannot test ranges\n>> +\n>> +\tEOF\n>> +\tgit add progress.txt &&\n>> +\tgit commit -m \"add a file for testing progress\" &&\n>> +\tGIT_PROGRESS_DELAY=0 \\\n>> +\tgit blame --progress progress.txt > /dev/null 2> full_progress.txt &&\n>> +\tgrep \"Blaming lines: 100% (6/6), done.\" full_progress.txt\n>> +'\n>> +\n>> +test_expect_success 'blame progress on a single range' '\n>> +\tGIT_PROGRESS_DELAY=0 \\\n>> +\tgit blame --progress -L 2,5 progress.txt > /dev/null 2> range_progress.txt &&\n>> +\tgrep \"Blaming lines: 100% (4/4), done.\" range_progress.txt\n>> +'\n>> +\n>> +test_expect_success 'blame progress on multiple ranges' '\n>> +\tGIT_PROGRESS_DELAY=0 \\\n>> +\tgit blame --progress -L 1,2 -L 4,6 progress.txt > /dev/null 2> range_progress.txt &&\n>> +\tgrep \"Blaming lines: 100% (5/5), done.\" range_progress.txt\n>> +'\n>> +\n>\n> Why not using test_i18ngrep?\n\nNothing should be using test_i18ngrep nowadays, just grep is better. We\nno longer test with the gettext \"poison\" mode which necessitated it.\n"},{"id":"453101","messageId":"CAOc6etYOHeiLU_u-WxEiZSszXRHfn6h32dcXHQtEQqu=d-87hQ@mail.gmail.com","threadId":"57658","inReplyTo":"220405.86wng4km5c.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH v2] blame: report correct number of lines in progress when using ranges","fromName":"Edmundo Carmona Antoranz","fromEmail":"eantoranz@gmail.com","sentAt":"2022-04-05T07:55:59Z","receivedAt":"2022-04-05T08:02:41Z","isPatch":true,"sender":{"key":"eantoranz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1491018?v=4"},"body":"On Tue, Apr 5, 2022 at 9:46 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:\n>\n>\n>\n> Nothing should be using test_i18ngrep nowadays, just grep is better. We\n> no longer test with the gettext \"poison\" mode which necessitated it.\n\nTaking a closer look at the already defined tests/files. Thank you all\nfor your feedback.\n"},{"id":"453104","messageId":"220405.86o81flve1.gmgdl@evledraar.gmail.com","threadId":"57658","inReplyTo":"20220404182129.33992-1-eantoranz@gmail.com","subject":"Re: [PATCH v2] blame: report correct number of lines in progress when using ranges","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-04-05T09:36:05Z","receivedAt":"2022-04-05T09:53:20Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Apr 04 2022, Edmundo Carmona Antoranz wrote:\n\n> When using ranges, use their sizes as the limit for progress\n> instead of the size of the full file.\n>\n> Before:\n> $ git blame --progress builtin/blame.c > /dev/null\n> Blaming lines: 100% (1210/1210), done.\n> $ git blame --progress -L 100,120 -L 200,300 builtin/blame.c > /dev/null\n> Blaming lines:  10% (122/1210), done.\n> $\n>\n> After:\n> $ ./git blame --progress builtin/blame.c > /dev/null\n> Blaming lines: 100% (1210/1210), done.\n> $ ./git blame --progress -L 100,120 -L 200,300 builtin/blame.c > /dev/null\n> Blaming lines: 100% (122/122), done.\n> $\n>\n> Signed-off-by: Edmundo Carmona Antoranz <eantoranz@gmail.com>\n> ---\n>  builtin/blame.c  |  6 +++++-\n>  t/t8002-blame.sh | 28 ++++++++++++++++++++++++++++\n>  2 files changed, 33 insertions(+), 1 deletion(-)\n>\n> diff --git a/builtin/blame.c b/builtin/blame.c\n> index 8d15b68afc..e33372c56b 100644\n> --- a/builtin/blame.c\n> +++ b/builtin/blame.c\n> @@ -898,6 +898,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n>  \tunsigned int range_i;\n>  \tlong anchor;\n>  \tconst int hexsz = the_hash_algo->hexsz;\n> +\tlong num_lines = 0;\n>  \n>  \tsetup_default_color_by_age();\n>  \tgit_config(git_blame_config, &output_option);\n> @@ -1129,7 +1130,10 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n>  \tfor (range_i = ranges.nr; range_i > 0; --range_i) {\n>  \t\tconst struct range *r = &ranges.ranges[range_i - 1];\n>  \t\tent = blame_entry_prepend(ent, r->start, r->end, o);\n> +\t\tnum_lines += (r->end - r->start);\n>  \t}\n> +\tif (!num_lines)\n> +\t\tnum_lines = sb.num_lines;\n>  \n>  \to->suspects = ent;\n>  \tprio_queue_put(&sb.commits, o->commit);\n> @@ -1158,7 +1162,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n>  \tsb.found_guilty_entry = &found_guilty_entry;\n>  \tsb.found_guilty_entry_data = &pi;\n>  \tif (show_progress)\n> -\t\tpi.progress = start_delayed_progress(_(\"Blaming lines\"), sb.num_lines);\n> +\t\tpi.progress = start_delayed_progress(_(\"Blaming lines\"), num_lines);\n>  \n>  \tassign_blame(&sb, opt);\n\nLooking good.\n\n> diff --git a/t/t8002-blame.sh b/t/t8002-blame.sh\n> index ee4fdd8f18..151a6fddfd 100755\n> --- a/t/t8002-blame.sh\n> +++ b/t/t8002-blame.sh\n> @@ -129,6 +129,34 @@ 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 'blame progress on a full file' '\n> +\tcat >progress.txt <<-\\EOF &&\n> +\ta simple test file\n> +\n> +\tno relevant content is expected here\n> +\n> +\tIf the file is too short, we cannot test ranges\n> +\n> +\tEOF\n> +\tgit add progress.txt &&\n> +\tgit commit -m \"add a file for testing progress\" &&\n\nLet's just skip this then and use existing test setup. A quick glance at\nthe state after this test shows that e.g. the \"hello.c\" we already\ncreated would be a good candidate.\n\n> +\tGIT_PROGRESS_DELAY=0 \\\n> +\tgit blame --progress progress.txt > /dev/null 2> full_progress.txt &&\n> +\tgrep \"Blaming lines: 100% (6/6), done.\" full_progress.txt\n\nLet's use test_cmp here instead, as we expect nothing else on stderr,\nand with grep one wonders why it's not ^$ anchored, but just:\n\n    echo \"Blaming lines: 100% (6/6), done.\" >expect &&\n    git blame ... 2>actual &&\n    test_cmp expect actual\n\nis better, both because it's more exhaustive as a test, and because\nit'll give better debug (diff) output on failure than grep will (just no\noutput at all).\n\n> +test_expect_success 'blame progress on a single range' '\n> +\tGIT_PROGRESS_DELAY=0 \\\n> +\tgit blame --progress -L 2,5 progress.txt > /dev/null 2> range_progress.txt &&\n> +\tgrep \"Blaming lines: 100% (4/4), done.\" range_progress.txt\n> +'\n> +\n> +test_expect_success 'blame progress on multiple ranges' '\n> +\tGIT_PROGRESS_DELAY=0 \\\n> +\tgit blame --progress -L 1,2 -L 4,6 progress.txt > /dev/null 2> range_progress.txt &&\n> +\tgrep \"Blaming lines: 100% (5/5), done.\" range_progress.txt\n> +'\n\nStyle nit, no space after \">\", so e.g. 2>err.\n\nAlso shorter names are easier to read, so just:\n\n    [...] 2>err\n\nOr \"actual\" per the suggestion above.\n\nAnd no need to redirect stdout to /dev/null, it's helpful to see it by\ndefault in the verbose test output, we let that take care of suppressing\nall of it ornot.\n\n\n>  test_expect_success 'blame with uncommitted edits in partial clone does not crash' '\n>  \tgit init server &&\n>  \techo foo >server/file.txt &&\n\n"},{"id":"453106","messageId":"8622e48c-9f34-c644-4016-02c3795ac1e9@iee.email","threadId":"57658","inReplyTo":"b2f5d6af-8da1-3a3a-cc21-848c14a8fb98@gmail.com","subject":"Re: [PATCH v2] blame: report correct number of lines in progress when using ranges","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2022-04-05T09:42:41Z","receivedAt":"2022-04-05T09:53:25Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"On 05/04/2022 08:34, Bagas Sanjaya wrote:\n> On 05/04/22 01.21, Edmundo Carmona Antoranz wrote:\n>> When using ranges, use their sizes as the limit for progress\n>> instead of the size of the full file.\n>>\n>\n> The progress limit is defined by number of affected lines, right?\n\nI'd also wondered about 'their', thinking it was 'the files', rather\nthan 'the ranges' [within those files].\n\nperhaps: s/their/range/  \n\n\"When using ranges, use the range sizes as the limit for progress' ..\n\nor maybe 'total range size'.\n--\nPhilip\n"},{"id":"453187","messageId":"xmqqsfqq1bx9.fsf@gitster.g","threadId":"57658","inReplyTo":"8622e48c-9f34-c644-4016-02c3795ac1e9@iee.email","subject":"Re: [PATCH v2] blame: report correct number of lines in progress when using ranges","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-04-06T15:14:42Z","receivedAt":"2022-04-06T17:16:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Philip Oakley <philipoakley@iee.email> writes:\n\n> On 05/04/2022 08:34, Bagas Sanjaya wrote:\n>> On 05/04/22 01.21, Edmundo Carmona Antoranz wrote:\n>>> When using ranges, use their sizes as the limit for progress\n>>> instead of the size of the full file.\n>>\n>> The progress limit is defined by number of affected lines, right?\n>\n> I'd also wondered about 'their', thinking it was 'the files', rather\n> than 'the ranges' [within those files].\n>\n> perhaps: s/their/range/\n\nI actually think that it is obvious that \"their\" refers to the\nranges and not the file.  Between \"the ranges\" and \"the file\", only\nthe former is plural that \"their\" could possibly refer to.  Also,\n\"instead ... the full file\" makes the sentence nonsensical if it\nreferred to the \"file\"---\"we must use the number of lines in the\nfile, instead of the number of lines in the file\" simply would not\nmake much sense.\n\nBut I do not object to being more explicit.\n\n> \"When using ranges, use the range sizes as the limit for progress' ..\n"},{"id":"453316","messageId":"9920b355-9003-e7c7-77ab-3432651674e9@iee.email","threadId":"57658","inReplyTo":"xmqqsfqq1bx9.fsf@gitster.g","subject":"Re: [PATCH v2] blame: report correct number of lines in progress when using ranges","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2022-04-08T08:03:55Z","receivedAt":"2022-04-08T08:04:07Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"On 06/04/2022 16:14, Junio C Hamano wrote:\n> Philip Oakley <philipoakley@iee.email> writes:\n>\n>> On 05/04/2022 08:34, Bagas Sanjaya wrote:\n>>> On 05/04/22 01.21, Edmundo Carmona Antoranz wrote:\n>>>> When using ranges, use their sizes as the limit for progress\n>>>> instead of the size of the full file.\n>>> The progress limit is defined by number of affected lines, right?\n>> I'd also wondered about 'their', thinking it was 'the files', rather\n>> than 'the ranges' [within those files].\n>>\n>> perhaps: s/their/range/\n> I actually think that it is obvious that \"their\" refers to the\n> ranges and not the file.  Between \"the ranges\" and \"the file\", only\n> the former is plural that \"their\" could possibly refer to.  Also,\n> \"instead ... the full file\" makes the sentence nonsensical if it\n> referred to the \"file\"---\"we must use the number of lines in the\n> file, instead of the number of lines in the file\" simply would not\n> make much sense.\nI'm on the 'context and guidelines' side of English comprehension, so it\nwas all about files being blamed.\n\n>\n> But I do not object to being more explicit.\n\nThe core point though was that it can be misunderstood, thus avoiding\nthe indirection, as you say, makes it more explicit for the reader.\n>\n>> \"When using ranges, use the range sizes as the limit for progress' ..\n--\nPhilip\n"},{"id":"453341","messageId":"xmqqy20fmoes.fsf@gitster.g","threadId":"57658","inReplyTo":"9920b355-9003-e7c7-77ab-3432651674e9@iee.email","subject":"Re: [PATCH v2] blame: report correct number of lines in progress when using ranges","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-04-08T18:16:11Z","receivedAt":"2022-04-08T18:16:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Philip Oakley <philipoakley@iee.email> writes:\n\n>> But I do not object to being more explicit.\n>\n> The core point though was that it can be misunderstood, thus avoiding\n> the indirection, as you say, makes it more explicit for the reader.\n\nYup.  FWIW, I was saying that what the author wrote was not _wrong_\nper-se.  I agree that being explicit here (instead of hiding behind\na pronoun) is an improvement.\n\nThanks.\n"}]}