{"thread":{"id":"50566","subject":"[PATCH 0/3] format-patch --no-clobber","startedAt":"2019-02-22T20:11:16Z","lastAt":"2019-03-12T20:53:26Z","messageCount":9,"participants":["Junio C Hamano","Eric Sunshine","Jeff King","Ntentos Stavros"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"369971","messageId":"20190222201111.98196-1-gitster@pobox.com","threadId":"50566","inReplyTo":null,"subject":"[PATCH 0/3] format-patch --no-clobber","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-02-22T20:11:08Z","receivedAt":"2019-02-22T20:11:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"If you keep an output for an older iteration of the same topic in\nthe same directory around and use \"git format-patch\" to prepare a\nnewer iteration of the topic, those commits that happen to be at the\nsame position in the series that have not been retitled will get the\nsame filename---and the command opens them for writing without any\ncheck.\n\nExisting \"-o outdir\" and \"-v number\" options are both good ways to\navoid such name collisions, and in general helps to give good ways\nto compare the latest iteration with older iteration(s), but let's\nsee if \"--no-clobber\" option that forbids overwrting existing files\nwould also help people.\n\nI meant to have just a single patch for this, but it ended up to be\na three-patch series.\n\n - Preliminary clean-up of builtin/log.c updates messages in the\n   file given to die()s and error()s that begin with capital by\n   downcasing them to match majority of the messages in the system.\n\n - When we fail to open a patchfile for writing, we immediately stop\n   and report failure, but we weren't careful about the cover letter\n   output, which is fixed by the second patch.\n\n - The last one adds an experimental \"--no-clobber\" option to forbid\n   the command from overwriting existing output files.  This is not\n   enabled by default, as suggeted by Brian Carlson in [1], at least\n   for now.\n\nJunio C Hamano (3):\n  builtin/log: downcase the beginning of error messages\n  format-patch: notice failure to open cover letter for writing\n  format-patch: --no-clobber refrains from overwriting output files\n\n Documentation/git-format-patch.txt |  8 +++-\n builtin/log.c                      | 72 +++++++++++++++++++-----------\n t/t4014-format-patch.sh            | 22 +++++++++\n 3 files changed, 75 insertions(+), 27 deletions(-)\n\n\n\n*1* <20190222001145.GD488342@genre.crustytoothpaste.net>\n\n\n\n-- \n2.21.0-rc2\n\n\n"},{"id":"369972","messageId":"20190222201111.98196-2-gitster@pobox.com","threadId":"50566","inReplyTo":"20190222201111.98196-1-gitster@pobox.com","subject":"[PATCH 1/3] builtin/log: downcase the beginning of error messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-02-22T20:11:09Z","receivedAt":"2019-02-22T20:11:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Signed-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/log.c | 38 +++++++++++++++++++-------------------\n 1 file changed, 19 insertions(+), 19 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 57869267d8..f2d1fbf18a 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -513,7 +513,7 @@ static int show_blob_object(const struct object_id *oid, struct rev_info *rev, c\n \tif (get_oid_with_context(the_repository, obj_name,\n \t\t\t\t GET_OID_RECORD_PATH,\n \t\t\t\t &oidc, &obj_context))\n-\t\tdie(_(\"Not a valid object name %s\"), obj_name);\n+\t\tdie(_(\"not a valid object name %s\"), obj_name);\n \tif (!obj_context.path ||\n \t    !textconv_object(the_repository, obj_context.path,\n \t\t\t     obj_context.mode, &oidc, 1, &buf, &size)) {\n@@ -537,7 +537,7 @@ static int show_tag_object(const struct object_id *oid, struct rev_info *rev)\n \tint offset = 0;\n \n \tif (!buf)\n-\t\treturn error(_(\"Could not read object %s\"), oid_to_hex(oid));\n+\t\treturn error(_(\"could not read object %s\"), oid_to_hex(oid));\n \n \tassert(type == OBJ_TAG);\n \twhile (offset < size && buf[offset] != '\\n') {\n@@ -631,7 +631,7 @@ int cmd_show(int argc, const char **argv, const char *prefix)\n \t\t\t\tbreak;\n \t\t\to = parse_object(the_repository, &t->tagged->oid);\n \t\t\tif (!o)\n-\t\t\t\tret = error(_(\"Could not read object %s\"),\n+\t\t\t\tret = error(_(\"could not read object %s\"),\n \t\t\t\t\t    oid_to_hex(&t->tagged->oid));\n \t\t\tobjects[i].item = o;\n \t\t\ti--;\n@@ -656,7 +656,7 @@ int cmd_show(int argc, const char **argv, const char *prefix)\n \t\t\tret = cmd_log_walk(&rev);\n \t\t\tbreak;\n \t\tdefault:\n-\t\t\tret = error(_(\"Unknown type: %d\"), o->type);\n+\t\t\tret = error(_(\"unknown type: %d\"), o->type);\n \t\t}\n \t}\n \tfree(objects);\n@@ -894,7 +894,7 @@ static int open_next_file(struct commit *commit, const char *subject,\n \t\tprintf(\"%s\\n\", filename.buf + outdir_offset);\n \n \tif ((rev->diffopt.file = fopen(filename.buf, \"w\")) == NULL) {\n-\t\terror_errno(_(\"Cannot open patch file %s\"), filename.buf);\n+\t\terror_errno(_(\"cannot open patch file %s\"), filename.buf);\n \t\tstrbuf_release(&filename);\n \t\treturn -1;\n \t}\n@@ -911,7 +911,7 @@ static void get_patch_ids(struct rev_info *rev, struct patch_ids *ids)\n \tunsigned flags1, flags2;\n \n \tif (rev->pending.nr != 2)\n-\t\tdie(_(\"Need exactly one range.\"));\n+\t\tdie(_(\"need exactly one range.\"));\n \n \to1 = rev->pending.objects[0].item;\n \to2 = rev->pending.objects[1].item;\n@@ -921,7 +921,7 @@ static void get_patch_ids(struct rev_info *rev, struct patch_ids *ids)\n \tc2 = lookup_commit_reference(the_repository, &o2->oid);\n \n \tif ((flags1 & UNINTERESTING) == (flags2 & UNINTERESTING))\n-\t\tdie(_(\"Not a range.\"));\n+\t\tdie(_(\"not a range.\"));\n \n \tinit_patch_ids(the_repository, ids);\n \n@@ -1044,7 +1044,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,\n \tstruct commit *head = list[0];\n \n \tif (!cmit_fmt_is_mail(rev->commit_format))\n-\t\tdie(_(\"Cover letter needs email format\"));\n+\t\tdie(_(\"cover letter needs email format\"));\n \n \tcommitter = git_committer_info(0);\n \n@@ -1214,7 +1214,7 @@ static int output_directory_callback(const struct option *opt, const char *arg,\n \tconst char **dir = (const char **)opt->value;\n \tBUG_ON_OPT_NEG(unset);\n \tif (*dir)\n-\t\tdie(_(\"Two output directories?\"));\n+\t\tdie(_(\"two output directories?\"));\n \t*dir = arg;\n \treturn 0;\n }\n@@ -1321,7 +1321,7 @@ static struct commit *get_base_commit(const char *base_commit,\n \tif (base_commit && strcmp(base_commit, \"auto\")) {\n \t\tbase = lookup_commit_reference_by_name(base_commit);\n \t\tif (!base)\n-\t\t\tdie(_(\"Unknown commit %s\"), base_commit);\n+\t\t\tdie(_(\"unknown commit %s\"), base_commit);\n \t} else if ((base_commit && !strcmp(base_commit, \"auto\")) || base_auto) {\n \t\tstruct branch *curr_branch = branch_get(NULL);\n \t\tconst char *upstream = branch_get_upstream(curr_branch, NULL);\n@@ -1331,16 +1331,16 @@ static struct commit *get_base_commit(const char *base_commit,\n \t\t\tstruct object_id oid;\n \n \t\t\tif (get_oid(upstream, &oid))\n-\t\t\t\tdie(_(\"Failed to resolve '%s' as a valid ref.\"), upstream);\n+\t\t\t\tdie(_(\"failed to resolve '%s' as a valid ref.\"), upstream);\n \t\t\tcommit = lookup_commit_or_die(&oid, \"upstream base\");\n \t\t\tbase_list = get_merge_bases_many(commit, total, list);\n \t\t\t/* There should be one and only one merge base. */\n \t\t\tif (!base_list || base_list->next)\n-\t\t\t\tdie(_(\"Could not find exact merge base.\"));\n+\t\t\t\tdie(_(\"could not find exact merge base.\"));\n \t\t\tbase = base_list->item;\n \t\t\tfree_commit_list(base_list);\n \t\t} else {\n-\t\t\tdie(_(\"Failed to get upstream, if you want to record base commit automatically,\\n\"\n+\t\t\tdie(_(\"failed to get upstream, if you want to record base commit automatically,\\n\"\n \t\t\t      \"please use git branch --set-upstream-to to track a remote branch.\\n\"\n \t\t\t      \"Or you could specify base commit by --base=<base-commit-id> manually.\"));\n \t\t}\n@@ -1360,7 +1360,7 @@ static struct commit *get_base_commit(const char *base_commit,\n \t\t\tstruct commit_list *merge_base;\n \t\t\tmerge_base = get_merge_bases(rev[2 * i], rev[2 * i + 1]);\n \t\t\tif (!merge_base || merge_base->next)\n-\t\t\t\tdie(_(\"Failed to find exact merge base\"));\n+\t\t\t\tdie(_(\"failed to find exact merge base\"));\n \n \t\t\trev[i] = merge_base->item;\n \t\t}\n@@ -1739,7 +1739,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\tif (use_stdout)\n \t\t\tdie(_(\"standard output, or directory, which one?\"));\n \t\tif (mkdir(output_directory, 0777) < 0 && errno != EEXIST)\n-\t\t\tdie_errno(_(\"Could not create directory '%s'\"),\n+\t\t\tdie_errno(_(\"could not create directory '%s'\"),\n \t\t\t\t  output_directory);\n \t}\n \n@@ -1941,7 +1941,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \n \t\tif (!use_stdout &&\n \t\t    open_next_file(rev.numbered_files ? NULL : commit, NULL, &rev, quiet))\n-\t\t\tdie(_(\"Failed to create output files\"));\n+\t\t\tdie(_(\"failed to create output files\"));\n \t\tshown = log_tree_commit(&rev, commit);\n \t\tfree_commit_buffer(the_repository->parsed_objects,\n \t\t\t\t   commit);\n@@ -2065,9 +2065,9 @@ int cmd_cherry(int argc, const char **argv, const char *prefix)\n \trevs.max_parents = 1;\n \n \tif (add_pending_commit(head, &revs, 0))\n-\t\tdie(_(\"Unknown commit %s\"), head);\n+\t\tdie(_(\"unknown commit %s\"), head);\n \tif (add_pending_commit(upstream, &revs, UNINTERESTING))\n-\t\tdie(_(\"Unknown commit %s\"), upstream);\n+\t\tdie(_(\"unknown commit %s\"), upstream);\n \n \t/* Don't say anything if head and upstream are the same. */\n \tif (revs.pending.nr == 2) {\n@@ -2079,7 +2079,7 @@ int cmd_cherry(int argc, const char **argv, const char *prefix)\n \tget_patch_ids(&revs, &ids);\n \n \tif (limit && add_pending_commit(limit, &revs, UNINTERESTING))\n-\t\tdie(_(\"Unknown commit %s\"), limit);\n+\t\tdie(_(\"unknown commit %s\"), limit);\n \n \t/* reverse the list of commits */\n \tif (prepare_revision_walk(&revs))\n-- \n2.21.0-rc2\n\n"},{"id":"369973","messageId":"20190222201111.98196-3-gitster@pobox.com","threadId":"50566","inReplyTo":"20190222201111.98196-1-gitster@pobox.com","subject":"[PATCH 2/3] format-patch: notice failure to open cover letter for writing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-02-22T20:11:10Z","receivedAt":"2019-02-22T20:11:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The make_cover_letter() function is supposed to open a new file for\nwriting, and let the caller write into it via FILE *rev->diffopt.file\nbut because the function does not return anything, the caller does not\nbother checking the return value.\n\nMake sure it dies, instead of keep going with a NULL output\nfilestream and relying on it to cause a crash, when it fails to\nopen the file.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/log.c           | 2 +-\n t/t4014-format-patch.sh | 6 ++++++\n 2 files changed, 7 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex f2d1fbf18a..ca86611efe 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1050,7 +1050,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,\n \n \tif (!use_stdout &&\n \t    open_next_file(NULL, rev->numbered_files ? NULL : \"cover-letter\", rev, quiet))\n-\t\treturn;\n+\t\tdie(_(\"failed to create cover-letter file\"));\n \n \tlog_write_email_headers(rev, head, &pp.after_subject, &need_8bit_cte, 0);\n \ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex 909c743c13..b6e2fdbc44 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -589,6 +589,12 @@ test_expect_success 'excessive subject' '\n \tls patches/0004-This-is-an-excessively-long-subject-line-for-a-messa.patch\n '\n \n+test_expect_success 'failure to write cover-letter aborts gracefully' '\n+\ttest_when_finished \"rmdir 0000-cover-letter.patch\" &&\n+\tmkdir 0000-cover-letter.patch &&\n+\ttest_must_fail git format-patch --no-renames --cover-letter -1\n+'\n+\n test_expect_success 'cover-letter inherits diff options' '\n \tgit mv file foo &&\n \tgit commit -m foo &&\n-- \n2.21.0-rc2\n\n"},{"id":"369974","messageId":"20190222201111.98196-4-gitster@pobox.com","threadId":"50566","inReplyTo":"20190222201111.98196-1-gitster@pobox.com","subject":"[PATCH 3/3] format-patch: --no-clobber refrains from overwriting output files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-02-22T20:11:11Z","receivedAt":"2019-02-22T20:11:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"If you keep an output for an older iteration of the same topic in\nthe same directory around and use \"git format-patch\" to prepare a\nnewer iteration of the topic, those commits that happen to be at the\nsame position in the series that have not been retitled will get the\nsame filename---and the command opens them for writing without any\ncheck.\n\nExisting \"-o outdir\" and \"-v number\" options are both good ways to\navoid such name collisions, and in general helps to give good ways\nto compare the latest iteration with older iteration(s), but let's\nsee if \"--no-clobber\" option that forbids overwrting existing files\nwould also help people.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/git-format-patch.txt |  8 +++++++-\n builtin/log.c                      | 32 ++++++++++++++++++++++++------\n t/t4014-format-patch.sh            | 16 +++++++++++++++\n 3 files changed, 49 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt\nindex 1af85d404f..540822b3b4 100644\n--- a/Documentation/git-format-patch.txt\n+++ b/Documentation/git-format-patch.txt\n@@ -25,7 +25,7 @@ SYNOPSIS\n \t\t   [--[no-]cover-letter] [--quiet] [--notes[=<ref>]]\n \t\t   [--interdiff=<previous>]\n \t\t   [--range-diff=<previous> [--creation-factor=<percent>]]\n-\t\t   [--progress]\n+\t\t   [--progress] [--[no-]clobber]\n \t\t   [<common diff options>]\n \t\t   [ <since> | <revision range> ]\n \n@@ -93,6 +93,12 @@ include::diff-options.txt[]\n \tUse <dir> to store the resulting files, instead of the\n \tcurrent working directory.\n \n+--clobber::\n+--no-clobber::\n+\t(experimental)\n+\tAllow overwriting existing files, which is the default.  To\n+\tmake the command refrain from overwriting, use `--no-clobber`.\n+\n -n::\n --numbered::\n \tName output in '[PATCH n/m]' format, even with a single patch.\ndiff --git a/builtin/log.c b/builtin/log.c\nindex ca86611efe..7421f1cc93 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -867,8 +867,16 @@ static int git_format_config(const char *var, const char *value, void *cb)\n static const char *output_directory = NULL;\n static int outdir_offset;\n \n+static FILE *fopen_excl(const char *filename)\n+{\n+\tint fd = open(filename, O_CREAT | O_EXCL | O_WRONLY, 0666);\n+\tif (fd < 0)\n+\t\treturn NULL;\n+\treturn fdopen(fd, \"w\");\n+}\n+\n static int open_next_file(struct commit *commit, const char *subject,\n-\t\t\t struct rev_info *rev, int quiet)\n+\t\t\t  struct rev_info *rev, int quiet, int clobber)\n {\n \tstruct strbuf filename = STRBUF_INIT;\n \tint suffix_len = strlen(rev->patch_suffix) + 1;\n@@ -893,7 +901,12 @@ static int open_next_file(struct commit *commit, const char *subject,\n \tif (!quiet)\n \t\tprintf(\"%s\\n\", filename.buf + outdir_offset);\n \n-\tif ((rev->diffopt.file = fopen(filename.buf, \"w\")) == NULL) {\n+\tif (clobber)\n+\t\trev->diffopt.file = fopen(filename.buf, \"w\");\n+\telse\n+\t\trev->diffopt.file = fopen_excl(filename.buf);\n+\n+\tif (!rev->diffopt.file) {\n \t\terror_errno(_(\"cannot open patch file %s\"), filename.buf);\n \t\tstrbuf_release(&filename);\n \t\treturn -1;\n@@ -1030,7 +1043,8 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,\n \t\t\t      struct commit *origin,\n \t\t\t      int nr, struct commit **list,\n \t\t\t      const char *branch_name,\n-\t\t\t      int quiet)\n+\t\t\t      int quiet,\n+\t\t\t      int clobber)\n {\n \tconst char *committer;\n \tconst char *body = \"*** SUBJECT HERE ***\\n\\n*** BLURB HERE ***\\n\";\n@@ -1049,7 +1063,8 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,\n \tcommitter = git_committer_info(0);\n \n \tif (!use_stdout &&\n-\t    open_next_file(NULL, rev->numbered_files ? NULL : \"cover-letter\", rev, quiet))\n+\t    open_next_file(NULL, rev->numbered_files ? NULL : \"cover-letter\",\n+\t\t\t   rev, quiet, clobber))\n \t\tdie(_(\"failed to create cover-letter file\"));\n \n \tlog_write_email_headers(rev, head, &pp.after_subject, &need_8bit_cte, 0);\n@@ -1509,6 +1524,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \tstruct strbuf buf = STRBUF_INIT;\n \tint use_patch_format = 0;\n \tint quiet = 0;\n+\tint clobber = 1;\n \tint reroll_count = -1;\n \tchar *branch_name = NULL;\n \tchar *base_commit = NULL;\n@@ -1595,6 +1611,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\tOPT__QUIET(&quiet, N_(\"don't print the patch filenames\")),\n \t\tOPT_BOOL(0, \"progress\", &show_progress,\n \t\t\t N_(\"show progress while generating patches\")),\n+\t\tOPT_BOOL(0, \"clobber\", &clobber,\n+\t\t\t N_(\"allow overwriting output files\")),\n \t\tOPT_CALLBACK(0, \"interdiff\", &idiff_prev, N_(\"rev\"),\n \t\t\t     N_(\"show changes against <rev> in cover letter or single patch\"),\n \t\t\t     parse_opt_object_name),\n@@ -1885,7 +1903,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\tif (thread)\n \t\t\tgen_message_id(&rev, \"cover\");\n \t\tmake_cover_letter(&rev, use_stdout,\n-\t\t\t\t  origin, nr, list, branch_name, quiet);\n+\t\t\t\t  origin, nr, list, branch_name,\n+\t\t\t\t  quiet, clobber);\n \t\tprint_bases(&bases, rev.diffopt.file);\n \t\tprint_signature(rev.diffopt.file);\n \t\ttotal++;\n@@ -1940,7 +1959,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t}\n \n \t\tif (!use_stdout &&\n-\t\t    open_next_file(rev.numbered_files ? NULL : commit, NULL, &rev, quiet))\n+\t\t    open_next_file(rev.numbered_files ? NULL : commit, NULL,\n+\t\t\t\t   &rev, quiet, clobber))\n \t\t\tdie(_(\"failed to create output files\"));\n \t\tshown = log_tree_commit(&rev, commit);\n \t\tfree_commit_buffer(the_repository->parsed_objects,\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex b6e2fdbc44..384a1fd9e7 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -595,6 +595,22 @@ test_expect_success 'failure to write cover-letter aborts gracefully' '\n \ttest_must_fail git format-patch --no-renames --cover-letter -1\n '\n \n+test_expect_success 'refrain from overwriting a patch with --no-clobber' '\n+\trm -f 000[01]-*.patch &&\n+\tgit format-patch --no-clobber --no-renames --cover-letter -1 >filelist &&\n+\t# empty the files output by the command ...\n+\tfor f in $(cat filelist)\n+\tdo\n+\t\t: >\"$f\" || return 1\n+\tdone &&\n+\ttest_must_fail git format-patch --no-clobber --cover-letter --no-renames -1 &&\n+\t# ... and make sure they stay empty\n+\tfor f in $(cat filelist)\n+\tdo\n+\t\t! test -s \"$f\" || return 1\n+\tdone\n+'\n+\n test_expect_success 'cover-letter inherits diff options' '\n \tgit mv file foo &&\n \tgit commit -m foo &&\n-- \n2.21.0-rc2\n\n"},{"id":"369975","messageId":"CAPig+cRhk-0LBCmUQ4LszJrnmnXCDz7HxWqDnKX53woBYhr-KQ@mail.gmail.com","threadId":"50566","inReplyTo":"20190222201111.98196-2-gitster@pobox.com","subject":"Re: [PATCH 1/3] builtin/log: downcase the beginning of error messages","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2019-02-22T20:24:15Z","receivedAt":"2019-02-22T20:24:28Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Feb 22, 2019 at 3:11 PM Junio C Hamano <gitster@pobox.com> wrote:\n> diff --git a/builtin/log.c b/builtin/log.c\n> @@ -911,7 +911,7 @@ static void get_patch_ids(struct rev_info *rev, struct patch_ids *ids)\n>         if (rev->pending.nr != 2)\n> -               die(_(\"Need exactly one range.\"));\n> +               die(_(\"need exactly one range.\"));\n\nPerhaps drop the trailing period while you're here?\n\n> @@ -921,7 +921,7 @@ static void get_patch_ids(struct rev_info *rev, struct patch_ids *ids)\n>         if ((flags1 & UNINTERESTING) == (flags2 & UNINTERESTING))\n> -               die(_(\"Not a range.\"));\n> +               die(_(\"not a range.\"));\n\nDitto.\n\n> @@ -1331,16 +1331,16 @@ static struct commit *get_base_commit(const char *base_commit,\n>                         if (get_oid(upstream, &oid))\n> -                               die(_(\"Failed to resolve '%s' as a valid ref.\"), upstream);\n> +                               die(_(\"failed to resolve '%s' as a valid ref.\"), upstream);\n\nDitto.\n\n>                         if (!base_list || base_list->next)\n> -                               die(_(\"Could not find exact merge base.\"));\n> +                               die(_(\"could not find exact merge base.\"));\n\nDitto.\n\n> -                       die(_(\"Failed to get upstream, if you want to record base commit automatically,\\n\"\n> +                       die(_(\"failed to get upstream, if you want to record base commit automatically,\\n\"\n>                               \"please use git branch --set-upstream-to to track a remote branch.\\n\"\n>                               \"Or you could specify base commit by --base=<base-commit-id> manually.\"));\n\nThe capitalized \"Or you...\" is odd after s/Failed/failed/. I wonder if\nthe period in the first sentence should be replaced by a semicolon and\nthen \"Or you...\" downcased (and the final period dropped):\n\n    die(_(\"failed to ... automatically,\\n\"\n    \"please use ... branch;\\n\"\n    \"or you ... manually\"));\n"},{"id":"369976","messageId":"CAPig+cSsONVNHsG+8Q5yZ9PU1=5059RKXBS+Ps5wX-5BihqPew@mail.gmail.com","threadId":"50566","inReplyTo":"20190222201111.98196-4-gitster@pobox.com","subject":"Re: [PATCH 3/3] format-patch: --no-clobber refrains from overwriting output files","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2019-02-22T20:38:16Z","receivedAt":"2019-02-22T20:38:28Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Feb 22, 2019 at 3:11 PM Junio C Hamano <gitster@pobox.com> wrote:\n> If you keep an output for an older iteration of the same topic in\n> the same directory around and use \"git format-patch\" to prepare a\n> newer iteration of the topic, those commits that happen to be at the\n> same position in the series that have not been retitled will get the\n> same filename---and the command opens them for writing without any\n> check.\n>\n> Existing \"-o outdir\" and \"-v number\" options are both good ways to\n> avoid such name collisions, and in general helps to give good ways\n> to compare the latest iteration with older iteration(s), but let's\n> see if \"--no-clobber\" option that forbids overwrting existing files\n> would also help people.\n\ns/overwrting/overwriting/\n\nMeh. I haven't particularly been following the thread, but this commit\nmessage doesn't necessarily provide sufficient justification for\nfurther bloating git-format-patch's set of options, its documentation,\nand implementation, not to mention potential user-brain overload. With\nthe possible exception of a 1-patch series, anyone who stores multiple\nversions of a patch series without using -o and/or -v is going to have\na mess to deal with regardless of this new option. (Just trying to\nfigure out which *.patch file belongs to which version of a patch\nseries will be a nightmare without use of -o and/or -v.)\n\n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n"},{"id":"369977","messageId":"xmqqmumnpneh.fsf@gitster-ct.c.googlers.com","threadId":"50566","inReplyTo":"CAPig+cRhk-0LBCmUQ4LszJrnmnXCDz7HxWqDnKX53woBYhr-KQ@mail.gmail.com","subject":"Re: [PATCH 1/3] builtin/log: downcase the beginning of error messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-02-22T20:40:38Z","receivedAt":"2019-02-22T20:40:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n>> -                       die(_(\"Failed to get upstream, if you want to record base commit automatically,\\n\"\n>> +                       die(_(\"failed to get upstream, if you want to record base commit automatically,\\n\"\n>>                               \"please use git branch --set-upstream-to to track a remote branch.\\n\"\n>>                               \"Or you could specify base commit by --base=<base-commit-id> manually.\"));\n>\n> The capitalized \"Or you...\" is odd after s/Failed/failed/.\n\nIt briefly bothered me, too, but after realizing that this is not\nthe whole sentence, it no longer looks so bad to me.  The thing is\nthat \"Failed\" (or \"failed\") is *not* what starts the message.  This\none is die(), so what you'd actually see is\n\n\tfatal: failed to get upstream, ... to track a remote branch.\n\tOr you could ...\n\nwith \"fatal:\" prefix.\n"},{"id":"370019","messageId":"20190223133418.GD22403@sigill.intra.peff.net","threadId":"50566","inReplyTo":"20190222201111.98196-4-gitster@pobox.com","subject":"Re: [PATCH 3/3] format-patch: --no-clobber refrains from overwriting output files","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-02-23T13:34:19Z","receivedAt":"2019-02-23T13:34:22Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 22, 2019 at 12:11:11PM -0800, Junio C Hamano wrote:\n\n> If you keep an output for an older iteration of the same topic in\n> the same directory around and use \"git format-patch\" to prepare a\n> newer iteration of the topic, those commits that happen to be at the\n> same position in the series that have not been retitled will get the\n> same filename---and the command opens them for writing without any\n> check.\n> \n> Existing \"-o outdir\" and \"-v number\" options are both good ways to\n> avoid such name collisions, and in general helps to give good ways\n> to compare the latest iteration with older iteration(s), but let's\n> see if \"--no-clobber\" option that forbids overwrting existing files\n> would also help people.\n\nI suspect it won't help much, because remembering to use --no-clobber is\njust as hard as remembering to clean up the stale patches in the first\nplace.\n\nIf we were starting from scratch, I'd suggest that --no-clobber be the\ndefault[1]. But at this point I wonder if people would be annoyed\n(because the clobbering behavior is convenient and works _most_ of the\ntime, as long as you don't add, remove, reorder, or retitle patches).\n\nI suppose that implies having a config option, so at least people who\nwant it only have to remember once.\n\n>  Documentation/git-format-patch.txt |  8 +++++++-\n>  builtin/log.c                      | 32 ++++++++++++++++++++++++------\n>  t/t4014-format-patch.sh            | 16 +++++++++++++++\n>  3 files changed, 49 insertions(+), 7 deletions(-)\n\nThe patch itself looks well done.\n\n-Peff\n\n[1] Actually, I'd suggest that --stdout be the default, which is what I\n    always use. But then I typically feed the result into mutt anyway.\n    Separate files is probably nicer if you're hand-editing.\n"},{"id":"371296","messageId":"a063547c-4958-aa62-cb76-e4970927730f@gmail.com","threadId":"50566","inReplyTo":"20190223133418.GD22403@sigill.intra.peff.net","subject":"Re: [PATCH 3/3] format-patch: --no-clobber refrains from overwriting output files","fromName":"Ntentos Stavros","fromEmail":"stdedos@gmail.com","sentAt":"2019-03-12T20:53:18Z","receivedAt":"2019-03-12T20:53:26Z","isPatch":true,"sender":{"key":"stdedos@gmail.com","avatar":null},"body":"Hello there,\n\nApologies for \"jumping in\". I was mentioned in [PATCH 0/3] but then for\na (good) reason or another, I wasn't CC-ed in the patches.\n\nI was the original \"suggester\" for this feature in the mailing list\n(https://public-inbox.org/git/CAHMHMxXxo4zXcriBJE2k3mWgwAj7KGA_AChuEmyciESGOC_7Bg@mail.gmail.com/)\n\nOn 22/2/2019 10:38 μ.μ., Eric Sunshine wrote:\n> Meh. I haven't particularly been following the thread, but this commit\n> message doesn't necessarily provide sufficient justification for\n> further bloating git-format-patch's set of options, its documentation,\n> and implementation, not to mention potential user-brain overload. With\n> the possible exception of a 1-patch series, anyone who stores multiple\n> versions of a patch series without using -o and/or -v is going to have\n> a mess to deal with regardless of this new option. (Just trying to\n> figure out which *.patch file belongs to which version of a patch\n> series will be a nightmare without use of -o and/or -v.)\n\nOn 23/2/2019 3:34 μ.μ., Jeff King wrote:\n> I suspect it won't help much, because remembering to use --no-clobber is\n> just as hard as remembering to clean up the stale patches in the first\n> place.\n> \n> If we were starting from scratch, I'd suggest that --no-clobber be the\n> default[1]. But at this point I wonder if people would be annoyed\n> (because the clobbering behavior is convenient and works _most_ of the\n> time, as long as you don't add, remove, reorder, or retitle patches).\n> \n> I suppose that implies having a config option, so at least people who\n> want it only have to remember once.\n> \n> The patch itself looks well done.\n> \n> -Peff\n\nOn the last mail of the thread (the one on the link) I mentioned that a\n\"set and forget\" setting was my original idea / wish.\n\n\"I have heard around\" that [PATCH 3/3] was really meh from the\nreviewers' point of view, and felt that it might not cut it. I thought a\nbenign nudge (and some disambiguation) could help tip the scales, since\nI am the one doing \"not nice things\" with git-format-patch :-)\n\n-- \nYours faithfully,\nNtentos Stavros\n\n---\nΑυτό το e-mail ελέγχθηκε για ιούς από το πρόγραμμα Avast antivirus.\nhttps://www.avast.com/antivirus\n\n"}]}