Sergey Organov <sorganov@xxxxxxxxx> writes:
> int cmd_diff_index(int argc, const char **argv, const char *prefix)
> {
> struct rev_info rev;
> unsigned int option = 0;
> - int i;
> int result;
>
> if (argc == 2 && !strcmp(argv[1], "-h"))
> @@ -27,17 +53,16 @@ int cmd_diff_index(int argc, const char **argv, const char *prefix)
> rev.abbrev = 0;
> prefix = precompose_argv_prefix(argc, argv, prefix);
>
> + /*
> + * It's essential to parse our distinct options before calling
> + * setup_revisions(), for the latter not to see "-m".
> + */
> + argc = parse_distinct_options(argc, argv, &rev, &option);
> argc = setup_revisions(argc, argv, &rev, NULL);
This change is risky, as the loop below (which this patch moves to
parse_distinct_options()) has no knowledge of other options that
setup_revisions() helper is prepared to handle and that takes an
argument. When parsing "git cmd --opt --cached A", setup_revisions()
may know that --opt takes an argument and eat both (i.e. the
"--cached" is not an option but an arg given to "--opt"), but the
new parse_distinct_options() helper does not; it will happily skip
"--opt" and leave it in, mistake "--cached" as an option and remove,
and instead make "A" the arg given to "--opt".
Picking up the remnant _after_ setup_revisions() ate what it
understands would not have such a downside, as long as none of our
"distinct options" take any argument.
Can't we make "-m means something special for diff-index" without
butchering the command line processing in this step? diff-index
does not care about --diff-merges, so letting setup_revisions()
remember only the fact that "-m" was given while parsing, and then
postprocess what "-m" means depending on the command (i.e. everybody
else would treat it as a short-hand for "--diff-merges=m" plus "we
need some form of diff output, while allowing "diff-index" to treat
it differently) should not be rocket science.
Thanks.
