* [PATCH] send-email: detect and offer to skip backup files
@ 2016-04-12 22:53 Junio C Hamano
2016-04-12 23:00 ` Eric Sunshine
2016-04-12 23:05 ` Stefan Beller
0 siblings, 2 replies; 5+ messages in thread
From: Junio C Hamano @ 2016-04-12 22:53 UTC (permalink / raw)
To: git
Diligent people save output from format-patch to files, proofread
and edit them and then finally send the result out. If the
resulting files are sent out with "git send-email 0*", this ends up
sending backup files (e.g. 0001-X.patch.backup or 0001-X.patch~)
left by their editors next to the final version. Sending them with
"git send-email 0*.patch" (if format-patch was run with the standard
suffix) would avoid such an embarrassment, but not everybody is
careful.
After collecting files to be sent (and sorting them if read from a
directory), notice when the file being sent out has the same name as
the previous file, plus some suffix (e.g. 0001-X.patch was sent, and
we are looking at 0001-X.patch.backup or 0001-X.patch~), and the
suffix begins with a non-alnum (e.g. ".backup" or "~") and ask if
the user really wants to send it out. Once the user skips sending
such a "backup" file, remember the suffix and stop asking the same
question (e.g. after skipping 0001-X.patch~, skip 0002-Y.patch~
without asking).
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* Just something I had lying around in my tree...
git-send-email.perl | 40 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)
diff --git a/git-send-email.perl b/git-send-email.perl
index d356901..74ed01a 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -621,6 +621,8 @@ sub is_format_patch_arg {
push @files, $repo->command('format-patch', '-o', tempdir(CLEANUP => 1), @rev_list_opts);
}
+@files = handle_backup_files(@files);
+
if ($validate) {
foreach my $f (@files) {
unless (-p $f) {
@@ -1726,6 +1728,44 @@ sub validate_patch {
return;
}
+sub handle_backup {
+ my ($last, $lastlen, $file, $known_suffix) = @_;
+ my ($suffix, $skip);
+
+ $skip = 0;
+ if (defined $last &&
+ ($lastlen < length($file)) &&
+ (substr($file, 0, $lastlen) eq $last) &&
+ ($suffix = substr($file, $lastlen)) !~ /^[a-z0-9]/i) {
+ if (defined $known_suffix && $suffix eq $known_suffix) {
+ print "Skipping $file with backup suffix '$known_suffix'.\n";
+ $skip = 1;
+ } else {
+ my $answer = ask("Do you really want to send $file? (y|N): ",
+ valid_re => qr/^(?:y|n)/i,
+ default => 'y');
+ $skip = ($answer ne 'y');
+ if ($skip) {
+ $known_suffix = $suffix;
+ }
+ }
+ }
+ return ($skip, $known_suffix);
+}
+
+sub handle_backup_files {
+ my @file = @_;
+ my ($last, $lastlen, $known_suffix, $skip, @result);
+ for my $file (@file) {
+ ($skip, $known_suffix) = handle_backup($last, $lastlen,
+ $file, $known_suffix);
+ push @result, $file unless $skip;
+ $last = $file;
+ $lastlen = length($file);
+ }
+ return @result;
+}
+
sub file_has_nonascii {
my $fn = shift;
open(my $fh, '<', $fn)
--
2.8.1-347-g322afaf
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] send-email: detect and offer to skip backup files
2016-04-12 22:53 [PATCH] send-email: detect and offer to skip backup files Junio C Hamano
@ 2016-04-12 23:00 ` Eric Sunshine
2016-04-13 1:36 ` Junio C Hamano
2016-04-12 23:05 ` Stefan Beller
1 sibling, 1 reply; 5+ messages in thread
From: Eric Sunshine @ 2016-04-12 23:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git List
On Tue, Apr 12, 2016 at 6:53 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Diligent people save output from format-patch to files, proofread
> and edit them and then finally send the result out. If the
> resulting files are sent out with "git send-email 0*", this ends up
> sending backup files (e.g. 0001-X.patch.backup or 0001-X.patch~)
> left by their editors next to the final version. Sending them with
> "git send-email 0*.patch" (if format-patch was run with the standard
> suffix) would avoid such an embarrassment, but not everybody is
> careful.
>
> After collecting files to be sent (and sorting them if read from a
> directory), notice when the file being sent out has the same name as
> the previous file, plus some suffix (e.g. 0001-X.patch was sent, and
> we are looking at 0001-X.patch.backup or 0001-X.patch~), and the
> suffix begins with a non-alnum (e.g. ".backup" or "~") and ask if
> the user really wants to send it out. Once the user skips sending
> such a "backup" file, remember the suffix and stop asking the same
> question (e.g. after skipping 0001-X.patch~, skip 0002-Y.patch~
> without asking).
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> diff --git a/git-send-email.perl b/git-send-email.perl
> +sub handle_backup {
> + my ($last, $lastlen, $file, $known_suffix) = @_;
> + my ($suffix, $skip);
> +
> + $skip = 0;
> + if (defined $last &&
> + ($lastlen < length($file)) &&
> + (substr($file, 0, $lastlen) eq $last) &&
> + ($suffix = substr($file, $lastlen)) !~ /^[a-z0-9]/i) {
> + if (defined $known_suffix && $suffix eq $known_suffix) {
> + print "Skipping $file with backup suffix '$known_suffix'.\n";
> + $skip = 1;
> + } else {
> + my $answer = ask("Do you really want to send $file? (y|N): ",
> + valid_re => qr/^(?:y|n)/i,
> + default => 'y');
I still have the same question[1] as last time I reviewed this patch:
Should default be 'n', or am I misunderstanding?
[1]: http://thread.gmane.org/gmane.comp.version-control.git/289063/focus=289197
> + $skip = ($answer ne 'y');
> + if ($skip) {
> + $known_suffix = $suffix;
> + }
> + }
> + }
> + return ($skip, $known_suffix);
> +}
> +
> +sub handle_backup_files {
> + my @file = @_;
> + my ($last, $lastlen, $known_suffix, $skip, @result);
> + for my $file (@file) {
> + ($skip, $known_suffix) = handle_backup($last, $lastlen,
> + $file, $known_suffix);
> + push @result, $file unless $skip;
> + $last = $file;
> + $lastlen = length($file);
> + }
> + return @result;
> +}
> +
> sub file_has_nonascii {
> my $fn = shift;
> open(my $fh, '<', $fn)
> --
> 2.8.1-347-g322afaf
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] send-email: detect and offer to skip backup files
2016-04-12 22:53 [PATCH] send-email: detect and offer to skip backup files Junio C Hamano
2016-04-12 23:00 ` Eric Sunshine
@ 2016-04-12 23:05 ` Stefan Beller
2016-04-13 1:38 ` Junio C Hamano
1 sibling, 1 reply; 5+ messages in thread
From: Stefan Beller @ 2016-04-12 23:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
On Tue, Apr 12, 2016 at 3:53 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Diligent people save output from format-patch to files, proofread
> and edit them and then finally send the result out. If the
> resulting files are sent out with "git send-email 0*", this ends up
> sending backup files (e.g. 0001-X.patch.backup or 0001-X.patch~)
> left by their editors next to the final version. Sending them with
> "git send-email 0*.patch" (if format-patch was run with the standard
> suffix) would avoid such an embarrassment, but not everybody is
> careful.
>
> After collecting files to be sent (and sorting them if read from a
> directory), notice when the file being sent out has the same name as
> the previous file, plus some suffix (e.g. 0001-X.patch was sent, and
> we are looking at 0001-X.patch.backup or 0001-X.patch~), and the
> suffix begins with a non-alnum (e.g. ".backup" or "~") and ask if
> the user really wants to send it out. Once the user skips sending
> such a "backup" file, remember the suffix and stop asking the same
> question (e.g. after skipping 0001-X.patch~, skip 0002-Y.patch~
> without asking).
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>
> * Just something I had lying around in my tree...
I think that is a valid use case. (Save the user from embarrassment).
Although I admit to being even more stupid than that.
Once I made the mistake to not remove a previous patch series I had,
such that there was:
0000-coverletter.patch (newly worded)
0001-bla-from-new-series
0001-foo-from-old-series
0002-...
...
`git send-email 0*` then sent out a totally bogus series for me.
As another user friendly helper we could warn/abort if the first <n> characters
are the same as the previous patch as well?
>
> git-send-email.perl | 40 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 40 insertions(+)
>
> diff --git a/git-send-email.perl b/git-send-email.perl
> index d356901..74ed01a 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -621,6 +621,8 @@ sub is_format_patch_arg {
> push @files, $repo->command('format-patch', '-o', tempdir(CLEANUP => 1), @rev_list_opts);
> }
>
> +@files = handle_backup_files(@files);
> +
> if ($validate) {
> foreach my $f (@files) {
> unless (-p $f) {
> @@ -1726,6 +1728,44 @@ sub validate_patch {
> return;
> }
>
> +sub handle_backup {
> + my ($last, $lastlen, $file, $known_suffix) = @_;
> + my ($suffix, $skip);
> +
> + $skip = 0;
> + if (defined $last &&
> + ($lastlen < length($file)) &&
> + (substr($file, 0, $lastlen) eq $last) &&
> + ($suffix = substr($file, $lastlen)) !~ /^[a-z0-9]/i) {
> + if (defined $known_suffix && $suffix eq $known_suffix) {
> + print "Skipping $file with backup suffix '$known_suffix'.\n";
> + $skip = 1;
> + } else {
> + my $answer = ask("Do you really want to send $file? (y|N): ",
> + valid_re => qr/^(?:y|n)/i,
> + default => 'y');
> + $skip = ($answer ne 'y');
> + if ($skip) {
> + $known_suffix = $suffix;
> + }
> + }
> + }
> + return ($skip, $known_suffix);
> +}
> +
> +sub handle_backup_files {
> + my @file = @_;
> + my ($last, $lastlen, $known_suffix, $skip, @result);
> + for my $file (@file) {
> + ($skip, $known_suffix) = handle_backup($last, $lastlen,
> + $file, $known_suffix);
> + push @result, $file unless $skip;
> + $last = $file;
> + $lastlen = length($file);
> + }
> + return @result;
> +}
> +
> sub file_has_nonascii {
> my $fn = shift;
> open(my $fh, '<', $fn)
> --
> 2.8.1-347-g322afaf
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] send-email: detect and offer to skip backup files
2016-04-12 23:00 ` Eric Sunshine
@ 2016-04-13 1:36 ` Junio C Hamano
0 siblings, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2016-04-13 1:36 UTC (permalink / raw)
To: Eric Sunshine; +Cc: Git List
Eric Sunshine <sunshine@sunshineco.com> writes:
> I still have the same question[1] as last time I reviewed this patch:
> Should default be 'n', or am I misunderstanding?
Yes, you are right and no, there is no misunderstanding.
Thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] send-email: detect and offer to skip backup files
2016-04-12 23:05 ` Stefan Beller
@ 2016-04-13 1:38 ` Junio C Hamano
0 siblings, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2016-04-13 1:38 UTC (permalink / raw)
To: Stefan Beller; +Cc: git
Stefan Beller <sbeller@google.com> writes:
> Once I made the mistake to not remove a previous patch series I had,
> such that there was:
>
> 0000-coverletter.patch (newly worded)
> 0001-bla-from-new-series
> 0001-foo-from-old-series
> 0002-...
> ...
>
> `git send-email 0*` then sent out a totally bogus series for me.
>
> As another user friendly helper we could warn/abort if the first <n> characters
> are the same as the previous patch as well?
I think that would be one of the many things that can be built on
top. For example, you could notice that all messages begin with
4-digit and a dash, and send such a set to a special case (where
patches having duplicated numbers are signalled as a potential
error).
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-04-13 1:38 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-12 22:53 [PATCH] send-email: detect and offer to skip backup files Junio C Hamano
2016-04-12 23:00 ` Eric Sunshine
2016-04-13 1:36 ` Junio C Hamano
2016-04-12 23:05 ` Stefan Beller
2016-04-13 1:38 ` Junio C Hamano
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).