{"thread":{"id":"59526","subject":"[PATCH] format-patch: correct documentation of --thread without an argument","startedAt":"2023-04-03T04:10:03Z","lastAt":"2023-04-03T17:50:15Z","messageCount":2,"participants":["Alex Henrie","Glen Choo"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"474672","messageId":"20230403040724.642513-1-alexhenrie24@gmail.com","threadId":"59526","inReplyTo":null,"subject":"[PATCH] format-patch: correct documentation of --thread without an argument","fromName":"Alex Henrie","fromEmail":"alexhenrie24@gmail.com","sentAt":"2023-04-03T04:07:24Z","receivedAt":"2023-04-03T04:10:03Z","isPatch":true,"sender":{"key":"alexhenrie24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5951993?v=4"},"body":"In Git, almost all command line flags unconditionally override the\ncorresponding config option.[1] Add a test to confirm that this is the\ncase for `git format-patch --thread`.\n\n[1] https://lore.kernel.org/git/CAMMLpeS3+NUQa2oqpHKVo3yWQNVMgkEXrs4U5_ggvk31yQbezQ@mail.gmail.com/\n\nSigned-off-by: Alex Henrie <alexhenrie24@gmail.com>\n---\n Documentation/git-format-patch.txt | 3 +--\n t/t4014-format-patch.sh            | 5 +++++\n 2 files changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex dfcc7da4c2..ed299e077d 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -173,8 +173,7 @@ series, where the head is chosen from the cover letter, the\n threading makes every mail a reply to the previous one.\n +\n The default is `--no-thread`, unless the `format.thread` configuration\n-is set.  If `--thread` is specified without a style, it defaults to the\n-style specified by `format.thread` if any, or else `shallow`.\n+is set.  `--thread` without an argument is equivalent to `--thread=shallow`.\n +\n Beware that the default for 'git send-email' is to thread emails\n itself.  If you want `git format-patch` to take care of threading, you\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex 8c3d06622a..b27a72f78a 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -470,6 +470,11 @@ test_expect_success 'thread' '\n \tcheck_threading expect.thread --thread main\n '\n \n+test_expect_success '--thread overrides format.thread=deep' '\n+\ttest_config format.thread deep &&\n+\tcheck_threading expect.thread --thread main\n+'\n+\n cat >expect.in-reply-to <<EOF\n ---\n Message-Id: <0>\n-- \n2.40.0\n\n"},{"id":"474699","messageId":"kl6lpm8kzwjz.fsf@chooglen-macbookpro.roam.corp.google.com","threadId":"59526","inReplyTo":"20230403040724.642513-1-alexhenrie24@gmail.com","subject":"Re: [PATCH] format-patch: correct documentation of --thread without an argument","fromName":"Glen Choo","fromEmail":"chooglen@google.com","sentAt":"2023-04-03T17:48:32Z","receivedAt":"2023-04-03T17:50:15Z","isPatch":true,"sender":{"key":"glencbz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58092771?v=4"},"body":"Alex Henrie <alexhenrie24@gmail.com> writes:\n\n> In Git, almost all command line flags unconditionally override the\n> corresponding config option.[1] Add a test to confirm that this is the\n> case for `git format-patch --thread`.\n\n[...]\n\n> diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\n> index dfcc7da4c2..ed299e077d 100644\n> --- a/Documentation/git-format-patch.txt\n> +++ b/Documentation/git-format-patch.txt\n> @@ -173,8 +173,7 @@ series, where the head is chosen from the cover letter, the\n>  threading makes every mail a reply to the previous one.\n>  +\n>  The default is `--no-thread`, unless the `format.thread` configuration\n> -is set.  If `--thread` is specified without a style, it defaults to the\n> -style specified by `format.thread` if any, or else `shallow`.\n> +is set.  `--thread` without an argument is equivalent to `--thread=shallow`.\n>  +\n\nPerhaps more important than the test is that you're fixing blatantly\nwrong documentation :)\n\nThis leads to the questions of:\n\na) what is the intended behavior is supposed to be: the documented one,\n or the actual one?\nb) if it is supposed to be the documented one, when did it break?\n\nAFAICT, this has _always_ been broken since it was introduced way way\nback in 30984ed2e9 (format-patch: support deep threading, 2009-02-19).\n\n\t\telse if (!strcmp(argv[i], \"--thread\")\n\t\t\t|| !strcmp(argv[i], \"--thread=shallow\"))\n\t\t\tthread = THREAD_SHALLOW;\n\t\telse if (!strcmp(argv[i], \"--thread=deep\"))\n\t\t\tthread = THREAD_DEEP;\n\nGiven how long this has been broken for without anyone noticing, we\nshould probably just document whatever everyone has been using instead\nof wondering what the original intended behavior is.\n\n>  Beware that the default for 'git send-email' is to thread emails\n>  itself.  If you want `git format-patch` to take care of threading, you\n> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\n> index 8c3d06622a..b27a72f78a 100755\n> --- a/t/t4014-format-patch.sh\n> +++ b/t/t4014-format-patch.sh\n> @@ -470,6 +470,11 @@ test_expect_success 'thread' '\n>  \tcheck_threading expect.thread --thread main\n>  '\n>  \n> +test_expect_success '--thread overrides format.thread=deep' '\n> +\ttest_config format.thread deep &&\n> +\tcheck_threading expect.thread --thread main\n> +'\n> +\n\nThe previous test is entirely in the context lines, and we can see that\n\"check_threading expect.thread --thread main\" passes regardless of the\nvalue of \"format.thread\". Nice.\n\nThanks!\n\nReviewed-By: Glen Choo <chooglen@google.com>\n\n"}]}