On Thu, Mar 16, 2023 at 4:39 PM Glen Choo <chooglen@xxxxxxxxxx> wrote: > > Alex Henrie <alexhenrie24@xxxxxxxxx> writes: > > >> Your doc patch explains the rules pretty clearly, but perhaps it doesn't > >> explain this mental model clearly enough, hence the confusion. If we > >> don't find a good way to communicate this (I think it is clear, but > >> other reviewers seem yet unconvinced), I wouldn't mind taking Phillip's > >> suggestion to have "--rebase-merges" override > >> "rebase.rebaseMerges='specific-mode'". > > > > I got the impression that everyone, including Phillip,[1] already > > agrees that the proposed documentation is clear about the interaction > > between the config option and the command line option. However, it is > > a little weird when you consider that other flags with optional > > arguments, like `git branch --track`, unconditionally override their > > corresponding config options.[2] > > Ah, I didn't consider other options like `git branch --track`. I haven't > looked into what is the norm, but I think we should follow it (whatever > it is). > > If other reviewers have a strong idea of what this norm is, I am happy > to defer to them. If not, I can look into it given some time. > > > Let me ask a different but related question: If we add a > > rebase-evil-merges mode, do you think that would be orthogonal to the > > rebase-cousins mode? > > I am not an expert on this, so perhaps my opinion isn't that important > ;) > > My understanding is that `[no-]rebase-cousins` affects which commits get > rebased and onto where, whereas `rebase-evil-merges` would affect how > the merge commits are generated (by rebasing the evil or by simply > recreating the merges). From that perspective, it seems like yes, the > two would be orthogonal. > > Hm. Maybe this means that we'd be introducing a new option, and that my > hunch that we would change the default to `rebase-evil-merges` is more > wrong than I expected. > > Though I guess this doesn't really affects what we do with the CLI > options _now_, since the discussion is about what we do about defaults, > and what the default is is quite immaterial. I looked through the code and documentation and found 12 good examples of command line flags with an optional argument that always override their corresponding config options whether or not the optional argument is given: git -c branch.autoSetupMerge=inherit branch --track mynewbranch git -c commit.gpgSign=mykey commit --gpg-sign git -c core.sharedRepository=0666 init --shared . git -c diff.ignoreSubmodules=untracked diff --ignore-submodules git -c diff.submodule=diff diff --submodule git -c format.attach=myboundary format-patch --attach HEAD^ git -c format.from='Dang Git <dang@xxxxxxxxxxx>' format-patch --from HEAD^ git -c format.thread=deep format-patch --thread HEAD~3 git -c gc.pruneExpire=1.week.ago gc --prune git -c log.decorate=full log --decorate git -c merge.log=1 merge --log mytopicbranch git -c pull.rebase=interactive pull --rebase I found 6 other similar examples where the config option is a yes/no/auto trilean, but I don't think those are good examples because it makes total sense for a command line flag without an argument to override a nonspecific "auto" value in the configuration: git -c color.diff=auto diff --color | less git -c color.grep=auto grep --color . | less git -c color.showbranch=auto show-branch --color master mytopicbranch | less git -c color.ui=auto log --color | less git -c fetch.recurseSubmodules=on-demand fetch --recurse-submodules git -c push.gpgSign=if-asked push --signed I found 1 good example where the config option does make a difference if the optional argument is omitted: git -c diff.colorMoved=plain diff --color-moved And I found 1 other similar example, but it's not good a example because the config option doesn't do anything unless the command line flag is specified: git -c diff.dirstat=lines diff --dirstat Given 12 good examples versus 1 good counterexample, I would say that the established convention is for the command line flag to always override the config option. Please let me know if there are other examples that I missed. It's worth noting that the documentation for `git format-patch --thread` explicitly says that format.thread takes precedence over --thread without an argument, but that is not correct: When I tested it, the command line argument always took precedence. Another problem is that the documentation for `git pack-objects` does not even mention that --unpack-unreachable has an optional argument, and it says that the argument to --cruft-expiration is required when it is not. I'm not going to fix all the documentation problems, at least not right now. However, in order to follow the established convention for flags with optional arguments, and because we probably aren't going to use rebase.rebaseMerges to facilitate a future change of defaults, I will redo the patch so that --rebase-merges without an argument takes precedence over rebase.rebaseMerges. Thanks for the feedback. I feel more confident about the way forward now. -Alex
