threads / patch / 42006

patchsend-email: detect and offer to skip backup files

Subject: [PATCH] send-email: detect and offer to skip backup files

## tl;dr

5 messages between Apr 12, 2016 and Apr 13, 2016. Diffs are folded; open one to read it.

replies: 4people: 3as markdown or json

Junio C Hamano· Apr 12, 2016, 22:53 UTC · lore

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(+)
Show changes to git-send-email.perl +40 −0
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
Eric Sunshine· Apr 12, 2016, 23:00 UTC · re: Junio C Hamano · lore

Re: [PATCH] send-email: detect and offer to skip backup files

On Tue, Apr 12, 2016 at 6:53 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 38 quoted lines
> 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
Show 27 quoted lines
> +                       $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
Junio C Hamano· Apr 13, 2016, 01:36 UTC · re: Eric Sunshine · lore

Re: [PATCH] send-email: detect and offer to skip backup files

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.
Stefan Beller· Apr 12, 2016, 23:05 UTC · re: Junio C Hamano · lore

Re: [PATCH] send-email: detect and offer to skip backup files

On Tue, Apr 12, 2016 at 3:53 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 23 quoted lines
> 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?

Show 69 quoted lines
>
>  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
Junio C Hamano· Apr 13, 2016, 01:38 UTC · re: Stefan Beller · lore

Re: [PATCH] send-email: detect and offer to skip backup files

Stefan Beller <sbeller@google.com> writes:
Show 13 quoted lines
> 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).

← back to recent threads