Git development
 help / color / mirror / Atom feed
* Re: [PATCH] whitespace: fix initial-indent checking
From: Jakub Narebski @ 2007-12-16  9:08 UTC (permalink / raw)
  To: git
In-Reply-To: <1197776919-16121-5-git-send-email-bfields@citi.umich.edu>

J. Bruce Fields wrote:

> This allows catching initial indents like '\t        ' (a tab followed
> by 8 spaces), while previously indent-with-non-tab caught only indents
> that consisted entirely of spaces.

I prefer to use tabs for indent, but _spaces_ for align. While previous,
less strict version of check catches indent using spaces, this one also
catches _align_ using spaces.

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ permalink raw reply

* Re: Windows binaries for qgit 2.0
From: Marco Costalba @ 2007-12-16  9:05 UTC (permalink / raw)
  To: Abdelrazak Younes; +Cc: git
In-Reply-To: <fk2p0f$961$1@ger.gmane.org>

On Dec 16, 2007 9:52 AM, Abdelrazak Younes <younes.a@free.fr> wrote:
>
> Hum, I can't imagine this is the reason, it might be for another reason,
> most certainly the static compiling.
>

Static compiling is indeed a highly suspect. I'm now compiling as
shared libraries, we will see...

>
> Well, for the LyX project we used to use and distribute the patched
> version of Qt in order to compile it with MSVC. Now, since Qt4.3.2 the
> MSVC compilation support is enabled in the official release so you don't
> even have to patch it anymore. I can send you precompiled dll if you
> want.

Sorry for the stupid question, but, what ddl ??

Marco

^ permalink raw reply

* Re: Windows binaries for qgit 2.0
From: Abdelrazak Younes @ 2007-12-16  8:52 UTC (permalink / raw)
  To: git
In-Reply-To: <e5bfff550712152355o7c8ef2f3j95f239697f77ccef@mail.gmail.com>

Marco Costalba wrote:
> On Dec 16, 2007 8:26 AM, Abdelrazak Younes <younes.a@free.fr> wrote:
>> Sorry for the intrusion in this list but you don't need to compile Qt
>> statically IMO. Just bundle the Qt dlls that you needs (most probably
>> QtCore and QtGui) with you executable and be done with it.
>>
> 
> Yes. That's another option, I will try that to see if things get better.
> 
>> If you are using debug dlls, those are known to be slower  because of
> 
> Actually I use debug dll for normal development and never experienced slowness.
> I switched to "release" library to create the qgit package and _now_ I
> have the thing much slower and also misbehaving. So for me are the
> release library that have problems, not the debug ones.

Hum, I can't imagine this is the reason, it might be for another reason, 
most certainly the static compiling.

> 
>> In any case, if you want to gain
>> speed, you should compile with MSVC (MSVC2005Express is free as in beer).
>>
> 
> Yes, I was thinking about this, but Qt don't officially supports MSVC
> for the Qt4 GPL libraries,

Well, for the LyX project we used to use and distribute the patched 
version of Qt in order to compile it with MSVC. Now, since Qt4.3.2 the 
MSVC compilation support is enabled in the official release so you don't 
even have to patch it anymore. I can send you precompiled dll if you 
want. Or you can use the one bundled in the last lyx installer 
(www.lyx.org).

> so that's the reason I was stick to mingw
> (although now it comes more and more clear to me that the shipped
> version has problems with release compiled libraries)

The first reason why we went away from mingw was because it was way too 
slow to compile Qt and LyX. With MSVC the binaries are much faster.

Abdel.

^ permalink raw reply

* Re: Windows binaries for qgit 2.0
From: Marco Costalba @ 2007-12-16  7:55 UTC (permalink / raw)
  To: Abdelrazak Younes; +Cc: git
In-Reply-To: <fk2juf$t25$1@ger.gmane.org>

On Dec 16, 2007 8:26 AM, Abdelrazak Younes <younes.a@free.fr> wrote:
>
> Sorry for the intrusion in this list but you don't need to compile Qt
> statically IMO. Just bundle the Qt dlls that you needs (most probably
> QtCore and QtGui) with you executable and be done with it.
>

Yes. That's another option, I will try that to see if things get better.

>
> If you are using debug dlls, those are known to be slower  because of

Actually I use debug dll for normal development and never experienced slowness.
I switched to "release" library to create the qgit package and _now_ I
have the thing much slower and also misbehaving. So for me are the
release library that have problems, not the debug ones.

> In any case, if you want to gain
> speed, you should compile with MSVC (MSVC2005Express is free as in beer).
>

Yes, I was thinking about this, but Qt don't officially supports MSVC
for the Qt4 GPL libraries, so that's the reason I was stick to mingw
(although now it comes more and more clear to me that the shipped
version has problems with release compiled libraries)

> Hope this helps,
> Abdel.
>

Yes, thanks a lot.
Marco

^ permalink raw reply

* [PATCH v2] Allow commit (and tag) messages to be edited when $EDITOR has arguments
From: Steven Grimm @ 2007-12-16  7:34 UTC (permalink / raw)
  To: git
In-Reply-To: <Pine.LNX.4.64.0712160139580.27959@racer.site>

Users who do EDITOR="/usr/bin/emacs -nw" or similar were left unable to
edit commit messages once commit became a builtin, because the editor
launch code assumed that $EDITOR was a single pathname.

This patch makes split_cmdline() a public function as suggested by
Johannes Schindelin, and renames an internal function in git.c to avoid
a name collision.

Signed-off-by: Steven Grimm <koreth@midwinter.com>
---
 builtin-tag.c |   14 +++++++++++-
 git.c         |   60 +++-------------------------------------------------
 run-command.c |   65 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 run-command.h |    2 +
 4 files changed, 84 insertions(+), 57 deletions(-)

diff --git a/builtin-tag.c b/builtin-tag.c
index 274901a..0a38724 100644
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -47,10 +47,22 @@ void launch_editor(const char *path, struct strbuf *buffer, const char *const *e
 		editor = "vi";
 
 	if (strcmp(editor, ":")) {
-		const char *args[] = { editor, path, NULL };
+		char *editor_copy = xstrdup(editor);
+		char **args;
+		int args_pos;
+
+		args_pos = split_cmdline(editor_copy, &args, 2);
+		if (args_pos < 0)
+			die("Couldn't parse the editor command %s.", editor);
+
+		args[args_pos++] = path;
+		args[args_pos++] = NULL;
 
 		if (run_command_v_opt_cd_env(args, 0, NULL, env))
 			die("There was a problem with the editor %s.", editor);
+
+		free(args);
+		free(editor_copy);
 	}
 
 	if (!buffer)
diff --git a/git.c b/git.c
index 15fec89..3d095ee 100644
--- a/git.c
+++ b/git.c
@@ -2,6 +2,7 @@
 #include "exec_cmd.h"
 #include "cache.h"
 #include "quote.h"
+#include "run-command.h"
 
 const char git_usage_string[] =
 	"git [--version] [--exec-path[=GIT_EXEC_PATH]] [-p|--paginate|--no-pager] [--bare] [--git-dir=GIT_DIR] [--work-tree=GIT_WORK_TREE] [--help] COMMAND [ARGS]";
@@ -98,59 +99,6 @@ static int git_alias_config(const char *var, const char *value)
 	return 0;
 }
 
-static int split_cmdline(char *cmdline, const char ***argv)
-{
-	int src, dst, count = 0, size = 16;
-	char quoted = 0;
-
-	*argv = xmalloc(sizeof(char*) * size);
-
-	/* split alias_string */
-	(*argv)[count++] = cmdline;
-	for (src = dst = 0; cmdline[src];) {
-		char c = cmdline[src];
-		if (!quoted && isspace(c)) {
-			cmdline[dst++] = 0;
-			while (cmdline[++src]
-					&& isspace(cmdline[src]))
-				; /* skip */
-			if (count >= size) {
-				size += 16;
-				*argv = xrealloc(*argv, sizeof(char*) * size);
-			}
-			(*argv)[count++] = cmdline + dst;
-		} else if(!quoted && (c == '\'' || c == '"')) {
-			quoted = c;
-			src++;
-		} else if (c == quoted) {
-			quoted = 0;
-			src++;
-		} else {
-			if (c == '\\' && quoted != '\'') {
-				src++;
-				c = cmdline[src];
-				if (!c) {
-					free(*argv);
-					*argv = NULL;
-					return error("cmdline ends with \\");
-				}
-			}
-			cmdline[dst++] = c;
-			src++;
-		}
-	}
-
-	cmdline[dst] = 0;
-
-	if (quoted) {
-		free(*argv);
-		*argv = NULL;
-		return error("unclosed quote");
-	}
-
-	return count;
-}
-
 static int handle_alias(int *argcp, const char ***argv)
 {
 	int nongit = 0, envchanged = 0, ret = 0, saved_errno = errno;
@@ -182,7 +130,7 @@ static int handle_alias(int *argcp, const char ***argv)
 			die("Failed to run '%s' when expanding alias '%s'\n",
 			    alias_string + 1, alias_command);
 		}
-		count = split_cmdline(alias_string, &new_argv);
+		count = split_cmdline(alias_string, &new_argv, 0);
 		option_count = handle_options(&new_argv, &count, &envchanged);
 		if (envchanged)
 			die("alias '%s' changes environment variables\n"
@@ -238,7 +186,7 @@ struct cmd_struct {
 	int option;
 };
 
-static int run_command(struct cmd_struct *p, int argc, const char **argv)
+static int run_git_command(struct cmd_struct *p, int argc, const char **argv)
 {
 	int status;
 	struct stat st;
@@ -380,7 +328,7 @@ static void handle_internal_command(int argc, const char **argv)
 		struct cmd_struct *p = commands+i;
 		if (strcmp(p->cmd, cmd))
 			continue;
-		exit(run_command(p, argc, argv));
+		exit(run_git_command(p, argc, argv));
 	}
 }
 
diff --git a/run-command.c b/run-command.c
index 476d00c..3ae55ec 100644
--- a/run-command.c
+++ b/run-command.c
@@ -237,3 +237,68 @@ int finish_async(struct async *async)
 		ret = error("waitpid (async) failed");
 	return ret;
 }
+
+/*
+ * Parses a command line into an array of char* representing the tokens
+ * on the command line.  Pass in a count to reserve some number of additional
+ * slots in the allocated array, e.g., so the caller can add a filename
+ * argument without having to reallocate the array.
+ *
+ * Returns the number of items in the array or -1 if an error occurred.
+ *
+ * Note that the command line will be altered (nulls will be inserted
+ * where the original had argument-delimiting whitespace.)
+ */
+int split_cmdline(char *cmdline, const char ***argv, int extra_slots)
+{
+	int src, dst, count = 0, size = extra_slots + 16;
+	char quoted = 0;
+
+	*argv = xmalloc(sizeof(char*) * size);
+
+	/* split alias_string */
+	(*argv)[count++] = cmdline;
+	for (src = dst = 0; cmdline[src];) {
+		char c = cmdline[src];
+		if (!quoted && isspace(c)) {
+			cmdline[dst++] = 0;
+			while (cmdline[++src]
+					&& isspace(cmdline[src]))
+				; /* skip */
+			if (count >= size) {
+				size += 16;
+				*argv = xrealloc(*argv, sizeof(char*) * size);
+			}
+			(*argv)[count++] = cmdline + dst;
+		} else if(!quoted && (c == '\'' || c == '"')) {
+			quoted = c;
+			src++;
+		} else if (c == quoted) {
+			quoted = 0;
+			src++;
+		} else {
+			if (c == '\\' && quoted != '\'') {
+				src++;
+				c = cmdline[src];
+				if (!c) {
+					free(*argv);
+					*argv = NULL;
+					return error("cmdline ends with \\");
+				}
+			}
+			cmdline[dst++] = c;
+			src++;
+		}
+	}
+
+	cmdline[dst] = 0;
+
+	if (quoted) {
+		free(*argv);
+		*argv = NULL;
+		return error("unclosed quote");
+	}
+
+	return count;
+}
+
diff --git a/run-command.h b/run-command.h
index 1fc781d..e2b5dea 100644
--- a/run-command.h
+++ b/run-command.h
@@ -66,4 +66,6 @@ struct async {
 int start_async(struct async *async);
 int finish_async(struct async *async);
 
+int split_cmdline(char *cmdline, const char ***argv, int extra_slots);
+
 #endif
-- 
1.5.4.rc0.37.g176bc

^ permalink raw reply related

* Re: Windows binaries for qgit 2.0
From: Abdelrazak Younes @ 2007-12-16  7:26 UTC (permalink / raw)
  To: git
In-Reply-To: <e5bfff550712150702p2675da8axed1f3db6273f619@mail.gmail.com>

Marco Costalba wrote:
> Due to a feature request on sourceforge qgit account:
> 
> http://sourceforge.net/tracker/index.php?func=detail&aid=1759403&group_id=139897&atid=744830
> 
> I have cooked and uploaded a qgit.exe file with Qt4.3.3 statically
> linked in so to let Windows users that just want to give a shot to
> qgit the possibility to try it easily.

Sorry for the intrusion in this list but you don't need to compile Qt 
statically IMO. Just bundle the Qt dlls that you needs (most probably 
QtCore and QtGui) with you executable and be done with it.

> Normally I use debug shared Qt libraries for development, so after
> some effort I finally managed to build the static ones and finally to
> build qgit.exe
> 
> Problem is that _this_ qgit.exe shows strange misbehaviour like 20X
> slower then the one I normally build for development,  there is also
> some strange artifacts with file history graph.

If you are using debug dlls, those are known to be slower  because of 
the additional checking (like vector bundaries, etc). Beware also of 
stdlib-debug if you're using gcc. In any case, if you want to gain 
speed, you should compile with MSVC (MSVC2005Express is free as in beer).

Hope this helps,
Abdel.

^ permalink raw reply

* Re: git-browse-help?
From: Jeff King @ 2007-12-16  7:21 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Shawn O. Pearce, Christian Couder, git
In-Reply-To: <7vabobn4i3.fsf@gitster.siamese.dyndns.org>

On Sat, Dec 15, 2007 at 11:26:44AM -0800, Junio C Hamano wrote:

> > Although I would have called it "git-help--browse" rather than
> > "git-browse--help" since
> 
> Good point.

And here is the patch (I based the change-sites off of the ones in your
previous patch, plus a grep to double-check).

-- >8 --
rename git-browse--help to git-help--browse

The convention for helper scripts has been
git-$TOOL--$HELPER. Since this is a "browse" helper for the
"help" tool, git-help--browse is a more sensible name.

Signed-off-by: Jeff King <peff@peff.net>
---
 .gitignore                                 |    2 +-
 Documentation/git-help.txt                 |    4 ++--
 Makefile                                   |    2 +-
 git-browse--help.sh => git-help--browse.sh |    0 
 help.c                                     |    2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)
 rename git-browse--help.sh => git-help--browse.sh (100%)

diff --git a/.gitignore b/.gitignore
index aef01c5..dab5bc2 100644
--- a/.gitignore
+++ b/.gitignore
@@ -12,7 +12,6 @@ git-archive
 git-bisect
 git-blame
 git-branch
-git-browse--help
 git-bundle
 git-cat-file
 git-check-attr
@@ -52,6 +51,7 @@ git-gc
 git-get-tar-commit-id
 git-grep
 git-hash-object
+git-help--browse
 git-http-fetch
 git-http-push
 git-imap-send
diff --git a/Documentation/git-help.txt b/Documentation/git-help.txt
index 8cd69e7..da3f718 100644
--- a/Documentation/git-help.txt
+++ b/Documentation/git-help.txt
@@ -47,13 +47,13 @@ OPTIONS
 +
 The web browser can be specified using the configuration variable
 'help.browser', or 'web.browser' if the former is not set. If none of
-these config variables is set, the 'git-browse--help' helper script
+these config variables is set, the 'git-help--browse' helper script
 (called by 'git-help') will pick a suitable default.
 +
 You can explicitly provide a full path to your prefered browser by
 setting the configuration variable 'browser.<tool>.path'. For example,
 you can configure the absolute path to firefox by setting
-'browser.firefox.path'. Otherwise, 'git-browse--help' assumes the tool
+'browser.firefox.path'. Otherwise, 'git-help--browse' assumes the tool
 is available in PATH.
 +
 Note that the script tries, as much as possible, to display the HTML
diff --git a/Makefile b/Makefile
index 62f1893..7776077 100644
--- a/Makefile
+++ b/Makefile
@@ -227,7 +227,7 @@ SCRIPT_SH = \
 	git-lost-found.sh git-quiltimport.sh git-submodule.sh \
 	git-filter-branch.sh \
 	git-stash.sh \
-	git-browse--help.sh
+	git-help--browse.sh
 
 SCRIPT_PERL = \
 	git-add--interactive.perl \
diff --git a/git-browse--help.sh b/git-help--browse.sh
similarity index 100%
rename from git-browse--help.sh
rename to git-help--browse.sh
diff --git a/help.c b/help.c
index f9ce6db..1302a61 100644
--- a/help.c
+++ b/help.c
@@ -331,7 +331,7 @@ static void show_info_page(const char *git_cmd)
 static void show_html_page(const char *git_cmd)
 {
 	const char *page = cmd_to_page(git_cmd);
-	execl_git_cmd("browse--help", page, NULL);
+	execl_git_cmd("help--browse", page, NULL);
 }
 
 void help_unknown_cmd(const char *cmd)
-- 
1.5.4.rc0.1122.g899d-dirty

^ permalink raw reply related

* Re: trim_common_tail bug?
From: Jeff King @ 2007-12-16  7:06 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Linus Torvalds, git
In-Reply-To: <20071215200202.GA3334@sigill.intra.peff.net>

On Sat, Dec 15, 2007 at 03:02:02PM -0500, Jeff King wrote:

> >   	for (i = 0, recovered = 0; recovered < trimmed && i <= ctx; i++) {
> > 		while (recovered < trimmed && ap[recovered++] != '\n')
> > 	        	; /* nothing */
> > 	}
> > 
> > then (warning: I haven't had my coffee yet)?
> 
> Yes, I think that is equivalent. My sleep-deprived brain keeps thinking
> there must be a more clear way of writing this whole loop, but it
> escapes me at the moment.

And this came to me in a dream. :) It fixes the bug, and I think it is a
bit simpler to see the termination conditions in a single loop. But
please double-check correctness, and that you agree that it is more
readable.

---
diff --git a/xdiff-interface.c b/xdiff-interface.c
index 700def2..98b02ed 100644
--- a/xdiff-interface.c
+++ b/xdiff-interface.c
@@ -110,7 +110,7 @@ int xdiff_outf(void *priv_, mmbuffer_t *mb, int nbuf)
 static void trim_common_tail(mmfile_t *a, mmfile_t *b, long ctx)
 {
 	const int blk = 1024;
-	long trimmed = 0, recovered = 0, i;
+	long trimmed = 0, recovered = 0;
 	char *ap = a->ptr + a->size;
 	char *bp = b->ptr + b->size;
 	long smaller = (a->size < b->size) ? a->size : b->size;
@@ -121,10 +121,9 @@ static void trim_common_tail(mmfile_t *a, mmfile_t *b, long ctx)
 		bp -= blk;
 	}
 
-	for (i = 0, recovered = 0; recovered < trimmed && i <= ctx; i++) {
-		while (recovered < trimmed && ap[recovered] != '\n')
-			recovered++;
-	}
+	while (recovered < trimmed && ctx)
+		if (ap[recovered++] == '\n')
+			ctx--;
 	a->size -= (trimmed - recovered);
 	b->size -= (trimmed - recovered);
 }

^ permalink raw reply related

* Re: git-svn rebase issues (the commiter gets changed)
From: Junio C Hamano @ 2007-12-16  5:50 UTC (permalink / raw)
  To: Eric Wong; +Cc: Kelvie Wong, git
In-Reply-To: <20071216033947.GB17666@muzzle>

Yes, I think Jeff's patch (and my simplification) should fix it.  Sorry
about the brown-paper-bag bug.

^ permalink raw reply

* [PATCH] Fix a memory leak
From: 李鸿 @ 2007-12-16  4:53 UTC (permalink / raw)
  To: git



Signed-off-by: Li Hong <leehong@pku.edu.cn>
---
 dir.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/dir.c b/dir.c
index 6b3273d..3e345c2 100644
--- a/dir.c
+++ b/dir.c
@@ -169,7 +169,10 @@ static int add_excludes_from_file_1(const char *fname,
 	}
 	buf = xmalloc(size+1);
 	if (read_in_full(fd, buf, size) != size)
+	{
+		free(buf);
 		goto err;
+	}
 	close(fd);
 
 	if (buf_p)
-- 
1.5.4.rc0.8.gaba5e

^ permalink raw reply related

* Re: [PATCH] whitespace: fix initial-indent checking
From: J. Bruce Fields @ 2007-12-16  3:54 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <1197776919-16121-5-git-send-email-bfields@citi.umich.edu>

On Sat, Dec 15, 2007 at 10:48:37PM -0500, J. Bruce Fields wrote:
> After this patch, "written" counts the number of bytes up to and
> including the most recently seen tab.  This allows us to detect (and
> count) spaces by comparing to "i".
> 
> This allows catching initial indents like '\t        ' (a tab followed
> by 8 spaces), while previously indent-with-non-tab caught only indents
> that consisted entirely of spaces.
> 
> This also allows fixing an indent-with-non-tab regression, so we can
> again detect indents like '\t \t'.
> 
> Also update tests to catch these cases.

One slightly weird thing about this: I'd expect indent-with-non-tab to
catch any sequence of 8 or more contiguous spaces, not just such
sequences at the end of the indent.  This doesn't quite do that.

You could make it do that with a few more lines of code.  But really I
don't think the combination of indent-with-non-tab without
space-before-tab makes any sense.

The only reason I didn't just modify it to turn on the latter whenever
the former is turned on is because I couldn't figure out how to modify
t/t4124-apply-ws-rule.sh to pass.....

But, whatever, that's an extremely minor point.  If people try that
combination who knows what they expect.

--b.

^ permalink raw reply

* [PATCH] whitespace: fix config.txt description of indent-with-non-tab
From: J. Bruce Fields @ 2007-12-16  3:48 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, J. Bruce Fields
In-Reply-To: <1197776919-16121-6-git-send-email-bfields@citi.umich.edu>

Fix garbled description.

Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
---
 Documentation/config.txt |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index fabe7f8..ce16fc7 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -307,7 +307,7 @@ core.whitespace::
   before a tab character in the initial indent part of the line as an
   error (enabled by default).
 * `indent-with-non-tab` treats a line that is indented with 8 or more
-  space characters that can be replaced with tab characters.
+  space characters as an error (not enabled by default).
 
 alias.*::
 	Command aliases for the gitlink:git[1] command wrapper - e.g.
-- 
1.5.4.rc0.41.gf723

^ permalink raw reply related

* [PATCH] whitespace: minor cleanup
From: J. Bruce Fields @ 2007-12-16  3:48 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, J. Bruce Fields
In-Reply-To: <1197776919-16121-3-git-send-email-bfields@citi.umich.edu>

The variable leading_space is initially used to represent the index of
the last space seen before a non-space.  Then later it represents the
index of the first non-indent character.

It will prove simpler to replace it by a variable representing a number
of bytes.  Eventually it will represent the number of bytes written so
far (in the stream != NULL case).

Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
---
 ws.c |   21 +++++++++------------
 1 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/ws.c b/ws.c
index 7165874..1b32e45 100644
--- a/ws.c
+++ b/ws.c
@@ -121,7 +121,7 @@ unsigned check_and_emit_line(const char *line, int len, unsigned ws_rule,
 			     const char *reset, const char *ws)
 {
 	unsigned result = 0;
-	int leading_space = -1;
+	int written = 0;
 	int trailing_whitespace = -1;
 	int trailing_newline = 0;
 	int i;
@@ -147,18 +147,18 @@ unsigned check_and_emit_line(const char *line, int len, unsigned ws_rule,
 	/* Check for space before tab in initial indent. */
 	for (i = 0; i < len; i++) {
 		if (line[i] == ' ') {
-			leading_space = i;
+			written = i + 1;
 			continue;
 		}
 		if (line[i] != '\t')
 			break;
-		if ((ws_rule & WS_SPACE_BEFORE_TAB) && (leading_space != -1))
+		if ((ws_rule & WS_SPACE_BEFORE_TAB) && (written != 0))
 			result |= WS_SPACE_BEFORE_TAB;
 		break;
 	}
 
 	/* Check for indent using non-tab. */
-	if ((ws_rule & WS_INDENT_WITH_NON_TAB) && leading_space >= 7)
+	if ((ws_rule & WS_INDENT_WITH_NON_TAB) && written >= 8)
 		result |= WS_INDENT_WITH_NON_TAB;
 
 	if (stream) {
@@ -166,23 +166,20 @@ unsigned check_and_emit_line(const char *line, int len, unsigned ws_rule,
 		if ((result & WS_SPACE_BEFORE_TAB) ||
 		    (result & WS_INDENT_WITH_NON_TAB)) {
 			fputs(ws, stream);
-			fwrite(line, leading_space + 1, 1, stream);
+			fwrite(line, written, 1, stream);
 			fputs(reset, stream);
-			leading_space++;
 		}
-		else
-			leading_space = 0;
 
-		/* Now the rest of the line starts at leading_space.
+		/* Now the rest of the line starts at written.
 		 * The non-highlighted part ends at trailing_whitespace. */
 		if (trailing_whitespace == -1)
 			trailing_whitespace = len;
 
 		/* Emit non-highlighted (middle) segment. */
-		if (trailing_whitespace - leading_space > 0) {
+		if (trailing_whitespace - written > 0) {
 			fputs(set, stream);
-			fwrite(line + leading_space,
-			    trailing_whitespace - leading_space, 1, stream);
+			fwrite(line + written,
+			    trailing_whitespace - written, 1, stream);
 			fputs(reset, stream);
 		}
 
-- 
1.5.4.rc0.41.gf723

^ permalink raw reply related

* [PATCH] whitespace: reorganize initial-indent check
From: J. Bruce Fields @ 2007-12-16  3:48 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, J. Bruce Fields
In-Reply-To: <1197776919-16121-2-git-send-email-bfields@citi.umich.edu>

Reorganize to emphasize the most complicated part of the code (the tab
case).

Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
---
 ws.c |   15 +++++++--------
 1 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/ws.c b/ws.c
index 5ebd109..7165874 100644
--- a/ws.c
+++ b/ws.c
@@ -146,16 +146,15 @@ unsigned check_and_emit_line(const char *line, int len, unsigned ws_rule,
 
 	/* Check for space before tab in initial indent. */
 	for (i = 0; i < len; i++) {
-		if (line[i] == '\t') {
-			if ((ws_rule & WS_SPACE_BEFORE_TAB) &&
-			    (leading_space != -1))
-				result |= WS_SPACE_BEFORE_TAB;
-			break;
-		}
-		else if (line[i] == ' ')
+		if (line[i] == ' ') {
 			leading_space = i;
-		else
+			continue;
+		}
+		if (line[i] != '\t')
 			break;
+		if ((ws_rule & WS_SPACE_BEFORE_TAB) && (leading_space != -1))
+			result |= WS_SPACE_BEFORE_TAB;
+		break;
 	}
 
 	/* Check for indent using non-tab. */
-- 
1.5.4.rc0.41.gf723

^ permalink raw reply related

* [PATCH] whitespace: fix initial-indent checking
From: J. Bruce Fields @ 2007-12-16  3:48 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, J. Bruce Fields
In-Reply-To: <1197776919-16121-4-git-send-email-bfields@citi.umich.edu>

After this patch, "written" counts the number of bytes up to and
including the most recently seen tab.  This allows us to detect (and
count) spaces by comparing to "i".

This allows catching initial indents like '\t        ' (a tab followed
by 8 spaces), while previously indent-with-non-tab caught only indents
that consisted entirely of spaces.

This also allows fixing an indent-with-non-tab regression, so we can
again detect indents like '\t \t'.

Also update tests to catch these cases.

Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
---
 t/t4015-diff-whitespace.sh |   15 +++++++++++++++
 ws.c                       |   10 ++++------
 2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index 0f16bca..d30169f 100755
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -125,6 +125,14 @@ test_expect_success 'check mixed spaces and tabs in indent' '
 
 '
 
+test_expect_success 'check mixed tabs and spaces in indent' '
+
+	# This is indented with HT SP HT.
+	echo "	 	foo();" > x &&
+	git diff --check | grep "space before tab in indent"
+
+'
+
 test_expect_success 'check with no whitespace errors' '
 
 	git commit -m "snapshot" &&
@@ -311,4 +319,11 @@ test_expect_success 'check spaces as indentation (indent-with-non-tab: on)' '
 
 '
 
+test_expect_success 'check tabs and spaces as indentation (indent-with-non-tab: on)' '
+
+	git config core.whitespace "indent-with-non-tab" &&
+	echo "	                foo ();" > x &&
+	! git diff --check
+
+'
 test_done
diff --git a/ws.c b/ws.c
index 1b32e45..aabd509 100644
--- a/ws.c
+++ b/ws.c
@@ -146,19 +146,17 @@ unsigned check_and_emit_line(const char *line, int len, unsigned ws_rule,
 
 	/* Check for space before tab in initial indent. */
 	for (i = 0; i < len; i++) {
-		if (line[i] == ' ') {
-			written = i + 1;
+		if (line[i] == ' ')
 			continue;
-		}
 		if (line[i] != '\t')
 			break;
-		if ((ws_rule & WS_SPACE_BEFORE_TAB) && (written != 0))
+		if ((ws_rule & WS_SPACE_BEFORE_TAB) && written < i)
 			result |= WS_SPACE_BEFORE_TAB;
-		break;
+		written = i + 1;
 	}
 
 	/* Check for indent using non-tab. */
-	if ((ws_rule & WS_INDENT_WITH_NON_TAB) && written >= 8)
+	if ((ws_rule & WS_INDENT_WITH_NON_TAB) && i - written >= 8)
 		result |= WS_INDENT_WITH_NON_TAB;
 
 	if (stream) {
-- 
1.5.4.rc0.41.gf723

^ permalink raw reply related

* [PATCH] whitespace: more accurate initial-indent highlighting
From: J. Bruce Fields @ 2007-12-16  3:48 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, J. Bruce Fields
In-Reply-To: <1197776919-16121-5-git-send-email-bfields@citi.umich.edu>

Instead of highlighting the entire initial indent, highlight only the
problematic spaces.

In the case of an indent like ' \t \t' there may be multiple problematic
ranges, so it's easiest to emit the highlighting as we go instead of
trying rember disjoint ranges and do it all at the end.

Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
---
 ws.c |   24 ++++++++++++++++--------
 1 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/ws.c b/ws.c
index aabd509..d09b9df 100644
--- a/ws.c
+++ b/ws.c
@@ -150,24 +150,32 @@ unsigned check_and_emit_line(const char *line, int len, unsigned ws_rule,
 			continue;
 		if (line[i] != '\t')
 			break;
-		if ((ws_rule & WS_SPACE_BEFORE_TAB) && written < i)
+		if ((ws_rule & WS_SPACE_BEFORE_TAB) && written < i) {
 			result |= WS_SPACE_BEFORE_TAB;
+			if (stream) {
+				fputs(ws, stream);
+				fwrite(line + written, i - written, 1, stream);
+				fputs(reset, stream);
+			}
+		} else if (stream)
+			fwrite(line + written, i - written, 1, stream);
+		if (stream)
+			fwrite(line + i, 1, 1, stream);
 		written = i + 1;
 	}
 
 	/* Check for indent using non-tab. */
-	if ((ws_rule & WS_INDENT_WITH_NON_TAB) && i - written >= 8)
+	if ((ws_rule & WS_INDENT_WITH_NON_TAB) && i - written >= 8) {
 		result |= WS_INDENT_WITH_NON_TAB;
-
-	if (stream) {
-		/* Highlight errors in leading whitespace. */
-		if ((result & WS_SPACE_BEFORE_TAB) ||
-		    (result & WS_INDENT_WITH_NON_TAB)) {
+		if (stream) {
 			fputs(ws, stream);
-			fwrite(line, written, 1, stream);
+			fwrite(line + written, i - written, 1, stream);
 			fputs(reset, stream);
 		}
+		written = i;
+	}
 
+	if (stream) {
 		/* Now the rest of the line starts at written.
 		 * The non-highlighted part ends at trailing_whitespace. */
 		if (trailing_whitespace == -1)
-- 
1.5.4.rc0.41.gf723

^ permalink raw reply related

* [PATCH] whitespace: fix off-by-one error in non-space-in-indent checking
From: J. Bruce Fields @ 2007-12-16  3:48 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, J. Bruce Fields
In-Reply-To: <1197776919-16121-1-git-send-email-bfields@citi.umich.edu>

If there were no tabs, and the last space was at position 7, then
positions 0..7 had spaces, so there were 8 spaces.

Update test to check exactly this case.

Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
---
 t/t4015-diff-whitespace.sh |    4 ++--
 ws.c                       |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index 9bff8f5..0f16bca 100755
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -298,7 +298,7 @@ test_expect_success 'check space before tab in indent (space-before-tab: on)' '
 test_expect_success 'check spaces as indentation (indent-with-non-tab: off)' '
 
 	git config core.whitespace "-indent-with-non-tab"
-	echo "                foo ();" > x &&
+	echo "        foo ();" > x &&
 	git diff --check
 
 '
@@ -306,7 +306,7 @@ test_expect_success 'check spaces as indentation (indent-with-non-tab: off)' '
 test_expect_success 'check spaces as indentation (indent-with-non-tab: on)' '
 
 	git config core.whitespace "indent-with-non-tab" &&
-	echo "                foo ();" > x &&
+	echo "        foo ();" > x &&
 	! git diff --check
 
 '
diff --git a/ws.c b/ws.c
index 46cbdd6..5ebd109 100644
--- a/ws.c
+++ b/ws.c
@@ -159,5 +159,5 @@ unsigned check_and_emit_line(const char *line, int len, unsigned ws_rule,
 	}
 
 	/* Check for indent using non-tab. */
-	if ((ws_rule & WS_INDENT_WITH_NON_TAB) && leading_space >= 8)
+	if ((ws_rule & WS_INDENT_WITH_NON_TAB) && leading_space >= 7)
 		result |= WS_INDENT_WITH_NON_TAB;
\ No newline at end of file
-- 
1.5.4.rc0.41.gf723

^ permalink raw reply related

* Re: [PATCH 3/2] core.whitespace: documentation updates.
From: J. Bruce Fields @ 2007-12-16  3:48 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vodd4fb2f.fsf@gitster.siamese.dyndns.org>

I wrote before:
> On Thu, Dec 06, 2007 at 01:04:56AM -0800, Junio C Hamano wrote:
> > "J. Bruce Fields" <bfields@fieldses.org> writes:
> > > OK, I will go wild, but... very slowly.
> >
> > How wild are you these days ;-)?  I know December is a busy time for
> > everybody, and I ended up doing this myself while I was writing up the
> > API documentation for gitattributes.
> 
> Wow, thanks!  Yes, I haven't done a thing on this.
> 
> > -- >8 --
> > [PATCH] Use gitattributes to define per-path whitespace rule
> >  
> > The `core.whitespace` configuration variable allows you to define what
> > `diff` and `apply` should consider whitespace errors for all paths in
> > the project (See gitlink:git-config[1]).  This attribute gives you
> > finer
> > control per path.
> 
> That looks like what I'd hoped for.
> 
> I'll set aside some time this weekend to play around with it.

Erm, well, some weekend anyway.  You can also pull the following from

	git://linux-nfs.org/~bfields/git.git master

if you'd like.

--b.

^ permalink raw reply

* Re: [PATCH] Add an "-i" option to git-reset, to confirm a reset.
From: Kelvie Wong @ 2007-12-16  3:46 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0712160332140.27959@racer.site>

On Dec 15, 2007 7:35 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Sat, 15 Dec 2007, Kelvie Wong wrote:
>
> > It shows a diffstat, and asks the user if they would like to continue,
> > or show a full diff of the things getting reset.
> >
> > I know that many times, I do a reset --hard thinking I had commited a
> > file already, but it turns out that I hadn't; and so this makes sure I
> > don't lose any work when the caffeine wears off.
> >
> > Maybe it should also be made that only hard resets take this option, as
> > I cannot see this being useful in other places.
>
> I am slightly negative on this patch.  Not only do I think that it is both
> easier and more natural to run diff/status/an-alias to see what a reset
> would do, but the patch only handles the index_file part (missing the --
> <file> part AFAICT).
>
> Besides, the code style is incompatible with the surrounding code.
>
> Ciao,
> Dscho
>
>
[forgot to hit Reply To All again, sorry!]

Ah, you're completely right about the index_file part (this is
actually the first time I've looked at the git-code :P)

Hrm.. I should have just used a shell script wrapper instead it seems.

I do think something like this would be nice though.

w.r.t. the style, you were referring to just the array initializers
right?  Or was there something else I did that doesn't look right?
-- 
Kelvie Wong

^ permalink raw reply

* Re: git-svn rebase issues (the commiter gets changed)
From: Eric Wong @ 2007-12-16  3:39 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Kelvie Wong, git
In-Reply-To: <20071216032523.GA17666@muzzle>

Eric Wong <normalperson@yhbt.net> wrote:
> From 84e99bffc72c10ec7db6d5ae7af6a795b75ef724 Mon Sep 17 00:00:00 2001
> Signed-off-by: Eric Wong <normalperson@yhbt.net>
> ---
>  git-svn.perl |   50 +++++++++++++++++++++++++++++++++++++++-----------
>  1 files changed, 39 insertions(+), 11 deletions(-)
> 
> diff --git a/git-svn.perl b/git-svn.perl
> index d411a34..7cd62fc 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -2089,5 +2116,6 @@ sub do_git_commit {
>  	defined(my $pid = open3(my $msg_fh, my $out_fh, '>&STDERR', @exec))
>  	                                                           or croak $!;
>  	print $msg_fh $log_entry->{log} or croak $!;
> +	restore_commit_header_env($old_env);
>  	unless ($self->no_metadata) {
>  		print $msg_fh "\ngit-svn-id: $log_entry->{metadata}\n"
> \ No newline at end of file

Hi Junio,

This "\ No newline at end of file" from git-format-patch seems wrong.

Bisecting reveals that it started happening at:

  913b45f51b151d8e29f86df67d3e10853d831470
  xdi_diff: trim common trailing lines

-- 
Eric Wong

^ permalink raw reply

* Re: [PATCH] Add an "-i" option to git-reset, to confirm a reset.
From: Johannes Schindelin @ 2007-12-16  3:35 UTC (permalink / raw)
  To: Kelvie Wong; +Cc: git
In-Reply-To: <1197775596-14329-1-git-send-email-kelvie@ieee.org>

Hi,

On Sat, 15 Dec 2007, Kelvie Wong wrote:

> It shows a diffstat, and asks the user if they would like to continue, 
> or show a full diff of the things getting reset.
> 
> I know that many times, I do a reset --hard thinking I had commited a 
> file already, but it turns out that I hadn't; and so this makes sure I 
> don't lose any work when the caffeine wears off.
> 
> Maybe it should also be made that only hard resets take this option, as 
> I cannot see this being useful in other places.

I am slightly negative on this patch.  Not only do I think that it is both 
easier and more natural to run diff/status/an-alias to see what a reset 
would do, but the patch only handles the index_file part (missing the -- 
<file> part AFAICT).

Besides, the code style is incompatible with the surrounding code.

Ciao,
Dscho

^ permalink raw reply

* Re: git-svn rebase issues (the commiter gets changed)
From: Eric Wong @ 2007-12-16  3:27 UTC (permalink / raw)
  To: Kelvie Wong; +Cc: Junio C Hamano, git
In-Reply-To: <94ccbe710711290816t7a6ba9b1o8f37ecf583305a51@mail.gmail.com>

Sorry for the late reply, I've been all over the place lately.

Kelvie Wong <kelvie@ieee.org> wrote:
> Just did it again this morning, with a clean test branch:
> 
> kelvie@mudd (text-edit) qt $ git checkout -b test git-svn
> Switched to a new branch "test"
> kelvie@mudd (test) qt $ touch test
> kelvie@mudd (test) qt $ git add test
> kelvie@mudd (test) qt $ git commit -a -m 'Test!'
> Created commit 05c4016: Test!
>  0 files changed, 0 insertions(+), 0 deletions(-)
>  create mode 100644 apps/qt/test
> kelvie@mudd (test) qt $ git-cat-file commit HEAD
> tree 867c0aa4c814542f0752b5d4c85fc96ba2279aac
> parent 831ffbf25057ed30274d4216269c572cfce12184
> author Kelvie Wong <Kelvie.Wong@safe.com> 1196352603 -0800
> committer Kelvie Wong <Kelvie.Wong@safe.com> 1196352603 -0800
> 
> Test!
> kelvie@mudd (test) qt $ git svn rebase
> <snip>
> HEAD is now at 7319c2a... (svn commit message)
> kelvie@mudd (test) qt $ git-cat-file commit HEAD
> tree 4edacbd41af76ac243099467b33350887c0fb03d
> parent 7319c2a810554aab25a688bcc2b16fc60529b59d
> author Kelvie Wong <Kelvie.Wong@safe.com> 1196352603 -0800
> committer ogibbins <ogibbins@e2d93294-a71b-0410-9dca-e2ea525a67c9>
> 1196346907 +0000
> 
> Test!
> kelvie@mudd (test) qt $ git --version
> git version 1.5.3.6.736.gb7f30
> 
> And again, the committer of a local commit gets changed.
> 
> Now, this part is more interesting:
> 
> kelvie@mudd (test) qt $ git checkout working
> Switched to branch "working"
> kelvie@mudd (working) qt $ git svn rebase
> <no fetch, just a checkout and rebase>
> 
> And when I cat-file the commit, this time it's preserved.  Wild guess
> here (this behaviour seems kind of inconsistent), but it has to do
> with the transition between fetch and rebase?  Or is this a bug in
> git-rebase somewhere?

Ah, it looks like a bug in git-svn.  The following should fix it:

>From 84e99bffc72c10ec7db6d5ae7af6a795b75ef724 Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Sat, 15 Dec 2007 19:08:22 -0800
Subject: [PATCH] git-svn: avoid leaving leftover committer/author info in rebase

We set the 6 environment variables for controlling
committer/author email/name/time for every commit.

We do this in the parent process to be passed to
git-commit-tree, because open3() doesn't afford us the control
of doing it only in the child process.  This means we leave them
hanging around in the main process until the next revision comes
around and all 6 environment variables are overwridden again.

Unfortunately, for the last commit, leaving them hanging around
means the git-rebase invocation will pick it up, rewriting the
rebased commit with incorrect author information.  This should fix
it.

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 git-svn.perl |   50 +++++++++++++++++++++++++++++++++++++++-----------
 1 files changed, 39 insertions(+), 11 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index d411a34..7cd62fc 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -2052,18 +2052,16 @@ sub full_url {
 	$self->{url} . (length $self->{path} ? '/' . $self->{path} : '');
 }
 
-sub do_git_commit {
-	my ($self, $log_entry) = @_;
-	my $lr = $self->last_rev;
-	if (defined $lr && $lr >= $log_entry->{revision}) {
-		die "Last fetched revision of ", $self->refname,
-		    " was r$lr, but we are about to fetch: ",
-		    "r$log_entry->{revision}!\n";
-	}
-	if (my $c = $self->rev_map_get($log_entry->{revision})) {
-		croak "$log_entry->{revision} = $c already exists! ",
-		      "Why are we refetching it?\n";
+
+sub set_commit_header_env {
+	my ($log_entry) = @_;
+	my %env;
+	foreach my $ned (qw/NAME EMAIL DATE/) {
+		foreach my $ac (qw/AUTHOR COMMITTER/) {
+			$env{"GIT_${ac}_${ned}"} = $ENV{"GIT_${ac}_${ned}"};
+		}
 	}
+
 	$ENV{GIT_AUTHOR_NAME} = $log_entry->{name};
 	$ENV{GIT_AUTHOR_EMAIL} = $log_entry->{email};
 	$ENV{GIT_AUTHOR_DATE} = $ENV{GIT_COMMITTER_DATE} = $log_entry->{date};
@@ -2074,7 +2072,36 @@ sub do_git_commit {
 	$ENV{GIT_COMMITTER_EMAIL} = (defined $log_entry->{commit_email})
 						? $log_entry->{commit_email}
 						: $log_entry->{email};
+	\%env;
+}
 
+sub restore_commit_header_env {
+	my ($env) = @_;
+	foreach my $ned (qw/NAME EMAIL DATE/) {
+		foreach my $ac (qw/AUTHOR COMMITTER/) {
+			my $k = "GIT_${ac}_${ned}";
+			if (defined $env->{$k}) {
+				$ENV{$k} = $env->{$k};
+			} else {
+				delete $ENV{$k};
+			}
+		}
+	}
+}
+
+sub do_git_commit {
+	my ($self, $log_entry) = @_;
+	my $lr = $self->last_rev;
+	if (defined $lr && $lr >= $log_entry->{revision}) {
+		die "Last fetched revision of ", $self->refname,
+		    " was r$lr, but we are about to fetch: ",
+		    "r$log_entry->{revision}!\n";
+	}
+	if (my $c = $self->rev_map_get($log_entry->{revision})) {
+		croak "$log_entry->{revision} = $c already exists! ",
+		      "Why are we refetching it?\n";
+	}
+	my $old_env = set_commit_header_env($log_entry);
 	my $tree = $log_entry->{tree};
 	if (!defined $tree) {
 		$tree = $self->tmp_index_do(sub {
@@ -2089,5 +2116,6 @@ sub do_git_commit {
 	defined(my $pid = open3(my $msg_fh, my $out_fh, '>&STDERR', @exec))
 	                                                           or croak $!;
 	print $msg_fh $log_entry->{log} or croak $!;
+	restore_commit_header_env($old_env);
 	unless ($self->no_metadata) {
 		print $msg_fh "\ngit-svn-id: $log_entry->{metadata}\n"
\ No newline at end of file
-- 
Eric Wong

^ permalink raw reply related

* [PATCH] Add an "-i" option to git-reset, to confirm a reset.
From: Kelvie Wong @ 2007-12-16  3:26 UTC (permalink / raw)
  To: git; +Cc: Kelvie Wong

It shows a diffstat, and asks the user if they would like to continue, or
show a full diff of the things getting reset.

I know that many times, I do a reset --hard thinking I had commited a file
already, but it turns out that I hadn't; and so this makes sure I don't
lose any work when the caffeine wears off.

Maybe it should also be made that only hard resets take this option, as
I cannot see this being useful in other places.

Signed-off-by: Kelvie Wong <kelvie@ieee.org>
---
 Documentation/git-reset.txt |    4 +++
 builtin-reset.c             |   46 ++++++++++++++++++++++++++++++++++++++----
 2 files changed, 45 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
index 050e4ea..0323d9d 100644
--- a/Documentation/git-reset.txt
+++ b/Documentation/git-reset.txt
@@ -48,6 +48,10 @@ OPTIONS
 -q::
 	Be quiet, only report errors.
 
+-i::
+	Show what is about to be reset, and ask for confirmation before doing
+	so.
+
 <commit>::
 	Commit to make the current HEAD.
 
diff --git a/builtin-reset.c b/builtin-reset.c
index 713c2d5..1086817 100644
--- a/builtin-reset.c
+++ b/builtin-reset.c
@@ -18,7 +18,8 @@
 #include "tree.h"
 
 static const char builtin_reset_usage[] =
-"git-reset [--mixed | --soft | --hard] [-q] [<commit-ish>] [ [--] <paths>...]";
+"git-reset [--mixed | --soft | --hard] [-q] [-i] [<commit-ish>] [ [--] "
+"<paths>...]";
 
 static char *args_to_str(const char **argv)
 {
@@ -56,17 +57,47 @@ static int unmerged_files(void)
 	return 0;
 }
 
-static int reset_index_file(const unsigned char *sha1, int is_hard_reset)
+static int reset_index_file(const unsigned char *sha1, int is_hard_reset,
+			    int confirm_reset)
 {
 	int i = 0;
 	const char *args[6];
+	struct strbuf buf;
+	char result = 0;
+	const char *ref = sha1_to_hex(sha1);
+	const char *diffstat_args[] = { "diff", "--stat", ref, NULL };
+	const char *diff_args[] = { "diff", ref, NULL };
 
 	args[i++] = "read-tree";
 	args[i++] = "-v";
 	args[i++] = "--reset";
+
+	/* Show the user what is about to be reset, and in more detail, if they
+	 * like. */
+	if(confirm_reset) {
+		printf("The following files will be reset:\n");
+		run_command_v_opt(diffstat_args, RUN_GIT_CMD);
+		strbuf_init(&buf, 0);
+		while(result != 'y') {
+			printf("Continue? ((y)es/(n)o/view (d)iff)\n");
+			strbuf_getline(&buf, stdin, '\n');
+			result = tolower(buf.buf[0]);
+			switch(result) {
+			case 'd':
+				run_command_v_opt(diff_args, RUN_GIT_CMD);
+				break;
+			case 'n':
+				return 1;
+				break;
+			};
+		}
+		strbuf_release(&buf);
+        }
+
+
 	if (is_hard_reset)
 		args[i++] = "-u";
-	args[i++] = sha1_to_hex(sha1);
+	args[i++] = ref;
 	args[i] = NULL;
 
 	return run_command_v_opt(args, RUN_GIT_CMD);
@@ -181,7 +212,8 @@ static const char *reset_type_names[] = { "mixed", "soft", "hard", NULL };
 
 int cmd_reset(int argc, const char **argv, const char *prefix)
 {
-	int i = 1, reset_type = NONE, update_ref_status = 0, quiet = 0;
+	int i = 1, reset_type = NONE, update_ref_status = 0, quiet = 0,
+		confirm = 0;
 	const char *rev = "HEAD";
 	unsigned char sha1[20], *orig = NULL, sha1_orig[20],
 				*old_orig = NULL, sha1_old_orig[20];
@@ -210,6 +242,10 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 			quiet = 1;
 			i++;
 		}
+		else if (!strcmp(argv[i], "-i")) {
+			confirm = 1;
+			i++;
+		}
 		else
 			break;
 	}
@@ -251,7 +287,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 		if (is_merge() || unmerged_files())
 			die("Cannot do a soft reset in the middle of a merge.");
 	}
-	else if (reset_index_file(sha1, (reset_type == HARD)))
+	else if (reset_index_file(sha1, (reset_type == HARD), confirm))
 		die("Could not reset index file to revision '%s'.", rev);
 
 	/* Any resets update HEAD to the head being switched to,
-- 
1.5.4-rc0.GIT

^ permalink raw reply related

* Re: [PATCH] Allow commit (and tag) messages to be edited when $EDITOR has arguments
From: Thomas Harning @ 2007-12-16  1:41 UTC (permalink / raw)
  To: Steven Grimm; +Cc: git
In-Reply-To: <20071216011201.GA10867@midwinter.com>

Steven Grimm wrote:
> Users who do EDITOR="/usr/bin/emacs -nw" or similar were left unable to
> edit commit messages once commit became a builtin, because the editor
> launch code assumed that $EDITOR was a single pathname.
>   
I see one problem with this code...  If you use quotes (single or 
double) then this will break it.  I suppose this isn't a major issue 
usually, but if not fixed should be documented.  One case that jumps out 
of my head is an executable path with spaces (quite stupid-and-ugly, but 
possible).

^ permalink raw reply

* Re: [PATCH] Allow commit (and tag) messages to be edited when $EDITOR has arguments
From: Johannes Schindelin @ 2007-12-16  1:41 UTC (permalink / raw)
  To: Steven Grimm; +Cc: git
In-Reply-To: <20071216011201.GA10867@midwinter.com>

Hi,

On Sat, 15 Dec 2007, Steven Grimm wrote:

> 	Looked around but didn't see an existing "build a char* array
> 	out of a delimited string" function in the git source; if one
> 	exists, of course it should be used instead of my pair of loops
> 	here.

Did you look for split_cmdline() in git.c?  IMHO it should move to 
run-command.c and be made public.

Thanks,
Dscho

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox