Denton Liu <liu.denton@xxxxxxxxx> writes:
> Instead of parsing the `--no-index` option with a plain strcmp, use
> parse_options() to parse options. This allows us to easily add more
> options in a future commit.
>
> As a result of this change, `--no-index` is removed from `argv` so, as a
> result, we no longer need to handle it in diff_no_index().
>
> Signed-off-by: Denton Liu <liu.denton@xxxxxxxxx>
> ---
> builtin/diff.c | 33 ++++++++++++++++++++++-----------
> diff-no-index.c | 15 +++------------
> 2 files changed, 25 insertions(+), 23 deletions(-)
>
> diff --git a/builtin/diff.c b/builtin/diff.c
> index cb98811c21..0e086ed7c4 100644
> --- a/builtin/diff.c
> +++ b/builtin/diff.c
> @@ -373,6 +373,13 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
> int nongit = 0, no_index = 0;
> int result = 0;
> struct symdiff sdiff;
> + struct option options[] = {
> + OPT_SET_INT_F(0, "no-index", &no_index,
> + N_("compare the given paths on the filesystem"),
> + DIFF_NO_INDEX_EXPLICIT,
> + PARSE_OPT_NONEG),
> + OPT_END(),
> + };
What's the reasoning behind teaching the "--merge-base" option only
to "diff" and not allowing "diff-index" and "diff-tree" to also
benefit from the new support? It should be taught to be handled by
diff.c::diff_opt_parse() instead, like all other "diff" options. I
simply do not see what makes "--merge-base" so special compared to
other options like "-R", "--color-words", etc.
And with that, this step will become totally unnecessary, because
the options[] array will still have only the "no-index" option and
nothing else.
UNLESS we are planning to use this array to parse all diff options
here, whether it is for the no-index mode or the normal invocation
of diff, but that is unlikely to happen---we'd still share many
options between the plumbing "diff-*" family and Porcelain "diff"
commands.
Thanks.
