Jacob Keller <jacob.e.keller@xxxxxxxxx> writes: > @@ -2305,6 +2311,15 @@ static void builtin_diff(const char *name_a, > struct strbuf header = STRBUF_INIT; > const char *line_prefix = diff_line_prefix(o); > > + diff_set_mnemonic_prefix(o, "a/", "b/"); > + if (DIFF_OPT_TST(o, REVERSE_DIFF)) { > + a_prefix = o->b_prefix; > + b_prefix = o->a_prefix; > + } else { > + a_prefix = o->a_prefix; > + b_prefix = o->b_prefix; > + } > + Hmph, is it safe to raise this when SUBMODULE_DIFF is not in effect? Not objecting, just asking. > if (DIFF_OPT_TST(o, SUBMODULE_LOG) && > ... > + } else if (DIFF_OPT_TST(o, SUBMODULE_DIFF) && This makes clear that SUBMODULE_LOG and SUBMODULE_DIFF should not be independent bits in the diff-opt flag word. We used to run something like "log --oneline --left-right A...B" and your new code runs "diff A B", but the next month somebody else would want to do "log -p --left-right A...B" or something else, and they are clearly mutually exclusive. > diff --git a/diff.h b/diff.h > index 6a91a1139686..65df44b1fcba 100644 > --- a/diff.h > +++ b/diff.h > @@ -69,7 +69,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data) > #define DIFF_OPT_FIND_COPIES_HARDER (1 << 6) > #define DIFF_OPT_FOLLOW_RENAMES (1 << 7) > #define DIFF_OPT_RENAME_EMPTY (1 << 8) > -/* (1 << 9) unused */ > +#define DIFF_OPT_SUBMODULE_DIFF (1 << 9) So I'd really prefer not to see this change; instead, we should move in the direction where we _REMOVE_ DIFF_OPT_SUBMODULE_LOG from these, and introduce an enum to hold "how would we show submodule changes" in the diff_options structure. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html