Hi Calvin
On 07/02/2023 18:17, Calvin Wan wrote:
Flatten out the if statements in match_stat_with_submodule so the
logic is more readable and easier for future patches to add to.
orig_flags didn't need to be set if the cache entry wasn't a
GITLINK so defer setting it.
Signed-off-by: Calvin Wan <calvinwan@xxxxxxxxxx>
---
diff-lib.c | 28 +++++++++++++++++-----------
1 file changed, 17 insertions(+), 11 deletions(-)
diff --git a/diff-lib.c b/diff-lib.c
index 7101cfda3f..e18c886a80 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -73,18 +73,24 @@ static int match_stat_with_submodule(struct diff_options *diffopt,
unsigned *dirty_submodule)
{
int changed = ie_match_stat(diffopt->repo->index, ce, st, ce_option);
- if (S_ISGITLINK(ce->ce_mode)) {
- struct diff_flags orig_flags = diffopt->flags;
- if (!diffopt->flags.override_submodule_config)
- set_diffopt_flags_from_submodule_config(diffopt, ce->name);
- if (diffopt->flags.ignore_submodules)
- changed = 0;
- else if (!diffopt->flags.ignore_dirty_submodules &&
- (!changed || diffopt->flags.dirty_submodules))
- *dirty_submodule = is_submodule_modified(ce->name,
- diffopt->flags.ignore_untracked_in_submodules);
- diffopt->flags = orig_flags;
+ struct diff_flags orig_flags;
+
+ if (!S_ISGITLINK(ce->ce_mode))
+ return changed;
+
+ orig_flags = diffopt->flags;
+ if (!diffopt->flags.override_submodule_config)
+ set_diffopt_flags_from_submodule_config(diffopt, ce->name);
+ if (diffopt->flags.ignore_submodules) {
+ changed = 0;
+ goto cleanup;
Looking ahead to patch 7 there are no new uses of the "cleanup" label so
I think it would be simpler to leave the code as it was, rather than
changing the "else if" below to "if" and adding the goto here.
Best Wishes
Phillip
}
+ if (!diffopt->flags.ignore_dirty_submodules &&
+ (!changed || diffopt->flags.dirty_submodules))
+ *dirty_submodule = is_submodule_modified(ce->name,
+ diffopt->flags.ignore_untracked_in_submodules);
+cleanup:
+ diffopt->flags = orig_flags;
return changed;
}
