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

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

[Thomas Rast]

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(-)

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)

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

[Jeff King]

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

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

[Thomas Rast]

[-- Attachment #1: Type: text/plain, Size: 763 bytes --]

Jeff King wrote:
> > +			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



[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

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

[SZEDER Gábor]

On Sun, Oct 26, 2008 at 12:28:09PM +0200, Thomas Rast wrote:
> 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

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

[Thomas Rast]

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:
> 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(-)

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)

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

[Jeff King]

On Sun, Oct 26, 2008 at 08:37:06PM +0100, Thomas Rast wrote:

> > What about
> > 
> >    Only binary files changed.
> > 
> > or something of the sort?
> 
> Yes, why not.

That is better, IMHO.

-Peff