git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Seyi Kuforiji <kuforiji98@gmail.com>
Cc: git@vger.kernel.org,  Patrick Steinhardt <ps@pks.im>,
	 Phillip Wood <phillip.wood@dunelm.org.uk>,
	 Taylor Blau <me@ttaylorr.com>,
	 Christian Couder <christian.couder@gmail.com>
Subject: Re: [FEATURE] Proposal: git format-patch with `--with-line-numbers` flag
Date: Mon, 18 Aug 2025 10:02:33 -0700	[thread overview]
Message-ID: <xmqqfrdok1g6.fsf@gitster.g> (raw)
In-Reply-To: <CAGedMtd_atWTAQXOPSJThB_tpHiOSY=PUhrfFxFZOEkgUtHf1w@mail.gmail.com> (Seyi Kuforiji's message of "Mon, 18 Aug 2025 11:08:43 +0100")

Seyi Kuforiji <kuforiji98@gmail.com> writes:

> While working on converting unit tests and sending patches, I ran into a
> pain point during review. The reviews by Junio, Patrick, and others pointed out
> issues in my patches, but without line numbers in the emailed code
> context, it was sometimes hard to know exactly which line was being
> referenced. I had to manually count through the diff hunks, which slowed
> things down.

Count through?  I do not usually see a review that talks line
numbers (e.g. "your change to line 772 is wrong and should look like
this"), so I am not sure which review comment against which patch
you had trouble with.  Can you give us an example or two?  URL into
the lore archive would be good.

One things I try in my reviews is, even though I trim my quotes
heavily and leave only the part I comment on, I try to leave the
filename part (i.e. "diff --git" line) and the hunk header (i.e. "@@
-L,K +M,N @@" line) in.  See

    https://lore.kernel.org/git/xmqqikla86id.fsf@gitster.g/

for an example.

> To address this, I’d like to propose adding an option to `git
> format-patch` (e.g., `--with-line-numbers`) that would include line numbers
> numbers alongside context lines in the generated patch. This would not
> affect patch application (`git am` / `git apply`), but would be a visual
> aid for mailing list readers.

"This would not affect" how?  If you show something like below, it
would break it so badly that the patch would not apply at all, so
you may have something else in mind, but I do not know what it would
be.

diff --git a/t/t0450-txt-doc-vs-help.sh b/t/t0450-txt-doc-vs-help.sh
index 980130be78..e12e18f97f 100755
--- a/t/t0450-txt-doc-vs-help.sh
+++ b/t/t0450-txt-doc-vs-help.sh
@@ -112,16 +112,19 @@ do
112 	adoc="$(builtin_to_adoc "$builtin")" &&
113 	preq="$(echo BUILTIN_ADOC_$builtin | tr '[:lower:]-' '[:upper:]_')" &&
114 
115-	# if and only if *.adoc is missing, builtin shall be listed in t0450/adoc-missing
116-	result=success
117+	# If and only if *.adoc is missing, builtin shall be listed in t0450/adoc-missing.
118 	if grep -q "^$builtin$" "$TEST_DIRECTORY"/t0450/adoc-missing
119 	then
120+		test_expect_success "$builtin appropriately marked as not having .adoc" '
121+			! test -f "$adoc"
122+		'
123+	else
124 		test_set_prereq "$preq"
125-		result=failure
126-	fi &&
127-	test_expect_$result "$builtin appropriately marked as having missing .adoc" '
128-		test -f "$adoc"
129-	'
130+
131+		test_expect_success "$builtin appropriately marked as having .adoc" '
132+			test -f "$adoc"
133+		'
134+	fi
135 
136 	# *.adoc output assertions
137 	test_expect_success "$preq" "$builtin *.adoc SYNOPSIS has dashed labels" '

  reply	other threads:[~2025-08-18 17:02 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-18 10:08 [FEATURE] Proposal: git format-patch with `--with-line-numbers` flag Seyi Kuforiji
2025-08-18 17:02 ` Junio C Hamano [this message]
2025-08-21 12:04   ` Seyi Kuforiji
2025-08-21 19:50     ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqfrdok1g6.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=kuforiji98@gmail.com \
    --cc=me@ttaylorr.com \
    --cc=phillip.wood@dunelm.org.uk \
    --cc=ps@pks.im \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).