threads / patch / 14682

patchgit-reset: Let -q hush "locally modified" messages

Subject: [PATCH] git-reset: Let -q hush "locally modified" messages

## tl;dr

5 messages between Jul 25, 2008 and Jul 26, 2008. Diffs are folded; open one to read it.

replies: 4people: 3as markdown or json

Stephan Beyer· Jul 25, 2008, 20:49 UTC · lore

git reset -q makes reset more quiet, but "locally modified" messages are still shown. This patch makes them disappear, too.

Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
---
Hi,

I don't know if this was actually a bug or a feature. I considered this a bug, so here's a patch.

Regards,
  Stephan
 builtin-reset.c |   15 +++++++++------
 1 files changed, 9 insertions(+), 6 deletions(-)
Show changes to builtin-reset.c +9 −6
diff --git a/builtin-reset.c b/builtin-reset.c
index 4d246c3..c24c219 100644
--- a/builtin-reset.c
+++ b/builtin-reset.c
@@ -85,7 +85,7 @@ static void print_new_head_line(struct commit *commit)
 		printf("\n");
 }
 
-static int update_index_refresh(int fd, struct lock_file *index_lock)
+static int update_index_refresh(int fd, struct lock_file *index_lock, int flags)
 {
 	int result;
 
@@ -96,7 +96,8 @@ static int update_index_refresh(int fd, struct lock_file *index_lock)
 
 	if (read_cache() < 0)
 		return error("Could not read index");
-	result = refresh_cache(REFRESH_SAY_CHANGED) ? 1 : 0;
+
+	result = refresh_cache(flags) ? 1 : 0;
 	if (write_cache(fd, active_cache, active_nr) ||
 			commit_locked_index(index_lock))
 		return error ("Could not refresh index");
@@ -128,7 +129,7 @@ static void update_index_from_diff(struct diff_queue_struct *q,
 }
 
 static int read_from_tree(const char *prefix, const char **argv,
-		unsigned char *tree_sha1)
+		unsigned char *tree_sha1, int refresh_flags)
 {
 	struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
 	int index_fd, index_was_discarded = 0;
@@ -152,7 +153,7 @@ static int read_from_tree(const char *prefix, const char **argv,
 	if (!index_was_discarded)
 		/* The index is still clobbered from do_diff_cache() */
 		discard_cache();
-	return update_index_refresh(index_fd, lock);
+	return update_index_refresh(index_fd, lock, refresh_flags);
 }
 
 static void prepend_reflog_action(const char *action, char *buf, size_t size)
@@ -246,7 +247,8 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 		else if (reset_type != NONE)
 			die("Cannot do %s reset with paths.",
 					reset_type_names[reset_type]);
-		return read_from_tree(prefix, argv + i, sha1);
+		return read_from_tree(prefix, argv + i, sha1,
+				quiet ? REFRESH_QUIET : REFRESH_SAY_CHANGED);
 	}
 	if (reset_type == NONE)
 		reset_type = MIXED; /* by default */
@@ -286,7 +288,8 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 	case SOFT: /* Nothing else to do. */
 		break;
 	case MIXED: /* Report what has not been updated. */
-		update_index_refresh(0, NULL);
+		update_index_refresh(0, NULL,
+				quiet ? REFRESH_QUIET : REFRESH_SAY_CHANGED);
 		break;
 	}
 

-- 
1.6.0.rc0.49.g3d2ce
Junio C Hamano· Jul 25, 2008, 21:24 UTC · re: Stephan Beyer · lore

Re: [PATCH] git-reset: Let -q hush "locally modified" messages

Stephan Beyer <s-beyer@gmx.net> writes:
> git reset -q makes reset more quiet, but "locally modified" messages are
> still shown.  This patch makes them disappear, too.

Files being "locally modified" is not and error, so I think it is in line with the spirit of what -q is meant to do.

It is an interface change, though.
Stephan Beyer· Jul 25, 2008, 21:38 UTC · re: Junio C Hamano · lore

Re: [PATCH] git-reset: Let -q hush "locally modified" messages

Hi,
Junio C Hamano wrote:
Show 9 quoted lines
> Stephan Beyer <s-beyer@gmx.net> writes:
> 
> > git reset -q makes reset more quiet, but "locally modified" messages are
> > still shown.  This patch makes them disappear, too.
> 
> Files being "locally modified" is not and error, so I think it is in line
> with the spirit of what -q is meant to do.
> 
> It is an interface change, though.
Yes, as "needs update" -> "locally modified" was.

However, scripts, that rely on filenames when doing -q (whyever), will break.

So should I stick to "git reset -q foo >/dev/null" if I want it quiet, or is there some chance of inclusion?

Regards,
  Stephan
-- 
Stephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F
Junio C Hamano· Jul 25, 2008, 22:07 UTC · re: Stephan Beyer · lore

Re: [PATCH] git-reset: Let -q hush "locally modified" messages

Stephan Beyer <s-beyer@gmx.net> writes:
Show 11 quoted lines
> Junio C Hamano wrote:
> ...
>> Files being "locally modified" is not and error, so I think it is in line
>> with the spirit of what -q is meant to do.
>> 
>> It is an interface change, though.
>
> Yes, as "needs update" -> "locally modified" was.
>
> However, scripts, that rely on filenames when doing -q (whyever),
> will break.

I was not worried about scripts. "reset" is a Porcelain, and it is Ok for the scripts to use it and rely on what it does (e.g. "git reset HEAD^" will move the HEAD one commit back and match your index to it), but they should not be depending on its output to be parsable. Perhaps it is Ok for them to detect if there is any change by observing if it spits out anything, but that is about it. So the earlier change to use "locall modified" does not really matter in that sense, but squelching the output altogether is a change with bigger impact. With your patch, we declare that the scripts should not assume anything about its output.

I was more worried about expectation from human users. We used to get a reminder of the paths that are different from the HEAD/index, even with the "-q" option, in the form of human-readable list of paths. Your patch changes that.

Having said all that (the only reason I said the above is not because I think this particular interface change is a big deal, but because I wanted to explain what kind of interface change I do and do _not_ care about when we talk about Porcelain), when the user explicitly gives "-q" to the command, I think it is Ok to assume that the user does not want to hear anything back. I do not see any fundamental problem with your patch in that sense.

It still is an interface change, though ;-)
Johannes Schindelin· Jul 26, 2008, 02:34 UTC · re: Stephan Beyer · lore

Re: [PATCH] git-reset: Let -q hush "locally modified" messages

Hi,
On Fri, 25 Jul 2008, Stephan Beyer wrote:
Show 12 quoted lines
> Junio C Hamano wrote:
> > Stephan Beyer <s-beyer@gmx.net> writes:
> > 
> > > git reset -q makes reset more quiet, but "locally modified" messages 
> > > are still shown.  This patch makes them disappear, too.
> > 
> > Files being "locally modified" is not and error, so I think it is in 
> > line with the spirit of what -q is meant to do.
> > 
> > It is an interface change, though.
> 
> Yes, as "needs update" -> "locally modified" was.

But that is not about scripts. Scripts have no business calling pure porcelain, so they still get "needs update" if done properly.

Hth, Dscho

← back to recent threads