* [PATCH 1/3] builtin/log: downcase the beginning of error messages
2019-02-22 20:11 [PATCH 0/3] format-patch --no-clobber Junio C Hamano
@ 2019-02-22 20:11 ` Junio C Hamano
2019-02-22 20:24 ` Eric Sunshine
2019-02-22 20:11 ` [PATCH 2/3] format-patch: notice failure to open cover letter for writing Junio C Hamano
2019-02-22 20:11 ` [PATCH 3/3] format-patch: --no-clobber refrains from overwriting output files Junio C Hamano
2 siblings, 1 reply; 9+ messages in thread
From: Junio C Hamano @ 2019-02-22 20:11 UTC (permalink / raw)
To: git
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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] builtin/log: downcase the beginning of error messages
2019-02-22 20:11 ` [PATCH 1/3] builtin/log: downcase the beginning of error messages Junio C Hamano
@ 2019-02-22 20:24 ` Eric Sunshine
2019-02-22 20:40 ` Junio C Hamano
0 siblings, 1 reply; 9+ messages in thread
From: Eric Sunshine @ 2019-02-22 20:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git List
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"));
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] builtin/log: downcase the beginning of error messages
2019-02-22 20:24 ` Eric Sunshine
@ 2019-02-22 20:40 ` Junio C Hamano
0 siblings, 0 replies; 9+ messages in thread
From: Junio C Hamano @ 2019-02-22 20:40 UTC (permalink / raw)
To: Eric Sunshine; +Cc: Git List
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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/3] format-patch: notice failure to open cover letter for writing
2019-02-22 20:11 [PATCH 0/3] format-patch --no-clobber Junio C Hamano
2019-02-22 20:11 ` [PATCH 1/3] builtin/log: downcase the beginning of error messages Junio C Hamano
@ 2019-02-22 20:11 ` Junio C Hamano
2019-02-22 20:11 ` [PATCH 3/3] format-patch: --no-clobber refrains from overwriting output files Junio C Hamano
2 siblings, 0 replies; 9+ messages in thread
From: Junio C Hamano @ 2019-02-22 20:11 UTC (permalink / raw)
To: git
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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/3] format-patch: --no-clobber refrains from overwriting output files
2019-02-22 20:11 [PATCH 0/3] format-patch --no-clobber Junio C Hamano
2019-02-22 20:11 ` [PATCH 1/3] builtin/log: downcase the beginning of error messages Junio C Hamano
2019-02-22 20:11 ` [PATCH 2/3] format-patch: notice failure to open cover letter for writing Junio C Hamano
@ 2019-02-22 20:11 ` Junio C Hamano
2019-02-22 20:38 ` Eric Sunshine
2019-02-23 13:34 ` Jeff King
2 siblings, 2 replies; 9+ messages in thread
From: Junio C Hamano @ 2019-02-22 20:11 UTC (permalink / raw)
To: git
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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] format-patch: --no-clobber refrains from overwriting output files
2019-02-22 20:11 ` [PATCH 3/3] format-patch: --no-clobber refrains from overwriting output files Junio C Hamano
@ 2019-02-22 20:38 ` Eric Sunshine
2019-02-23 13:34 ` Jeff King
1 sibling, 0 replies; 9+ messages in thread
From: Eric Sunshine @ 2019-02-22 20:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git List
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>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] format-patch: --no-clobber refrains from overwriting output files
2019-02-22 20:11 ` [PATCH 3/3] format-patch: --no-clobber refrains from overwriting output files Junio C Hamano
2019-02-22 20:38 ` Eric Sunshine
@ 2019-02-23 13:34 ` Jeff King
2019-03-12 20:53 ` Ntentos Stavros
1 sibling, 1 reply; 9+ messages in thread
From: Jeff King @ 2019-02-23 13:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] format-patch: --no-clobber refrains from overwriting output files
2019-02-23 13:34 ` Jeff King
@ 2019-03-12 20:53 ` Ntentos Stavros
0 siblings, 0 replies; 9+ messages in thread
From: Ntentos Stavros @ 2019-03-12 20:53 UTC (permalink / raw)
To: Jeff King, Junio C Hamano, Eric Sunshine; +Cc: git
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
^ permalink raw reply [flat|nested] 9+ messages in thread