diff mbox series

[v4,2/4] diff: add option to skip resolving diff statuses

Message ID 20250228002604.3859939-3-jltobler@gmail.com (mailing list archive)
State Superseded
Headers show
Series batch blob diff generation | expand

Commit Message

Justin Tobler Feb. 28, 2025, 12:26 a.m. UTC
By default, `diffcore_std()` resolves the statuses for queued diff file
pairs by calling `diff_resolve_rename_copy()`. If status information is
already manually set, invoking `diffcore_std()` may change the status
value.

Introduce the `skip_resolving_statuses` diff option that prevents
`diffcore_std()` from resolving file pair statuses when enabled.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
---
 diff.c | 2 +-
 diff.h | 8 ++++++++
 2 files changed, 9 insertions(+), 1 deletion(-)

Comments

Patrick Steinhardt Feb. 28, 2025, 8:29 a.m. UTC | #1
On Thu, Feb 27, 2025 at 06:26:02PM -0600, Justin Tobler wrote:
> By default, `diffcore_std()` resolves the statuses for queued diff file
> pairs by calling `diff_resolve_rename_copy()`. If status information is
> already manually set, invoking `diffcore_std()` may change the status
> value.
> 
> Introduce the `skip_resolving_statuses` diff option that prevents
> `diffcore_std()` from resolving file pair statuses when enabled.

You mentioned to me that there was another user that basically abused
`found_follow` to skip over this, which seems to be in "tree-diff.c".
Would it make sense to convert that user to use the new mechanism, as
well, so that we don't mix up options and state?

Patrick
Justin Tobler Feb. 28, 2025, 5:10 p.m. UTC | #2
On 25/02/28 09:29AM, Patrick Steinhardt wrote:
> On Thu, Feb 27, 2025 at 06:26:02PM -0600, Justin Tobler wrote:
> > By default, `diffcore_std()` resolves the statuses for queued diff file
> > pairs by calling `diff_resolve_rename_copy()`. If status information is
> > already manually set, invoking `diffcore_std()` may change the status
> > value.
> > 
> > Introduce the `skip_resolving_statuses` diff option that prevents
> > `diffcore_std()` from resolving file pair statuses when enabled.
> 
> You mentioned to me that there was another user that basically abused
> `found_follow` to skip over this, which seems to be in "tree-diff.c".
> Would it make sense to convert that user to use the new mechanism, as
> well, so that we don't mix up options and state?

I was mixed up with something else and mistaken. There is only the one
expected existing user of the `found_follow` option. Apoligies for any
confusion.

-Justin
diff mbox series

Patch

diff --git a/diff.c b/diff.c
index b5a779f997..37cc88c75b 100644
--- a/diff.c
+++ b/diff.c
@@ -7081,7 +7081,7 @@  void diffcore_std(struct diff_options *options)
 		diffcore_order(options->orderfile);
 	if (options->rotate_to)
 		diffcore_rotate(options);
-	if (!options->found_follow)
+	if (!options->found_follow && !options->skip_resolving_statuses)
 		/* See try_to_follow_renames() in tree-diff.c */
 		diff_resolve_rename_copy();
 	diffcore_apply_filter(options);
diff --git a/diff.h b/diff.h
index 63afa17e84..fc791ee2cc 100644
--- a/diff.h
+++ b/diff.h
@@ -353,6 +353,14 @@  struct diff_options {
 	/* to support internal diff recursion by --follow hack*/
 	int found_follow;
 
+	/*
+	 * By default, diffcore_std() resolves the statuses for queued diff file
+	 * pairs by calling diff_resolve_rename_copy(). If status information
+	 * has already been manually set, this option prevents diffcore_std()
+	 * from resetting statuses.
+	 */
+	int skip_resolving_statuses;
+
 	/* Callback which allows tweaking the options in diff_setup_done(). */
 	void (*set_default)(struct diff_options *);