{"thread":{"id":"62953","subject":"[PATCH RFC] builtin/log: include From in git show --format=email","startedAt":"2025-02-14T16:32:27Z","lastAt":"2025-02-18T21:45:37Z","messageCount":4,"participants":["Antonin Godard","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"512428","messageId":"20250214-git-show-from-email-v1-1-df6469635454@bootlin.com","threadId":"62953","inReplyTo":null,"subject":"[PATCH RFC] builtin/log: include From in git show --format=email","fromName":"Antonin Godard","fromEmail":"antonin.godard@bootlin.com","sentAt":"2025-02-14T16:31:13Z","receivedAt":"2025-02-14T16:32:27Z","isPatch":true,"sender":{"key":"antonin.godard@bootlin.com","avatar":null},"body":"Currently, when the format.from and format.forceInBodyFrom options are\nconfigured, the command `git show --format=email ` command does\nnot include \"From: user \" in the body, even though I believe it\nis expected when using this format.\n\nWhile the code exists in log-tree.c to take the identity into account:\n\n if (opt->from_ident.mail_begin && opt->from_ident.name_begin)\n ctx.from_ident = &opt->from_ident;\n\nthe mail_begin and name_begin would always be null pointers\nbecause opt->from_ident is never filled in with the identity from\ncmd_show.\n\nThis commit adds the `from` member to struct log_config, and reads the\nuser configuration to fill in rev.from_ident. It also reuses\nthe existing force_in_body_from variable to take this option into\naccount.\n\nSigned-off-by: Antonin Godard \n---\nThis is probably not the best solution, as we already have the from\nmember in struct format_config, so adding it in struct log_config is a bit\nredundant. However, this is an RFC that hopefully will get my question\nanswered: is my belief to have the From field shown with this command\ncorrect?\n---\n builtin/log.c | 21 +++++++++++++++++++++\n 1 file changed, 21 insertions(+)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex e41f88945e..16a4889c01 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -113,6 +113,7 @@ struct log_config {\n \tint fmt_patch_name_max;\n \tchar *fmt_pretty;\n \tchar *default_date_mode;\n+\tchar *from;\n };\n \n static void log_config_init(struct log_config *cfg)\n@@ -592,6 +593,19 @@ static int git_log_config(const char *var, const char *value,\n \t\tcfg->default_encode_email_headers = git_config_bool(var, value);\n \t\treturn 0;\n \t}\n+\tif (!strcmp(var, \"format.from\")) {\n+\t\tint b = git_parse_maybe_bool(value);\n+\t\tFREE_AND_NULL(cfg->from);\n+\t\tif (b < 0)\n+\t\t\tcfg->from = xstrdup(value);\n+\t\telse if (b)\n+\t\t\tcfg->from = xstrdup(git_committer_info(IDENT_NO_DATE));\n+\t\treturn 0;\n+\t}\n+\tif (!strcmp(var, \"format.forceinbodyfrom\")) {\n+\t\tforce_in_body_from = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n \tif (!strcmp(var, \"log.abbrevcommit\")) {\n \t\tcfg->default_abbrev_commit = git_config_bool(var, value);\n \t\treturn 0;\n@@ -799,6 +813,13 @@ int cmd_show(int argc,\n \t\treturn ret;\n \t}\n \n+\tif (cfg.from) {\n+\t\tif (split_ident_line(&rev.from_ident, cfg.from, strlen(cfg.from)))\n+\t\t\tdie(_(\"invalid ident line: %s\"), cfg.from);\n+\t}\n+\n+\trev.force_in_body_from = force_in_body_from;\n+\n \trev.diffopt.no_free = 1;\n \tfor (i = 0; i < rev.pending.nr && !ret; i++) {\n \t\tstruct object *o = rev.pending.objects[i].item;\n\n---\nbase-commit: 9520f7d9985d8879bddd157309928fc0679c8e92\nchange-id: 20250117-git-show-from-email-62f0e4004d7d\n\nBest regards,\n-- \nAntonin Godard \n\n"},{"id":"512437","messageId":"xmqqh64wl0wl.fsf@gitster.g","threadId":"62953","inReplyTo":"20250214-git-show-from-email-v1-1-df6469635454@bootlin.com","subject":"Re: [PATCH RFC] builtin/log: include From in git show --format=email","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-02-14T20:35:54Z","receivedAt":"2025-02-14T20:35:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Antonin Godard writes:\n\n> Currently, when the format.from and format.forceInBodyFrom options are\n> configured, the command `git show --format=email ` command does\n> not include \"From: user \" in the body, even though I believe it\n> is expected when using this format.\n\nAren't \"format.*\" configuration variables for \"git format-patch\",\nand not \"git show\" or \"git log\"?\n\nI do not see there is anything that needs fixing, but I may be\nmissing something.\n"},{"id":"512509","messageId":"D7UQ9PQOTOLT.26JEMSNOV4WP6@bootlin.com","threadId":"62953","inReplyTo":"xmqqh64wl0wl.fsf@gitster.g","subject":"Re: [PATCH RFC] builtin/log: include From in git show --format=email","fromName":"Antonin Godard","fromEmail":"antonin.godard@bootlin.com","sentAt":"2025-02-17T12:42:47Z","receivedAt":"2025-02-17T12:42:50Z","isPatch":true,"sender":{"key":"antonin.godard@bootlin.com","avatar":null},"body":"Hi Junio,\n\nOn Fri Feb 14, 2025 at 9:35 PM CET, Junio C Hamano wrote:\n> Antonin Godard writes:\n>\n>> Currently, when the format.from and format.forceInBodyFrom options are\n>> configured, the command `git show --format=email ` command does\n>> not include \"From: user \" in the body, even though I believe it\n>> is expected when using this format.\n>\n> Aren't \"format.*\" configuration variables for \"git format-patch\",\n> and not \"git show\" or \"git log\"?\n>\n> I do not see there is anything that needs fixing, but I may be\n> missing something.\n\nThis is what the documentation seems to imply, but builtin/log.c uses these\nconfiguration variables in git_log_config(), for example. In the same file,\ncmd_show() uses git_log_config().\n\ngit show can be used with --format=email, and you can use the format.* options\nto control the output of git show --format=email . For example:\n\n git -c format.subjectPrefix=FOO show --format=email HEAD\n\nWill affect the subject.\n\nWith this reasoning in mind, I thought \"git show --format=email\" should also\nbenefit from the format.from and format.forceInBodyFrom variables, to correctly\ndisplay the output.\n\nI should also mention that b4[1] uses `git show --format=email`[2] to generate the\npatches. So at the moment any user using b4 does not have their format.from\nvariables satisfied. This is how I spotted this behavior, initially.\n\n[1]: https://b4.docs.kernel.org/en/latest\n[2]: https://git.kernel.org/pub/scm/utils/b4/b4.git/tree/src/b4/__init__.py#n3487\n\nAntonin\n\n-- \nAntonin Godard, Bootlin\nEmbedded Linux and Kernel engineering\nhttps://bootlin.com\n"},{"id":"512642","messageId":"xmqqh64rj5a9.fsf@gitster.g","threadId":"62953","inReplyTo":"D7UQ9PQOTOLT.26JEMSNOV4WP6@bootlin.com","subject":"Re: [PATCH RFC] builtin/log: include From in git show --format=email","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-02-18T21:45:34Z","receivedAt":"2025-02-18T21:45:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Antonin Godard\" writes:\n\n> This is what the documentation seems to imply, but builtin/log.c uses these\n> configuration variables in git_log_config(), for example. In the same file,\n> cmd_show() uses git_log_config().\n\n\"imply\"? The documentation says so because the command was designed\nto work like so when the feature was added in mid 2022.\n\n 34bc1b10 (format-patch: allow forcing the use of in-body From: header, 2022-08-29)\n d5fc07df (format-patch: learn format.forceInBodyFrom configuration variable, 2022-08-29)\n\nPlease refrain from using words that imply value-judgement from your\nanalysis and stick to the facts; I'll try to do so in my messages,\ntoo.\n\nAs the log family of commands happen to share much of the\nimplementation, their code paths pass the log-config structure which\nis a mixed bag. It does not necessarily mean all commands in the\nfamily use all the members in the structure.\n\n> With this reasoning in mind, I thought \"git show --format=email\" should also\n> benefit from the format.from and format.forceInBodyFrom variables, to correctly\n> display the output.\n\n\"correctly\"? \n\nChanging the behaviour retroactively would mean that those who have\nbeen relying on the fact that in-body header configuration meant for\n\"git format-patch\" does not affect \"git log\" would suddenly start to\nsee their output differently. So I'd worry how bad the downside\nwould be.\n\nHaving said all that.\n\nIn hindsight, or if we pretend we were in 2005 and starting the\nproject again from scratch, if we did not have any \"format.*\"\nconfiguration variables and instead all of these were \"log.*\", I'd\nagree with everything you said in the message I am responding to.\nIn such a hypothetical world, we may not even have added a separate\n\"format-patch\" command, but used \"git log\" with an option to spit\nout its output into an individual file for every commit.\n\n"}]}