git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: git@vger.kernel.org
Subject: bug in git-revert
Date: Sun, 2 Mar 2008 01:44:49 -0500	[thread overview]
Message-ID: <20080302064449.GA6334@coredump.intra.peff.net> (raw)

In 9509af6 (Make git-revert & git-cherry-pick a builtin), you introduced
the following code:

   252          } else {
   253                  struct wt_status s;
   254
   255                  if (get_sha1("HEAD", head))
   256                          die ("You do not have a valid HEAD");
   257                  wt_status_prepare(&s);
   258                  if (s.commitable || s.workdir_dirty)
   259                          die ("Dirty index: cannot %s", me);
   260                  discard_cache();
   261          }

(where the die condition was later changed to allow a dirty workdir). Am
I crazy, or is it impossible to trigger either of those conditions?
wt_status_prepare is about setting up the object to do the actual diff.
At that point it hasn't actually looked at the index or file contents,
and so commitable is _always_ 0. And if it did do the right thing, it
would have spewed a bunch of stuff to stdout.

I think it should be fixable by something like the patch below, but it
feels a little hack-ish. Should there perhaps be an equivalent to
"wt_status_print_updated" that just finds the commitable flag?

diff --git a/builtin-revert.c b/builtin-revert.c
index b6dee6a..64f381c 100644
--- a/builtin-revert.c
+++ b/builtin-revert.c
@@ -279,6 +279,8 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 		if (get_sha1("HEAD", head))
 			die ("You do not have a valid HEAD");
 		wt_status_prepare(&s);
+		s.fp = NULL;
+		wt_status_print_updated(&s);
 		if (s.commitable)
 			die ("Dirty index: cannot %s", me);
 		discard_cache();
diff --git a/color.c b/color.c
index 12a6453..6dba762 100644
--- a/color.c
+++ b/color.c
@@ -160,6 +160,8 @@ static int color_vfprintf(FILE *fp, const char *color, const char *fmt,
 {
 	int r = 0;
 
+	if (!fp)
+		return 0;
 	if (*color)
 		r += fprintf(fp, "%s", color);
 	r += vfprintf(fp, fmt, args);
diff --git a/wt-status.c b/wt-status.c
index 32d780a..bf0934a 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -169,7 +169,8 @@ static void wt_status_print_filepair(struct wt_status *s,
 	default:
 		die("bug: unhandled diff status %c", p->status);
 	}
-	fprintf(s->fp, "\n");
+	if (s->fp)
+		fprintf(s->fp, "\n");
 	strbuf_release(&onebuf);
 	strbuf_release(&twobuf);
 }
@@ -238,7 +239,7 @@ static void wt_status_print_initial(struct wt_status *s)
 	strbuf_release(&buf);
 }
 
-static void wt_status_print_updated(struct wt_status *s)
+void wt_status_print_updated(struct wt_status *s)
 {
 	struct rev_info rev;
 	init_revisions(&rev, NULL);
@@ -369,8 +370,10 @@ void wt_status_print(struct wt_status *s)
 	if (s->verbose && !s->is_initial)
 		wt_status_print_verbose(s);
 	if (!s->commitable) {
-		if (s->amend)
-			fprintf(s->fp, "# No changes\n");
+		if (s->amend) {
+			if (s->fp)
+				fprintf(s->fp, "# No changes\n");
+		}
 		else if (s->nowarn)
 			; /* nothing */
 		else if (s->workdir_dirty)
diff --git a/wt-status.h b/wt-status.h
index 02afaa6..7d40a57 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -32,5 +32,6 @@ int wt_status_use_color;
 int wt_status_relative_paths;
 void wt_status_prepare(struct wt_status *s);
 void wt_status_print(struct wt_status *s);
+void wt_status_print_updated(struct wt_status *s);
 
 #endif /* STATUS_H */

             reply	other threads:[~2008-03-02  6:46 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-03-02  6:44 Jeff King [this message]
2008-03-02  7:00 ` bug in git-revert Johannes Schindelin
2008-03-02  7:02 ` Jeff King
2008-03-02  7:22 ` [PATCH] revert: actually check for a dirty index Jeff King
2008-03-02 10:37   ` Alex Riesen
2008-03-03  6:30     ` Jeff King
2008-03-03  7:11       ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080302064449.GA6334@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).