# [PATCH] format-patch: correct documentation of --thread without an argument 2 messages from 2023-04-03 to 2023-04-03. Participants: Alex Henrie, Glen Choo. Thread: https://gitlist.dev/t/59526 ## Alex Henrie, 2023-04-03 04:07 Subject: [PATCH] format-patch: correct documentation of --thread without an argument Message-ID: <20230403040724.642513-1-alexhenrie24@gmail.com> URL: https://gitlist.dev/e/20230403040724.642513-1-alexhenrie24%40gmail.com ``` 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`. [1] https://lore.kernel.org/git/CAMMLpeS3+NUQa2oqpHKVo3yWQNVMgkEXrs4U5_ggvk31yQbezQ@mail.gmail.com/ Signed-off-by: Alex Henrie --- Documentation/git-format-patch.txt | 3 +-- t/t4014-format-patch.sh | 5 +++++ 2 files changed, 6 insertions(+), 2 deletions(-) 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`. + 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 +' + cat >expect.in-reply-to < -- 2.40.0 ``` ## Glen Choo, 2023-04-03 17:48 Subject: Re: [PATCH] format-patch: correct documentation of --thread without an argument Message-ID: URL: https://gitlist.dev/e/kl6lpm8kzwjz.fsf%40chooglen-macbookpro.roam.corp.google.com In-Reply-To: <20230403040724.642513-1-alexhenrie24@gmail.com> ``` Alex Henrie 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 ```