On 1/5/2018 5:22 PM, Junio C Hamano wrote:
Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes:
diff --git a/diff-lib.c b/diff-lib.c
index 8104603a3..13ff00d81 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -95,6 +95,9 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
diff_set_mnemonic_prefix(&revs->diffopt, "i/", "w/");
+ if (!(option & DIFF_SKIP_FSMONITOR))
+ refresh_fsmonitor(&the_index);
+
if (diff_unmerged_stage < 0)
diff_unmerged_stage = 2;
I read over this hunk five times, and only now am I able to wrap my head
around this: if we do *not* want to skip the fsmonitor data, we refresh
the fsmonitor data in the index.
That feels a bit like an unneeded double negation. Speaking for myself, I
would prefore `DIFF_IGNORE_FSMONITOR` instead, it would feel less like a
double negation then. But I am not a native speaker, so I might be wrong.
I do find the logic a bit convoluted with double negative.
It's great to see more use of the fsmonitor data. Thanks for doing this!
I agree with the sentiment that the logic as written is confusing. I'll
also point out that DIFF_IGNORE_FSMONITOR would be more consistent with
the similar CE_MATCH_IGNORE_FSMONITOR flag and logic.
I'm also confused why we would not want to use the fsmonitor data in the
'add' case. When would you ever need to add a file that had not been
modified?
