Hi Calvin
On 07/02/2023 18:17, Calvin Wan wrote:
Refactor out logic that sets up the diff_change call into a helper
function for a future patch.
Signed-off-by: Calvin Wan <calvinwan@xxxxxxxxxx>
---
diff-lib.c | 46 +++++++++++++++++++++++++++++-----------------
1 file changed, 29 insertions(+), 17 deletions(-)
diff --git a/diff-lib.c b/diff-lib.c
index dec040c366..7101cfda3f 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -88,6 +88,31 @@ static int match_stat_with_submodule(struct diff_options *diffopt,
return changed;
}
+static int diff_change_helper(struct diff_options *options,
+ unsigned newmode, unsigned dirty_submodule,
+ int changed,
I worry that having three integer parameters next to each other makes it
very easy to mix them up with out getting any errors from the compiler
because the types are all compatible. Could the last two be combined
into a flags argument? A similar issues occurs in
match_stat_with_submodule() in patch 7
Best Wishes
Phillip
struct index_state *istate,
+ struct cache_entry *ce)
+{
+ unsigned int oldmode;
+ const struct object_id *old_oid, *new_oid;
+
+ if (!changed && !dirty_submodule) {
+ ce_mark_uptodate(ce);
+ mark_fsmonitor_valid(istate, ce);
+ if (!options->flags.find_copies_harder)
+ return 1;
+ }
+ oldmode = ce->ce_mode;
+ old_oid = &ce->oid;
+ new_oid = changed ? null_oid() : &ce->oid;
+ diff_change(options, oldmode, newmode,
+ old_oid, new_oid,
+ !is_null_oid(old_oid),
+ !is_null_oid(new_oid),
+ ce->name, 0, dirty_submodule);
+ return 0;
+}
+
int run_diff_files(struct rev_info *revs, unsigned int option)
{
int entries, i;
@@ -105,11 +130,10 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
diff_unmerged_stage = 2;
entries = istate->cache_nr;
for (i = 0; i < entries; i++) {
- unsigned int oldmode, newmode;
+ unsigned int newmode;
struct cache_entry *ce = istate->cache[i];
int changed;
unsigned dirty_submodule = 0;
- const struct object_id *old_oid, *new_oid;
if (diff_can_quit_early(&revs->diffopt))
break;
@@ -245,21 +269,9 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
newmode = ce_mode_from_stat(ce, st.st_mode);
}
- if (!changed && !dirty_submodule) {
- ce_mark_uptodate(ce);
- mark_fsmonitor_valid(istate, ce);
- if (!revs->diffopt.flags.find_copies_harder)
- continue;
- }
- oldmode = ce->ce_mode;
- old_oid = &ce->oid;
- new_oid = changed ? null_oid() : &ce->oid;
- diff_change(&revs->diffopt, oldmode, newmode,
- old_oid, new_oid,
- !is_null_oid(old_oid),
- !is_null_oid(new_oid),
- ce->name, 0, dirty_submodule);
-
+ if (diff_change_helper(&revs->diffopt, newmode, dirty_submodule,
+ changed, istate, ce))
+ continue;
}
diffcore_std(&revs->diffopt);
diff_flush(&revs->diffopt);
