Git development
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Muhammed Dilshad A <dilsheddilu123@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v2 0/2] combine-diff: honor relative paths consistently
Date: Wed, 07 Oct 2026 10:53:05 -0700	[thread overview]
Message-ID: <xmqqh5ix8kji.fsf@gitster.g> (raw)
In-Reply-To: <cover.1791390459.git.dilsheddilu123@gmail.com> (Muhammed Dilshad A.'s message of "Wed, 7 Oct 2026 22:05:12 +0530")

Muhammed Dilshad A <dilsheddilu123@gmail.com> writes:

> The revised patch prints there/file as ../there/file when the prefix is
> here/. All displayed names then use the same base. The tests cover both
> discovery paths, including raw and NUL-separated output.

This sounds like the most sensible behaviour, within the constraint
that --relative must give a relative path to the prefix.

> The index rejects repeated separators, but tree entry parsing does not
> enforce the same check. The helper now skips all separators at the prefix
> boundary, so it does not rely on there being only one. Explicit prefix
> arguments stay literal, matching ordinary diff's filtering behavior.
> I also wrapped the added C lines to fit the coding guidelines.

Do *not* respond to review comments in your cover letter.  Nobody
reading the above, other than those who have seen our earlier
exchange of you sending v1 patch with I commenting on it, would not
know what you are talking about in the above, and especially what is
so special about "repeated separators" without context.  The cover
letter should aim to welcome even those late-comming reviewers who
missed an earlier round.

Review response should be done as a response to a review message,
unrelated to your rerolled patches.

> While checking the two discovery paths, I found that the fast multi-tree
> scan bypasses the relative-prefix filter entirely. Patch 2 fixes that
> separately and adds tests for outside paths and repeated separators in
> an explicit prefix.

Great.

>
> Changes since v1:
>
> * Use relative_path() for parent names outside the prefix while keeping
>   ordinary diff's literal-prefix behavior for matching names.
> * Preserve /dev/null, skip all boundary separators, and wrap long lines.
> * Add cross-directory rename tests and the separate fast-scan fix.
>
> The developer build with SANITIZE=leak succeeds. The affected suites pass
> all 77 normal tests with SHA-1 and SHA-256. A separate run with
> LSAN_OPTIONS=detect_leaks=1 also passes without a leak report. The existing
> three-parent coalescing failure in t4038 remains an expected failure.
>
> Muhammed Dilshad A (2):
>   combine-diff: honor --relative when printing paths
>   combine-diff: filter the fast scan by the relative prefix
>
>  combine-diff.c           |  70 ++++++++++++++++++---
>  t/t4038-diff-combined.sh | 130 +++++++++++++++++++++++++++++++++++++++
>  t/t4045-diff-relative.sh |  62 ++++++++++++++++++-
>  3 files changed, 251 insertions(+), 11 deletions(-)
>
>
> base-commit: 6de20f6092dcf9bdb1c8efe03db4b70c82b423dd

  parent reply	other threads:[~2026-10-07 17:53 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-07  5:17 [PATCH] combine-diff: honor --relative when printing paths Muhammed Dilshad A
2026-10-07 14:45 ` Junio C Hamano
2026-10-07 16:35   ` [PATCH v2 0/2] combine-diff: honor relative paths consistently Muhammed Dilshad A
2026-10-07 16:35     ` [PATCH v2 1/2] combine-diff: honor --relative when printing paths Muhammed Dilshad A
2026-10-07 16:35     ` [PATCH v2 2/2] combine-diff: filter the fast scan by the relative prefix Muhammed Dilshad A
2026-10-07 17:53     ` Junio C Hamano [this message]
2026-10-09 11:08       ` [PATCH v2 0/2] combine-diff: honor relative paths consistently Muhammed Dilshad A

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=xmqqh5ix8kji.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=dilsheddilu123@gmail.com \
    --cc=git@vger.kernel.org \
    /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