[PATCH 0/3] format-patch --no-clobber

[PATCH 0/3] format-patch --no-clobber

[Junio C Hamano]

If you keep an output for an older iteration of the same topic in
the same directory around and use "git format-patch" to prepare a
newer iteration of the topic, those commits that happen to be at the
same position in the series that have not been retitled will get the
same filename---and the command opens them for writing without any
check.

Existing "-o outdir" and "-v number" options are both good ways to
avoid such name collisions, and in general helps to give good ways
to compare the latest iteration with older iteration(s), but let's
see if "--no-clobber" option that forbids overwrting existing files
would also help people.

I meant to have just a single patch for this, but it ended up to be
a three-patch series.

 - Preliminary clean-up of builtin/log.c updates messages in the
   file given to die()s and error()s that begin with capital by
   downcasing them to match majority of the messages in the system.

 - When we fail to open a patchfile for writing, we immediately stop
   and report failure, but we weren't careful about the cover letter
   output, which is fixed by the second patch.

 - The last one adds an experimental "--no-clobber" option to forbid
   the command from overwriting existing output files.  This is not
   enabled by default, as suggeted by Brian Carlson in [1], at least
   for now.

Junio C Hamano (3):
  builtin/log: downcase the beginning of error messages
  format-patch: notice failure to open cover letter for writing
  format-patch: --no-clobber refrains from overwriting output files

 Documentation/git-format-patch.txt |  8 +++-
 builtin/log.c                      | 72 +++++++++++++++++++-----------
 t/t4014-format-patch.sh            | 22 +++++++++
 3 files changed, 75 insertions(+), 27 deletions(-)



*1* <20190222001145.GD488342@genre.crustytoothpaste.net>



-- 
2.21.0-rc2

[PATCH 1/3] builtin/log: downcase the beginning of error messages

[Junio C Hamano]

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/log.c | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index 57869267d8..f2d1fbf18a 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -513,7 +513,7 @@ static int show_blob_object(const struct object_id *oid, struct rev_info *rev, c
 	if (get_oid_with_context(the_repository, obj_name,
 				 GET_OID_RECORD_PATH,
 				 &oidc, &obj_context))
-		die(_("Not a valid object name %s"), obj_name);
+		die(_("not a valid object name %s"), obj_name);
 	if (!obj_context.path ||
 	    !textconv_object(the_repository, obj_context.path,
 			     obj_context.mode, &oidc, 1, &buf, &size)) {
@@ -537,7 +537,7 @@ static int show_tag_object(const struct object_id *oid, struct rev_info *rev)
 	int offset = 0;
 
 	if (!buf)
-		return error(_("Could not read object %s"), oid_to_hex(oid));
+		return error(_("could not read object %s"), oid_to_hex(oid));
 
 	assert(type == OBJ_TAG);
 	while (offset < size && buf[offset] != '\n') {
@@ -631,7 +631,7 @@ int cmd_show(int argc, const char **argv, const char *prefix)
 				break;
 			o = parse_object(the_repository, &t->tagged->oid);
 			if (!o)
-				ret = error(_("Could not read object %s"),
+				ret = error(_("could not read object %s"),
 					    oid_to_hex(&t->tagged->oid));
 			objects[i].item = o;
 			i--;
@@ -656,7 +656,7 @@ int cmd_show(int argc, const char **argv, const char *prefix)
 			ret = cmd_log_walk(&rev);
 			break;
 		default:
-			ret = error(_("Unknown type: %d"), o->type);
+			ret = error(_("unknown type: %d"), o->type);
 		}
 	}
 	free(objects);
@@ -894,7 +894,7 @@ static int open_next_file(struct commit *commit, const char *subject,
 		printf("%s\n", filename.buf + outdir_offset);
 
 	if ((rev->diffopt.file = fopen(filename.buf, "w")) == NULL) {
-		error_errno(_("Cannot open patch file %s"), filename.buf);
+		error_errno(_("cannot open patch file %s"), filename.buf);
 		strbuf_release(&filename);
 		return -1;
 	}
@@ -911,7 +911,7 @@ static void get_patch_ids(struct rev_info *rev, struct patch_ids *ids)
 	unsigned flags1, flags2;
 
 	if (rev->pending.nr != 2)
-		die(_("Need exactly one range."));
+		die(_("need exactly one range."));
 
 	o1 = rev->pending.objects[0].item;
 	o2 = rev->pending.objects[1].item;
@@ -921,7 +921,7 @@ static void get_patch_ids(struct rev_info *rev, struct patch_ids *ids)
 	c2 = lookup_commit_reference(the_repository, &o2->oid);
 
 	if ((flags1 & UNINTERESTING) == (flags2 & UNINTERESTING))
-		die(_("Not a range."));
+		die(_("not a range."));
 
 	init_patch_ids(the_repository, ids);
 
@@ -1044,7 +1044,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
 	struct commit *head = list[0];
 
 	if (!cmit_fmt_is_mail(rev->commit_format))
-		die(_("Cover letter needs email format"));
+		die(_("cover letter needs email format"));
 
 	committer = git_committer_info(0);
 
@@ -1214,7 +1214,7 @@ static int output_directory_callback(const struct option *opt, const char *arg,
 	const char **dir = (const char **)opt->value;
 	BUG_ON_OPT_NEG(unset);
 	if (*dir)
-		die(_("Two output directories?"));
+		die(_("two output directories?"));
 	*dir = arg;
 	return 0;
 }
@@ -1321,7 +1321,7 @@ static struct commit *get_base_commit(const char *base_commit,
 	if (base_commit && strcmp(base_commit, "auto")) {
 		base = lookup_commit_reference_by_name(base_commit);
 		if (!base)
-			die(_("Unknown commit %s"), base_commit);
+			die(_("unknown commit %s"), base_commit);
 	} else if ((base_commit && !strcmp(base_commit, "auto")) || base_auto) {
 		struct branch *curr_branch = branch_get(NULL);
 		const char *upstream = branch_get_upstream(curr_branch, NULL);
@@ -1331,16 +1331,16 @@ static struct commit *get_base_commit(const char *base_commit,
 			struct object_id oid;
 
 			if (get_oid(upstream, &oid))
-				die(_("Failed to resolve '%s' as a valid ref."), upstream);
+				die(_("failed to resolve '%s' as a valid ref."), upstream);
 			commit = lookup_commit_or_die(&oid, "upstream base");
 			base_list = get_merge_bases_many(commit, total, list);
 			/* There should be one and only one merge base. */
 			if (!base_list || base_list->next)
-				die(_("Could not find exact merge base."));
+				die(_("could not find exact merge base."));
 			base = base_list->item;
 			free_commit_list(base_list);
 		} else {
-			die(_("Failed to get upstream, if you want to record base commit automatically,\n"
+			die(_("failed to get upstream, if you want to record base commit automatically,\n"
 			      "please use git branch --set-upstream-to to track a remote branch.\n"
 			      "Or you could specify base commit by --base=<base-commit-id> manually."));
 		}
@@ -1360,7 +1360,7 @@ static struct commit *get_base_commit(const char *base_commit,
 			struct commit_list *merge_base;
 			merge_base = get_merge_bases(rev[2 * i], rev[2 * i + 1]);
 			if (!merge_base || merge_base->next)
-				die(_("Failed to find exact merge base"));
+				die(_("failed to find exact merge base"));
 
 			rev[i] = merge_base->item;
 		}
@@ -1739,7 +1739,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		if (use_stdout)
 			die(_("standard output, or directory, which one?"));
 		if (mkdir(output_directory, 0777) < 0 && errno != EEXIST)
-			die_errno(_("Could not create directory '%s'"),
+			die_errno(_("could not create directory '%s'"),
 				  output_directory);
 	}
 
@@ -1941,7 +1941,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 
 		if (!use_stdout &&
 		    open_next_file(rev.numbered_files ? NULL : commit, NULL, &rev, quiet))
-			die(_("Failed to create output files"));
+			die(_("failed to create output files"));
 		shown = log_tree_commit(&rev, commit);
 		free_commit_buffer(the_repository->parsed_objects,
 				   commit);
@@ -2065,9 +2065,9 @@ int cmd_cherry(int argc, const char **argv, const char *prefix)
 	revs.max_parents = 1;
 
 	if (add_pending_commit(head, &revs, 0))
-		die(_("Unknown commit %s"), head);
+		die(_("unknown commit %s"), head);
 	if (add_pending_commit(upstream, &revs, UNINTERESTING))
-		die(_("Unknown commit %s"), upstream);
+		die(_("unknown commit %s"), upstream);
 
 	/* Don't say anything if head and upstream are the same. */
 	if (revs.pending.nr == 2) {
@@ -2079,7 +2079,7 @@ int cmd_cherry(int argc, const char **argv, const char *prefix)
 	get_patch_ids(&revs, &ids);
 
 	if (limit && add_pending_commit(limit, &revs, UNINTERESTING))
-		die(_("Unknown commit %s"), limit);
+		die(_("unknown commit %s"), limit);
 
 	/* reverse the list of commits */
 	if (prepare_revision_walk(&revs))
-- 
2.21.0-rc2

[PATCH 2/3] format-patch: notice failure to open cover letter for writing

[Junio C Hamano]

The make_cover_letter() function is supposed to open a new file for
writing, and let the caller write into it via FILE *rev->diffopt.file
but because the function does not return anything, the caller does not
bother checking the return value.

Make sure it dies, instead of keep going with a NULL output
filestream and relying on it to cause a crash, when it fails to
open the file.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/log.c           | 2 +-
 t/t4014-format-patch.sh | 6 ++++++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/builtin/log.c b/builtin/log.c
index f2d1fbf18a..ca86611efe 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1050,7 +1050,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
 
 	if (!use_stdout &&
 	    open_next_file(NULL, rev->numbered_files ? NULL : "cover-letter", rev, quiet))
-		return;
+		die(_("failed to create cover-letter file"));
 
 	log_write_email_headers(rev, head, &pp.after_subject, &need_8bit_cte, 0);
 
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 909c743c13..b6e2fdbc44 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -589,6 +589,12 @@ test_expect_success 'excessive subject' '
 	ls patches/0004-This-is-an-excessively-long-subject-line-for-a-messa.patch
 '
 
+test_expect_success 'failure to write cover-letter aborts gracefully' '
+	test_when_finished "rmdir 0000-cover-letter.patch" &&
+	mkdir 0000-cover-letter.patch &&
+	test_must_fail git format-patch --no-renames --cover-letter -1
+'
+
 test_expect_success 'cover-letter inherits diff options' '
 	git mv file foo &&
 	git commit -m foo &&
-- 
2.21.0-rc2

[PATCH 3/3] format-patch: --no-clobber refrains from overwriting output files

[Junio C Hamano]

If you keep an output for an older iteration of the same topic in
the same directory around and use "git format-patch" to prepare a
newer iteration of the topic, those commits that happen to be at the
same position in the series that have not been retitled will get the
same filename---and the command opens them for writing without any
check.

Existing "-o outdir" and "-v number" options are both good ways to
avoid such name collisions, and in general helps to give good ways
to compare the latest iteration with older iteration(s), but let's
see if "--no-clobber" option that forbids overwrting existing files
would also help people.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-format-patch.txt |  8 +++++++-
 builtin/log.c                      | 32 ++++++++++++++++++++++++------
 t/t4014-format-patch.sh            | 16 +++++++++++++++
 3 files changed, 49 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index 1af85d404f..540822b3b4 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -25,7 +25,7 @@ SYNOPSIS
 		   [--[no-]cover-letter] [--quiet] [--notes[=<ref>]]
 		   [--interdiff=<previous>]
 		   [--range-diff=<previous> [--creation-factor=<percent>]]
-		   [--progress]
+		   [--progress] [--[no-]clobber]
 		   [<common diff options>]
 		   [ <since> | <revision range> ]
 
@@ -93,6 +93,12 @@ include::diff-options.txt[]
 	Use <dir> to store the resulting files, instead of the
 	current working directory.
 
+--clobber::
+--no-clobber::
+	(experimental)
+	Allow overwriting existing files, which is the default.  To
+	make the command refrain from overwriting, use `--no-clobber`.
+
 -n::
 --numbered::
 	Name output in '[PATCH n/m]' format, even with a single patch.
diff --git a/builtin/log.c b/builtin/log.c
index ca86611efe..7421f1cc93 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -867,8 +867,16 @@ static int git_format_config(const char *var, const char *value, void *cb)
 static const char *output_directory = NULL;
 static int outdir_offset;
 
+static FILE *fopen_excl(const char *filename)
+{
+	int fd = open(filename, O_CREAT | O_EXCL | O_WRONLY, 0666);
+	if (fd < 0)
+		return NULL;
+	return fdopen(fd, "w");
+}
+
 static int open_next_file(struct commit *commit, const char *subject,
-			 struct rev_info *rev, int quiet)
+			  struct rev_info *rev, int quiet, int clobber)
 {
 	struct strbuf filename = STRBUF_INIT;
 	int suffix_len = strlen(rev->patch_suffix) + 1;
@@ -893,7 +901,12 @@ static int open_next_file(struct commit *commit, const char *subject,
 	if (!quiet)
 		printf("%s\n", filename.buf + outdir_offset);
 
-	if ((rev->diffopt.file = fopen(filename.buf, "w")) == NULL) {
+	if (clobber)
+		rev->diffopt.file = fopen(filename.buf, "w");
+	else
+		rev->diffopt.file = fopen_excl(filename.buf);
+
+	if (!rev->diffopt.file) {
 		error_errno(_("cannot open patch file %s"), filename.buf);
 		strbuf_release(&filename);
 		return -1;
@@ -1030,7 +1043,8 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
 			      struct commit *origin,
 			      int nr, struct commit **list,
 			      const char *branch_name,
-			      int quiet)
+			      int quiet,
+			      int clobber)
 {
 	const char *committer;
 	const char *body = "*** SUBJECT HERE ***\n\n*** BLURB HERE ***\n";
@@ -1049,7 +1063,8 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
 	committer = git_committer_info(0);
 
 	if (!use_stdout &&
-	    open_next_file(NULL, rev->numbered_files ? NULL : "cover-letter", rev, quiet))
+	    open_next_file(NULL, rev->numbered_files ? NULL : "cover-letter",
+			   rev, quiet, clobber))
 		die(_("failed to create cover-letter file"));
 
 	log_write_email_headers(rev, head, &pp.after_subject, &need_8bit_cte, 0);
@@ -1509,6 +1524,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	struct strbuf buf = STRBUF_INIT;
 	int use_patch_format = 0;
 	int quiet = 0;
+	int clobber = 1;
 	int reroll_count = -1;
 	char *branch_name = NULL;
 	char *base_commit = NULL;
@@ -1595,6 +1611,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		OPT__QUIET(&quiet, N_("don't print the patch filenames")),
 		OPT_BOOL(0, "progress", &show_progress,
 			 N_("show progress while generating patches")),
+		OPT_BOOL(0, "clobber", &clobber,
+			 N_("allow overwriting output files")),
 		OPT_CALLBACK(0, "interdiff", &idiff_prev, N_("rev"),
 			     N_("show changes against <rev> in cover letter or single patch"),
 			     parse_opt_object_name),
@@ -1885,7 +1903,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		if (thread)
 			gen_message_id(&rev, "cover");
 		make_cover_letter(&rev, use_stdout,
-				  origin, nr, list, branch_name, quiet);
+				  origin, nr, list, branch_name,
+				  quiet, clobber);
 		print_bases(&bases, rev.diffopt.file);
 		print_signature(rev.diffopt.file);
 		total++;
@@ -1940,7 +1959,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		}
 
 		if (!use_stdout &&
-		    open_next_file(rev.numbered_files ? NULL : commit, NULL, &rev, quiet))
+		    open_next_file(rev.numbered_files ? NULL : commit, NULL,
+				   &rev, quiet, clobber))
 			die(_("failed to create output files"));
 		shown = log_tree_commit(&rev, commit);
 		free_commit_buffer(the_repository->parsed_objects,
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index b6e2fdbc44..384a1fd9e7 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -595,6 +595,22 @@ test_expect_success 'failure to write cover-letter aborts gracefully' '
 	test_must_fail git format-patch --no-renames --cover-letter -1
 '
 
+test_expect_success 'refrain from overwriting a patch with --no-clobber' '
+	rm -f 000[01]-*.patch &&
+	git format-patch --no-clobber --no-renames --cover-letter -1 >filelist &&
+	# empty the files output by the command ...
+	for f in $(cat filelist)
+	do
+		: >"$f" || return 1
+	done &&
+	test_must_fail git format-patch --no-clobber --cover-letter --no-renames -1 &&
+	# ... and make sure they stay empty
+	for f in $(cat filelist)
+	do
+		! test -s "$f" || return 1
+	done
+'
+
 test_expect_success 'cover-letter inherits diff options' '
 	git mv file foo &&
 	git commit -m foo &&
-- 
2.21.0-rc2

Re: [PATCH 1/3] builtin/log: downcase the beginning of error messages

[Eric Sunshine]

On Fri, Feb 22, 2019 at 3:11 PM Junio C Hamano <gitster@pobox.com> wrote:
> diff --git a/builtin/log.c b/builtin/log.c
> @@ -911,7 +911,7 @@ static void get_patch_ids(struct rev_info *rev, struct patch_ids *ids)
>         if (rev->pending.nr != 2)
> -               die(_("Need exactly one range."));
> +               die(_("need exactly one range."));

Perhaps drop the trailing period while you're here?

> @@ -921,7 +921,7 @@ static void get_patch_ids(struct rev_info *rev, struct patch_ids *ids)
>         if ((flags1 & UNINTERESTING) == (flags2 & UNINTERESTING))
> -               die(_("Not a range."));
> +               die(_("not a range."));

Ditto.

> @@ -1331,16 +1331,16 @@ static struct commit *get_base_commit(const char *base_commit,
>                         if (get_oid(upstream, &oid))
> -                               die(_("Failed to resolve '%s' as a valid ref."), upstream);
> +                               die(_("failed to resolve '%s' as a valid ref."), upstream);

Ditto.

>                         if (!base_list || base_list->next)
> -                               die(_("Could not find exact merge base."));
> +                               die(_("could not find exact merge base."));

Ditto.

> -                       die(_("Failed to get upstream, if you want to record base commit automatically,\n"
> +                       die(_("failed to get upstream, if you want to record base commit automatically,\n"
>                               "please use git branch --set-upstream-to to track a remote branch.\n"
>                               "Or you could specify base commit by --base=<base-commit-id> manually."));

The capitalized "Or you..." is odd after s/Failed/failed/. I wonder if
the period in the first sentence should be replaced by a semicolon and
then "Or you..." downcased (and the final period dropped):

    die(_("failed to ... automatically,\n"
    "please use ... branch;\n"
    "or you ... manually"));

Re: [PATCH 3/3] format-patch: --no-clobber refrains from overwriting output files

[Eric Sunshine]

On Fri, Feb 22, 2019 at 3:11 PM Junio C Hamano <gitster@pobox.com> wrote:
> If you keep an output for an older iteration of the same topic in
> the same directory around and use "git format-patch" to prepare a
> newer iteration of the topic, those commits that happen to be at the
> same position in the series that have not been retitled will get the
> same filename---and the command opens them for writing without any
> check.
>
> Existing "-o outdir" and "-v number" options are both good ways to
> avoid such name collisions, and in general helps to give good ways
> to compare the latest iteration with older iteration(s), but let's
> see if "--no-clobber" option that forbids overwrting existing files
> would also help people.

s/overwrting/overwriting/

Meh. I haven't particularly been following the thread, but this commit
message doesn't necessarily provide sufficient justification for
further bloating git-format-patch's set of options, its documentation,
and implementation, not to mention potential user-brain overload. With
the possible exception of a 1-patch series, anyone who stores multiple
versions of a patch series without using -o and/or -v is going to have
a mess to deal with regardless of this new option. (Just trying to
figure out which *.patch file belongs to which version of a patch
series will be a nightmare without use of -o and/or -v.)

> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Re: [PATCH 1/3] builtin/log: downcase the beginning of error messages

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

>> -                       die(_("Failed to get upstream, if you want to record base commit automatically,\n"
>> +                       die(_("failed to get upstream, if you want to record base commit automatically,\n"
>>                               "please use git branch --set-upstream-to to track a remote branch.\n"
>>                               "Or you could specify base commit by --base=<base-commit-id> manually."));
>
> The capitalized "Or you..." is odd after s/Failed/failed/.

It briefly bothered me, too, but after realizing that this is not
the whole sentence, it no longer looks so bad to me.  The thing is
that "Failed" (or "failed") is *not* what starts the message.  This
one is die(), so what you'd actually see is

	fatal: failed to get upstream, ... to track a remote branch.
	Or you could ...

with "fatal:" prefix.

Re: [PATCH 3/3] format-patch: --no-clobber refrains from overwriting output files

[Jeff King]

On Fri, Feb 22, 2019 at 12:11:11PM -0800, Junio C Hamano wrote:

> If you keep an output for an older iteration of the same topic in
> the same directory around and use "git format-patch" to prepare a
> newer iteration of the topic, those commits that happen to be at the
> same position in the series that have not been retitled will get the
> same filename---and the command opens them for writing without any
> check.
> 
> Existing "-o outdir" and "-v number" options are both good ways to
> avoid such name collisions, and in general helps to give good ways
> to compare the latest iteration with older iteration(s), but let's
> see if "--no-clobber" option that forbids overwrting existing files
> would also help people.

I suspect it won't help much, because remembering to use --no-clobber is
just as hard as remembering to clean up the stale patches in the first
place.

If we were starting from scratch, I'd suggest that --no-clobber be the
default[1]. But at this point I wonder if people would be annoyed
(because the clobbering behavior is convenient and works _most_ of the
time, as long as you don't add, remove, reorder, or retitle patches).

I suppose that implies having a config option, so at least people who
want it only have to remember once.

>  Documentation/git-format-patch.txt |  8 +++++++-
>  builtin/log.c                      | 32 ++++++++++++++++++++++++------
>  t/t4014-format-patch.sh            | 16 +++++++++++++++
>  3 files changed, 49 insertions(+), 7 deletions(-)

The patch itself looks well done.

-Peff

[1] Actually, I'd suggest that --stdout be the default, which is what I
    always use. But then I typically feed the result into mutt anyway.
    Separate files is probably nicer if you're hand-editing.

Re: [PATCH 3/3] format-patch: --no-clobber refrains from overwriting output files

[Ntentos Stavros]

Hello there,

Apologies for "jumping in". I was mentioned in [PATCH 0/3] but then for
a (good) reason or another, I wasn't CC-ed in the patches.

I was the original "suggester" for this feature in the mailing list
(https://public-inbox.org/git/CAHMHMxXxo4zXcriBJE2k3mWgwAj7KGA_AChuEmyciESGOC_7Bg@mail.gmail.com/)

On 22/2/2019 10:38 μ.μ., Eric Sunshine wrote:
> Meh. I haven't particularly been following the thread, but this commit
> message doesn't necessarily provide sufficient justification for
> further bloating git-format-patch's set of options, its documentation,
> and implementation, not to mention potential user-brain overload. With
> the possible exception of a 1-patch series, anyone who stores multiple
> versions of a patch series without using -o and/or -v is going to have
> a mess to deal with regardless of this new option. (Just trying to
> figure out which *.patch file belongs to which version of a patch
> series will be a nightmare without use of -o and/or -v.)

On 23/2/2019 3:34 μ.μ., Jeff King wrote:
> I suspect it won't help much, because remembering to use --no-clobber is
> just as hard as remembering to clean up the stale patches in the first
> place.
> 
> If we were starting from scratch, I'd suggest that --no-clobber be the
> default[1]. But at this point I wonder if people would be annoyed
> (because the clobbering behavior is convenient and works _most_ of the
> time, as long as you don't add, remove, reorder, or retitle patches).
> 
> I suppose that implies having a config option, so at least people who
> want it only have to remember once.
> 
> The patch itself looks well done.
> 
> -Peff

On the last mail of the thread (the one on the link) I mentioned that a
"set and forget" setting was my original idea / wish.

"I have heard around" that [PATCH 3/3] was really meh from the
reviewers' point of view, and felt that it might not cut it. I thought a
benign nudge (and some disambiguation) could help tip the scales, since
I am the one doing "not nice things" with git-format-patch :-)

-- 
Yours faithfully,
Ntentos Stavros

---
Αυτό το e-mail ελέγχθηκε για ιούς από το πρόγραμμα Avast antivirus.
https://www.avast.com/antivirus