threads / patch / 16037

patchadd -p: warn if only binary changes present

Subject: [PATCH] add -p: warn if only binary changes present

## tl;dr

6 messages between Oct 24, 2008 and Oct 26, 2008. Diffs are folded; open one to read it.

replies: 5people: 3as markdown or json

Thomas Rast· Oct 24, 2008, 21:48 UTC · lore

Current 'git add -p' will say "No changes." if there are no changes to text files, which can be confusing if there _are_ changes to binary files. Add some code to distinguish the two cases, and give a different message in the latter one.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 git-add--interactive.perl |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)
Show changes to git-add--interactive.perl +7 −2
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index da768ee..a560133 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -811,11 +811,16 @@ EOF
 }
 
 sub patch_update_cmd {
-	my @mods = grep { !($_->{BINARY}) } list_modified('file-only');
+	my @all_mods = list_modified('file-only');
+	my @mods = grep { !($_->{BINARY}) } @all_mods;
 	my @them;
 
 	if (!@mods) {
-		print STDERR "No changes.\n";
+		if (@all_mods) {
+			print STDERR "No changes except to binary files.\n";
+		} else {
+			print STDERR "No changes.\n";
+		}
 		return 0;
 	}
 	if ($patch_mode) {

-- 
tg: (759ad19..) t/add-p-binary-warn (depends on: origin/master)
Jeff King· Oct 26, 2008, 05:10 UTC · re: Thomas Rast · lore

Re: [PATCH] add -p: warn if only binary changes present

On Fri, Oct 24, 2008 at 11:48:36PM +0200, Thomas Rast wrote:
> Current 'git add -p' will say "No changes." if there are no changes to
> text files, which can be confusing if there _are_ changes to binary
> files.  Add some code to distinguish the two cases, and give a
> different message in the latter one.

Having just looked at this code (for potentially handling "git add -p" of new files), I think this change is sane.

> +			print STDERR "No changes except to binary files.\n";
This wording seems a little awkward to me, though. Maybe
  No changed text files.
?
-Peff
Thomas Rast· Oct 26, 2008, 10:28 UTC · re: Jeff King · lore

Re: [PATCH] add -p: warn if only binary changes present

Jeff King wrote:
Show 7 quoted lines
> > +			print STDERR "No changes except to binary files.\n";
> 
> This wording seems a little awkward to me, though. Maybe
> 
>   No changed text files.
> 
> ?

I tried to make a more precise statement. "No changed text files" also holds if no files at all were changed. A user can only infer that there _are_ binary changes if he knows that the message in the latter case would have been "No changes".

That being said, it is somewhat awkward, I just couldn't come up with a better message with this meaning. I considered some other options, such as giving a hint to use git-add or running git-status for the user, but these would have been far more verbose.

- Thomas
-- 
Thomas Rast
trast@{inf,student}.ethz.ch
SZEDER Gábor· Oct 26, 2008, 10:40 UTC · re: Thomas Rast · lore

Re: [PATCH] add -p: warn if only binary changes present

On Sun, Oct 26, 2008 at 12:28:09PM +0200, Thomas Rast wrote:
Show 10 quoted lines
> Jeff King wrote:
> > > +			print STDERR "No changes except to binary files.\n";
> >   No changed text files.
> I tried to make a more precise statement.  "No changed text files"
> also holds if no files at all were changed.  A user can only infer
> that there _are_ binary changes if he knows that the message in the
> latter case would have been "No changes".
> 
> That being said, it is somewhat awkward, I just couldn't come up with
> a better message with this meaning.
What about
   Only binary files changed.
or something of the sort?
Gábor
Thomas Rast· Oct 26, 2008, 19:37 UTC · re: SZEDER Gábor · lore

[PATCH v2] add -p: warn if only binary changes present

Current 'git add -p' will say "No changes." if there are no changes to text files, which can be confusing if there _are_ changes to binary files. Add some code to distinguish the two cases, and give a different message in the latter one.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
SZEDER Gábor wrote:
Show 5 quoted lines
> What about
> 
>    Only binary files changed.
> 
> or something of the sort?
Yes, why not.
 git-add--interactive.perl |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)
Show changes to git-add--interactive.perl +7 −2
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index da768ee..b0223c3 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -811,11 +811,16 @@ EOF
 }
 
 sub patch_update_cmd {
-	my @mods = grep { !($_->{BINARY}) } list_modified('file-only');
+	my @all_mods = list_modified('file-only');
+	my @mods = grep { !($_->{BINARY}) } @all_mods;
 	my @them;
 
 	if (!@mods) {
-		print STDERR "No changes.\n";
+		if (@all_mods) {
+			print STDERR "Only binary files changed.\n";
+		} else {
+			print STDERR "No changes.\n";
+		}
 		return 0;
 	}
 	if ($patch_mode) {

-- 
tg: (759ad19..) t/add-p-binary-warn (depends on: origin/master)
Jeff King· Oct 26, 2008, 19:38 UTC · re: Thomas Rast · lore

Re: [PATCH v2] add -p: warn if only binary changes present

On Sun, Oct 26, 2008 at 08:37:06PM +0100, Thomas Rast wrote:
Show 7 quoted lines
> > What about
> > 
> >    Only binary files changed.
> > 
> > or something of the sort?
> 
> Yes, why not.
That is better, IMHO.
-Peff

← back to recent threads