Git development
 help / color / mirror / Atom feed
* [PATCH v7 3/6] fast-import: add feature command
From: Sverre Rabbelier @ 2009-09-06 14:35 UTC (permalink / raw)
  To: Junio C Hamano, Shawn O. Pearce, Johannes Schindelin, Git List,
	Ian Clatworthy <ian.cla
  Cc: Sverre Rabbelier
In-Reply-To: <1252247748-14507-3-git-send-email-srabbelier@gmail.com>

This allows the fronted to require a specific feature to be supported
by the frontend, or abort.

Also add support for the first feature, date-format=.

Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
---

  Unchanged since v6.

 Documentation/git-fast-import.txt |   16 ++++++++++++++++
 fast-import.c                     |   13 +++++++++++++
 2 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
index c2f483a..1e293f2 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -303,6 +303,10 @@ and control the current import process.  More detailed discussion
 	standard output.  This command is optional and is not needed
 	to perform an import.
 
+`feature`::
+	Require that fast-import supports the specified feature, or
+	abort if it does not.
+
 `commit`
 ~~~~~~~~
 Create or update a branch with a new commit, recording one logical
@@ -813,6 +817,18 @@ Placing a `progress` command immediately after a `checkpoint` will
 inform the reader when the `checkpoint` has been completed and it
 can safely access the refs that fast-import updated.
 
+`feature`
+~~~~~~~~~
+Require that fast-import supports the specified feature, or abort if
+it does not.
+
+....
+	'feature' SP <feature> LF
+....
+
+The <feature> part of the command may be any string matching
+[a-zA-Z-] and should be understood by a version of fast-import.
+
 Crash Reports
 -------------
 If fast-import is supplied invalid input it will terminate with a
diff --git a/fast-import.c b/fast-import.c
index 812fcf0..9bf06a4 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2450,6 +2450,17 @@ static void parse_one_option(const char *option)
 	}
 }
 
+static void parse_feature(void)
+{
+	char *feature = command_buf.buf + 8;
+
+	if (!prefixcmp(feature, "date-format=")) {
+		option_date_format(feature + 12);
+	} else {
+		die("This version of fast-import does not support feature %s.", feature);
+	}
+}
+
 static int git_pack_config(const char *k, const char *v, void *cb)
 {
 	if (!strcmp(k, "pack.depth")) {
@@ -2526,6 +2537,8 @@ int main(int argc, const char **argv)
 			parse_checkpoint();
 		else if (!prefixcmp(command_buf.buf, "progress "))
 			parse_progress();
+		else if (!prefixcmp(command_buf.buf, "feature "))
+			parse_feature();
 		else
 			die("Unsupported command: %s", command_buf.buf);
 	}
-- 
1.6.4.16.g72c66.dirty

^ permalink raw reply related

* [PATCH v7 4/6] fast-import: test the new feature command
From: Sverre Rabbelier @ 2009-09-06 14:35 UTC (permalink / raw)
  To: Junio C Hamano, Shawn O. Pearce, Johannes Schindelin, Git List,
	Ian Clatworthy <ian.cla
  Cc: Sverre Rabbelier
In-Reply-To: <1252247748-14507-4-git-send-email-srabbelier@gmail.com>

Test that an unknown feature causes fast-import to abort, and that a
known feature is accepted.

Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
---

  Unchanged since v6.

 t/t9300-fast-import.sh |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 821be7c..564ed6b 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -1088,4 +1088,24 @@ INPUT_END
 test_expect_success 'P: fail on blob mark in gitlink' '
     test_must_fail git fast-import <input'
 
+###
+### series R (feature)
+###
+
+cat >input <<EOF
+feature no-such-feature-exists
+EOF
+
+test_expect_success 'R: abort on unsupported feature' '
+	test_must_fail git fast-import <input
+'
+
+cat >input <<EOF
+feature date-format=now
+EOF
+
+test_expect_success 'R: supported feature is accepted' '
+	git fast-import <input
+'
+
 test_done
-- 
1.6.4.16.g72c66.dirty

^ permalink raw reply related

* [PATCH v7 1/6] fast-import: put option parsing code in separate functions
From: Sverre Rabbelier @ 2009-09-06 14:35 UTC (permalink / raw)
  To: Junio C Hamano, Shawn O. Pearce, Johannes Schindelin, Git List,
	Ian Clatworthy <ian.cla
  Cc: Sverre Rabbelier
In-Reply-To: <1252247748-14507-1-git-send-email-srabbelier@gmail.com>

Putting the options in their own functions increases readability of
the option parsing block and makes it easier to reuse the option
parsing code later on.

Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
---

  Unchanged again.

 fast-import.c |  115 +++++++++++++++++++++++++++++++++++++--------------------
 1 files changed, 75 insertions(+), 40 deletions(-)

diff --git a/fast-import.c b/fast-import.c
index 7ef9865..b904f20 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -291,6 +291,7 @@ static unsigned long branch_count;
 static unsigned long branch_load_count;
 static int failure;
 static FILE *pack_edges;
+static unsigned int show_stats = 1;
 
 /* Memory pools */
 static size_t mem_pool_alloc = 2*1024*1024 - sizeof(struct mem_pool);
@@ -2337,7 +2338,7 @@ static void parse_progress(void)
 	skip_optional_lf();
 }
 
-static void import_marks(const char *input_file)
+static void option_import_marks(const char *input_file)
 {
 	char line[512];
 	FILE *f = fopen(input_file, "r");
@@ -2372,6 +2373,76 @@ static void import_marks(const char *input_file)
 	fclose(f);
 }
 
+static void option_date_format(const char *fmt)
+{
+	if (!strcmp(fmt, "raw"))
+		whenspec = WHENSPEC_RAW;
+	else if (!strcmp(fmt, "rfc2822"))
+		whenspec = WHENSPEC_RFC2822;
+	else if (!strcmp(fmt, "now"))
+		whenspec = WHENSPEC_NOW;
+	else
+		die("unknown --date-format argument %s", fmt);
+}
+
+static void option_max_pack_size(const char *packsize)
+{
+	max_packsize = strtoumax(packsize, NULL, 0) * 1024 * 1024;
+}
+
+static void option_depth(const char *depth)
+{
+	max_depth = strtoul(depth, NULL, 0);
+	if (max_depth > MAX_DEPTH)
+		die("--depth cannot exceed %u", MAX_DEPTH);
+}
+
+static void option_active_branches(const char *branches)
+{
+	max_active_branches = strtoul(branches, NULL, 0);
+}
+
+static void option_export_marks(const char *marks)
+{
+	mark_file = xstrdup(marks);
+}
+
+static void option_export_pack_edges(const char *edges)
+{
+	if (pack_edges)
+		fclose(pack_edges);
+	pack_edges = fopen(edges, "a");
+	if (!pack_edges)
+		die_errno("Cannot open '%s'", edges);
+}
+
+static void parse_one_option(const char *option)
+{
+	if (!prefixcmp(option, "date-format=")) {
+		option_date_format(option + 12);
+	} else if (!prefixcmp(option, "max-pack-size=")) {
+		option_max_pack_size(option + 14);
+	} else if (!prefixcmp(option, "depth=")) {
+		option_depth(option + 6);
+	} else if (!prefixcmp(option, "active-branches=")) {
+		option_active_branches(option + 16);
+	} else if (!prefixcmp(option, "import-marks=")) {
+		option_import_marks(option + 13);
+	} else if (!prefixcmp(option, "export-marks=")) {
+		option_export_marks(option + 13);
+	} else if (!prefixcmp(option, "export-pack-edges=")) {
+		option_export_pack_edges(option + 18);
+	} else if (!prefixcmp(option, "force")) {
+		force_update = 1;
+	} else if (!prefixcmp(option, "quiet")) {
+		show_stats = 0;
+	} else if (!prefixcmp(option, "stats")) {
+		show_stats = 1;
+	} else {
+		die("Unsupported option: %s", option);
+	}
+}
+
 static int git_pack_config(const char *k, const char *v, void *cb)
 {
 	if (!strcmp(k, "pack.depth")) {
@@ -2398,7 +2469,7 @@ static const char fast_import_usage[] =
 
 int main(int argc, const char **argv)
 {
-	unsigned int i, show_stats = 1;
+	unsigned int i;
 
 	git_extract_argv0_path(argv[0]);
 
@@ -2419,44 +2490,8 @@ int main(int argc, const char **argv)
 
 		if (*a != '-' || !strcmp(a, "--"))
 			break;
-		else if (!prefixcmp(a, "--date-format=")) {
-			const char *fmt = a + 14;
-			if (!strcmp(fmt, "raw"))
-				whenspec = WHENSPEC_RAW;
-			else if (!strcmp(fmt, "rfc2822"))
-				whenspec = WHENSPEC_RFC2822;
-			else if (!strcmp(fmt, "now"))
-				whenspec = WHENSPEC_NOW;
-			else
-				die("unknown --date-format argument %s", fmt);
-		}
-		else if (!prefixcmp(a, "--max-pack-size="))
-			max_packsize = strtoumax(a + 16, NULL, 0) * 1024 * 1024;
-		else if (!prefixcmp(a, "--depth=")) {
-			max_depth = strtoul(a + 8, NULL, 0);
-			if (max_depth > MAX_DEPTH)
-				die("--depth cannot exceed %u", MAX_DEPTH);
-		}
-		else if (!prefixcmp(a, "--active-branches="))
-			max_active_branches = strtoul(a + 18, NULL, 0);
-		else if (!prefixcmp(a, "--import-marks="))
-			import_marks(a + 15);
-		else if (!prefixcmp(a, "--export-marks="))
-			mark_file = a + 15;
-		else if (!prefixcmp(a, "--export-pack-edges=")) {
-			if (pack_edges)
-				fclose(pack_edges);
-			pack_edges = fopen(a + 20, "a");
-			if (!pack_edges)
-				die_errno("Cannot open '%s'", a + 20);
-		} else if (!strcmp(a, "--force"))
-			force_update = 1;
-		else if (!strcmp(a, "--quiet"))
-			show_stats = 0;
-		else if (!strcmp(a, "--stats"))
-			show_stats = 1;
-		else
-			die("unknown option %s", a);
+
+		parse_one_option(a + 2);
 	}
 	if (i != argc)
 		usage(fast_import_usage);
-- 
1.6.4.16.g72c66.dirty

^ permalink raw reply related

* [PATCH v7 2/6] fast-import: put marks reading in it's own function
From: Sverre Rabbelier @ 2009-09-06 14:35 UTC (permalink / raw)
  To: Junio C Hamano, Shawn O. Pearce, Johannes Schindelin, Git List,
	Ian Clatworthy <ian.cla
  Cc: Sverre Rabbelier
In-Reply-To: <1252247748-14507-2-git-send-email-srabbelier@gmail.com>

All options do nothing but set settings, with the exception of the
--input-marks option. Delay the reading of the marks file till after
all options have been parsed.

Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
---

  Unchanged again.

 fast-import.c |   73 ++++++++++++++++++++++++++++++++-------------------------
 1 files changed, 41 insertions(+), 32 deletions(-)

diff --git a/fast-import.c b/fast-import.c
index b904f20..812fcf0 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -315,6 +315,7 @@ static struct object_entry_pool *blocks;
 static struct object_entry *object_table[1 << 16];
 static struct mark_set *marks;
 static const char *mark_file;
+static const char *input_file;
 
 /* Our last blob */
 static struct last_object last_blob = { STRBUF_INIT, 0, 0, 0 };
@@ -1643,6 +1644,42 @@ static void dump_marks(void)
 	}
 }
 
+static void read_marks(void)
+{
+	char line[512];
+	FILE *f = fopen(input_file, "r");
+	if (!f)
+		die_errno("cannot read '%s'", input_file);
+	while (fgets(line, sizeof(line), f)) {
+		uintmax_t mark;
+		char *end;
+		unsigned char sha1[20];
+		struct object_entry *e;
+
+		end = strchr(line, '\n');
+		if (line[0] != ':' || !end)
+			die("corrupt mark line: %s", line);
+		*end = 0;
+		mark = strtoumax(line + 1, &end, 10);
+		if (!mark || end == line + 1
+			|| *end != ' ' || get_sha1(end + 1, sha1))
+			die("corrupt mark line: %s", line);
+		e = find_object(sha1);
+		if (!e) {
+			enum object_type type = sha1_object_info(sha1, NULL);
+			if (type < 0)
+				die("object not found: %s", sha1_to_hex(sha1));
+			e = insert_object(sha1);
+			e->type = type;
+			e->pack_id = MAX_PACK_ID;
+			e->offset = 1; /* just not zero! */
+		}
+		insert_mark(mark, e);
+	}
+	fclose(f);
+}
+
+
 static int read_next_command(void)
 {
 	static int stdin_eof = 0;
@@ -2338,39 +2375,9 @@ static void parse_progress(void)
 	skip_optional_lf();
 }
 
-static void option_import_marks(const char *input_file)
+static void option_import_marks(const char *marks)
 {
-	char line[512];
-	FILE *f = fopen(input_file, "r");
-	if (!f)
-		die_errno("cannot read '%s'", input_file);
-	while (fgets(line, sizeof(line), f)) {
-		uintmax_t mark;
-		char *end;
-		unsigned char sha1[20];
-		struct object_entry *e;
-
-		end = strchr(line, '\n');
-		if (line[0] != ':' || !end)
-			die("corrupt mark line: %s", line);
-		*end = 0;
-		mark = strtoumax(line + 1, &end, 10);
-		if (!mark || end == line + 1
-			|| *end != ' ' || get_sha1(end + 1, sha1))
-			die("corrupt mark line: %s", line);
-		e = find_object(sha1);
-		if (!e) {
-			enum object_type type = sha1_object_info(sha1, NULL);
-			if (type < 0)
-				die("object not found: %s", sha1_to_hex(sha1));
-			e = insert_object(sha1);
-			e->type = type;
-			e->pack_id = MAX_PACK_ID;
-			e->offset = 1; /* just not zero! */
-		}
-		insert_mark(mark, e);
-	}
-	fclose(f);
+	input_file = xstrdup(marks);
 }
 
 static void option_date_format(const char *fmt)
@@ -2495,6 +2502,8 @@ int main(int argc, const char **argv)
 	}
 	if (i != argc)
 		usage(fast_import_usage);
+	if (input_file)
+		read_marks();
 
 	rc_free = pool_alloc(cmd_save * sizeof(*rc_free));
 	for (i = 0; i < (cmd_save - 1); i++)
-- 
1.6.4.16.g72c66.dirty

^ permalink raw reply related

* [PATCH v7 0/6] fast-import: add new feature and option command
From: Sverre Rabbelier @ 2009-09-06 14:35 UTC (permalink / raw)
  To: Junio C Hamano, Shawn O. Pearce, Johannes Schindelin, Git List,
	Ian Clatworthy <ian.cla

Only changes are in 5/6 and 6/6, as suggested by Junio we now ignore
git options that we do not recognise. If desired we could add a
feature command for each option so that the frontend can make it
required with 'feature git-option-marks'.

Sverre Rabbelier (6):
      fast-import: put option parsing code in separate functions
      fast-import: put marks reading in it's own function
      fast-import: add feature command
      fast-import: test the new feature command
      fast-import: add option command
      fast-import: test the new option command

 Documentation/git-fast-import.txt |   40 ++++++
 fast-import.c                     |  264 ++++++++++++++++++++++++++-----------
 t/t9300-fast-import.sh            |  107 +++++++++++++++
 3 files changed, 334 insertions(+), 77 deletions(-)

^ permalink raw reply

* Re: Use case I don't know how to address
From: Alan Chandler @ 2009-09-06 12:50 UTC (permalink / raw)
  Cc: git
In-Reply-To: <200909051923.28831.j6t@kdbg.org>

Johannes Sixt wrote:
> On Samstag, 5. September 2009, Alan Chandler wrote:
>> The problem comes when I want to now merge back further work that I did
>> on the master branch (the 5-6 transition) to the fan club site
>>
>>
>>         2' - 2a - 3' - 4' ----------------- 6' SITE
>>        /         /    /                    /
>> 1 -  2  ------ 3  - 4  ------------6'''- 6a TEST
>>                       \            /
>>                         5  ------ 6  MASTER
>>                          \         \
>>                            5''- 5a- 6'' DEMO
>>
>>
>> What will happen is the changes made in 4->5 will get applied to the
>> (now) Test branch as part of the 6->6'' merge, and I will be left having
>> to add a new commit, 6a, to undo them all again.  Given this is likely
>> to be quite a substantial change I want to try and avoid it if possible.
>>
>> Is there any way I could use git to remember the 4->5 transition,
>> reverse it and apply it back to the Test branch before hand.
> 
> Basically, your mistake was to rename master to test and continue development 
> on the demo branch. So what you should do instead is this:
> 
>         2' - 2a - 3' - 4' ------ 6' SITE
>        /         /    /         /
> 1 -  2  ------ 3  - 4  ------- 6  MASTER
>                       \         \
>                         5 - 5a - 6'' DEMO
> 


I understand what you are saying.  I think I need to consider a slight 
change to it however.  Master is currently the branch that I push to my 
public repository and that should ideally have my NON FAN CLUB skinning

So actually I think I am like this

          2' - 2a - 3' - 4' ------ 6' SITE
         /         /    /         /
  1 -  2  ------ 3  - 4  ------- 6  MASTER
                        \         \
                          5 - -- - 6'' PUBLIC REPO
                           \        \
                            5'''-5a -6''' DEMO






-- 
Alan Chandler
http://www.chandlerfamily.org.uk

^ permalink raw reply

* Re: [BUG] 'add -u' doesn't work from untracked subdir
From: Matthieu Moy @ 2009-09-06 12:32 UTC (permalink / raw)
  To: Clemens Buchacher; +Cc: Junio C Hamano, Jeff King, SZEDER Gábor, git
In-Reply-To: <20090905084641.GA24865@darc.dnsalias.org>

Clemens Buchacher <drizzd@aon.at> writes:

> On Sat, Sep 05, 2009 at 12:02:35AM -0700, Junio C Hamano wrote:
>
>> I personally find "add -u" that defaults to the current directory more
>> natural than always going to the root; same preference for "grep".
>> Besides, "add -u subdir" must add subdir relative to the cwd, without
>> going to the root.  Why should "add -u" sans argument behave drastically
>> differently?
>
> Sorry for stating the obvious here, but the following commands affect the
> entire repository, even though they limit themselves to the current
> directory, if passed a '.'.
>
> 	git commit
> 	git log
> 	git diff
> 	git checkout
> 	git reset

You have to add "git add -e", "git add -i" and "git add -p" here.

I completely agree that "git add -u" should have been a full-tree
oriented command, just like other "git add" variants and other Git
commands, from the beginning.

Now, I'm unconfortable with both a behavior change and a config
option. Someone used to the cwd-limited behavior typing "git add -u"
on a machine configured to git the full-tree behavior could be really
annoyed (not even mentionning scripts).

I think it has already been proposed to introduce "git add -a" doing
what "git add -u" do, but for the full tree. The "-a" option here
being analogous to the one of "git commit": roughly, "git add -a; git
commit" would be equivalent to "git commit -a". This would allow a
long deprecation period for "git add -u". I find the proposal
sensible, but IIRC it has already been rejected.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply

* Re: [RFC/PATCH 0/4] make helpful messages optional
From: Matthieu Moy @ 2009-09-06 11:53 UTC (permalink / raw)
  To: Jeff King; +Cc: Nanako Shiraishi, Junio C Hamano, Teemu Likonen, git
In-Reply-To: <20090906064454.GA1643@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

>   [1/4]: push: fix english in non-fast-forward message
>   [2/4]: push: re-flow non-fast-forward message

Sounds OK to me.

>   [3/4]: push: make non-fast-forward help message configurable
>   [4/4]: status: make "how to stage" messages optional

I didn't review the code in details, but I like the direction where
it's going. I'm not sure what's the best name for the configuration
option, but probably your proposal of "message" is good because it can
encompass many different cases (very detailed error messages, advices,
informative messages, ...).

And BTW, as the initial author of the push help message, I fully agree
that it is unnecessarily eating 3 lines of my terminal ;-). The
message is clearly targeted to newbies (but I do think it's very
helpfull to them).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply

* Re: [PATCH 3/4] push: make non-fast-forward help message configurable
From: Sverre Rabbelier @ 2009-09-06 11:30 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jeff King, Nanako Shiraishi, Matthieu Moy, Teemu Likonen, git
In-Reply-To: <7vzl98fr22.fsf@alter.siamese.dyndns.org>

Heya,

On Sun, Sep 6, 2009 at 09:52, Junio C Hamano<gitster@pobox.com> wrote:
> Resetting is done because I want to build an alternate history starting
> from an earlier point, and when I am done, I may want to feed this to "git
> diff" or whatever to sanity check the result, without having to go through
> the reflog.

I agree, the 'you are now at ...' is not very helpful, I just TOLD you
where I want to go to! However, 'you were at ....' is also not
optimal, since what I really want to know is where I was at "plus any
old data I had" (going back to the 'git reset --hard' should create a
snapshot of the current content before doing it's resetting'). I would
definitely prefer 'you were at' over 'you are now at' though.

WRT the 'none of your business', it sounds like 'I [already] know
[that]' might be approriate?

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* [PATCH] githooks.txt: put hooks into subsections
From: Bert Wesarg @ 2009-09-06 10:22 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Christian Couder, git, Bert Wesarg

All hooks are currently in its own section. Which may confuse users,
because the section name serves as the hook file name and sections are
all caps for man pages. Putting them into a new HOOKS section and each
hook into a subsection keeps the case to lower case.

Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
---
 Documentation/githooks.txt |   33 ++++++++++++++++++---------------
 1 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index 1c73673..acc408d 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -26,8 +26,11 @@ executable by default.
 
 This document describes the currently defined hooks.
 
+HOOKS
+-----
+
 applypatch-msg
---------------
+~~~~~~~~~~~~~~
 
 This hook is invoked by 'git-am' script.  It takes a single
 parameter, the name of the file that holds the proposed commit
@@ -43,7 +46,7 @@ The default 'applypatch-msg' hook, when enabled, runs the
 'commit-msg' hook, if the latter is enabled.
 
 pre-applypatch
---------------
+~~~~~~~~~~~~~~
 
 This hook is invoked by 'git-am'.  It takes no parameter, and is
 invoked after the patch is applied, but before a commit is made.
@@ -58,7 +61,7 @@ The default 'pre-applypatch' hook, when enabled, runs the
 'pre-commit' hook, if the latter is enabled.
 
 post-applypatch
----------------
+~~~~~~~~~~~~~~~
 
 This hook is invoked by 'git-am'.  It takes no parameter,
 and is invoked after the patch is applied and a commit is made.
@@ -67,7 +70,7 @@ This hook is meant primarily for notification, and cannot affect
 the outcome of 'git-am'.
 
 pre-commit
-----------
+~~~~~~~~~~
 
 This hook is invoked by 'git-commit', and can be bypassed
 with `\--no-verify` option.  It takes no parameter, and is
@@ -84,7 +87,7 @@ variable `GIT_EDITOR=:` if the command will not bring up an editor
 to modify the commit message.
 
 prepare-commit-msg
-------------------
+~~~~~~~~~~~~~~~~~~
 
 This hook is invoked by 'git-commit' right after preparing the
 default log message, and before the editor is started.
@@ -109,7 +112,7 @@ The sample `prepare-commit-msg` hook that comes with git comments
 out the `Conflicts:` part of a merge's commit message.
 
 commit-msg
-----------
+~~~~~~~~~~
 
 This hook is invoked by 'git-commit', and can be bypassed
 with `\--no-verify` option.  It takes a single parameter, the
@@ -126,7 +129,7 @@ The default 'commit-msg' hook, when enabled, detects duplicate
 "Signed-off-by" lines, and aborts the commit if one is found.
 
 post-commit
------------
+~~~~~~~~~~~
 
 This hook is invoked by 'git-commit'.  It takes no
 parameter, and is invoked after a commit is made.
@@ -135,14 +138,14 @@ This hook is meant primarily for notification, and cannot affect
 the outcome of 'git-commit'.
 
 pre-rebase
-----------
+~~~~~~~~~~
 
 This hook is called by 'git-rebase' and can be used to prevent a branch
 from getting rebased.
 
 
 post-checkout
------------
+~~~~~~~~~~~~~
 
 This hook is invoked when a 'git-checkout' is run after having updated the
 worktree.  The hook is given three parameters: the ref of the previous HEAD,
@@ -160,7 +163,7 @@ differences from the previous HEAD if different, or set working dir metadata
 properties.
 
 post-merge
------------
+~~~~~~~~~~
 
 This hook is invoked by 'git-merge', which happens when a 'git-pull'
 is done on a local repository.  The hook takes a single parameter, a status
@@ -175,7 +178,7 @@ for an example of how to do this.
 
 [[pre-receive]]
 pre-receive
------------
+~~~~~~~~~~~
 
 This hook is invoked by 'git-receive-pack' on the remote repository,
 which happens when a 'git-push' is done on a local repository.
@@ -204,7 +207,7 @@ for the user.
 
 [[update]]
 update
-------
+~~~~~~
 
 This hook is invoked by 'git-receive-pack' on the remote repository,
 which happens when a 'git-push' is done on a local repository.
@@ -247,7 +250,7 @@ unannotated tags to be pushed.
 
 [[post-receive]]
 post-receive
-------------
+~~~~~~~~~~~~
 
 This hook is invoked by 'git-receive-pack' on the remote repository,
 which happens when a 'git-push' is done on a local repository.
@@ -277,7 +280,7 @@ emails.
 
 [[post-update]]
 post-update
------------
+~~~~~~~~~~~
 
 This hook is invoked by 'git-receive-pack' on the remote repository,
 which happens when a 'git-push' is done on a local repository.
@@ -308,7 +311,7 @@ Both standard output and standard error output are forwarded to
 for the user.
 
 pre-auto-gc
------------
+~~~~~~~~~~~
 
 This hook is invoked by 'git-gc --auto'. It takes no parameter, and
 exiting with non-zero status from this script causes the 'git-gc --auto'
-- 
1.6.4.GIT

^ permalink raw reply related

* Re: Strange merge failure (would be overwritten by merge / cannot merge)
From: Junio C Hamano @ 2009-09-06  8:21 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git
In-Reply-To: <7vzl98vrmt.fsf@alter.siamese.dyndns.org>

I was looking at this loop in traverse_trees() from tree-walk.c

	for (i = 0; i < n; i++) {
		if (!t[i].size)
			continue;
		entry_extract(t+i, entry+i);
		if (last >= 0) {
			int cmp = entry_compare(entry+i, entry+last);

			/*
			 * Is the new name bigger than the old one?
			 * Ignore it
			 */
			if (cmp > 0)
				continue;
			/*
			 * Is the new name smaller than the old one?
			 * Ignore all old ones
			 */
			if (cmp < 0)
				mask = 0;
		}
		mask |= 1ul << i;
		if (S_ISDIR(entry[i].mode))
			dirmask |= 1ul << i;
		last = i;
	}

The logic to update last breaks down while merging these three trees:

    Tree #1: tree "t"
    Tree #2: blob "t"
             blob "t-f"
    Tree #3: blob "t-f"
             tree "t"

When looking at Tree #3, "last" points at "t" (blob) from Tree #2
and we decide "t-f" comes later than that to ignore it because "cmp > 0".

We instead somehow need to look ahead and find "t", while remembering to
revisit "t-f" from it in the later rounds (of an outer loop that has this
loop inside).

Also the second comparison to update last breaks down while merging these:

    Tree #1: blob "t-f" 
             tree "t"
    Tree #2: blob "t"
    Tree #3: blob "t"
             blob "t-f"

When looking at Tree #2, "last" points at "t-f" (blob) from Tree #1 and we
decide to use "t" because it sorts earlier.  We will match "t" from Tree #3
but we somehow need to go back to Tree #1 and look beyond "t-f" to find
the matchig "t" from there as well, while remembering that we need to
revisit "t-f" from it in the later rounds..

I am thinking about changing the function this way.

 (0) take an optional "candidate for the earliest name" from the caller,
     in a new member in traverse_info structure. unpack_callback() would
     give the path of the index entry it is looking at after stripping the
     leading paths as necessary.

 (1) find the "earliest name" N among the given trees (and the optional
     additional candidate from the above).  The "earliest" is computed by
     comparing names as if everything were a blob;

 (2) for each tree t[i], if the current entry entry[i] compares
     differently with N between the cases where the N were a blob and
     where N were a tree, and if entry[i] is a blob, a tree with the name
     N might be hidden behind it.  remember the current position so we can
     come back, and scan forward to find such tree (this does not have to
     run to the end of the tree), and if we do find one, then instead of
     saying "we do not have an entry with that name", use that tree in
     entry[i].  This will collect entries with name N from all trees in
     the entry[] array;

 (3) make the callback as usual; unpack_callback(), after processing this
     round with entries with name N, may or may not advance o->pos but if
     it did so, it would also update the "candidate for the earliest name"
     field passed back in the traverse_info structure to prepare for the
     next round.

 (4) traverse_trees() will go back to step (0).

The tricky part is step (1) and the latter half of step (2) to skip,
rewind, and avoid re-processing what was processed already after
rewinding, and the progress is much slower than I would like.

^ permalink raw reply

* Re: [PATCH/RFC 3/6] status: refactor short-mode printing to its own function
From: Junio C Hamano @ 2009-09-06  8:05 UTC (permalink / raw)
  To: Jeff King; +Cc: David Aguilar, Junio C Hamano, Johannes Sixt, bill lam, git
In-Reply-To: <20090905085348.GC13157@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> I am tempted to move all of the short-printing code to its own file, and
> move "cmd_status" to its own builtin-status.c, as well. I don't know if
> that is a cleanup that makes sense to others, as well, or if it is too
> much churn for too little good.

Earlier in the series when "git commit --dry-run" and "git status" still
were the same thing, I checked if the above was feasible and then decided
against it, because they needed to share the option parsing and the index
preparation, and exporting these functions inherently internal to "git
commit" only to use them in "git status" did not make much sense.

But once "git status" does not have anything to do with "git commit", I
think such a separation would become much more sensible.

^ permalink raw reply

* Re: [PATCH/RFC 2/6] docs: note that status configuration affects only long format
From: Junio C Hamano @ 2009-09-06  8:04 UTC (permalink / raw)
  To: Jeff King; +Cc: David Aguilar, Johannes Sixt, bill lam, git
In-Reply-To: <20090905085218.GB13157@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> Combined with the --short/--porcelain distinction introduced later in
> the series, should short perhaps respect status.relativePaths and
> status.submoduleSummary?

I think that makes sense. 

^ permalink raw reply

* Re: Use case I don't know how to address
From: Junio C Hamano @ 2009-09-06  8:03 UTC (permalink / raw)
  To: Alan Chandler; +Cc: git
In-Reply-To: <4AA20CEC.8060408@chandlerfamily.org.uk>

Alan Chandler <alan@chandlerfamily.org.uk> writes:

>        2' - 2a - 3' - 4' ----------------- 6' SITE
>       /         /    /                    /
> 1 -  2  ------ 3  - 4  ------------6'''- 6a TEST
>                      \            /
>                        5  ------ 6  MASTER
>                         \         \
>                           5''- 5a- 6'' DEMO
>
>
> What will happen is the changes made in 4->5 will get applied to the
> (now) Test branch as part of the 6->6'' merge, and I will be left
> having to add a new commit, 6a, to undo them all again.  Given this is
> likely to be quite a substantial change I want to try and avoid it if
> possible.

I presume 6'''-6a has the revert of 4-5?  If so, the next merge should
work just fine.

You have arranged TEST->SITE transition correctly to limiting the SITE
customization to 2a and never merging SITE back to TEST, so we can ignore
SITE branch altogether from now on.  Similarly we can ignore DEMO branch,
since its customization is limited to 5a and it never gets merged back to
MASTER.

    1 -  2  ------ 3  - 4  ------------6'''- 6a-----7a??? TEST
                         \            /            /    
                           5  ------ 6------------7 MASTER

Now, 6-7 is a new feature built on MASTER.  What would happen when it is
merged to TEST to produce 7a?

The merge base for this merge is 6, and since that commit to the tip of
the TEST branch 6a, there is a "skin from vanilla to FanClub" change and
nothing else.  On the other hand, since the merge base to the tip of the
MASTER branch 7, there is a "feature enhancement" but no skin related
changes.

So bog-standard three-way merge should say:

 - One branch, MASTER, added these features, but TEST branch did not do
   anything with these feature changes since the merge base.  We'll use
   the feature change done on MASTER.

 - The other branch, TEST, changed the skin from the merge base, but
   MASTER branch did not change any skinning.  We'll keep the skin change
   done on TEST.

And everything should be fine.

^ permalink raw reply

* Re: [PATCH 3/4] push: make non-fast-forward help message configurable
From: Junio C Hamano @ 2009-09-06  7:52 UTC (permalink / raw)
  To: Jeff King; +Cc: Nanako Shiraishi, Matthieu Moy, Teemu Likonen, git
In-Reply-To: <20090906072322.GA29949@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> Thinking on it more, I think "advice" is the right word. It is not about
> arbitrary messages; it is about particular messages which try to advise.
> You would never want this feature to cover messages that are
> informational about a particular state or action that has occurred. Only
> "maybe you should try this" messages.

Speaking of which, has anybody felt annoyed by this message?

    $ git reset --hard HEAD^^
    HEAD is now at 3fb9d58 Do not scramble password read from .cvspass

This is not "maybe you should try this", but I would consider that it
falls into the same "I see you are trying to be helpful, but I know what I
am doing, and you are stealing screen real estate from me without helping
me at all, thank you very much" category.

Besides, if you want to use "where you are" to your next command, you can
just say HEAD without knowing exactly where you are.

It might be slightly more useful if the message said this instead:

    $ git reset --hard HEAD^^
    HEAD was at d1fe49f push: re-flow non-fast-forward message

Resetting is done because I want to build an alternate history starting
from an earlier point, and when I am done, I may want to feed this to "git
diff" or whatever to sanity check the result, without having to go through
the reflog.

^ permalink raw reply

* Re: [PATCH 3/4] push: make non-fast-forward help message configurable
From: Jeff King @ 2009-09-06  7:32 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nanako Shiraishi, Matthieu Moy, Teemu Likonen, git
In-Reply-To: <7vocpoh6nt.fsf@alter.siamese.dyndns.org>

On Sun, Sep 06, 2009 at 12:30:14AM -0700, Junio C Hamano wrote:

> And no need to hurry.  I've decided to queue 1 and 2 to 'master' (was
> Nana's reroll of Matthieu's patch on 'maint'???) and 3 and 4 merged to

It was on 'maint', which surprised me, too. But v1.6.4.1 got it.

-Peff

^ permalink raw reply

* Re: [PATCH 3/4] push: make non-fast-forward help message configurable
From: Junio C Hamano @ 2009-09-06  7:30 UTC (permalink / raw)
  To: Jeff King; +Cc: Nanako Shiraishi, Matthieu Moy, Teemu Likonen, git
In-Reply-To: <20090906072322.GA29949@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> I'll re-roll 3 and 4 based on that, but I will wait a bit to see any
> more comments. Probably you should consider patches 1 and 2 as a
> potential series for 'maint', and 3 and 4 should be spun off into their
> own series (they really only rely textually on the first two).

Yeah, thanks.

And no need to hurry.  I've decided to queue 1 and 2 to 'master' (was
Nana's reroll of Matthieu's patch on 'maint'???) and 3 and 4 merged to
'pu', but I seem to be having a problem ssh'ing into k.org, and it appears
that no pushout will happen tonight.

^ permalink raw reply

* Re: [PATCH 3/4] push: make non-fast-forward help message configurable
From: Jeff King @ 2009-09-06  7:23 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nanako Shiraishi, Matthieu Moy, Teemu Likonen, git
In-Reply-To: <7v8wgsk0rw.fsf@alter.siamese.dyndns.org>

On Sun, Sep 06, 2009 at 12:09:07AM -0700, Junio C Hamano wrote:

> I hate to start off with a digression, but one expression from Japanese I
> have long been frustrated with, because I cannot find a good counterpart
> in English, is "大きなお世話".
> 
> J-E dictionaries often translate this phrase as "none of your business",
> and it indeed is meant to be thrown at somebody who gives an unsolicited
> and an unwanted advice to you, but at the same time it strongly implies
> that the reason why such an advice is unwanted is because you very well
> knows the issue and does not _need_ (not want) the help on the topic at
> all.  It is not about _who_ gives the unwanted advice (which I think the
> expression "none of YOUR business" talks about---if the same advice came
> from somebody else, it might be appreciated), but it is all about on what
> topic the advice is about, and you feel that you know about the topic very
> well yourself and do not need any advice from anybody.

The best related English phrase I can think of is that the person is a
"back seat driver". The phrase "armchair $X" also comes to mind (e.g.,
"armchair critic"), though that is usually to indicate that the armchair
critic is giving _bad_ advice, not being well-versed in the field they
are advising about. And I think you are not so much saying the advice is
wrong as much as it is unwanted because you already know it.

I can't think of a noun that describes the advice itself, though
(nagging? ;) ).

> I have always felt that many of the messages we have added since the
> "newbie friendliness" drive around 1.3.0 deserve to be labeled with the
> expression 大きなお世話.
> 
> Of course, "unsolicited-unneeded-advice.*" is too long as a variable name,
> but I personally would very much welcome changes along this line and I
> think "advice.*" is a good name for the category.

Git config files are utf8, right? You can always internationalize this
feature. :)

Thinking on it more, I think "advice" is the right word. It is not about
arbitrary messages; it is about particular messages which try to advise.
You would never want this feature to cover messages that are
informational about a particular state or action that has occurred. Only
"maybe you should try this" messages.

I'll re-roll 3 and 4 based on that, but I will wait a bit to see any
more comments. Probably you should consider patches 1 and 2 as a
potential series for 'maint', and 3 and 4 should be spun off into their
own series (they really only rely textually on the first two).

-Peff

^ permalink raw reply

* Re: [PATCH 3/4] push: make non-fast-forward help message configurable
From: Junio C Hamano @ 2009-09-06  7:09 UTC (permalink / raw)
  To: Jeff King; +Cc: Nanako Shiraishi, Matthieu Moy, Teemu Likonen, git
In-Reply-To: <20090906064816.GC28941@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> Probably "messages" is too vague a term to use in the code and config.
> Maybe "advice.*"?

I hate to start off with a digression, but one expression from Japanese I
have long been frustrated with, because I cannot find a good counterpart
in English, is "大きなお世話".

J-E dictionaries often translate this phrase as "none of your business",
and it indeed is meant to be thrown at somebody who gives an unsolicited
and an unwanted advice to you, but at the same time it strongly implies
that the reason why such an advice is unwanted is because you very well
knows the issue and does not _need_ (not want) the help on the topic at
all.  It is not about _who_ gives the unwanted advice (which I think the
expression "none of YOUR business" talks about---if the same advice came
from somebody else, it might be appreciated), but it is all about on what
topic the advice is about, and you feel that you know about the topic very
well yourself and do not need any advice from anybody.

Enough digression.

I have always felt that many of the messages we have added since the
"newbie friendliness" drive around 1.3.0 deserve to be labeled with the
expression 大きなお世話.

Of course, "unsolicited-unneeded-advice.*" is too long as a variable name,
but I personally would very much welcome changes along this line and I
think "advice.*" is a good name for the category.

^ permalink raw reply

* [PATCH 4/4] status: make "how to stage" messages optional
From: Jeff King @ 2009-09-06  6:50 UTC (permalink / raw)
  To: Nanako Shiraishi; +Cc: Junio C Hamano, Matthieu Moy, Teemu Likonen, git
In-Reply-To: <20090906064454.GA1643@coredump.intra.peff.net>

These messages are nice for new users, but experienced git
users know how to manipulate the index, and these messages
waste a lot of screen real estate.

Signed-off-by: Jeff King <peff@peff.net>
---
This also actually cuts out the blank line after the instructions, so:

  # Changed but not updated:
  #   (use "git add <file>..." to update what will be committed)
  #   (use "git checkout -- <file>..." to discard changes in working directory)
  #
  #       modified:   file
  #

becomes:

  # Changed but not updated:
  #       modified:   file
  #

Arguably the blank should be left in.

 Documentation/config.txt |    4 ++++
 messages.c               |    1 +
 messages.h               |    1 +
 wt-status.c              |    8 ++++++++
 4 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index ec308a6..cadbfc9 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1215,6 +1215,10 @@ message.*::
 	pushNonFastForward::
 		Help message shown when linkgit:git-push[1] refuses
 		non-fast-forward refs. Default: true.
+	statusAdvice::
+		Directions on how to stage/unstage/add shown in the
+		output of linkgit:git-status[1] and the template shown
+		when writing commit messages. Default: true.
 --
 
 pack.window::
diff --git a/messages.c b/messages.c
index 00fc196..d2785b2 100644
--- a/messages.c
+++ b/messages.c
@@ -3,6 +3,7 @@
 
 struct message_preference messages[] = {
 	{ "pushnonfastforward", 1 },
+	{ "statusadvice", 1 },
 };
 
 int git_default_message_config(const char *var, const char *value)
diff --git a/messages.h b/messages.h
index f175747..8edd08e 100644
--- a/messages.h
+++ b/messages.h
@@ -2,6 +2,7 @@
 #define MESSAGE_H
 
 #define MESSAGE_PUSH_NONFASTFORWARD 0
+#define MESSAGE_STATUS_ADVICE 1
 
 struct message_preference {
 	const char *name;
diff --git a/wt-status.c b/wt-status.c
index 85f3fcb..716d8cc 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -48,6 +48,8 @@ static void wt_status_print_unmerged_header(struct wt_status *s)
 {
 	const char *c = color(WT_STATUS_HEADER, s);
 	color_fprintf_ln(s->fp, c, "# Unmerged paths:");
+	if (!messages[MESSAGE_STATUS_ADVICE].preference)
+		return;
 	if (!s->is_initial)
 		color_fprintf_ln(s->fp, c, "#   (use \"git reset %s <file>...\" to unstage)", s->reference);
 	else
@@ -60,6 +62,8 @@ static void wt_status_print_cached_header(struct wt_status *s)
 {
 	const char *c = color(WT_STATUS_HEADER, s);
 	color_fprintf_ln(s->fp, c, "# Changes to be committed:");
+	if (!messages[MESSAGE_STATUS_ADVICE].preference)
+		return;
 	if (!s->is_initial) {
 		color_fprintf_ln(s->fp, c, "#   (use \"git reset %s <file>...\" to unstage)", s->reference);
 	} else {
@@ -73,6 +77,8 @@ static void wt_status_print_dirty_header(struct wt_status *s,
 {
 	const char *c = color(WT_STATUS_HEADER, s);
 	color_fprintf_ln(s->fp, c, "# Changed but not updated:");
+	if (!messages[MESSAGE_STATUS_ADVICE].preference)
+		return;
 	if (!has_deleted)
 		color_fprintf_ln(s->fp, c, "#   (use \"git add <file>...\" to update what will be committed)");
 	else
@@ -85,6 +91,8 @@ static void wt_status_print_untracked_header(struct wt_status *s)
 {
 	const char *c = color(WT_STATUS_HEADER, s);
 	color_fprintf_ln(s->fp, c, "# Untracked files:");
+	if (!messages[MESSAGE_STATUS_ADVICE].preference)
+		return;
 	color_fprintf_ln(s->fp, c, "#   (use \"git add <file>...\" to include in what will be committed)");
 	color_fprintf_ln(s->fp, c, "#");
 }
-- 
1.6.4.2.266.g981efc.dirty

^ permalink raw reply related

* [PATCH 3/4] push: make non-fast-forward help message configurable
From: Jeff King @ 2009-09-06  6:48 UTC (permalink / raw)
  To: Nanako Shiraishi; +Cc: Junio C Hamano, Matthieu Moy, Teemu Likonen, git
In-Reply-To: <20090906064454.GA1643@coredump.intra.peff.net>

This message is designed to help new users understand what
has happened when refs fail to push. However, it does not
help experienced users at all, and significantly clutters
the output, frequently dwarfing the regular status table and
making it harder to see.

This patch introduces a general configuration mechanism for
optional messages, with this push message as the first
example.

Signed-off-by: Jeff King <peff@peff.net>
---
Probably "messages" is too vague a term to use in the code and config.
Maybe "advice.*"?

 Documentation/config.txt |   16 ++++++++++++++++
 Makefile                 |    2 ++
 builtin-push.c           |    3 ++-
 cache.h                  |    1 +
 config.c                 |    3 +++
 messages.c               |   28 ++++++++++++++++++++++++++++
 messages.h               |   15 +++++++++++++++
 7 files changed, 67 insertions(+), 1 deletions(-)
 create mode 100644 messages.c
 create mode 100644 messages.h

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 5256c7f..ec308a6 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1201,6 +1201,22 @@ mergetool.keepTemporaries::
 mergetool.prompt::
 	Prompt before each invocation of the merge resolution program.
 
+message.all::
+	When set to 'true', display all optional help messages. When set
+	to 'false', do not display any. Defaults vary with individual
+	messages (see message.* below).
+
+message.*::
+	When set to 'true', display the given optional help message.
+	When set to 'false', do not display. The configuration variables
+	are:
++
+--
+	pushNonFastForward::
+		Help message shown when linkgit:git-push[1] refuses
+		non-fast-forward refs. Default: true.
+--
+
 pack.window::
 	The size of the window used by linkgit:git-pack-objects[1] when no
 	window size is given on the command line. Defaults to 10.
diff --git a/Makefile b/Makefile
index a614347..0785977 100644
--- a/Makefile
+++ b/Makefile
@@ -424,6 +424,7 @@ LIB_H += ll-merge.h
 LIB_H += log-tree.h
 LIB_H += mailmap.h
 LIB_H += merge-recursive.h
+LIB_H += messages.h
 LIB_H += object.h
 LIB_H += pack.h
 LIB_H += pack-refs.h
@@ -506,6 +507,7 @@ LIB_OBJS += mailmap.o
 LIB_OBJS += match-trees.o
 LIB_OBJS += merge-file.o
 LIB_OBJS += merge-recursive.o
+LIB_OBJS += messages.o
 LIB_OBJS += name-hash.o
 LIB_OBJS += object.o
 LIB_OBJS += pack-check.o
diff --git a/builtin-push.c b/builtin-push.c
index 787011f..dceac9f 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -157,7 +157,8 @@ static int do_push(const char *repo, int flags)
 			continue;
 
 		error("failed to push some refs to '%s'", url[i]);
-		if (nonfastforward) {
+		if (nonfastforward &&
+		    messages[MESSAGE_PUSH_NONFASTFORWARD].preference) {
 			printf("To prevent you from losing history, non-fast-forward updates were rejected\n"
 			       "Merge the remote changes before pushing again.  See the 'non-fast forward'\n"
 			       "section of 'git push --help' for details.\n");
diff --git a/cache.h b/cache.h
index 5fad24c..32d1a27 100644
--- a/cache.h
+++ b/cache.h
@@ -4,6 +4,7 @@
 #include "git-compat-util.h"
 #include "strbuf.h"
 #include "hash.h"
+#include "messages.h"
 
 #include SHA1_HEADER
 #ifndef git_SHA_CTX
diff --git a/config.c b/config.c
index e87edea..aed1547 100644
--- a/config.c
+++ b/config.c
@@ -627,6 +627,9 @@ int git_default_config(const char *var, const char *value, void *dummy)
 	if (!prefixcmp(var, "mailmap."))
 		return git_default_mailmap_config(var, value);
 
+	if (!prefixcmp(var, "message."))
+		return git_default_message_config(var, value);
+
 	if (!strcmp(var, "pager.color") || !strcmp(var, "color.pager")) {
 		pager_use_color = git_config_bool(var,value);
 		return 0;
diff --git a/messages.c b/messages.c
new file mode 100644
index 0000000..00fc196
--- /dev/null
+++ b/messages.c
@@ -0,0 +1,28 @@
+#include "cache.h"
+#include "messages.h"
+
+struct message_preference messages[] = {
+	{ "pushnonfastforward", 1 },
+};
+
+int git_default_message_config(const char *var, const char *value)
+{
+	const char *k = skip_prefix(var, "message.");
+	int i;
+
+	if (!strcmp(k, "all")) {
+		int v = git_config_bool(var, value);
+		for (i = 0; i < ARRAY_SIZE(messages); i++)
+			messages[i].preference = v;
+		return 0;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(messages); i++) {
+		if (strcmp(k, messages[i].name))
+			continue;
+		messages[i].preference = git_config_bool(var, value);
+		return 0;
+	}
+
+	return 0;
+}
diff --git a/messages.h b/messages.h
new file mode 100644
index 0000000..f175747
--- /dev/null
+++ b/messages.h
@@ -0,0 +1,15 @@
+#ifndef MESSAGE_H
+#define MESSAGE_H
+
+#define MESSAGE_PUSH_NONFASTFORWARD 0
+
+struct message_preference {
+	const char *name;
+	int preference;
+};
+
+extern struct message_preference messages[];
+
+int git_default_message_config(const char *var, const char *value);
+
+#endif /* MESSAGE_H */
-- 
1.6.4.2.266.g981efc.dirty

^ permalink raw reply related

* [PATCH 2/4] push: re-flow non-fast-forward message
From: Jeff King @ 2009-09-06  6:47 UTC (permalink / raw)
  To: Nanako Shiraishi; +Cc: Junio C Hamano, Matthieu Moy, Teemu Likonen, git
In-Reply-To: <20090906064454.GA1643@coredump.intra.peff.net>

The extreme raggedness of the right edge make this jarring
to read. Let's re-flow the text to fill the lines in a more
even way.

Signed-off-by: Jeff King <peff@peff.net>
---
I am actually not all that fond of the particular wording, either, but I
didn't want to re-open a wording bikeshed debate.

 builtin-push.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin-push.c b/builtin-push.c
index 8d72cc2..787011f 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -158,9 +158,9 @@ static int do_push(const char *repo, int flags)
 
 		error("failed to push some refs to '%s'", url[i]);
 		if (nonfastforward) {
-			printf("To prevent you from losing history, non-fast-forward updates were rejected.\n"
-			       "Merge the remote changes before pushing again.\n"
-			       "See the 'non-fast forward' section of 'git push --help' for details.\n");
+			printf("To prevent you from losing history, non-fast-forward updates were rejected\n"
+			       "Merge the remote changes before pushing again.  See the 'non-fast forward'\n"
+			       "section of 'git push --help' for details.\n");
 		}
 		errs++;
 	}
-- 
1.6.4.2.266.g981efc.dirty

^ permalink raw reply related

* [PATCH 1/4] push: fix english in non-fast-forward message
From: Jeff King @ 2009-09-06  6:46 UTC (permalink / raw)
  To: Nanako Shiraishi; +Cc: Junio C Hamano, Matthieu Moy, Teemu Likonen, git
In-Reply-To: <20090906064454.GA1643@coredump.intra.peff.net>

We must use an article when referring to the section
because it is a non-proper noun, and it must be the definite
article because we are referring to a specific section.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin-push.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/builtin-push.c b/builtin-push.c
index 67f6d96..8d72cc2 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -160,7 +160,7 @@ static int do_push(const char *repo, int flags)
 		if (nonfastforward) {
 			printf("To prevent you from losing history, non-fast-forward updates were rejected.\n"
 			       "Merge the remote changes before pushing again.\n"
-			       "See 'non-fast forward' section of 'git push --help' for details.\n");
+			       "See the 'non-fast forward' section of 'git push --help' for details.\n");
 		}
 		errs++;
 	}
-- 
1.6.4.2.266.g981efc.dirty

^ permalink raw reply related

* [RFC/PATCH 0/4] make helpful messages optional
From: Jeff King @ 2009-09-06  6:44 UTC (permalink / raw)
  To: Nanako Shiraishi; +Cc: Junio C Hamano, Matthieu Moy, Teemu Likonen, git
In-Reply-To: <20090811120313.6117@nanako3.lavabit.com>

On Tue, Aug 11, 2009 at 12:03:13PM +0900, Nanako Shiraishi wrote:

>  		error("failed to push some refs to '%s'", url[i]);
> +		if (nonfastforward) {
> +			printf("To prevent you from losing history, non-fast-forward updates were rejected.\n"
> +			       "Merge the remote changes before pushing again.\n"
> +			       "See 'non-fast forward' section of 'git push --help' for details.\n");
> +		}

I saw this message in regular use for the first time today, and I
thought it was terribly ugly. The sheer number of lines, coupled with
the extremely ragged right edge made it much harder to pick out the
nicely formatted status table. And of course the information in the
message was totally worthless to me, as an experienced git user.

So rather than re-open the debate on whether this message should exist
or not, here is a patch series which tries to address the issue:

  [1/4]: push: fix english in non-fast-forward message

    Hopefully non-controversial.

  [2/4]: push: re-flow non-fast-forward message

    This makes it prettier, IMHO.  I suspect the message was flowed as
    it was originally on purpose so that each sentence began on its own
    line. I think making it easier on the eyes is more important,
    though.

  [3/4]: push: make non-fast-forward help message configurable

    I feel like we have had this exact sort of tension before: we want
    one thing to help new users and experienced users want another
    thing. We have resisted an "expert user" config variable in the past
    because proposals usually involved a change of behavior, which could
    lead to quite confusing results. However, I think the case of
    "helpful messages" is much simpler. The message is meant purely for
    human consumption and is redundant with information already given,
    so an expert sitting at a novice's terminal (or vice versa) will not
    encounter any surprises.

    This actually introduces infrastructure for other, similar messages,
    so that we can make existing ones optional, or build new ones as
    appropriate.

  [4/4]: status: make "how to stage" messages optional

    This uses the infrastructure in 3/4 to lose the "use git add to add
    untracked files" advice.

I think (1) and (2) are pretty straightforward. (3) and (4) are more
questionable, and I am undecided whether I am over-reacting to being
annoyed by the message. I do know this is not the first time I have had
the urge to write such a patch, so maybe others feel the same.

-Peff

^ permalink raw reply

* Re: [PATCH 0/9] War on blank-at-eof
From: Junio C Hamano @ 2009-09-06  6:13 UTC (permalink / raw)
  To: Thell Fowler; +Cc: git
In-Reply-To: <alpine.WNT.2.00.0909051534380.7040@GWNotebook>

Thell Fowler <git@tbfowler.name> writes:

> While thinking about what appeared in:
>
> http://article.gmane.org/gmane.comp.version-control.git/124138

Oh, I forgot all about that one.  The suggestion does include two very
good points, one being "git apply" which I did, and the other being what I
completely forgot.  Introduction of blank-at-eol and blank-at-eof, and
make trailing-space a convenience synonym that triggers both.

Thanks for a reminder.  The following patch can come on top of the
series.

-- >8 --
Subject: core.whitespace: split trailing-space into blank-at-{eol,eof}

People who configured trailing-space depended on it to catch both extra
white space at the end of line, and extra blank lines at the end of file.
Earlier attempt to introduce only blank-at-eof gave them an escape hatch
to keep the old behaviour, but it is a regression until they explicitly
specify the new error class.

This introduces a blank-at-eol that only catches extra white space at the
end of line, and makes the traditional trailing-space a convenient synonym
to catch both blank-at-eol and blank-at-eof.  This way, people who used
trailing-space continue to catch both classes of errors.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/config.txt |    5 ++++-
 cache.h                  |    5 +++--
 ws.c                     |   24 +++++++++++++++---------
 3 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 871384e..0e245a7 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -382,7 +382,7 @@ core.whitespace::
 	consider them as errors.  You can prefix `-` to disable
 	any of them (e.g. `-trailing-space`):
 +
-* `trailing-space` treats trailing whitespaces at the end of the line
+* `blank-at-eol` treats trailing whitespaces at the end of the line
   as an error (enabled by default).
 * `space-before-tab` treats a space character that appears immediately
   before a tab character in the initial indent part of the line as an
@@ -391,11 +391,14 @@ core.whitespace::
   space characters as an error (not enabled by default).
 * `blank-at-eof` treats blank lines added at the end of file as an error
   (enabled by default).
+* `trailing-space` is a short-hand to cover both `blank-at-eol` and
+  `blank-at-eof`.
 * `cr-at-eol` treats a carriage-return at the end of line as
   part of the line terminator, i.e. with it, `trailing-space`
   does not trigger if the character before such a carriage-return
   is not a whitespace (not enabled by default).
 
+
 core.fsyncobjectfiles::
 	This boolean will enable 'fsync()' when writing object files.
 +
diff --git a/cache.h b/cache.h
index 7152fea..ee12e74 100644
--- a/cache.h
+++ b/cache.h
@@ -841,12 +841,13 @@ void shift_tree(const unsigned char *, const unsigned char *, unsigned char *, i
  * whitespace rules.
  * used by both diff and apply
  */
-#define WS_TRAILING_SPACE	01
+#define WS_BLANK_AT_EOL         01
 #define WS_SPACE_BEFORE_TAB	02
 #define WS_INDENT_WITH_NON_TAB	04
 #define WS_CR_AT_EOL           010
 #define WS_BLANK_AT_EOF        020
-#define WS_DEFAULT_RULE (WS_TRAILING_SPACE|WS_SPACE_BEFORE_TAB|WS_BLANK_AT_EOF)
+#define WS_TRAILING_SPACE      (WS_BLANK_AT_EOL|WS_BLANK_AT_EOF)
+#define WS_DEFAULT_RULE (WS_TRAILING_SPACE|WS_SPACE_BEFORE_TAB)
 extern unsigned whitespace_rule_cfg;
 extern unsigned whitespace_rule(const char *);
 extern unsigned parse_whitespace_rule(const char *);
diff --git a/ws.c b/ws.c
index d56636b..cd03bc0 100644
--- a/ws.c
+++ b/ws.c
@@ -15,6 +15,7 @@ static struct whitespace_rule {
 	{ "space-before-tab", WS_SPACE_BEFORE_TAB },
 	{ "indent-with-non-tab", WS_INDENT_WITH_NON_TAB },
 	{ "cr-at-eol", WS_CR_AT_EOL },
+	{ "blank-at-eol", WS_BLANK_AT_EOL },
 	{ "blank-at-eof", WS_BLANK_AT_EOF },
 };
 
@@ -101,9 +102,19 @@ unsigned whitespace_rule(const char *pathname)
 char *whitespace_error_string(unsigned ws)
 {
 	struct strbuf err;
+
 	strbuf_init(&err, 0);
-	if (ws & WS_TRAILING_SPACE)
+	if ((ws & WS_TRAILING_SPACE) == WS_TRAILING_SPACE)
 		strbuf_addstr(&err, "trailing whitespace");
+	else {
+		if (ws & WS_BLANK_AT_EOL)
+			strbuf_addstr(&err, "trailing whitespace");
+		if (ws & WS_BLANK_AT_EOF) {
+			if (err.len)
+				strbuf_addstr(&err, ", ");
+			strbuf_addstr(&err, "new blank line at EOF");
+		}
+	}
 	if (ws & WS_SPACE_BEFORE_TAB) {
 		if (err.len)
 			strbuf_addstr(&err, ", ");
@@ -114,11 +125,6 @@ char *whitespace_error_string(unsigned ws)
 			strbuf_addstr(&err, ", ");
 		strbuf_addstr(&err, "indent with spaces");
 	}
-	if (ws & WS_BLANK_AT_EOF) {
-		if (err.len)
-			strbuf_addstr(&err, ", ");
-		strbuf_addstr(&err, "new blank line at EOF");
-	}
 	return strbuf_detach(&err, NULL);
 }
 
@@ -146,11 +152,11 @@ static unsigned ws_check_emit_1(const char *line, int len, unsigned ws_rule,
 	}
 
 	/* Check for trailing whitespace. */
-	if (ws_rule & WS_TRAILING_SPACE) {
+	if (ws_rule & WS_BLANK_AT_EOL) {
 		for (i = len - 1; i >= 0; i--) {
 			if (isspace(line[i])) {
 				trailing_whitespace = i;
-				result |= WS_TRAILING_SPACE;
+				result |= WS_BLANK_AT_EOL;
 			}
 			else
 				break;
@@ -266,7 +272,7 @@ int ws_fix_copy(char *dst, const char *src, int len, unsigned ws_rule, int *erro
 	/*
 	 * Strip trailing whitespace
 	 */
-	if ((ws_rule & WS_TRAILING_SPACE) &&
+	if ((ws_rule & WS_BLANK_AT_EOL) &&
 	    (2 <= len && isspace(src[len-2]))) {
 		if (src[len - 1] == '\n') {
 			add_nl_to_tail = 1;
-- 
1.6.4.2.313.g0425f

^ permalink raw reply related


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