{"thread":{"id":"63976","subject":"[FEATURE] Proposal: git format-patch with `--with-line-numbers` flag","startedAt":"2025-08-18T10:08:56Z","lastAt":"2025-08-21T19:50:37Z","messageCount":4,"participants":["Seyi Kuforiji","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"524340","messageId":"CAGedMtd_atWTAQXOPSJThB_tpHiOSY=PUhrfFxFZOEkgUtHf1w@mail.gmail.com","threadId":"63976","inReplyTo":null,"subject":"[FEATURE] Proposal: git format-patch with `--with-line-numbers` flag","fromName":"Seyi Kuforiji","fromEmail":"kuforiji98@gmail.com","sentAt":"2025-08-18T10:08:43Z","receivedAt":"2025-08-18T10:08:56Z","isPatch":false,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"Hi Everyone,\n\nWhile working on converting unit tests and sending patches, I ran into a\npain point during review. The reviews by Junio, Patrick, and others pointed out\nissues in my patches, but without line numbers in the emailed code\ncontext, it was sometimes hard to know exactly which line was being\nreferenced. I had to manually count through the diff hunks, which slowed\nthings down.\n\nTo address this, I’d like to propose adding an option to `git\nformat-patch` (e.g., `--with-line-numbers`) that would include line numbers\nnumbers alongside context lines in the generated patch. This would not\naffect patch application (`git am` / `git apply`), but would be a visual\naid for mailing list readers.\n\nBenefits:\n- Makes reviews on the mailing list clearer and faster.\n- Let reviewers point out \"line 52 has an off-by-one\" for easy review.\n- Reduces friction for new contributors.\n\nPossible concerns:\n- Could clutter diffs if not formatted cleanly.\n\nI wanted to ask for feedback: would this kind of feature be welcomed?\nIf so, I’d be happy to draft a patch implementing it.\n\nBest regards,\nSeyi Kuforiji\n"},{"id":"524352","messageId":"xmqqfrdok1g6.fsf@gitster.g","threadId":"63976","inReplyTo":"CAGedMtd_atWTAQXOPSJThB_tpHiOSY=PUhrfFxFZOEkgUtHf1w@mail.gmail.com","subject":"Re: [FEATURE] Proposal: git format-patch with `--with-line-numbers` flag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-08-18T17:02:33Z","receivedAt":"2025-08-18T17:02:36Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Seyi Kuforiji <kuforiji98@gmail.com> writes:\n\n> While working on converting unit tests and sending patches, I ran into a\n> pain point during review. The reviews by Junio, Patrick, and others pointed out\n> issues in my patches, but without line numbers in the emailed code\n> context, it was sometimes hard to know exactly which line was being\n> referenced. I had to manually count through the diff hunks, which slowed\n> things down.\n\nCount through?  I do not usually see a review that talks line\nnumbers (e.g. \"your change to line 772 is wrong and should look like\nthis\"), so I am not sure which review comment against which patch\nyou had trouble with.  Can you give us an example or two?  URL into\nthe lore archive would be good.\n\nOne things I try in my reviews is, even though I trim my quotes\nheavily and leave only the part I comment on, I try to leave the\nfilename part (i.e. \"diff --git\" line) and the hunk header (i.e. \"@@\n-L,K +M,N @@\" line) in.  See\n\n    https://lore.kernel.org/git/xmqqikla86id.fsf@gitster.g/\n\nfor an example.\n\n> To address this, I’d like to propose adding an option to `git\n> format-patch` (e.g., `--with-line-numbers`) that would include line numbers\n> numbers alongside context lines in the generated patch. This would not\n> affect patch application (`git am` / `git apply`), but would be a visual\n> aid for mailing list readers.\n\n\"This would not affect\" how?  If you show something like below, it\nwould break it so badly that the patch would not apply at all, so\nyou may have something else in mind, but I do not know what it would\nbe.\n\ndiff --git a/t/t0450-txt-doc-vs-help.sh b/t/t0450-txt-doc-vs-help.sh\nindex 980130be78..e12e18f97f 100755\n--- a/t/t0450-txt-doc-vs-help.sh\n+++ b/t/t0450-txt-doc-vs-help.sh\n@@ -112,16 +112,19 @@ do\n112 \tadoc=\"$(builtin_to_adoc \"$builtin\")\" &&\n113 \tpreq=\"$(echo BUILTIN_ADOC_$builtin | tr '[:lower:]-' '[:upper:]_')\" &&\n114 \n115-\t# if and only if *.adoc is missing, builtin shall be listed in t0450/adoc-missing\n116-\tresult=success\n117+\t# If and only if *.adoc is missing, builtin shall be listed in t0450/adoc-missing.\n118 \tif grep -q \"^$builtin$\" \"$TEST_DIRECTORY\"/t0450/adoc-missing\n119 \tthen\n120+\t\ttest_expect_success \"$builtin appropriately marked as not having .adoc\" '\n121+\t\t\t! test -f \"$adoc\"\n122+\t\t'\n123+\telse\n124 \t\ttest_set_prereq \"$preq\"\n125-\t\tresult=failure\n126-\tfi &&\n127-\ttest_expect_$result \"$builtin appropriately marked as having missing .adoc\" '\n128-\t\ttest -f \"$adoc\"\n129-\t'\n130+\n131+\t\ttest_expect_success \"$builtin appropriately marked as having .adoc\" '\n132+\t\t\ttest -f \"$adoc\"\n133+\t\t'\n134+\tfi\n135 \n136 \t# *.adoc output assertions\n137 \ttest_expect_success \"$preq\" \"$builtin *.adoc SYNOPSIS has dashed labels\" '\n"},{"id":"524645","messageId":"CAGedMtf2CW_L8uSc1KRqmAoJ=2Sw4t5AL2AC0uKQJb5keX63ZA@mail.gmail.com","threadId":"63976","inReplyTo":"xmqqfrdok1g6.fsf@gitster.g","subject":"Re: [FEATURE] Proposal: git format-patch with `--with-line-numbers` flag","fromName":"Seyi Kuforiji","fromEmail":"kuforiji98@gmail.com","sentAt":"2025-08-21T12:04:02Z","receivedAt":"2025-08-21T12:04:16Z","isPatch":false,"sender":{"key":"kuforiji98@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94905626?v=4"},"body":"Hi Junio,\n\nOn Mon, 18 Aug 2025 at 18:02, Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Seyi Kuforiji <kuforiji98@gmail.com> writes:\n>\n> > While working on converting unit tests and sending patches, I ran into a\n> > pain point during review. The reviews by Junio, Patrick, and others pointed out\n> > issues in my patches, but without line numbers in the emailed code\n> > context, it was sometimes hard to know exactly which line was being\n> > referenced. I had to manually count through the diff hunks, which slowed\n> > things down.\n>\n> Count through?  I do not usually see a review that talks line\n> numbers (e.g. \"your change to line 772 is wrong and should look like\n> this\"), so I am not sure which review comment against which patch\n> you had trouble with.  Can you give us an example or two?  URL into\n> the lore archive would be good.\n>\n> One things I try in my reviews is, even though I trim my quotes\n> heavily and leave only the part I comment on, I try to leave the\n> filename part (i.e. \"diff --git\" line) and the hunk header (i.e. \"@@\n> -L,K +M,N @@\" line) in.  See\n>\n>     https://lore.kernel.org/git/xmqqikla86id.fsf@gitster.g/\n>\n> for an example.\n>\nAh, thanks for the clarification; that makes sense now. Up until this\npoint, I didn't know the hunk headers \"(@@ -L,K +M,N @@ lines)\"\nprovided enough context in terms of the lines the changes were made. I\njust never read them and usually just jump to the reviews on the code\nchanges, and I try to locate the changes locally :(. I agree this\nalready provides sufficient context, and I've definitely learned\nsomething new here :). I am wondering if a description of this is\ncovered in our documentation. If not, maybe I could add it, since I\nimagine others might have the same question.\n\n> > To address this, I’d like to propose adding an option to `git\n> > format-patch` (e.g., `--with-line-numbers`) that would include line numbers\n> > numbers alongside context lines in the generated patch. This would not\n> > affect patch application (`git am` / `git apply`), but would be a visual\n> > aid for mailing list readers.\n>\n> \"This would not affect\" how?  If you show something like below, it\n> would break it so badly that the patch would not apply at all, so\n> you may have something else in mind, but I do not know what it would\n> be.\n>\n> diff --git a/t/t0450-txt-doc-vs-help.sh b/t/t0450-txt-doc-vs-help.sh\n> index 980130be78..e12e18f97f 100755\n> --- a/t/t0450-txt-doc-vs-help.sh\n> +++ b/t/t0450-txt-doc-vs-help.sh\n> @@ -112,16 +112,19 @@ do\n> 112     adoc=\"$(builtin_to_adoc \"$builtin\")\" &&\n> 113     preq=\"$(echo BUILTIN_ADOC_$builtin | tr '[:lower:]-' '[:upper:]_')\" &&\n> 114\n> 115-    # if and only if *.adoc is missing, builtin shall be listed in t0450/adoc-missing\n> 116-    result=success\n> 117+    # If and only if *.adoc is missing, builtin shall be listed in t0450/adoc-missing.\n> 118     if grep -q \"^$builtin$\" \"$TEST_DIRECTORY\"/t0450/adoc-missing\n> 119     then\n> 120+            test_expect_success \"$builtin appropriately marked as not having .adoc\" '\n> 121+                    ! test -f \"$adoc\"\n> 122+            '\n> 123+    else\n> 124             test_set_prereq \"$preq\"\n> 125-            result=failure\n> 126-    fi &&\n> 127-    test_expect_$result \"$builtin appropriately marked as having missing .adoc\" '\n> 128-            test -f \"$adoc\"\n> 129-    '\n> 130+\n> 131+            test_expect_success \"$builtin appropriately marked as having .adoc\" '\n> 132+                    test -f \"$adoc\"\n> 133+            '\n> 134+    fi\n> 135\n> 136     # *.adoc output assertions\n> 137     test_expect_success \"$preq\" \"$builtin *.adoc SYNOPSIS has dashed labels\" '\n\nThere isn't a need anymore for my proposed changes to the way `git\nformat-patch` operates.\n\nBest regards,\nSeyi Kuforiji\n"},{"id":"524679","messageId":"xmqqa53s1mk5.fsf@gitster.g","threadId":"63976","inReplyTo":"CAGedMtf2CW_L8uSc1KRqmAoJ=2Sw4t5AL2AC0uKQJb5keX63ZA@mail.gmail.com","subject":"Re: [FEATURE] Proposal: git format-patch with `--with-line-numbers` flag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-08-21T19:50:34Z","receivedAt":"2025-08-21T19:50:37Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Seyi Kuforiji <kuforiji98@gmail.com> writes:\n\n>> One things I try in my reviews is, even though I trim my quotes\n>> heavily and leave only the part I comment on, I try to leave the\n>> filename part (i.e. \"diff --git\" line) and the hunk header (i.e. \"@@\n>> -L,K +M,N @@\" line) in.  See\n>>\n>>     https://lore.kernel.org/git/xmqqikla86id.fsf@gitster.g/\n>>\n>> for an example.\n>>\n> Ah, thanks for the clarification; that makes sense now. Up until this\n> point, I didn't know the hunk headers \"(@@ -L,K +M,N @@ lines)\"\n> provided enough context in terms of the lines the changes were made. I\n> just never read them and usually just jump to the reviews on the code\n> changes, and I try to locate the changes locally :(. I agree this\n> already provides sufficient context, and I've definitely learned\n> something new here :). I am wondering if a description of this is\n> covered in our documentation. If not, maybe I could add it, since I\n> imagine others might have the same question.\n\nI do not think our documentation (documentation proper, not\nhandholding newbies tutorials) wants to repeat what is in\n\nhttps://pubs.opengroup.org/onlinepubs/9799919799/utilities/diff.html#tag_20_34_10_07\n\nand explain what different parts of a \"patch\" output look like, but\nperhaps MyFirstContribution would be a good place to add it, if not\ndone already.\n\nThanks.\n\n\n"}]}