On Wed, Dec 9, 2020 at 11:44 AM Sergey Organov <sorganov@xxxxxxxxx> wrote: > > Junio C Hamano <gitster@xxxxxxxxx> writes: > > [...] > > > By the time the change to make "--cc" imply "-p" was introduced, it > > was pretty much given that "-m -p" was useful to anybody, unless you > > are consuming these individual patches in a script or something like > > that. So simply I didn't even think of making "-m" imply "-p". It > > would be logical to make it so, but it would not add much practical > > value, I would have to say. > > I need some help here. > > Looking at the code and trying to follow the flow, I can't figure what > rev->diff flag is for? Why rev->diffopt.output_format, that actually > affects the output, is not enough? I'm not a revision walking expert, but to the best of my understanding... Showing a diff is not the only reason you might need to compute one. You may also need to compute them if you are filtering commits by pathspec (-- $filename), using the pickaxe (-S foo), checking if commits are cherry-picks (--cherry-mark), checking for commits with certain type of file changes (--diff-filter=A), selecting commits that modified a certain function (-L :funcname:filename --no-patch), or others I've overlooked. None of these cause a diff to be shown. I don't know if all these set rev->diff to 1 or if they special case some other way, but I suspect that rev->diff exists as a shorthand for "need a diff", so that the code can check for it without having to check a half dozen special conditions. >> My confusion originates from the fact that the code in revision.c sets > rev->diff to 1 for -c/--cc , while it doesn't set it for -m, and this > was the case *before* -c/--cc started to imply -p and -m. > > It seems that the only place where rev->diff is tested is at the start > of log_tree_diff(), and even there its absence could be ignored when > rev->diffopt.flags.exit_with_status is set. rev->diffopt.flags.exit_with_status seems quite unlikely to be set, though. That setting was added with the --exit-code flag to git log in 2008 (in the pm/log-exit-code topic), but was never documented (other than to say it's incompatible with --check), the commit message adding it doesn't say what behavior was intended, and the commit message which added it added no regression tests either. I know what diff --exit-code does, but I'm really not sure what git-log's --exit-code does (random guess: sets the exit code to an OR of what git diff would have shown for any one of the commits shown?). Since I don't know and can't even figure it out looking at the commits in question, I suspect there aren't too many users out there using it. As such, I suspect rev->diffopt.flags.exit_with_status will be 0 most of the time and that the relevant check at the top of log_tree_diff() really is the "if (!opt->diff)" part of it. > Is rev->diff an optimization, does it play another significant role, or > is it a remnant?
