Junio C Hamano <gitster@xxxxxxxxx> writes: > Sergey Organov <sorganov@xxxxxxxxx> writes: > >> Move specific handling of "-m" for diff-index to diff-index.c, so >> diff-merges is left to handle only diff for merges options. >> >> Being a better design by itself, this is especially essential in >> preparation for letting -m imply -p, as "diff-index -m" obviously >> should not imply -p, as it's entirely unrelated. >> >> To handle this, in addition to moving specific diff-index "-m" code >> out of diff-merges, we introduce new >> >> diff_merges_suppress_options_parsing() >> >> and call it before generic options processing in cmd_diff_index(). > > This change has a small but obvious fallout. > > $ git diff-index -c --cached HEAD^ > > now starts failing loudly. Earlier, it silently fell back to > "combined" diff of one parent, which is "-p". > > I think the end result is good (and luckily, "DIFF FORMAT FOR > MERGES" section explicitly limits "-c" and "--cc" to diff-tree, > diff-files and diff (and by implication excludes diff-index) so I am > sure there are small but non-zero number of people somewhere in the > world who has "diff-index -c" in their scripts that suddenly starts > failing with the version of Git with this change, but we can just > say their use was broken ;-) > > Having said all that, I have to wonder if it still is needed to keep > the "diff-index -m" working, or we would be better off breaking it > to avoid a change like this that makes us bend over backwards to > work around the command line parsing infrastructure. > > The only reason why "diff-index -m" exists is because it was part of > the idea Linus had for the merge implementation that we ended up > deciding not taking, where merges and possibly other bulk operations > that would affect the working tree is done in a separate, temporary > directory that is sparsely populated, the user is asked to edit away > conflicts in the temporary directory and expected to monitor his or > her own progress using "diff-index -m". Our plan was to populate > such a temporary directory with only paths that are involved in the > operation in progress, without instantiating paths that are not > touched, so "treat missing files as if they haven't been modified" > was a handy ingredient for such a mode of operation. > > But we ended up going with a different design, in which the main > working tree area is used to perform merges and to resolve > conflicts, which made this "pretend missing files as unmodified" > unnecessary feature. In the end, we made a good move, as the > current design allows users to verify their changes in the context > of a full checkout (e.g. "make" would not have been a good way to > validate the conflict resolution if it is done in a separate > temporary directory that is sparsely populated with only the paths > involved in the merge---you need all files for building, including > the ones that are not modified, and "make" does not know to treat > missing files as if they are unmodified). This could be a good thing to do, but as I wrote in the description, there are in fact other commands that don't need diff-merges options and currently just eat them silently instead of bailing out. It's likely that 90% of uses of setup_revisions() don't need diff-merges, so a feature to exclude diff-merges parsing seems to be useful by itself. Thanks, -- Sergey Organov
