{"thread":{"id":"55635","subject":"[PATCH v1 0/8] format-patch: introduce --confirm-overwrite","startedAt":"2021-05-06T16:52:27Z","lastAt":"2021-05-11T06:37:13Z","messageCount":34,"participants":["Firmin Martin","Junio C Hamano","Felipe Contreras","Bagas Sanjaya","Jeff King","Denton Liu","Sergey Organov","Ævar Arnfjörð Bjarmason"],"isPatch":true,"patchVersion":1,"patchTotal":8},"messages":[{"id":"423800","messageId":"20210506165102.123739-1-firminmartin24@gmail.com","threadId":"55635","inReplyTo":null,"subject":"[PATCH v1 0/8] format-patch: introduce --confirm-overwrite","fromName":"Firmin Martin","fromEmail":"firminmartin24@gmail.com","sentAt":"2021-05-06T16:50:54Z","receivedAt":"2021-05-06T16:52:27Z","isPatch":true,"sender":{"key":"firminmartin24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20540328?v=4"},"body":"Hi all,\n\nThis patch series aims to prevent git-format-patch from overwriting cover\nletter and patches silently. It also tries to fix git_prompt but there\nare still some works to do (see NEED HELPS).\n\nBACKGROUND\n==========\n\nCurrently, git-format-patch, along with the option --cover-letter,\nunconditionally overwrites a cover letter with the same name (if\npresent). Although this is a desired behaviour for patches which are\nauto-generated from Git commits log, it might not be the case for a\ncover letter whose the content is meticulously written manually.\n\nParticulary, this behaviour could be awkward in the following\nhypothetical situations:\n\n* The user can easily erase a cover letter coming from prior versions or\nanother patch series by reusing an old command line (e.g. autocompleted\nfrom the shell history).\n\n* Assuming that the user is writing a cover letter and realizes that\nsmall changes should be made. They make the change, amend and\nformat-patch again to regenerate patches. If it happens that they use\nthe same command again (e.g. with --cover-letter), the cover letter\nbeing written is gone.\n\nThis patch series addresses this kind of issue by asking confirmation\nfrom the user whenever a cover letter or a patch is subject to be\noverwritten.\n\nCHANGES\n=======\n\nSubstantial changes include:\n\n* New options format.confirmOverwrite and --confirm-overwrite=\n which decide when to prompt the user for confirmation to overwrite\n patches or cover letter. There are three possible values \"always\",\n \"never\", and \"cover\". The default value is \"cover\", which means \"only\n prompt when a cover letter already exists\". This is a breaking change:\n prior this patch series, the behaviour of Git corresponds to\n format.confirmOverwrite = never.\n* Some tests (t4014) who overwrites cover letter in silence are fixed to\n address this breaking change.\n* compat/terminal.c::git_terminal_prompt is modified to accept input\n from pipe. This makes Git subcommands using prompt.c::git_prompt\n testable. As far as I know, the two occurrences of git_prompt are from\n credentials.c and builtin/git-bisect--helper.c, and they are not\n tested so far (see BUG below).\n\nNEED HELPS\n==========\n\nI would appreciate some guidance on the following points.\n\ngit_prompt\n~~~~~~~~~~\nThere are currently three issues related to changes made into\ngit_terminal_prompt (see patch #1).\n\n1. All tests have passed in my machine (Ubuntu 20.04), but CI reported tests\n in t4014 that all failed at the same point:\n\n fopen(\"/dev/tty\", \"w\") in Linux and OSX, and fopen(\"CONOUT$\", \"wt\") in Windows.\n \n - Linux error: No such device or address\n - Windows error: Invalid argument\n - OSX error: Device not configured\n\n I also observed that one cannot write into /dev/stderr. Is this a CI\n specific issue ? (see patch #5 for the new tests).\n\n2. (BUG) While all tests passed locally, I realized that \"git push\" (to\n Github) can't read my credentials. That's because, for some obscure\n reason, isatty(0) returns 0 when \"git push\" is asking for credentials.\n Consequently, the new code will read input from stdin, which is wrong.\n\n What would be the reason that causes isatty(0) returns 0 when\n git-push calls credential.c::credential_getpass ?\n\n3. Finally, while testing git_prompt's caller works, it uglifies the\n output of \"make prove\" (as git_prompt writes into /dev/tty). I tried to\n \"redirect\" the output of /dev/tty to /dev/null to silent it using the\n \"script\" command (along the line of \"script -e -c 'echo Y | git ...'\n /dev/stderr 2>/dev/null\"). Unfortunately, it is not portable\n (specifically, OSX doesn't have the option -e, and \"script\" is not\n available under Windows). Are there other ways to hide the output of\n the prompts ?\n\nI will squash patches #2-#8 for the last version.\n\nThanks,\n\nFirmin\n\n\nFirmin Martin (8):\n compat/terminal: let prompt accept input from pipe\n format-patch: confirmation whenever patches exist\n format-patch: add config option confirmOverwrite\n format-patch: add the option --confirm-overwrite\n t4014: test patches overwrite confirmation\n t4014: fix tests overwriting cover letter in silent\n doc/git-format-patch: describe --confirm-overwrite\n config/format: describe format.confirmOverwrite\n\n Documentation/config/format.txt | 5 +++\n Documentation/git-format-patch.txt | 20 +++++\n builtin/log.c | 65 ++++++++++++--\n compat/terminal.c | 47 ++++++----\n t/t4014-format-patch.sh | 140 +++++++++++++++++++++++++++--\n 5 files changed, 247 insertions(+), 30 deletions(-)\n\n-- \n2.31.1.449.gf23dcf53bc\n\n"},{"id":"423801","messageId":"20210506165102.123739-2-firminmartin24@gmail.com","threadId":"55635","inReplyTo":"20210506165102.123739-1-firminmartin24@gmail.com","subject":"[PATCH v1 1/8] compat/terminal: let prompt accept input from pipe","fromName":"Firmin Martin","fromEmail":"firminmartin24@gmail.com","sentAt":"2021-05-06T16:50:55Z","receivedAt":"2021-05-06T16:52:44Z","isPatch":true,"sender":{"key":"firminmartin24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20540328?v=4"},"body":"Currently, git_prompt ignores input coming from anywhere other than\nterminal (pipe, redirection etc.) meaning that standard prompt\nauto-answering methods would have no effect:\n\n echo 'Y' | git ...\n yes 'Y' | git ...\n git ... \n---\n compat/terminal.c | 47 ++++++++++++++++++++++++++++-------------------\n 1 file changed, 28 insertions(+), 19 deletions(-)\n\ndiff --git a/compat/terminal.c b/compat/terminal.c\nindex 43b73ddc75..c12e0b9ab9 100644\n--- a/compat/terminal.c\n+++ b/compat/terminal.c\n@@ -202,41 +202,50 @@ static int mingw_getchar(void)\n char *git_terminal_prompt(const char *prompt, int echo)\n {\n \tstatic struct strbuf buf = STRBUF_INIT;\n-\tint r;\n-\tFILE *input_fh, *output_fh;\n+\tint r, input_not_from_tty = !isatty(STDIN_FILENO);\n+\tFILE *input_fh = NULL, *output_fh = NULL;\n+\tchar* ret = NULL;\n+\n+\tif (input_not_from_tty) \n+\t\tinput_fh = stdin;\n+\telse\n+\t\tinput_fh = fopen(INPUT_PATH, \"r\" FORCE_TEXT);\n \n-\tinput_fh = fopen(INPUT_PATH, \"r\" FORCE_TEXT);\n \tif (!input_fh)\n-\t\treturn NULL;\n+\t\tgoto done;\n \n \toutput_fh = fopen(OUTPUT_PATH, \"w\" FORCE_TEXT);\n-\tif (!output_fh) {\n-\t\tfclose(input_fh);\n-\t\treturn NULL;\n-\t}\n \n-\tif (!echo && disable_echo()) {\n-\t\tfclose(input_fh);\n-\t\tfclose(output_fh);\n-\t\treturn NULL;\n-\t}\n+\tif (!output_fh) \n+\t\tgoto done;\n+\n+\tif (!echo && disable_echo()) \n+\t\tgoto done;\n \n \tfputs(prompt, output_fh);\n \tfflush(output_fh);\n \n \tr = strbuf_getline_lf(&buf, input_fh);\n-\tif (!echo) {\n+\n+\tif (input_not_from_tty) \n+\t\tfputs(buf.buf, output_fh);\n+\n+\tif (!echo || input_not_from_tty) {\n \t\tputc('\\n', output_fh);\n \t\tfflush(output_fh);\n \t}\n \n \trestore_term();\n-\tfclose(input_fh);\n-\tfclose(output_fh);\n \n-\tif (r == EOF)\n-\t\treturn NULL;\n-\treturn buf.buf;\n+\tif (r != EOF)\n+\t\tret = buf.buf;\n+done:\n+\tif (input_fh && input_fh != stdin) \n+\t\tfclose(input_fh);\n+\tif (output_fh)\n+\t\tfclose(output_fh);\n+\n+\treturn ret;\n }\n \n /*\n-- \n2.31.1.449.g4a44fa8106\n\n"},{"id":"423802","messageId":"20210506165102.123739-3-firminmartin24@gmail.com","threadId":"55635","inReplyTo":"20210506165102.123739-1-firminmartin24@gmail.com","subject":"[PATCH v1 2/8] format-patch: confirmation whenever patches exist","fromName":"Firmin Martin","fromEmail":"firminmartin24@gmail.com","sentAt":"2021-05-06T16:50:56Z","receivedAt":"2021-05-06T16:52:45Z","isPatch":true,"sender":{"key":"firminmartin24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20540328?v=4"},"body":"Currently, git-format-patch, along with the option --cover-letter,\nunconditionaly overwrites a cover letter with the same name (if\npresent). Although this is a desired behaviour for patches which are\nauto-generated from Git commits log, it might not be the case for a\ncover letter whose the content is meticulously written manually.\n\nParticulary, this behaviour could be awkward in the following\nhypothetical situations:\n\n* The user can easily erase a cover letter coming from prior versions or\n another patch series by reusing an old command line (e.g. autocompleted\n from the shell history).\n\n* Assuming that the user is writing a cover letter and realizes that\n small changes should be made. They make the change, amend and\n format-patch again to regenerate patches. If it happens that they use\n the same command again (e.g. with --cover-letter), the cover letter\n being written is gone.\n\nThis patch addresses this issue by asking confirmation from the user\nwhenever a cover letter or a patch with the same name already exists.\n\nSigned-off-by: Firmin Martin \n---\n builtin/log.c | 33 ++++++++++++++++++++++++++++-----\n 1 file changed, 28 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 6102893fcc..bada3db9eb 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -35,6 +35,7 @@\n #include \"repository.h\"\n #include \"commit-reach.h\"\n #include \"range-diff.h\"\n+#include \"prompt.h\"\n \n #define MAIL_DEFAULT_WRAP 72\n #define COVER_FROM_AUTO_MAX_SUBJECT_LEN 100\n@@ -959,6 +960,10 @@ static int open_next_file(struct commit *commit, const char *subject,\n \t\t\t struct rev_info *rev, int quiet)\n {\n \tstruct strbuf filename = STRBUF_INIT;\n+\tstruct strbuf file_exists_prompt = STRBUF_INIT;\n+\tconst char *yesno;\n+\tstatic int not_prompted = 1;\n+\tint res = 0;\n \n \tif (output_directory) {\n \t\tstrbuf_addstr(&filename, output_directory);\n@@ -972,17 +977,35 @@ static int open_next_file(struct commit *commit, const char *subject,\n \telse\n \t\tfmt_output_subject(&filename, subject, rev);\n \n-\tif (!quiet)\n-\t\tprintf(\"%s\\n\", filename.buf + outdir_offset);\n+\tif (not_prompted && !access(filename.buf, F_OK)) {\n+\n+\t\t/*\n+\t\t * TRANSLATORS: Make sure to include [Y] and [n] in your\n+\t\t * translation. The program will only accept English input\n+\t\t * at this point.\n+\t\t */\n+\t\tstrbuf_addf(&file_exists_prompt, _(\"The file '%s' already exists.\\n\"\n+\t\t\t\"Would you overwrite this file and subsequent ones [Y/n]? \"), filename.buf);\n+\t\tyesno = git_prompt(file_exists_prompt.buf, PROMPT_ECHO);\n+\t\tnot_prompted = 0;\n+\t\tif (tolower(*yesno) == 'n') {\n+\t\t\tres = -1;\n+\t\t\tgoto done;\n+\t\t}\n+\t}\n \n \tif ((rev->diffopt.file = fopen(filename.buf, \"w\")) == NULL) {\n \t\terror_errno(_(\"cannot open patch file %s\"), filename.buf);\n-\t\tstrbuf_release(&filename);\n-\t\treturn -1;\n+\t\tres = -1;\n+\t\tgoto done;\n \t}\n \n+\tif (!quiet)\n+\t\tprintf(\"%s\\n\", filename.buf + outdir_offset);\n+done:\n \tstrbuf_release(&filename);\n-\treturn 0;\n+\tstrbuf_release(&file_exists_prompt);\n+\treturn res;\n }\n \n static void get_patch_ids(struct rev_info *rev, struct patch_ids *ids)\n-- \n2.31.1.449.g4a44fa8106\n\n"},{"id":"423803","messageId":"20210506165102.123739-4-firminmartin24@gmail.com","threadId":"55635","inReplyTo":"20210506165102.123739-1-firminmartin24@gmail.com","subject":"[PATCH v1 3/8] format-patch: add config option confirmOverwrite","fromName":"Firmin Martin","fromEmail":"firminmartin24@gmail.com","sentAt":"2021-05-06T16:50:57Z","receivedAt":"2021-05-06T16:52:47Z","isPatch":true,"sender":{"key":"firminmartin24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20540328?v=4"},"body":"Provide the configuration option format.confirmOverwrite. This option\nwill decide whether a confirmation is required to overwrite cover letter\nor patches issued by git-format-patch.\n\nIt accepts three values.\n* \"never\"/\"always\": never/always ask confirmation whenever cover letter\nor patches are subject to be overwritten.\n* \"cover\": ask confirmation only if a cover letter is subject to be\noverwritten.\n\nformat.confirmOverwrite defaults to \"cover\" to avoid cover letter\nbeing written be overwritten mistakenly.\n\nSigned-off-by: Firmin Martin \n---\n builtin/log.c | 28 +++++++++++++++++++++++++++-\n 1 file changed, 27 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex bada3db9eb..ec9848da70 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -816,6 +816,12 @@ enum auto_base_setting {\n \tAUTO_BASE_WHEN_ABLE\n };\n \n+enum confirm_overwrite_setting {\n+\tCONFIRM_OVERWRITE_NEVER,\n+\tCONFIRM_OVERWRITE_ALWAYS,\n+\tCONFIRM_OVERWRITE_COVER\n+};\n+\n static enum thread_level thread;\n static int do_signoff;\n static enum auto_base_setting auto_base;\n@@ -827,6 +833,7 @@ static const char *config_output_directory;\n static enum cover_from_description cover_from_description_mode = COVER_FROM_MESSAGE;\n static int show_notes;\n static struct display_notes_opt notes_opt;\n+static enum confirm_overwrite_setting confirm_overwrite = CONFIRM_OVERWRITE_COVER;\n \n static enum cover_from_description parse_cover_from_description(const char *arg)\n {\n@@ -844,6 +851,18 @@ static enum cover_from_description parse_cover_from_description(const char *arg)\n \t\tdie(_(\"%s: invalid cover from description mode\"), arg);\n }\n \n+static enum confirm_overwrite_setting parse_confirm_overwrite(const char *arg)\n+{\n+\tif (!arg || !strcasecmp(arg, \"cover\"))\n+\t\treturn CONFIRM_OVERWRITE_COVER;\n+\telse if (!strcasecmp(arg, \"always\"))\n+\t\treturn CONFIRM_OVERWRITE_ALWAYS;\n+\telse if (!strcasecmp(arg, \"never\"))\n+\t\treturn CONFIRM_OVERWRITE_NEVER;\n+\telse\n+\t\tdie(_(\"%s: invalid file overwrite setting\"), arg);\n+}\n+\n static int git_format_config(const char *var, const char *value, void *cb)\n {\n \tif (!strcmp(var, \"format.headers\")) {\n@@ -949,6 +968,10 @@ static int git_format_config(const char *var, const char *value, void *cb)\n \t\tcover_from_description_mode = parse_cover_from_description(value);\n \t\treturn 0;\n \t}\n+\tif (!strcmp(var, \"format.confirmoverwrite\")) {\n+\t\tconfirm_overwrite = parse_confirm_overwrite(value);\n+\t\treturn 0;\n+\t}\n \n \treturn git_log_config(var, value, cb);\n }\n@@ -977,7 +1000,10 @@ static int open_next_file(struct commit *commit, const char *subject,\n \telse\n \t\tfmt_output_subject(&filename, subject, rev);\n \n-\tif (not_prompted && !access(filename.buf, F_OK)) {\n+\tif (not_prompted &&\n+ ((rev->nr == 0 && confirm_overwrite == CONFIRM_OVERWRITE_COVER) || \n+ confirm_overwrite == CONFIRM_OVERWRITE_ALWAYS) &&\n+\t !access(filename.buf, F_OK)) {\n \n \t\t/*\n \t\t * TRANSLATORS: Make sure to include [Y] and [n] in your\n-- \n2.31.1.449.g4a44fa8106\n\n"},{"id":"423804","messageId":"20210506165102.123739-5-firminmartin24@gmail.com","threadId":"55635","inReplyTo":"20210506165102.123739-1-firminmartin24@gmail.com","subject":"[PATCH v1 4/8] format-patch: add the option --confirm-overwrite","fromName":"Firmin Martin","fromEmail":"firminmartin24@gmail.com","sentAt":"2021-05-06T16:50:58Z","receivedAt":"2021-05-06T16:52:47Z","isPatch":true,"sender":{"key":"firminmartin24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20540328?v=4"},"body":"This command line option acts in the same way as the configuration option\nformat.confirmOverwrite.\n\nSigned-off-by: Firmin Martin \n---\n builtin/log.c | 6 ++++++\n 1 file changed, 6 insertions(+)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex ec9848da70..0ce8778338 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1776,6 +1776,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tint quiet = 0;\n \tconst char *reroll_count = NULL;\n \tchar *cover_from_description_arg = NULL;\n+\tchar *confirm_overwrite_arg = NULL;\n \tchar *branch_name = NULL;\n \tchar *base_commit = NULL;\n \tstruct base_tree_info bases;\n@@ -1818,6 +1819,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\tOPT_STRING(0, \"cover-from-description\", &cover_from_description_arg,\n \t\t\t N_(\"cover-from-description-mode\"),\n \t\t\t N_(\"generate parts of a cover letter based on a branch's description\")),\n+\t\tOPT_STRING(0, \"confirm-overwrite\", &confirm_overwrite_arg, N_(\"when\"),\n+\t\t\t N_(\"overwrite cover letter/patches with or without confirmation\")),\n \t\tOPT_CALLBACK_F(0, \"subject-prefix\", &rev, N_(\"prefix\"),\n \t\t\t N_(\"use [] instead of [PATCH]\"),\n \t\t\t PARSE_OPT_NONEG, subject_prefix_callback),\n@@ -1919,6 +1922,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tif (cover_from_description_arg)\n \t\tcover_from_description_mode = parse_cover_from_description(cover_from_description_arg);\n \n+\tif (confirm_overwrite_arg)\n+\t\tconfirm_overwrite = parse_confirm_overwrite(confirm_overwrite_arg);\n+\n \tif (reroll_count) {\n \t\tstruct strbuf sprefix = STRBUF_INIT;\n \n-- \n2.31.1.449.g4a44fa8106\n\n"},{"id":"423805","messageId":"20210506165102.123739-6-firminmartin24@gmail.com","threadId":"55635","inReplyTo":"20210506165102.123739-1-firminmartin24@gmail.com","subject":"[PATCH v1 5/8] t4014: test patches overwrite confirmation","fromName":"Firmin Martin","fromEmail":"firminmartin24@gmail.com","sentAt":"2021-05-06T16:50:59Z","receivedAt":"2021-05-06T16:52:48Z","isPatch":true,"sender":{"key":"firminmartin24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20540328?v=4"},"body":"Signed-off-by: Firmin Martin \n---\n t/t4014-format-patch.sh | 123 ++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 123 insertions(+)\n\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex 712d4b5ddf..cf7e48f4de 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -2256,6 +2256,129 @@ test_expect_success 'format-patch --pretty=mboxrd' '\n \ttest_cmp expect actual\n '\n \n+# $1 = git format-patch extra arguments\n+confirm_overwrite_setup () {\n+\ttest_when_finished \"rm -rf confirm-overwrite\" &&\n+\tgit format-patch $1 -o confirm-overwrite main..side &&\n+\tfor patch in confirm-overwrite/*; do echo 'APPENDUM' >>$patch; done\n+}\n+\n+# $1 = format.confirmOverwrite value\n+# $2 = git format-patch extra arguments\n+# $3 = git format-patch with prompt (Y/N) or without it\n+confirm_overwrite_test_body () {\n+\tif test ! -z $1; \n+\tthen\n+\t\ttest_config format.confirmOverwrite $1\n+\tfi &&\n+\tcase \"$3\" in\n+\tY)\n+\t\techo Y | git format-patch $2 -o confirm-overwrite main..side \n+\t\t;;\n+\tN)\n+\t\techo N | test_must_fail git format-patch $2 -o confirm-overwrite main..side\n+\t\t;;\n+\t*)\n+\t\tgit format-patch $2 -o confirm-overwrite main..side \n+\t\t;;\n+\tesac\n+}\n+\n+# true if all patches are overwritten, false otherwise\n+confirm_overwrite_all_overwritten () {\n+\tfor patch in confirm-overwrite/*; do test_i18ngrep ! \"^APPENDUM$\" $patch; done \n+}\n+\n+test_expect_success 'format-patch overwrite unconditionally patch series without cover letter' '\n+\tconfirm_overwrite_setup &&\n+\tconfirm_overwrite_test_body &&\n+\tconfirm_overwrite_all_overwritten\n+'\n+\n+test_expect_success 'format-patch overwrites present cover letter (prompt/Y)' '\n+\tconfirm_overwrite_setup \"--cover-letter\" &&\n+\tconfirm_overwrite_test_body \"\" \"--cover-letter\" \"Y\" &&\n+\tconfirm_overwrite_all_overwritten\n+'\n+\n+test_expect_success 'format-patch does not overwrite present cover letter (prompt/N)' '\n+\tconfirm_overwrite_setup \"--cover-letter\" &&\n+\tconfirm_overwrite_test_body \"\" \"--cover-letter\" \"N\" &&\n+\t! confirm_overwrite_all_overwritten\n+'\n+\n+test_expect_success 'format-patch --numbered-files overwrites existing cover letter (prompt/Y)' '\n+\tconfirm_overwrite_setup \"--cover-letter --numbered-files\" &&\n+\tconfirm_overwrite_test_body \"\" \"--cover-letter --numbered-files\" \"Y\" &&\n+\tconfirm_overwrite_all_overwritten\n+'\n+\n+test_expect_success 'format-patch --numbered-files does not overwrite existing cover letter (prompt/N)' '\n+\tconfirm_overwrite_setup \"--cover-letter --numbered-files\" &&\n+\tconfirm_overwrite_test_body \"\" \"--cover-letter --numbered-files\" \"N\" &&\n+\t! confirm_overwrite_all_overwritten\n+'\n+\n+test_expect_success 'format-patch overwrites existing cover letter (format.confirmOverwrite = never)' '\n+\tconfirm_overwrite_setup \"--cover-letter\" &&\n+\tconfirm_overwrite_test_body \"never\" \"--cover-letter\" &&\n+\tconfirm_overwrite_all_overwritten\n+'\n+\n+test_expect_success 'format-patch: the user disagrees to overwrite existing cover letter (format.confirmOverwrite = always)' '\n+\tconfirm_overwrite_setup \"--cover-letter\" &&\n+\tconfirm_overwrite_test_body \"always\" \"--cover-letter\" \"N\" &&\n+\t! confirm_overwrite_all_overwritten\n+'\n+\n+test_expect_success 'format-patch: the user agrees to overwrite existing cover letter (format.confirmOverwrite = always)' '\n+\tconfirm_overwrite_setup \"--cover-letter\" &&\n+\tconfirm_overwrite_test_body \"always\" \"--cover-letter\" \"Y\" &&\n+\tconfirm_overwrite_all_overwritten\n+'\n+\n+test_expect_success 'format-patch --confirm-overwrite has higher priority than format.confirmOverwrite' '\n+\tconfirm_overwrite_setup &&\n+\tconfirm_overwrite_test_body \"always\" \"--confirm-overwrite never\" &&\n+\tconfirm_overwrite_all_overwritten\n+'\n+\n+test_expect_success 'format-patch --confirm-overwrite cover: the user agrees to overwrite existing cover letter' '\n+\tconfirm_overwrite_setup \"--cover-letter\" &&\n+\tconfirm_overwrite_test_body \"never\" \"--cover-letter --confirm-overwrite cover\" \"Y\" &&\n+\tconfirm_overwrite_all_overwritten\n+'\n+\n+test_expect_success 'format-patch --confirm-overwrite cover: the user disagrees to overwrite existing cover letter' '\n+\tconfirm_overwrite_setup \"--cover-letter\" &&\n+\tconfirm_overwrite_test_body \"never\" \"--cover-letter --confirm-overwrite cover\" \"N\" &&\n+\t! confirm_overwrite_all_overwritten\n+'\n+\n+test_expect_success 'format-patch --confirm-overwrite always: the user agrees to overwrite existing patches' '\n+\tconfirm_overwrite_setup &&\n+\tconfirm_overwrite_test_body \"never\" \"--confirm-overwrite always\" \"Y\" &&\n+\tconfirm_overwrite_all_overwritten\n+'\n+\n+test_expect_success 'format-patch --confirm-overwrite always: the user disagrees to overwrite existing patches' '\n+\tconfirm_overwrite_setup &&\n+\tconfirm_overwrite_test_body \"never\" \"--confirm-overwrite always\" \"N\" &&\n+\t! confirm_overwrite_all_overwritten\n+'\n+\n+test_expect_success 'format-patch --confirm-overwrite never: overwrite cover letter unconditionally' '\n+\tconfirm_overwrite_setup \"--cover-letter\" &&\n+\tconfirm_overwrite_test_body \"always\" \"--cover-letter --confirm-overwrite never\" &&\n+\tconfirm_overwrite_all_overwritten\n+'\n+\n+test_expect_success 'format-patch --confirm-overwrite never: overwrite patches unconditionally' '\n+\tconfirm_overwrite_setup &&\n+\tconfirm_overwrite_test_body \"always\" \"--confirm-overwrite never\" &&\n+\tconfirm_overwrite_all_overwritten\n+'\n+\n test_expect_success 'interdiff: setup' '\n \tgit checkout -b boop main &&\n \ttest_commit fnorp blorp &&\n-- \n2.31.1.449.g4a44fa8106\n\n"},{"id":"423806","messageId":"20210506165102.123739-7-firminmartin24@gmail.com","threadId":"55635","inReplyTo":"20210506165102.123739-1-firminmartin24@gmail.com","subject":"[PATCH v1 6/8] t4014: fix tests overwriting cover letter in silent","fromName":"Firmin Martin","fromEmail":"firminmartin24@gmail.com","sentAt":"2021-05-06T16:51:00Z","receivedAt":"2021-05-06T16:52:50Z","isPatch":true,"sender":{"key":"firminmartin24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20540328?v=4"},"body":"These tests are broken due to the new configuration option\nformat.confirmOvewrite whose the default value requires confirmation\nfrom user whenever an existing cover letter is subject to be\noverwritten.\n\nAlso change some pairs of git config and git config --unset to a single\ntest_config.\n\nSigned-off-by: Firmin Martin \n---\n t/t4014-format-patch.sh | 17 +++++++++++------\n 1 file changed, 11 insertions(+), 6 deletions(-)\n\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex cf7e48f4de..a34598beca 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -669,11 +669,13 @@ test_expect_success 'failure to write cover-letter aborts gracefully' '\n '\n \n test_expect_success 'cover-letter inherits diff options' '\n+\ttest_when_finished \"rm 0000-cover-letter.patch\" &&\n \tgit mv file foo &&\n \tgit commit -m foo &&\n \tgit format-patch --no-renames --cover-letter -1 &&\n \tcheck_patch 0000-cover-letter.patch &&\n \t! grep \"file => foo .* 0 *\\$\" 0000-cover-letter.patch &&\n+\trm 0000-cover-letter.patch &&\n \tgit format-patch --cover-letter -1 -M &&\n \tgrep \"file => foo .* 0 *\\$\" 0000-cover-letter.patch\n '\n@@ -1917,10 +1919,9 @@ test_expect_success 'cover letter with nothing' '\n \n test_expect_success 'cover letter auto' '\n \tmkdir -p tmp &&\n-\ttest_when_finished \"rm -rf tmp;\n-\t\tgit config --unset format.coverletter\" &&\n+\ttest_when_finished \"rm -rf tmp\" &&\n \n-\tgit config format.coverletter auto &&\n+\ttest_config format.coverletter auto &&\n \tgit format-patch -o tmp -1 >list &&\n \ttest_line_count = 1 list &&\n \tgit format-patch -o tmp -2 >list &&\n@@ -1929,10 +1930,10 @@ test_expect_success 'cover letter auto' '\n \n test_expect_success 'cover letter auto user override' '\n \tmkdir -p tmp &&\n-\ttest_when_finished \"rm -rf tmp;\n-\t\tgit config --unset format.coverletter\" &&\n+\ttest_when_finished \"rm -rf tmp\" &&\n \n-\tgit config format.coverletter auto &&\n+\ttest_config format.confirmOverwrite never &&\n+\ttest_config format.coverletter auto &&\n \tgit format-patch -o tmp --cover-letter -1 >list &&\n \ttest_line_count = 2 list &&\n \tgit format-patch -o tmp --cover-letter -2 >list &&\n@@ -2386,6 +2387,7 @@ test_expect_success 'interdiff: setup' '\n '\n \n test_expect_success 'interdiff: cover-letter' '\n+\ttest_when_finished \"rm 0000-cover-letter.patch\" &&\n \tsed \"y/q/ /\" >expect <<-\\EOF &&\n \t+fleep\n \t--q\n@@ -2398,16 +2400,19 @@ test_expect_success 'interdiff: cover-letter' '\n '\n \n test_expect_success 'interdiff: reroll-count' '\n+\ttest_when_finished \"rm v2-0000-cover-letter.patch\" &&\n \tgit format-patch --cover-letter --interdiff=boop~2 -v2 -1 boop &&\n \ttest_i18ngrep \"^Interdiff ..* v1:$\" v2-0000-cover-letter.patch\n '\n \n test_expect_success 'interdiff: reroll-count with a non-integer' '\n+\ttest_when_finished \"rm v2.2-0000-cover-letter.patch\" &&\n \tgit format-patch --cover-letter --interdiff=boop~2 -v2.2 -1 boop &&\n \ttest_i18ngrep \"^Interdiff:$\" v2.2-0000-cover-letter.patch\n '\n \n test_expect_success 'interdiff: reroll-count with a integer' '\n+\ttest_when_finished \"rm v2-0000-cover-letter.patch\" &&\n \tgit format-patch --cover-letter --interdiff=boop~2 -v2 -1 boop &&\n \ttest_i18ngrep \"^Interdiff ..* v1:$\" v2-0000-cover-letter.patch\n '\n-- \n2.31.1.449.g4a44fa8106\n\n"},{"id":"423807","messageId":"20210506165102.123739-8-firminmartin24@gmail.com","threadId":"55635","inReplyTo":"20210506165102.123739-1-firminmartin24@gmail.com","subject":"[PATCH v1 7/8] doc/format-patch: describe --confirm-overwrite","fromName":"Firmin Martin","fromEmail":"firminmartin24@gmail.com","sentAt":"2021-05-06T16:51:01Z","receivedAt":"2021-05-06T16:52:51Z","isPatch":true,"sender":{"key":"firminmartin24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20540328?v=4"},"body":"Signed-off-by: Firmin Martin \n---\n Documentation/git-format-patch.txt | 20 ++++++++++++++++++++\n 1 file changed, 20 insertions(+)\n\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex 911da181a1..49f08b5e51 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -20,6 +20,7 @@ SYNOPSIS\n \t\t [--in-reply-to=] [--suffix=.]\n \t\t [--ignore-if-in-upstream]\n \t\t [--cover-from-description=]\n+\t\t [--confirm-overwrite=]\n \t\t [--rfc] [--subject-prefix=]\n \t\t [(--reroll-count|-v) ]\n \t\t [--to=] [--cc=]\n@@ -195,6 +196,7 @@ will want to ensure that threading is disabled for `git send-email`.\n --cover-from-description=::\n \tControls which parts of the cover letter will be automatically\n \tpopulated using the branch's description.\n+\n +\n If `` is `message` or `default`, the cover letter subject will be\n populated with placeholder text. The body of the cover letter will be\n@@ -212,6 +214,23 @@ is greater than 100 bytes, then the mode will be `message`, otherwise\n If `` is `none`, both the cover letter subject and body will be\n populated with placeholder text.\n \n+`--confirm-overwrite`=::\n+\tSpecifies when Git must ask the user to confirm whether existing\n+\tpatches or cover letter of the same name should be overwritten.\n+\t possible values are:\n++\n+--\n+always;;\n+never;;\n+\tAlways/never prompt for confirmation whenever patches or a cover letter\n+\tare subject to be overwritten.\n+cover;;\n+\tAsk confirmation whenever a cover letter is subject to be overwritten.\n+--\n++\n+Defaults to the value of the `format.confirmOverwrite` variable, or\n+\"cover\" if unconfigured.\n+\n --subject-prefix=::\n \tInstead of the standard '[PATCH]' prefix in the subject\n \tline, instead use '[]'. This\n@@ -409,6 +428,7 @@ with configuration variables.\n \toutputDirectory = \n \tcoverLetter = auto\n \tcoverFromDescription = auto\n+\tconfirmOverwrite = onlyCover\n ------------\n \n \n-- \n2.31.1.449.g4a44fa8106\n\n"},{"id":"423808","messageId":"20210506165102.123739-9-firminmartin24@gmail.com","threadId":"55635","inReplyTo":"20210506165102.123739-1-firminmartin24@gmail.com","subject":"[PATCH v1 8/8] config/format: describe format.confirmOverwrite","fromName":"Firmin Martin","fromEmail":"firminmartin24@gmail.com","sentAt":"2021-05-06T16:51:02Z","receivedAt":"2021-05-06T16:52:54Z","isPatch":true,"sender":{"key":"firminmartin24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20540328?v=4"},"body":"Signed-off-by: Firmin Martin \n---\n Documentation/config/format.txt | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/Documentation/config/format.txt b/Documentation/config/format.txt\nindex fdbc06a4d2..bc189e3ec8 100644\n--- a/Documentation/config/format.txt\n+++ b/Documentation/config/format.txt\n@@ -42,6 +42,11 @@ format.coverFromDescription::\n \tdescription. See the `--cover-from-description` option in\n \tlinkgit:git-format-patch[1].\n \n+format.confirmOverwrite::\n+\tDetermines when the user must be prompted for confirmation whenever a\n+\tcover letter or a patch is subject to be overwritten. See the\n+\t`--confirm-overwrite` option in linkgit:git-format-patch[1].\n+\n format.signature::\n \tThe default for format-patch is to output a signature containing\n \tthe Git version number. Use this variable to change that default.\n-- \n2.31.1.449.g4a44fa8106\n\n"},{"id":"423828","messageId":"xmqqv97vwkzo.fsf@gitster.g","threadId":"55635","inReplyTo":"20210506165102.123739-1-firminmartin24@gmail.com","subject":"Re: [PATCH v1 0/8] format-patch: introduce --confirm-overwrite","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-06T22:33:31Z","receivedAt":"2021-05-06T22:33:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Firmin Martin writes:\n\n> Particulary, this behaviour could be awkward in the following\n> hypothetical situations:\n>\n> * The user can easily erase a cover letter coming from prior versions or\n> another patch series by reusing an old command line (e.g. autocompleted\n> from the shell history).\n\n\"prior versions\" implies that the user is better off using -v$n\nwhere $n is the number greater than the one used for the prior\niteration by one, and there won't be any overwriting, so this is not\na very compelling use case.\n\nBut the next one is real.\n\n> * Assuming that the user is writing a cover letter and realizes that\n> small changes should be made. They make the change, amend and\n> format-patch again to regenerate patches. If it happens that they use\n> the same command again (e.g. with --cover-letter), the cover letter\n> being written is gone.\n\nYes, after preparing, say, -v2, but before sending them out, it is\nvery plausible that proofreading of your own patches may make you\nrealize more issues in the series, which may make you go back to your\ncommits, \"rebase -i\" to improve them and re-run \"format-patch -v2\".\n\nWe do want to encourage such careful preparation of your patch\nseries before sending it out, and we want to support it well with\nour tools. Preventing overwriting of the cover (which will have the\nsame filename, with the same v2- prefix) is very valuable here.\n\nThere is another thing that I suspect people may find irritating in\nthe same workflow. If you fix the commit title while \"rebase -i\" to\npolish your v2 patch, it would result in a different filename from\nthe original v2, so you'd end up with something like\n\n v2-0000-cover-letter.patch\n v2-0001-thes-forny-change.patch\n v2-0001-this-phoney-change.patch\n v2-0002-another-sample-change.patch\n\nwhile redoing a two-patch series. The \"thes-forny\" thing is a\nleftover from the first \"format-patch -v2\" run, you fixed typoes\nwith \"rebase -i\" after a self-review and other three files have the\nresult of the second \"format-patch -v2\" run. You need a way to\nsomehow exclude that stale file when driving send-email; in other\nwords, before running\n\n git send-email v2-*.patch\n\nyou would want to move away v2-0001-thes-forny-change.patch that no\nlonger is part of the series. I wonder if format-patch can help by\nlooking at the output directory before writing its output and move\nthe old files away, say, to \"old-v2-*.patch\" or something? That\nincidentally would solve your \"files getting overwritten is\nirritating\" issue at the same time.\n\nComing back to the topic of cover letter, even when there is no risk\nof ovetwriting, there is another thing we may want to improve to\nhelp our users. Suppose you are preparing your v2 patch after\nsending out the v1. The cover letter we generate for v2 will have\nthe same **BOILERPLATE** placeholders that need to be filled from\nscratch. As many things you wrote for the cover letter in the\nprevious round should be reusable, even though the list of titles of\nthe patch should be generated afresh, it would be nice if\nformat-patch can carry forward what you wrote in the cover letter\nfor the v1 iteration to the cover letter for this v2 iteration.\n\nAnd when we have such a \"reuse description in the existing cover\nletter\" support, the value of \"don't overwrite\" knob will mostly go\naway. Instead of failing the command by refusing to overwrite, you\ncan read what is in the existing cover-letter that is about to be\noverwritten, combine the hand-written description with the material\nautomatically generated, and ovewrite the existing file, and as long\nas you do a good job of preserving, nobody would complain that you\noverwrote the file.\n\n"},{"id":"423829","messageId":"xmqqr1ijwi0i.fsf@gitster.g","threadId":"55635","inReplyTo":"20210506165102.123739-2-firminmartin24@gmail.com","subject":"Re: [PATCH v1 1/8] compat/terminal: let prompt accept input from pipe","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-06T23:37:49Z","receivedAt":"2021-05-06T23:37:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Firmin Martin writes:\n\n> Currently, git_prompt ignores input coming from anywhere other than\n> terminal (pipe, redirection etc.) meaning that standard prompt\n> auto-answering methods would have no effect:\n>\n> echo 'Y' | git ...\n> yes 'Y' | git ...\n> git ... \n> It also prevents git subcommands using git_prompt to be tested using\n> such methods.\n\nFor testing, wouldn't lib-terminal.sh be usable for your purpose?\nIf not, what is the reason why it is insufficient? Can we fix that\ninstead?\n\nAllowing prompter to read from pipe has a big downside in the\nproduction code: you cannot pipe data into our command, and let it\nask interactive questions from the end user by opening /dev/tty.\n\n\n> This patch fixes this issue by considering standard input when !isatty(0).\n> It also rearranges the control flow to close input and output file handlers.\n\nSo this \"fix\" is probably very unwelcome, especially if done\nunconditionally.\n\nThanks.\n"},{"id":"423830","messageId":"xmqqk0obwhi2.fsf@gitster.g","threadId":"55635","inReplyTo":"20210506165102.123739-3-firminmartin24@gmail.com","subject":"Re: [PATCH v1 2/8] format-patch: confirmation whenever patches exist","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-06T23:48:53Z","receivedAt":"2021-05-06T23:48:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Firmin Martin writes:\n\n> Currently, git-format-patch, along with the option --cover-letter,\n> unconditionaly overwrites a cover letter with the same name (if\n> present). Although this is a desired behaviour for patches which are\n> auto-generated from Git commits log, it might not be the case for a\n> cover letter whose the content is meticulously written manually.\n\nTrue. But if we require confirmation before overwriting patches,\nthat would be overall worsening the end-user experience, I am\nafraid. In a 5-patch series with a cover-letter that was formatted,\nproofread, corrected with \"rebase -i\" and then re-formatted, unless\nyou rephrased the titles of the patches, you'd get prompted once for\nthe cover letter (which *IS* valuable) and five-times for patches\n(which is annoying).\n\nIt also is unfortunate that this change does not address another\nannoyance after retitling a patch---the stale output from the\nprevious run is not overwritten with the updated one but is left in\nthe same directory as the output files from the latest run.\n\nSo, while I very much do welcome the motivation, I am not onboard\nwith this particular design.\n\n> diff --git a/builtin/log.c b/builtin/log.c\n> index 6102893fcc..bada3db9eb 100644\n> --- a/builtin/log.c\n> +++ b/builtin/log.c\n> @@ -35,6 +35,7 @@\n> #include \"repository.h\"\n> #include \"commit-reach.h\"\n> #include \"range-diff.h\"\n> +#include \"prompt.h\"\n> \n> #define MAIL_DEFAULT_WRAP 72\n> #define COVER_FROM_AUTO_MAX_SUBJECT_LEN 100\n> @@ -959,6 +960,10 @@ static int open_next_file(struct commit *commit, const char *subject,\n> \t\t\t struct rev_info *rev, int quiet)\n> {\n> \tstruct strbuf filename = STRBUF_INIT;\n> +\tstruct strbuf file_exists_prompt = STRBUF_INIT;\n> +\tconst char *yesno;\n> +\tstatic int not_prompted = 1;\n\nWhen the API passes a structure that keeps track of state (like\n\"struct rev_info *rev\", used to hold rev->diffopt.file which is\nclearly a state), add a member to it, instead of inventing a\nfunction local static that will hurt reuse of the API you are\ntouching. This static variable will make it impossible from a\nsingle process to format two patch series, but if it is made a part\nof rev_info, you do not have to introduce such a limitation.\n"},{"id":"423834","messageId":"60949be8613c1_8c2220882@natae.notmuch","threadId":"55635","inReplyTo":"20210506165102.123739-1-firminmartin24@gmail.com","subject":"RE: [PATCH v1 0/8] format-patch: introduce --confirm-overwrite","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-07T01:46:16Z","receivedAt":"2021-05-07T01:46:21Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Firmin Martin wrote:\n> Currently, git-format-patch, along with the option --cover-letter,\n> unconditionally overwrites a cover letter with the same name (if\n> present). Although this is a desired behaviour for patches which are\n> auto-generated from Git commits log, it might not be the case for a\n> cover letter whose the content is meticulously written manually.\n\nThis is one of the reasons I never use git format-patch directly, but I\nuse a tool on top: git send-series[1].\n\nIt would be nice if git format-patch grabbed the text of the body from\nsomewhere, and even better if git branch learned --edit-cover-letter.\n\nNone of this invalidates the usefulness of your patches, of course.\n\nCheers.\n\n[1] https://github.com/felipec/git-send-series\n\n-- \nFelipe Contreras\n"},{"id":"423840","messageId":"e0bc4aef-a92a-20f2-0b9f-d65917710e3b@gmail.com","threadId":"55635","inReplyTo":"20210506165102.123739-8-firminmartin24@gmail.com","subject":"Re: [PATCH v1 7/8] doc/format-patch: describe --confirm-overwrite","fromName":"Bagas Sanjaya","fromEmail":"bagasdotme@gmail.com","sentAt":"2021-05-07T03:32:17Z","receivedAt":"2021-05-07T03:32:24Z","isPatch":true,"sender":{"key":"bagasdotme@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40219486?v=4"},"body":"On 06/05/21 23.51, Firmin Martin wrote:\n> +always;;\n> +never;;\n> +\tAlways/never prompt for confirmation whenever patches or a cover letter\n> +\tare subject to be overwritten.\n> +cover;;\n> +\tAsk confirmation whenever a cover letter is subject to be overwritten.\n> +--\n\nFor `always` and `never`, I think s/patches or/patches and\\/or\nFor `cover`, I think s/whenever/whenever only (add `only` after `whenever`\n\n-- \nAn old man doll... just what I always wanted! - Clara\n"},{"id":"423854","messageId":"YJTH+sTP/O5Nxtp9@coredump.intra.peff.net","threadId":"55635","inReplyTo":"xmqqr1ijwi0i.fsf@gitster.g","subject":"Re: [PATCH v1 1/8] compat/terminal: let prompt accept input from pipe","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-05-07T04:54:18Z","receivedAt":"2021-05-07T04:54:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, May 07, 2021 at 08:37:49AM +0900, Junio C Hamano wrote:\n\n> Firmin Martin writes:\n> \n> > Currently, git_prompt ignores input coming from anywhere other than\n> > terminal (pipe, redirection etc.) meaning that standard prompt\n> > auto-answering methods would have no effect:\n> >\n> > echo 'Y' | git ...\n> > yes 'Y' | git ...\n> > git ... >\n> > It also prevents git subcommands using git_prompt to be tested using\n> > such methods.\n> \n> For testing, wouldn't lib-terminal.sh be usable for your purpose?\n> If not, what is the reason why it is insufficient? Can we fix that\n> instead?\n\nThat doesn't work, because it insists on reading from /dev/tty and not\nthe pty that lib-terminal will set up as stdin. But...\n\n> Allowing prompter to read from pipe has a big downside in the\n> production code: you cannot pipe data into our command, and let it\n> ask interactive questions from the end user by opening /dev/tty.\n\nRight. The main purpose of the function was to let git-remote-https,\nwhose stdin is connected to git-fetch, get a password from the user.\nReading from stdin would break things badly there[1].\n\nLooking at the second patch, the motivation here seems to be to use\ngit_prompt() for another run-of-the-mill prompt. But the right answer\nis: don't do that. In fact, we recently-ish removed a similar case in\n97387c8bdd (am: read interactive input from stdin, 2019-05-20) that was\nlikewise causing problems with the test suite.\n\nI think we might consider renaming git_prompt(), or adding an\nexplanatory comment above it.\n\n-Peff\n\n[1] Sadly I don't think our test suite could notice the breakage\n introduced by this function. It uses the askpass feature to avoid\n triggering this code at all, because of course we can not reliably\n read from /dev/tty in the script. But with just this patch applied,\n and no credential helpers defined, trying \"git ls-remote\n https://github.com/you/some-private-repo\" shows the problem: you get\n prompted, but it never reads your input.\n"},{"id":"423857","messageId":"xmqq1rajw1xf.fsf@gitster.g","threadId":"55635","inReplyTo":"YJTH+sTP/O5Nxtp9@coredump.intra.peff.net","subject":"Re: [PATCH v1 1/8] compat/terminal: let prompt accept input from pipe","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-07T05:25:16Z","receivedAt":"2021-05-07T05:26:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n>> For testing, wouldn't lib-terminal.sh be usable for your purpose?\n>> If not, what is the reason why it is insufficient? Can we fix that\n>> instead?\n>\n> That doesn't work, because it insists on reading from /dev/tty and not\n> the pty that lib-terminal will set up as stdin. But...\n\nI somehow thought that lib-terminal was a bit more complete in that\nit would make the pty we allocate to the controlling terminal for\nthe process that runs the test program. Sigh.\n"},{"id":"423867","messageId":"YJUAmUJiHmJRQBMj@generichostname","threadId":"55635","inReplyTo":"60949be8613c1_8c2220882@natae.notmuch","subject":"Re: [PATCH v1 0/8] format-patch: introduce --confirm-overwrite","fromName":"Denton Liu","fromEmail":"liu.denton@gmail.com","sentAt":"2021-05-07T08:55:53Z","receivedAt":"2021-05-07T08:55:58Z","isPatch":true,"sender":{"key":"liu.denton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9620836?v=4"},"body":"Hi Felipe,\n\nOn Thu, May 06, 2021 at 08:46:16PM -0500, Felipe Contreras wrote:\n> Firmin Martin wrote:\n> > Currently, git-format-patch, along with the option --cover-letter,\n> > unconditionally overwrites a cover letter with the same name (if\n> > present). Although this is a desired behaviour for patches which are\n> > auto-generated from Git commits log, it might not be the case for a\n> > cover letter whose the content is meticulously written manually.\n> \n> This is one of the reasons I never use git format-patch directly, but I\n> use a tool on top: git send-series[1].\n\nIt seems like everyone has written some sort of tooling on top of\nformat-patch at this point. Taking a cursory look at your tool, perhaps\na feature like `--previous-cover-letter ` might provide most of\nthe functionality that most tooling that I've seen gives.\n\nPerhaps this option could parse a cover letter from a previous version\nof a patch and use it to populate the next version number, In-Reply-To,\ncover letter subject/body, To/Cc lists and maybe more. I think that\nextracting the information would be pretty easy but designing the UI it\nin a non-obtuse way would be pretty challenging.\n\n> It would be nice if git format-patch grabbed the text of the body from\n> somewhere, and even better if git branch learned --edit-cover-letter.\n\nWell, you're in luck! I wanted the same thing a couple of years back so\nI implemented the --cover-from-description option[0]. It allows the cover\nletter to be populated by the text given in\n`git branch --edit-description`.\n\n-Denton\n\n[0]: https://git-scm.com/docs/git-format-patch#Documentation/git-format-patch.txt---cover-from-descriptionltmodegt\n"},{"id":"423873","messageId":"87sg2ybq0h.fsf@osv.gnss.ru","threadId":"55635","inReplyTo":"60949be8613c1_8c2220882@natae.notmuch","subject":"Re: [PATCH v1 0/8] format-patch: introduce --confirm-overwrite","fromName":"Sergey Organov","fromEmail":"sorganov@gmail.com","sentAt":"2021-05-07T14:02:54Z","receivedAt":"2021-05-07T14:03:02Z","isPatch":true,"sender":{"key":"sorganov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8501568?v=4"},"body":"Felipe Contreras writes:\n\n> Firmin Martin wrote:\n>> Currently, git-format-patch, along with the option --cover-letter,\n>> unconditionally overwrites a cover letter with the same name (if\n>> present). Although this is a desired behaviour for patches which are\n>> auto-generated from Git commits log, it might not be the case for a\n>> cover letter whose the content is meticulously written manually.\n>\n> This is one of the reasons I never use git format-patch directly, but I\n> use a tool on top: git send-series[1].\n>\n> It would be nice if git format-patch grabbed the text of the body from\n> somewhere,\n\nIt does already. I use:\n\n git format-patch --cover-letter --cover-from-description=auto\n\nthat takes both subject and text for cover letter from branch\ndescription.\n\n> and even better if git branch learned --edit-cover-letter.\n\nIt reads:\n\n git branch --edit-description\n\nWorks for me.\n\n-- Sergey Organov\n"},{"id":"423989","messageId":"878s4ngta2.fsf@Inspiron.i-did-not-set--mail-host-address--so-tickle-me","threadId":"55635","inReplyTo":"xmqqk0obwhi2.fsf@gitster.g","subject":"Re: [PATCH v1 2/8] format-patch: confirmation whenever patches exist","fromName":"Firmin Martin","fromEmail":"firminmartin24@gmail.com","sentAt":"2021-05-10T03:30:13Z","receivedAt":"2021-05-10T03:30:26Z","isPatch":true,"sender":{"key":"firminmartin24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20540328?v=4"},"body":"\nHi Junio,\n\nJunio C Hamano writes:\n\n> Firmin Martin writes:\n>\n>> Currently, git-format-patch, along with the option --cover-letter,\n>> unconditionaly overwrites a cover letter with the same name (if\n>> present). Although this is a desired behaviour for patches which are\n>> auto-generated from Git commits log, it might not be the case for a\n>> cover letter whose the content is meticulously written manually.\n>\n> True. But if we require confirmation before overwriting patches,\n> that would be overall worsening the end-user experience, I am\n> afraid. In a 5-patch series with a cover-letter that was formatted,\n> proofread, corrected with \"rebase -i\" and then re-formatted, unless\n> you rephrased the titles of the patches, you'd get prompted once for\n> the cover letter (which *IS* valuable) and five-times for patches\n> (which is annoying).\nThis is true for this patch, but the semantics changed after the patch\n#3. I really should have squashed them together to not create\nconfusion. Sorry about that.\n\nAfter patch #3:\n\n- The prompt happens only one time so that the user could decide whether they\nwant overwrite the existing file and subsequent ones. In your example,\nthe session would go like this:\n\n git format-patch --cover-letter -o patch upstream/master\n\n The file 'patch/0000-cover-letter.patch' already exists.\n Would you overwrite this file and subsequent ones [Y/n]? n\n fatal: failed to create cover-letter file\n\n(replying Y would overwrite the cover letter and the patches as usual)\n \n- The default behaviour affects only cover letters, meaning that, in\n your example, if the user employs instead \n\n git format-patch -o patch upstream/master\n\n(without --cover-letter) the second time, the patches are overwritten\nwithout any disturbance.\n\nAfter all, the point of this patch series is to prevent overwriting an\nexisting cover letter by using a cover letter template\n(--cover-letter). \n\n>\n> It also is unfortunate that this change does not address another\n> annoyance after retitling a patch---the stale output from the\n> previous run is not overwritten with the updated one but is left in\n> the same directory as the output files from the latest run.\n\n\n> So, while I very much do welcome the motivation, I am not onboard\n> with this particular design.\n>\n>> diff --git a/builtin/log.c b/builtin/log.c\n>> index 6102893fcc..bada3db9eb 100644\n>> --- a/builtin/log.c\n>> +++ b/builtin/log.c\n>> @@ -35,6 +35,7 @@\n>> #include \"repository.h\"\n>> #include \"commit-reach.h\"\n>> #include \"range-diff.h\"\n>> +#include \"prompt.h\"\n>> \n>> #define MAIL_DEFAULT_WRAP 72\n>> #define COVER_FROM_AUTO_MAX_SUBJECT_LEN 100\n>> @@ -959,6 +960,10 @@ static int open_next_file(struct commit *commit, const char *subject,\n>> \t\t\t struct rev_info *rev, int quiet)\n>> {\n>> \tstruct strbuf filename = STRBUF_INIT;\n>> +\tstruct strbuf file_exists_prompt = STRBUF_INIT;\n>> +\tconst char *yesno;\n>> +\tstatic int not_prompted = 1;\n>\n> When the API passes a structure that keeps track of state (like\n> \"struct rev_info *rev\", used to hold rev->diffopt.file which is\n> clearly a state), add a member to it, instead of inventing a\n> function local static that will hurt reuse of the API you are\n> touching. This static variable will make it impossible from a\n> single process to format two patch series, but if it is made a part\n> of rev_info, you do not have to introduce such a limitation.\n\nOK, I will keep in mind of that. But after the discussion on git_prompt,\nI think this variable will be dropped.\n\nMany thanks for your comments,\n\nFirmin\n"},{"id":"423992","messageId":"875yzrgr1f.fsf@Inspiron.i-did-not-set--mail-host-address--so-tickle-me","threadId":"55635","inReplyTo":"YJTH+sTP/O5Nxtp9@coredump.intra.peff.net","subject":"Re: [PATCH v1 1/8] compat/terminal: let prompt accept input from pipe","fromName":"Firmin Martin","fromEmail":"firminmartin24@gmail.com","sentAt":"2021-05-10T04:18:36Z","receivedAt":"2021-05-10T04:18:41Z","isPatch":true,"sender":{"key":"firminmartin24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20540328?v=4"},"body":"Jeff King writes:\n\nHi Peff,\n\n> On Fri, May 07, 2021 at 08:37:49AM +0900, Junio C Hamano wrote:\n>\n>> Firmin Martin writes:\n>> \n>> > Currently, git_prompt ignores input coming from anywhere other than\n>> > terminal (pipe, redirection etc.) meaning that standard prompt\n>> > auto-answering methods would have no effect:\n>> >\n>> > echo 'Y' | git ...\n>> > yes 'Y' | git ...\n>> > git ... > >\n>> > It also prevents git subcommands using git_prompt to be tested using\n>> > such methods.\n>> \n>> For testing, wouldn't lib-terminal.sh be usable for your purpose?\n>> If not, what is the reason why it is insufficient? Can we fix that\n>> instead?\n>\n> That doesn't work, because it insists on reading from /dev/tty and not\n> the pty that lib-terminal will set up as stdin. But...\n>\n>> Allowing prompter to read from pipe has a big downside in the\n>> production code: you cannot pipe data into our command, and let it\n>> ask interactive questions from the end user by opening /dev/tty.\n>\n> Right. The main purpose of the function was to let git-remote-https,\n> whose stdin is connected to git-fetch, get a password from the user.\n> Reading from stdin would break things badly there[1].\n>\n> Looking at the second patch, the motivation here seems to be to use\n> git_prompt() for another run-of-the-mill prompt. But the right answer\n> is: don't do that. In fact, we recently-ish removed a similar case in\n> 97387c8bdd (am: read interactive input from stdin, 2019-05-20) that was\n> likewise causing problems with the test suite.\n\nI actually inspired myself from the two occurrences of git_prompt in\nbuiltin/bisect--helper.c introduced in 09535f056b (bisect--helper:\nreimplement `bisect_autostart` shell function in C, 2020-09-24).\nNot sure if they should also be converted to a simple fgets.\nI will do that in the v2 of this series if the prompt is still proven useful.\n\n>\n> I think we might consider renaming git_prompt(), or adding an\n> explanatory comment above it.\n\nI would be happy to do that. A comment along the line of 97387c8bdd (am:\nread interactive input from stdin, 2019-05-20) and a \"CAUTION: don't use\nit for regular prompt\" would suffice ?\n\n>\n> -Peff\n>\n> [1] Sadly I don't think our test suite could notice the breakage\n> introduced by this function. It uses the askpass feature to avoid\n> triggering this code at all, because of course we can not reliably\n> read from /dev/tty in the script. But with just this patch applied,\n> and no credential helpers defined, trying \"git ls-remote\n> https://github.com/you/some-private-repo\" shows the problem: you get\n> prompted, but it never reads your input.\n\nMany thanks for your comments,\n\nFirmin\n"},{"id":"423993","messageId":"8735uvgqv7.fsf@Inspiron.i-did-not-set--mail-host-address--so-tickle-me","threadId":"55635","inReplyTo":"e0bc4aef-a92a-20f2-0b9f-d65917710e3b@gmail.com","subject":"Re: [PATCH v1 7/8] doc/format-patch: describe --confirm-overwrite","fromName":"Firmin Martin","fromEmail":"firminmartin24@gmail.com","sentAt":"2021-05-10T04:22:20Z","receivedAt":"2021-05-10T04:22:26Z","isPatch":true,"sender":{"key":"firminmartin24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20540328?v=4"},"body":"Hi Bagas,\n\nBagas Sanjaya writes:\n\n> On 06/05/21 23.51, Firmin Martin wrote:\n>> +always;;\n>> +never;;\n>> +\tAlways/never prompt for confirmation whenever patches or a cover letter\n>> +\tare subject to be overwritten.\n>> +cover;;\n>> +\tAsk confirmation whenever a cover letter is subject to be overwritten.\n>> +--\n>\n> For `always` and `never`, I think s/patches or/patches and\\/or\n> For `cover`, I think s/whenever/whenever only (add `only` after `whenever`\n\nMakes sense. I will change it if this text remains in v2.\n\n>\n> -- \n> An old man doll... just what I always wanted! - Clara\n\nThanks,\n\nFirmin\n"},{"id":"424009","messageId":"xmqq8s4nqc0u.fsf@gitster.g","threadId":"55635","inReplyTo":"878s4ngta2.fsf@Inspiron.i-did-not-set--mail-host-address--so-tickle-me","subject":"Re: [PATCH v1 2/8] format-patch: confirmation whenever patches exist","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-10T07:32:49Z","receivedAt":"2021-05-10T07:32:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Firmin Martin writes:\n\n>> True. But if we require confirmation before overwriting patches,\n>> that would be overall worsening the end-user experience, I am\n>> afraid. In a 5-patch series with a cover-letter that was formatted,\n>> proofread, corrected with \"rebase -i\" and then re-formatted, unless\n>> you rephrased the titles of the patches, you'd get prompted once for\n>> the cover letter (which *IS* valuable) and five-times for patches\n>> (which is annoying).\n> This is true for this patch, but the semantics changed after the patch\n> #3. I really should have squashed them together to not create\n> confusion. Sorry about that.\n\nNo, please keep them separate. What we can do to avoid confusion\nlike I showed is to make a note on the earlier one, saying \"with\nthis the user experience looks like this, which may be suboptimal\nfor such and such reasons, but in a later step it will be improved\nin this and that way\".\n"},{"id":"424025","messageId":"87r1ieu6q4.fsf@evledraar.gmail.com","threadId":"55635","inReplyTo":"20210506165102.123739-1-firminmartin24@gmail.com","subject":"Re: [PATCH v1 0/8] format-patch: introduce --confirm-overwrite","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-05-10T12:02:19Z","receivedAt":"2021-05-10T12:51:14Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, May 06 2021, Firmin Martin wrote:\n\n> BACKGROUND\n> ==========\n>\n> Currently, git-format-patch, along with the option --cover-letter,\n> unconditionally overwrites a cover letter with the same name (if\n> present). Although this is a desired behaviour for patches which are\n> auto-generated from Git commits log, it might not be the case for a\n> cover letter whose the content is meticulously written manually.\n>\n> Particulary, this behaviour could be awkward in the following\n> hypothetical situations:\n>\n> * The user can easily erase a cover letter coming from prior versions or\n> another patch series by reusing an old command line (e.g. autocompleted\n> from the shell history).\n>\n> * Assuming that the user is writing a cover letter and realizes that\n> small changes should be made. They make the change, amend and\n> format-patch again to regenerate patches. If it happens that they use\n> the same command again (e.g. with --cover-letter), the cover letter\n> being written is gone.\n>\n> This patch series addresses this kind of issue by asking confirmation\n> from the user whenever a cover letter or a patch is subject to be\n> overwritten.\n\nI like the goal here, I'm another person with ad-hoc tooling around\nformat-patch to manage / avoid this particular scenario and related\nissues (e.g. I have a wrapper to rm -rf the output directory & re-crete\nit, in case I want to rebase but use the same -vN version).\n\nI wonder if you've considered some ways to automatically and more gently\ndetect these cases, such as:\n\n 1. When we generate the patch, set the mtime manually to the time in\n the commit object. When clobbering a file see if they correspond. If\n mtime != expected => *boom* and ask for confirmation.\n\n 2. We already include a blurb like \"2.31.1.838.g7ac6e98bb53\" (the git\n version) if you have format.signature set. How about making that\n include a short hash of the preceding lines. If it doesn't match we\n can ask, but if it does we clobber.\n\n This has the added benefit that other could script their MUAs to\n highlight manually edited patches.\n\n 3. Ditto #2 but generate the new file in a tempfile, diff them, if\n they're different complain. This also opens the door to make this\n neatly integrate with git-diff's -I option, so you could specify\n regexes to ignore. By default we could ignore lines that change\n known headers we expect to change.\n\n 4. The format we output would need parsing, but it's not that\n hard. I.e. we expect something like:\n\n [...] \n Subject: [PATCH 0/1] *** SUBJECT HERE ***\n MIME-Version: 1.0\n Content-Type: text/plain; charset=UTF-8\n Content-Transfer-Encoding: 8bit\n \n *** BLURB HERE ***\n \n Ævar Arnfjörð Bjarmason (1):\n [...]\n\n\n And similarly for patches we could narrow things to look between the\n \"---\" and an expected diffstat. So we could complain just about\n changes there.\n\n But maybe that's a fool's errand, e.g. it would be hard to catch\n manually commented-on range-diffs without implementing a full parser\n for that format...\n\nNone of these suggestions should be read as making perfect the enemy of\nthe good. I *do* rely on the behavior of the setting you're introducing\nand \"breaking\", but I think the user-base of advanced format-patch users\nis small enough that we could just configure things to \"never\" and move\non, but accidentally losing data (as happens now) is a worse default...\n"},{"id":"424079","messageId":"YJmmXZdwSoR+vxjw@coredump.intra.peff.net","threadId":"55635","inReplyTo":"875yzrgr1f.fsf@Inspiron.i-did-not-set--mail-host-address--so-tickle-me","subject":"Re: [PATCH v1 1/8] compat/terminal: let prompt accept input from pipe","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-05-10T21:32:13Z","receivedAt":"2021-05-10T21:32:17Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, May 10, 2021 at 06:18:36AM +0200, Firmin Martin wrote:\n\n> > Looking at the second patch, the motivation here seems to be to use\n> > git_prompt() for another run-of-the-mill prompt. But the right answer\n> > is: don't do that. In fact, we recently-ish removed a similar case in\n> > 97387c8bdd (am: read interactive input from stdin, 2019-05-20) that was\n> > likewise causing problems with the test suite.\n> \n> I actually inspired myself from the two occurrences of git_prompt in\n> builtin/bisect--helper.c introduced in 09535f056b (bisect--helper:\n> reimplement `bisect_autostart` shell function in C, 2020-09-24).\n> Not sure if they should also be converted to a simple fgets.\n\nYes, I think they should be switched.\n\nIt looks like since my earlier patches to \"am\" we have grown a\ngit_read_line_interactively() helper. See:\n\n https://lore.kernel.org/git/pull.755.git.git.1586374380709.gitgitgadget@gmail.com/\n\nThey should probably use that (and likewise, it would make sense for\ngit-am to switch to it).\n\n> > I think we might consider renaming git_prompt(), or adding an\n> > explanatory comment above it.\n> \n> I would be happy to do that. A comment along the line of 97387c8bdd (am:\n> read interactive input from stdin, 2019-05-20) and a \"CAUTION: don't use\n> it for regular prompt\" would suffice ?\n\nYeah. You might want to point people at git_read_line_interactively() in\nthe same header file, too.\n\n-Peff\n"},{"id":"424084","messageId":"87zgx2f7h9.fsf@Inspiron.i-did-not-set--mail-host-address--so-tickle-me","threadId":"55635","inReplyTo":"xmqqv97vwkzo.fsf@gitster.g","subject":"Re: [PATCH v1 0/8] format-patch: introduce --confirm-overwrite","fromName":"Firmin Martin","fromEmail":"firminmartin24@gmail.com","sentAt":"2021-05-11T00:18:42Z","receivedAt":"2021-05-11T00:18:47Z","isPatch":true,"sender":{"key":"firminmartin24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20540328?v=4"},"body":"Junio C Hamano writes:\n\n> Firmin Martin writes:\n>\n>> Particulary, this behaviour could be awkward in the following\n>> hypothetical situations:\n>>\n>> * The user can easily erase a cover letter coming from prior versions or\n>> another patch series by reusing an old command line (e.g. autocompleted\n>> from the shell history).\n>\n> \"prior versions\" implies that the user is better off using -v$n\n> where $n is the number greater than the one used for the prior\n> iteration by one, and there won't be any overwriting, so this is not\n> a very compelling use case.\n\nTrue, silly of me.\n\n>\n> But the next one is real.\n>\n>> * Assuming that the user is writing a cover letter and realizes that\n>> small changes should be made. They make the change, amend and\n>> format-patch again to regenerate patches. If it happens that they use\n>> the same command again (e.g. with --cover-letter), the cover letter\n>> being written is gone.\n>\n> Yes, after preparing, say, -v2, but before sending them out, it is\n> very plausible that proofreading of your own patches may make you\n> realize more issues in the series, which may make you go back to your\n> commits, \"rebase -i\" to improve them and re-run \"format-patch -v2\".\n>\n> We do want to encourage such careful preparation of your patch\n> series before sending it out, and we want to support it well with\n> our tools. Preventing overwriting of the cover (which will have the\n> same filename, with the same v2- prefix) is very valuable here.\n>\n> There is another thing that I suspect people may find irritating in\n> the same workflow. If you fix the commit title while \"rebase -i\" to\n> polish your v2 patch, it would result in a different filename from\n> the original v2, so you'd end up with something like\n>\n> v2-0000-cover-letter.patch\n> v2-0001-thes-forny-change.patch\n> v2-0001-this-phoney-change.patch\n> v2-0002-another-sample-change.patch\n>\n> while redoing a two-patch series. The \"thes-forny\" thing is a\n> leftover from the first \"format-patch -v2\" run, you fixed typoes\n> with \"rebase -i\" after a self-review and other three files have the\n> result of the second \"format-patch -v2\" run. You need a way to\n> somehow exclude that stale file when driving send-email; in other\n> words, before running\n>\n> git send-email v2-*.patch\n>\n> you would want to move away v2-0001-thes-forny-change.patch that no\n> longer is part of the series. I wonder if format-patch can help by\n> looking at the output directory before writing its output and move\n> the old files away, say, to \"old-v2-*.patch\" or something? That\n\nI would go with v2-*.patch.old as it won't be matched by *.patch\n(something tempting to do when only one version is present). Or, we can\neven keep their filenames unchanged and move them into a subdirectory\nold_/.\n\nThe case of --numbered-files should also be taking into account. In the\nsame fashion, the files that will be renamed/moved would be those which\nare constituted with numbers.\n\n> incidentally would solve your \"files getting overwritten is\n> irritating\" issue at the same time.\n\nI will work towards this.\n\n>\n> Coming back to the topic of cover letter, even when there is no risk\n> of ovetwriting, there is another thing we may want to improve to\n> help our users. Suppose you are preparing your v2 patch after\n> sending out the v1. The cover letter we generate for v2 will have\n> the same **BOILERPLATE** placeholders that need to be filled from\n> scratch. As many things you wrote for the cover letter in the\n> previous round should be reusable, even though the list of titles of\n> the patch should be generated afresh, it would be nice if\n> format-patch can carry forward what you wrote in the cover letter\n> for the v1 iteration to the cover letter for this v2 iteration.\n>\n> And when we have such a \"reuse description in the existing cover\n> letter\" support, the value of \"don't overwrite\" knob will mostly go\n> away. Instead of failing the command by refusing to overwrite, you\n> can read what is in the existing cover-letter that is about to be\n> overwritten, combine the hand-written description with the material\n> automatically generated, and ovewrite the existing file, and as long\n> as you do a good job of preserving, nobody would complain that you\n> overwrote the file.\n\nI have thought about that before this series and agree that this is the\nway to go, but I found it a bit challenging: how should I distinguish\nthe cover letter body and the shortlog (i.e. any content we don't want\nto keep to the next cover letter), given that the user could do anything\nthey want (e.g. remove the shortlog, write after the shortlog, etc.) ?\n\nTo easier this task, should we introduce a delimiter between the letter\nbody and the shortlog as we already do with patches right after the\ncommit message and before the diffstat (e.g. \"---\") ? I ignore whether\nthere are functions we can directly reuse in trailer.c for this purpose.\n\nThanks,\n\nFirmin\n"},{"id":"424085","messageId":"87wns6f66n.fsf@Inspiron.i-did-not-set--mail-host-address--so-tickle-me","threadId":"55635","inReplyTo":"60949be8613c1_8c2220882@natae.notmuch","subject":"RE: [PATCH v1 0/8] format-patch: introduce --confirm-overwrite","fromName":"Firmin Martin","fromEmail":"firminmartin24@gmail.com","sentAt":"2021-05-11T00:46:40Z","receivedAt":"2021-05-11T00:46:44Z","isPatch":true,"sender":{"key":"firminmartin24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20540328?v=4"},"body":"Hi Felipe,\n\nFelipe Contreras writes:\n\n> Firmin Martin wrote:\n>> Currently, git-format-patch, along with the option --cover-letter,\n>> unconditionally overwrites a cover letter with the same name (if\n>> present). Although this is a desired behaviour for patches which are\n>> auto-generated from Git commits log, it might not be the case for a\n>> cover letter whose the content is meticulously written manually.\n>\n> This is one of the reasons I never use git format-patch directly, but I\n> use a tool on top: git send-series[1].\n\nThis is good to know. As a newcomer to email-based workflow, I ignored\nhow people use git format-patch/send-mail efficiently.\n\n> It would be nice if git format-patch grabbed the text of the body from\n> somewhere,\nIn v2, I planned to grab the letter body from the cover letter subject\nto being overwritten. Maybe if such a letter doesn't exist, we can instead\ninherit the content of the cover letter from prior series.\n\n> and even better if git branch learned --edit-cover-letter.\n> None of this invalidates the usefulness of your patches, of course.\n\n>\n> Cheers.\n>\n> [1] https://github.com/felipec/git-send-series\n>\n> -- \n> Felipe Contreras\n\nThanks for your comment,\n\nFirmin\n"},{"id":"424086","messageId":"87tunaf54u.fsf@Inspiron.i-did-not-set--mail-host-address--so-tickle-me","threadId":"55635","inReplyTo":"YJUAmUJiHmJRQBMj@generichostname","subject":"Re: [PATCH v1 0/8] format-patch: introduce --confirm-overwrite","fromName":"Firmin Martin","fromEmail":"firminmartin24@gmail.com","sentAt":"2021-05-11T01:09:21Z","receivedAt":"2021-05-11T01:09:27Z","isPatch":true,"sender":{"key":"firminmartin24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20540328?v=4"},"body":"Hi Denton,\n\nDenton Liu writes:\n\n> Hi Felipe,\n>\n> On Thu, May 06, 2021 at 08:46:16PM -0500, Felipe Contreras wrote:\n>> Firmin Martin wrote:\n>> > Currently, git-format-patch, along with the option --cover-letter,\n>> > unconditionally overwrites a cover letter with the same name (if\n>> > present). Although this is a desired behaviour for patches which are\n>> > auto-generated from Git commits log, it might not be the case for a\n>> > cover letter whose the content is meticulously written manually.\n>> \n>> This is one of the reasons I never use git format-patch directly, but I\n>> use a tool on top: git send-series[1].\n>\n> It seems like everyone has written some sort of tooling on top of\n> format-patch at this point. Taking a cursory look at your tool, perhaps\n> a feature like `--previous-cover-letter ` might provide most of\n> the functionality that most tooling that I've seen gives.\n\nThis is a good idea. We can default to the target cover letter\n(e.g., if -v2 is passed, v2-0000-cover-letter.patch or if\n--numbered-files is passed, 0) if present, or the previous series' cover\nletter.\n\n> Perhaps this option could parse a cover letter from a previous version\n> of a patch and use it to populate the next version number, In-Reply-To,\n> cover letter subject/body, To/Cc lists and maybe more.\n\nAbsolutely.\n\n> I think that extracting the information would be pretty easy but\n> designing the UI it in a non-obtuse way would be pretty challenging.\n>\n>> It would be nice if git format-patch grabbed the text of the body from\n>> somewhere, and even better if git branch learned --edit-cover-letter.\n>\n> Well, you're in luck! I wanted the same thing a couple of years back so\n> I implemented the --cover-from-description option[0]. It allows the cover\n> letter to be populated by the text given in\n> `git branch --edit-description`.\n\nThis is the reason I CCed you!\n\nThanks for your comment,\n\nFirmin\n\n\n>\n> -Denton\n>\n> [0]: https://git-scm.com/docs/git-format-patch#Documentation/git-format-patch.txt---cover-from-descriptionltmodegt\n"},{"id":"424093","messageId":"87pmxyez7k.fsf@Inspiron.i-did-not-set--mail-host-address--so-tickle-me","threadId":"55635","inReplyTo":"xmqq8s4nqc0u.fsf@gitster.g","subject":"Re: [PATCH v1 2/8] format-patch: confirmation whenever patches exist","fromName":"Firmin Martin","fromEmail":"firminmartin24@gmail.com","sentAt":"2021-05-11T03:17:19Z","receivedAt":"2021-05-11T03:17:24Z","isPatch":true,"sender":{"key":"firminmartin24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20540328?v=4"},"body":"Junio C Hamano writes:\n\n> Firmin Martin writes:\n>\n>>> True. But if we require confirmation before overwriting patches,\n>>> that would be overall worsening the end-user experience, I am\n>>> afraid. In a 5-patch series with a cover-letter that was formatted,\n>>> proofread, corrected with \"rebase -i\" and then re-formatted, unless\n>>> you rephrased the titles of the patches, you'd get prompted once for\n>>> the cover letter (which *IS* valuable) and five-times for patches\n>>> (which is annoying).\n>> This is true for this patch, but the semantics changed after the patch\n>> #3. I really should have squashed them together to not create\n>> confusion. Sorry about that.\n>\n> No, please keep them separate. What we can do to avoid confusion\n> like I showed is to make a note on the earlier one, saying \"with\n> this the user experience looks like this, which may be suboptimal\n> for such and such reasons, but in a later step it will be improved\n> in this and that way\".\n\nOk, it's noted.\n"},{"id":"424095","messageId":"xmqq1raeos7x.fsf@gitster.g","threadId":"55635","inReplyTo":"YJmmXZdwSoR+vxjw@coredump.intra.peff.net","subject":"Re: [PATCH v1 1/8] compat/terminal: let prompt accept input from pipe","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-11T03:38:10Z","receivedAt":"2021-05-11T03:38:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Mon, May 10, 2021 at 06:18:36AM +0200, Firmin Martin wrote:\n>\n>> > Looking at the second patch, the motivation here seems to be to use\n>> > git_prompt() for another run-of-the-mill prompt. But the right answer\n>> > is: don't do that. In fact, we recently-ish removed a similar case in\n>> > 97387c8bdd (am: read interactive input from stdin, 2019-05-20) that was\n>> > likewise causing problems with the test suite.\n>> \n>> I actually inspired myself from the two occurrences of git_prompt in\n>> builtin/bisect--helper.c introduced in 09535f056b (bisect--helper:\n>> reimplement `bisect_autostart` shell function in C, 2020-09-24).\n>> Not sure if they should also be converted to a simple fgets.\n>\n> Yes, I think they should be switched.\n\nOK, that is because in the context of a \"bisect\" session, we won't\nbe feeding any real data from its standard input, unlike \"git am\"\nthat may well be eating a patch stream from its standard input\nstream. If so, makes sense.\n\n"},{"id":"424101","messageId":"609a1007ec851_5b0e1208e5@natae.notmuch","threadId":"55635","inReplyTo":"YJUAmUJiHmJRQBMj@generichostname","subject":"Re: [PATCH v1 0/8] format-patch: introduce --confirm-overwrite","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-11T05:03:03Z","receivedAt":"2021-05-11T05:03:09Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Denton Liu wrote:\n> Hi Felipe,\n> \n> On Thu, May 06, 2021 at 08:46:16PM -0500, Felipe Contreras wrote:\n> > Firmin Martin wrote:\n> > > Currently, git-format-patch, along with the option --cover-letter,\n> > > unconditionally overwrites a cover letter with the same name (if\n> > > present). Although this is a desired behaviour for patches which are\n> > > auto-generated from Git commits log, it might not be the case for a\n> > > cover letter whose the content is meticulously written manually.\n> > \n> > This is one of the reasons I never use git format-patch directly, but I\n> > use a tool on top: git send-series[1].\n> \n> It seems like everyone has written some sort of tooling on top of\n> format-patch at this point. Taking a cursory look at your tool, perhaps\n> a feature like `--previous-cover-letter ` might provide most of\n> the functionality that most tooling that I've seen gives.\n\nIf that worked correctly, maybe, but not for my tool.\n\nSome of the features still missing:\n\n * List of people to cc\n * refs of where the branch was in vX\n * Automatic rangediff\n * Storing other metadata like last Message-Id\n\n> Perhaps this option could parse a cover letter from a previous version\n> of a patch and use it to populate the next version number, In-Reply-To,\n> cover letter subject/body, To/Cc lists and maybe more. I think that\n> extracting the information would be pretty easy but designing the UI it\n> in a non-obtuse way would be pretty challenging.\n\nWhere would you put the Cc list for example?\n\n> > It would be nice if git format-patch grabbed the text of the body from\n> > somewhere, and even better if git branch learned --edit-cover-letter.\n> \n> Well, you're in luck! I wanted the same thing a couple of years back so\n> I implemented the --cover-from-description option[0]. It allows the cover\n> letter to be populated by the text given in\n> `git branch --edit-description`.\n\nI did see --cover-from-description and thought of making use of it on my\ntool, but I thought it only updated the subject of the cover-letter. Now\nI see --cover-from-description=subject does exactly what I would want.\nNice! Although I've no idea why that option is called \"subject\".\n\nMy only comment is: why doesn't --cover-from-description do something\nuseful?\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"424102","messageId":"609a124a3cdd6_5b0e12081c@natae.notmuch","threadId":"55635","inReplyTo":"87tunaf54u.fsf@Inspiron.i-did-not-set--mail-host-address--so-tickle-me","subject":"Re: [PATCH v1 0/8] format-patch: introduce --confirm-overwrite","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-11T05:12:42Z","receivedAt":"2021-05-11T05:12:47Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Firmin Martin wrote:\n> Denton Liu writes:\n> > It seems like everyone has written some sort of tooling on top of\n> > format-patch at this point. Taking a cursory look at your tool, perhaps\n> > a feature like `--previous-cover-letter ` might provide most of\n> > the functionality that most tooling that I've seen gives.\n> \n> This is a good idea. We can default to the target cover letter\n> (e.g., if -v2 is passed, v2-0000-cover-letter.patch or if\n> --numbered-files is passed, 0) if present, or the previous series' cover\n> letter.\n\nThat's a good idea.\n\nActually I don't see any reason why by default it isn't\n0000-v2-cover-letter.patch.\n\nIf it was it would diminish the need to warn people before overwriting\n0000-cover-letter.patch.\n\n-- \nFelipe Contreras\n"},{"id":"424106","messageId":"YJof0/Eq18QnLumV@coredump.intra.peff.net","threadId":"55635","inReplyTo":"xmqq1raeos7x.fsf@gitster.g","subject":"Re: [PATCH v1 1/8] compat/terminal: let prompt accept input from pipe","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-05-11T06:10:27Z","receivedAt":"2021-05-11T06:10:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, May 11, 2021 at 12:38:10PM +0900, Junio C Hamano wrote:\n\n> >> I actually inspired myself from the two occurrences of git_prompt in\n> >> builtin/bisect--helper.c introduced in 09535f056b (bisect--helper:\n> >> reimplement `bisect_autostart` shell function in C, 2020-09-24).\n> >> Not sure if they should also be converted to a simple fgets.\n> >\n> > Yes, I think they should be switched.\n> \n> OK, that is because in the context of a \"bisect\" session, we won't\n> be feeding any real data from its standard input, unlike \"git am\"\n> that may well be eating a patch stream from its standard input\n> stream. If so, makes sense.\n\nYes, though even in \"git am\", we forbid using interactive mode with\npatches on stdin (and did so even when we were reading from the tty;\npresumably the rule dates back to when it was a shell script and was\nusing stdin).\n\n-Peff\n"},{"id":"424109","messageId":"xmqq35utokuu.fsf@gitster.g","threadId":"55635","inReplyTo":"YJof0/Eq18QnLumV@coredump.intra.peff.net","subject":"Re: [PATCH v1 1/8] compat/terminal: let prompt accept input from pipe","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-11T06:17:13Z","receivedAt":"2021-05-11T06:17:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Tue, May 11, 2021 at 12:38:10PM +0900, Junio C Hamano wrote:\n>\n>> >> I actually inspired myself from the two occurrences of git_prompt in\n>> >> builtin/bisect--helper.c introduced in 09535f056b (bisect--helper:\n>> >> reimplement `bisect_autostart` shell function in C, 2020-09-24).\n>> >> Not sure if they should also be converted to a simple fgets.\n>> >\n>> > Yes, I think they should be switched.\n>> \n>> OK, that is because in the context of a \"bisect\" session, we won't\n>> be feeding any real data from its standard input, unlike \"git am\"\n>> that may well be eating a patch stream from its standard input\n>> stream. If so, makes sense.\n>\n> Yes, though even in \"git am\", we forbid using interactive mode with\n> patches on stdin (and did so even when we were reading from the tty;\n> presumably the rule dates back to when it was a shell script and was\n> using stdin).\n\nAs long as the \"prompt and accept an single-line answer from the end\nuser\" is restricted to \"git am -i\", I'll be perfectly OK with that.\nI just do not want my regular \"type '|' in my MUA to pipe the\ncurrent article to a command, and give 'git am -s' as the command\"\nworkflow to get broken in the future when somebody blindly follows a\ncarelessly written direction to use a helper that reads from the\nstandard input for confirmation. The condition under which use of\nthat helper is appropriate needs to be clearly spelled out.\n\nThanks.\n"},{"id":"424111","messageId":"YJomFMGsurmmEetB@coredump.intra.peff.net","threadId":"55635","inReplyTo":"xmqq35utokuu.fsf@gitster.g","subject":"Re: [PATCH v1 1/8] compat/terminal: let prompt accept input from pipe","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-05-11T06:37:08Z","receivedAt":"2021-05-11T06:37:13Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, May 11, 2021 at 03:17:13PM +0900, Junio C Hamano wrote:\n\n> >> OK, that is because in the context of a \"bisect\" session, we won't\n> >> be feeding any real data from its standard input, unlike \"git am\"\n> >> that may well be eating a patch stream from its standard input\n> >> stream. If so, makes sense.\n> >\n> > Yes, though even in \"git am\", we forbid using interactive mode with\n> > patches on stdin (and did so even when we were reading from the tty;\n> > presumably the rule dates back to when it was a shell script and was\n> > using stdin).\n> \n> As long as the \"prompt and accept an single-line answer from the end\n> user\" is restricted to \"git am -i\", I'll be perfectly OK with that.\n> I just do not want my regular \"type '|' in my MUA to pipe the\n> current article to a command, and give 'git am -s' as the command\"\n> workflow to get broken in the future when somebody blindly follows a\n> carelessly written direction to use a helper that reads from the\n> standard input for confirmation. The condition under which use of\n> that helper is appropriate needs to be clearly spelled out.\n\nYeah, I don't think anybody is proposing to change the behavior of \"git\nam\" here (we might swap out the current fgets(stdin) for a helper which\ndoes the equivalent). But I agree that any comment recommending one\nversus the other should probably remind people to think about how stdin\nis otherwise used in the program, and whether that will cause any\nconflicts.\n\n-Peff\n"}]}