Alex Henrie <alexhenrie24@xxxxxxxxx> writes: > In Git, almost all command line flags unconditionally override the > corresponding config option.[1] Add a test to confirm that this is the > case for `git format-patch --thread`. [...] > diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt > index dfcc7da4c2..ed299e077d 100644 > --- a/Documentation/git-format-patch.txt > +++ b/Documentation/git-format-patch.txt > @@ -173,8 +173,7 @@ series, where the head is chosen from the cover letter, the > threading makes every mail a reply to the previous one. > + > The default is `--no-thread`, unless the `format.thread` configuration > -is set. If `--thread` is specified without a style, it defaults to the > -style specified by `format.thread` if any, or else `shallow`. > +is set. `--thread` without an argument is equivalent to `--thread=shallow`. > + Perhaps more important than the test is that you're fixing blatantly wrong documentation :) This leads to the questions of: a) what is the intended behavior is supposed to be: the documented one, or the actual one? b) if it is supposed to be the documented one, when did it break? AFAICT, this has _always_ been broken since it was introduced way way back in 30984ed2e9 (format-patch: support deep threading, 2009-02-19). else if (!strcmp(argv[i], "--thread") || !strcmp(argv[i], "--thread=shallow")) thread = THREAD_SHALLOW; else if (!strcmp(argv[i], "--thread=deep")) thread = THREAD_DEEP; Given how long this has been broken for without anyone noticing, we should probably just document whatever everyone has been using instead of wondering what the original intended behavior is. > Beware that the default for 'git send-email' is to thread emails > itself. If you want `git format-patch` to take care of threading, you > diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh > index 8c3d06622a..b27a72f78a 100755 > --- a/t/t4014-format-patch.sh > +++ b/t/t4014-format-patch.sh > @@ -470,6 +470,11 @@ test_expect_success 'thread' ' > check_threading expect.thread --thread main > ' > > +test_expect_success '--thread overrides format.thread=deep' ' > + test_config format.thread deep && > + check_threading expect.thread --thread main > +' > + The previous test is entirely in the context lines, and we can see that "check_threading expect.thread --thread main" passes regardless of the value of "format.thread". Nice. Thanks! Reviewed-By: Glen Choo <chooglen@xxxxxxxxxx>
