Git development
 help / color / mirror / Atom feed
* Re: [PATCH v2 1/4] Add date formatting and parsing functions relative  to a given time
From: Alex Riesen @ 2009-08-31  6:08 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20090831022605.GA5507@coredump.intra.peff.net>

On Mon, Aug 31, 2009 at 04:26, Jeff King<peff@peff.net> wrote:
> From: Alex Riesen <raa.lkml@gmail.com>
>
> The main purpose is to allow predictable testing of the code.
>
> Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> Same as previous 1/3, but rebased onto lt/approxidate topic. The merge
> ended up as quite a mess because of textual differences, so I had to
> fix up a fair bit by hand; Alex, please confirm that I didn't screw
> anything up too badly right before putting your name at the top. ;)

Looks good.

^ permalink raw reply

* Re: Merging in Subversion 1.5 (was: Re: Using git to track my PhD thesis, couple of questions)
From: Dmitry Potapov @ 2009-08-31  5:47 UTC (permalink / raw)
  To: Sam Vilain; +Cc: Jakub Narebski, Matthias Andree, git, Matthieu Moy
In-Reply-To: <1251661316.25764.4.camel@maia.lan>

On Mon, Aug 31, 2009 at 07:41:56AM +1200, Sam Vilain wrote:
> On Fri, 2009-08-28 at 08:12 -0700, Jakub Narebski wrote:
> 
> > Also IIRC there is warning (well, at least there was in Subversion 1.5
> > release notes) that merge tracking doesn't work entirely correctly in
> > the face of criss-cross merges (multiple merge bases) and renaming
> > (although I do hope that they fixed problem with silent corruption if
> > there is rename during merge).
> 
> Not sure about that one.  I also heard - unconfirmed - that things start
> to go awry if you start branching off branches and merging around the
> place.  But if that happens it's likely a bug rather than a design flaw
> (I think).

Some of the initial issues that existed in SVN 1.5.0 have been resolved,
but some others remain. Here is one bug report related to merge:
http://subversion.tigris.org/issues/show_bug.cgi?id=2897
It was reported two years ago, but the problem is still not fixed.
And there is a few others (some of them even older but even with less
prospect of being fixed any time soon):
http://subversion.tigris.org/issues/show_bug.cgi?id=2837
http://subversion.tigris.org/issues/show_bug.cgi?id=2898
http://subversion.tigris.org/issues/show_bug.cgi?id=3056
http://subversion.tigris.org/issues/show_bug.cgi?id=3157

I don't think they would exist for long if they were ease to fix.  Merge
in Subversion is essence automatic cherry-picking, and it is not easy to
implement that in the way it would be reasonably fast and work correctly
in a general case.

Darcs is probably the best when it comes to cherry-picking but clearly
it is not a speed demon. In case of Subversion, the problem is worse,
because it has to make decision on a per file basis rather than operate
each patch as a unit. So, it is even more difficult to implement that
correctly and efficiently.

What you can do relatively simple is to handle a of one directional
merge, and that was the primary design goal of Subversion merge
tracking feature.

Here is what Daniel Berlin wrote about it:
<<<
The initial merge tracking implementation was not meant to handle
repeated bidirectional merging, at least, as designed.

It was designed to allow cherry picks, and mainly for maintaining
feature branches that were mostly one way merges, with the very
occasional merge in the other direction and then branch death :).

For these cases, it works out fine.

For more complex cyclical merge patterns, you really can't use what
we've got. Trying to work around these cases, or build algorithms
that handle them, is just going to lead you into 20 years of edge
cases that made people come up with changeset dags in the first place.
>>>
Source: http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=892215

So, I do not think that SVN merge will ever work correctly for those
edge cases.

But even if Subversion learns how to handle all those complex cases
correctly, it will still come with some surprises. One of the main
advantage of the simple 3-way merge is that it is easy to understand
and it makes the right thing most of time. Linus provided a really good
explanation of it here:
http://thread.gmane.org/gmane.comp.version-control.git/60457/focus=60644


Dmitry

^ permalink raw reply

* Re: [BUG] git stash refuses to save after "add -N"
From: Jeff King @ 2009-08-31  5:05 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Yann Dirson, git
In-Reply-To: <7vljl0lgms.fsf@alter.siamese.dyndns.org>

On Sun, Aug 30, 2009 at 10:03:07PM -0700, Junio C Hamano wrote:

> >   - "-f" is kind of vague. Would people expect it to force aspects of
> >     the stash? Should it be "--intent-as-empty"?
> 
> I am not sure if asking for positive confirmation with "-f" is even worth
> it.  As you pointed out in your earlier message, which prompted me to
> respond with a patch, when this codepath is exercised, the user is in a
> rush, and I do not see what else the user would want to do other than
> including it in the stash by rerunning with -f.

I guess it was just to mitigate my fear that we are somehow creating a
stash that will confuse people when they apply it. But really that fear
is probably unjustified.

-Peff

^ permalink raw reply

* Re: [BUG] git stash refuses to save after "add -N"
From: Junio C Hamano @ 2009-08-31  5:03 UTC (permalink / raw)
  To: Jeff King; +Cc: Yann Dirson, git
In-Reply-To: <20090831042724.GA16646@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> My concerns are:
>
>   - "-f" is kind of vague. Would people expect it to force aspects of
>     the stash? Should it be "--intent-as-empty"?
>
>   - the error message is still a bit muddled, because you get the "not
>     yet added" files _first_, then some failure cruft from write-tree,
>     and _then_ the trying-to-be-helpful message
>
> I dunno. Honestly I am a bit lukewarm about this whole thing, as it
> seems like something that just wouldn't come up that often, and while
> the current error message is a bit disorganized, I think a user who has
> used "git add -N" can figure out that it is related (the only report we
> have is from Yann, who _did_ figure it out, but wanted to know how to
> make git handle the situation better).

I am not sure if asking for positive confirmation with "-f" is even worth
it.  As you pointed out in your earlier message, which prompted me to
respond with a patch, when this codepath is exercised, the user is in a
rush, and I do not see what else the user would want to do other than
including it in the stash by rerunning with -f.

^ permalink raw reply

* Re: [BUG] git stash refuses to save after "add -N"
From: Jeff King @ 2009-08-31  4:36 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Yann Dirson, git
In-Reply-To: <7v63c5f4vs.fsf@alter.siamese.dyndns.org>

On Sun, Aug 30, 2009 at 01:01:11PM -0700, Junio C Hamano wrote:

> One thing I noticed was that while unstashing without --index, we add full
> contents to the index of new files.  I think it is because back then when
> stash was written there was no other way, but now we have intent-to-add
> and a way to stash such an entry, I think we should add only the intent to
> add them in that codepath.
> 
> Of course we will not do this when unstashing with --index.

And btw, I think your suggestion is reasonable, though I don't feel
strongly either way. I don't know that the current behavior is really
bothering anybody.

-Peff

^ permalink raw reply

* Re: [BUG] git stash refuses to save after "add -N"
From: Jeff King @ 2009-08-31  4:27 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Yann Dirson, git
In-Reply-To: <7v63c5f4vs.fsf@alter.siamese.dyndns.org>

On Sun, Aug 30, 2009 at 01:01:11PM -0700, Junio C Hamano wrote:

> > And all of this while they are trying to quickly switch contexts to
> > whatever it was that caused them to stash in the first place.
> 
> Ok, then probably the "how about" patch would be a part of the right
> solution.

And then something like this on top (assuming you cut the change to
git-stash.sh from yours).

My concerns are:

  - "-f" is kind of vague. Would people expect it to force aspects of
    the stash? Should it be "--intent-as-empty"?

  - the error message is still a bit muddled, because you get the "not
    yet added" files _first_, then some failure cruft from write-tree,
    and _then_ the trying-to-be-helpful message

I dunno. Honestly I am a bit lukewarm about this whole thing, as it
seems like something that just wouldn't come up that often, and while
the current error message is a bit disorganized, I think a user who has
used "git add -N" can figure out that it is related (the only report we
have is from Yann, who _did_ figure it out, but wanted to know how to
make git handle the situation better).

diff --git a/git-stash.sh b/git-stash.sh
index d61c9d0..963cad0 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -20,6 +20,7 @@ TMP="$GIT_DIR/.git-stash.$$"
 trap 'rm -f "$TMP-*"' 0
 
 ref_stash=refs/stash
+force=
 
 no_changes () {
 	git diff-index --quiet --cached HEAD --ignore-submodules -- &&
@@ -63,7 +64,14 @@ create_stash () {
 	msg=$(printf '%s: %s' "$branch" "$head")
 
 	# state of the index
-	i_tree=$(git write-tree) &&
+	if ! i_tree=$(git write-tree ${force:+--intent-as-empty}); then
+		case "$force" in
+		t) die 'Cannot save the current index state';;
+		*) echo >&2 'fatal: unable to create tree; if some files are marked as'
+		   echo >&2 '"not added yet", you may override with "git stash save -f"'
+		   exit 1
+		esac
+	fi
 	i_commit=$(printf 'index on %s\n' "$msg" |
 		git commit-tree $i_tree -p $b_commit) ||
 		die "Cannot save the current index state"
@@ -104,6 +112,9 @@ save_stash () {
 		-q|--quiet)
 			GIT_QUIET=t
 			;;
+		-f|--force)
+			force=t
+			;;
 		*)
 			break
 			;;
diff --git a/t/t3904-stash-intent.sh b/t/t3904-stash-intent.sh
new file mode 100755
index 0000000..ec7dd12
--- /dev/null
+++ b/t/t3904-stash-intent.sh
@@ -0,0 +1,30 @@
+#!/bin/sh
+
+test_description='stash with intent-to-add index entries'
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	echo content >base &&
+	git add base &&
+	git commit -m base &&
+	echo foo content >foo &&
+	echo bar content >bar &&
+	git add foo &&
+	git add -N bar
+'
+
+test_expect_success 'stash save refuses intent-to-add entry' '
+	test_must_fail git stash save
+'
+
+test_expect_success 'stash save -f allows intent-to-add' '
+	git stash save -f &&
+	git show stash^2:foo >foo.stash &&
+	echo foo content >expect &&
+	test_cmp expect foo.stash &&
+	>expect &&
+	git show stash^2:bar >bar.stash &&
+	test_cmp expect bar.stash
+'
+
+test_done

^ permalink raw reply related

* Re: [msysGit] Re: Using VC build git (split patch)
From: Frank Li @ 2009-08-31  2:45 UTC (permalink / raw)
  To: Thiago Farina
  Cc: Erik Faye-Lund, Marius Storm-Olsen, git, msysGit,
	Johannes Schindelin
In-Reply-To: <a4c8a6d00908301324o25d1cb48u12b527ea68e43680@mail.gmail.com>

> Yep, an imap-send vcproject with one file(imap-send.c) was added to
> gitbuild.sln.

Sorry, I add imap-send to sln just for testing at solution.

You can omit this project.

best regards
Frank Li

^ permalink raw reply

* [PATCH v2 4/4] fix approxidate parsing of relative months and years
From: Jeff King @ 2009-08-31  2:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Linus Torvalds, Alex Riesen, git
In-Reply-To: <20090831022228.GA4833@coredump.intra.peff.net>

These were broken by b5373e9. The problem is that the code
marks the month and year with "-1" for "we don't know it
yet", but the month and year code paths were not adjusted to
fill in the current time before doing their calculations
(whereas other units follow a different code path and are
fine).

Signed-off-by: Jeff King <peff@peff.net>
---
This one is new from the last series, as it fixes bugs only found in
lt/approxidate.

 date.c          |    5 ++++-
 t/t0006-date.sh |    4 ++--
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/date.c b/date.c
index 8e57e5e..e9ee4aa 100644
--- a/date.c
+++ b/date.c
@@ -857,7 +857,9 @@ static const char *approxidate_alpha(const char *date, struct tm *tm, struct tm
 	}
 
 	if (match_string(date, "months") >= 5) {
-		int n = tm->tm_mon - *num;
+		int n;
+		update_tm(tm, now, 0); /* fill in date fields if needed */
+		n = tm->tm_mon - *num;
 		*num = 0;
 		while (n < 0) {
 			n += 12;
@@ -868,6 +870,7 @@ static const char *approxidate_alpha(const char *date, struct tm *tm, struct tm
 	}
 
 	if (match_string(date, "years") >= 4) {
+		update_tm(tm, now, 0); /* fill in date fields if needed */
 		tm->tm_year -= *num;
 		*num = 0;
 		return end;
diff --git a/t/t0006-date.sh b/t/t0006-date.sh
index 02cd565..a4d8fa8 100755
--- a/t/t0006-date.sh
+++ b/t/t0006-date.sh
@@ -53,8 +53,8 @@ check_approxidate 10.minutes.ago '2009-08-30 19:10:00'
 check_approxidate yesterday '2009-08-29 19:20:00'
 check_approxidate 3.days.ago '2009-08-27 19:20:00'
 check_approxidate 3.weeks.ago '2009-08-09 19:20:00'
-check_approxidate 3.months.ago '2009-05-30 19:20:00' failure
-check_approxidate 2.years.3.months.ago '2007-05-30 19:20:00' failure
+check_approxidate 3.months.ago '2009-05-30 19:20:00'
+check_approxidate 2.years.3.months.ago '2007-05-30 19:20:00'
 
 check_approxidate '6am yesterday' '2009-08-29 06:00:00'
 check_approxidate '6pm yesterday' '2009-08-29 18:00:00'
-- 
1.6.4.2.373.g5881fd

^ permalink raw reply related

* [PATCH v2 3/4] tests: add date printing and parsing tests
From: Jeff King @ 2009-08-31  2:30 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Linus Torvalds, Alex Riesen, git
In-Reply-To: <20090831022228.GA4833@coredump.intra.peff.net>

Until now, there was no coverage of relative date printing
or approxidate parsing routines (mainly because we had no
way of faking the "now" time for relative date calculations,
which made consistent testing impossible).

This new script tries to exercise the basic features of
show_date and approxidate. Most of the tests are just "this
obvious thing works" to prevent future regressions, with a
few exceptions:

  - We confirm the fix in 607a9e8 that relative year/month
    dates in the latter half of a year round correctly.

  - We confirm that the improvements in b5373e9 and 1bddb25
    work.

  - A few tests are marked to expect failure, which are
    regressions recently introduced by the two commits
    above.

Signed-off-by: Jeff King <peff@peff.net>
---
Similar to earlier 3/3, but improvements and regressions from
lt/approxidate included.

Linus, when you posted the approxidate fixes earlier, you mentioned you
might have some other corner cases. I think you were just referring to
the stuff you improved in the followup patch, but if you know of more
broken-ness, we should probably include it here.

 t/t0006-date.sh |   75 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 75 insertions(+), 0 deletions(-)
 create mode 100755 t/t0006-date.sh

diff --git a/t/t0006-date.sh b/t/t0006-date.sh
new file mode 100755
index 0000000..02cd565
--- /dev/null
+++ b/t/t0006-date.sh
@@ -0,0 +1,75 @@
+#!/bin/sh
+
+test_description='test date parsing and printing'
+. ./test-lib.sh
+
+# arbitrary reference time: 2009-08-30 19:20:00
+TEST_DATE_NOW=1251660000; export TEST_DATE_NOW
+
+check_show() {
+	t=$(($TEST_DATE_NOW - $1))
+	echo "$t -> $2" >expect
+	test_expect_${3:-success} "relative date ($2)" "
+	test-date show $t >actual &&
+	test_cmp expect actual
+	"
+}
+
+check_show 5 '5 seconds ago'
+check_show 300 '5 minutes ago'
+check_show 18000 '5 hours ago'
+check_show 432000 '5 days ago'
+check_show 1728000 '3 weeks ago'
+check_show 13000000 '5 months ago'
+check_show 37500000 '1 year, 2 months ago'
+check_show 55188000 '1 year, 9 months ago'
+check_show 630000000 '20 years ago'
+
+check_parse() {
+	echo "$1 -> $2" >expect
+	test_expect_${3:-success} "parse date ($1)" "
+	test-date parse '$1' >actual &&
+	test_cmp expect actual
+	"
+}
+
+check_parse 2008 bad
+check_parse 2008-02 bad
+check_parse 2008-02-14 bad
+check_parse '2008-02-14 20:30:45' '2008-02-14 20:30:45 +0000'
+
+check_approxidate() {
+	echo "$1 -> $2 +0000" >expect
+	test_expect_${3:-success} "parse approxidate ($1)" "
+	test-date approxidate '$1' >actual &&
+	test_cmp expect actual
+	"
+}
+
+check_approxidate now '2009-08-30 19:20:00'
+check_approxidate '5 seconds ago' '2009-08-30 19:19:55'
+check_approxidate 5.seconds.ago '2009-08-30 19:19:55'
+check_approxidate 10.minutes.ago '2009-08-30 19:10:00'
+check_approxidate yesterday '2009-08-29 19:20:00'
+check_approxidate 3.days.ago '2009-08-27 19:20:00'
+check_approxidate 3.weeks.ago '2009-08-09 19:20:00'
+check_approxidate 3.months.ago '2009-05-30 19:20:00' failure
+check_approxidate 2.years.3.months.ago '2007-05-30 19:20:00' failure
+
+check_approxidate '6am yesterday' '2009-08-29 06:00:00'
+check_approxidate '6pm yesterday' '2009-08-29 18:00:00'
+check_approxidate '3:00' '2009-08-30 03:00:00'
+check_approxidate '15:00' '2009-08-30 15:00:00'
+check_approxidate 'noon today' '2009-08-30 12:00:00'
+check_approxidate 'noon yesterday' '2009-08-29 12:00:00'
+
+check_approxidate 'last tuesday' '2009-08-25 19:20:00'
+check_approxidate 'July 5th' '2009-07-05 19:20:00'
+check_approxidate '06/05/2009' '2009-06-05 19:20:00'
+check_approxidate '06.05.2009' '2009-05-06 19:20:00'
+
+check_approxidate 'Jun 6, 5AM' '2009-06-06 05:00:00'
+check_approxidate '5AM Jun 6' '2009-06-06 05:00:00'
+check_approxidate '6AM, June 7, 2009' '2009-06-07 06:00:00'
+
+test_done
-- 
1.6.4.2.373.g5881fd

^ permalink raw reply related

* [PATCH v2 2/4] refactor test-date interface
From: Jeff King @ 2009-08-31  2:26 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Alex Riesen, git
In-Reply-To: <20090831022228.GA4833@coredump.intra.peff.net>

The test-date program goes back to the early days of git,
where it was presumably used to do manual sanity checks on
changes to the date code. However, it is not actually used
by the test suite to do any sort of automatic of systematic
tests.

This patch refactors the interface to the program to try to
make it more suitable for use by the test suite. There
should be no fallouts to changing the interface since it is
not actually installed and is not internally called by any
other programs.

The changes are:

  - add a "mode" parameter so the caller can specify which
    operation to test

  - add a mode to test relative date output from show_date

  - allow faking a fixed time via the TEST_DATE_NOW
    environment variable, which allows consistent automated
    testing

  - drop the use of ctime for showing dates in favor of our
    internal iso8601 printing routines. The ctime output is
    somewhat redundant (because of the day-of-week) which
    makes writing test cases more annoying.

Signed-off-by: Jeff King <peff@peff.net>
---
Same as earlier 2/3.

 test-date.c |   86 +++++++++++++++++++++++++++++++++++++++++++++-------------
 1 files changed, 66 insertions(+), 20 deletions(-)
 rewrite test-date.c (63%)

diff --git a/test-date.c b/test-date.c
dissimilarity index 63%
index 62e8f23..5b0a220 100644
--- a/test-date.c
+++ b/test-date.c
@@ -1,20 +1,66 @@
-#include "cache.h"
-
-int main(int argc, char **argv)
-{
-	int i;
-
-	for (i = 1; i < argc; i++) {
-		char result[100];
-		time_t t;
-
-		memcpy(result, "bad", 4);
-		parse_date(argv[i], result, sizeof(result));
-		t = strtoul(result, NULL, 0);
-		printf("%s -> %s -> %s", argv[i], result, ctime(&t));
-
-		t = approxidate(argv[i]);
-		printf("%s -> %s\n", argv[i], ctime(&t));
-	}
-	return 0;
-}
+#include "cache.h"
+
+static const char *usage_msg = "\n"
+"  test-date show [time_t]...\n"
+"  test-date parse [date]...\n"
+"  test-date approxidate [date]...\n";
+
+static void show_dates(char **argv, struct timeval *now)
+{
+	char buf[128];
+
+	for (; *argv; argv++) {
+		time_t t = atoi(*argv);
+		show_date_relative(t, 0, now, buf, sizeof(buf));
+		printf("%s -> %s\n", *argv, buf);
+	}
+}
+
+static void parse_dates(char **argv, struct timeval *now)
+{
+	for (; *argv; argv++) {
+		char result[100];
+		time_t t;
+
+		parse_date(*argv, result, sizeof(result));
+		t = strtoul(result, NULL, 0);
+		printf("%s -> %s\n", *argv,
+			t ? show_date(t, 0, DATE_ISO8601) : "bad");
+	}
+}
+
+static void parse_approxidate(char **argv, struct timeval *now)
+{
+	for (; *argv; argv++) {
+		time_t t;
+		t = approxidate_relative(*argv, now);
+		printf("%s -> %s\n", *argv, show_date(t, 0, DATE_ISO8601));
+	}
+}
+
+int main(int argc, char **argv)
+{
+	struct timeval now;
+	const char *x;
+
+	x = getenv("TEST_DATE_NOW");
+	if (x) {
+		now.tv_sec = atoi(x);
+		now.tv_usec = 0;
+	}
+	else
+		gettimeofday(&now, NULL);
+
+	argv++;
+	if (!*argv)
+		usage(usage_msg);
+	if (!strcmp(*argv, "show"))
+		show_dates(argv+1, &now);
+	else if (!strcmp(*argv, "parse"))
+		parse_dates(argv+1, &now);
+	else if (!strcmp(*argv, "approxidate"))
+		parse_approxidate(argv+1, &now);
+	else
+		usage(usage_msg);
+	return 0;
+}
-- 
1.6.4.2.373.g5881fd

^ permalink raw reply related

* [PATCH v2 1/4] Add date formatting and parsing functions relative to a given time
From: Jeff King @ 2009-08-31  2:26 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Alex Riesen, git
In-Reply-To: <20090831022228.GA4833@coredump.intra.peff.net>

From: Alex Riesen <raa.lkml@gmail.com>

The main purpose is to allow predictable testing of the code.

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
---
Same as previous 1/3, but rebased onto lt/approxidate topic. The merge
ended up as quite a mess because of textual differences, so I had to
fix up a fair bit by hand; Alex, please confirm that I didn't screw
anything up too badly right before putting your name at the top. ;)

 cache.h |    5 ++
 date.c  |  150 ++++++++++++++++++++++++++++++++++++--------------------------
 2 files changed, 92 insertions(+), 63 deletions(-)

diff --git a/cache.h b/cache.h
index 808daba..5fad24c 100644
--- a/cache.h
+++ b/cache.h
@@ -731,9 +731,14 @@ enum date_mode {
 };
 
 const char *show_date(unsigned long time, int timezone, enum date_mode mode);
+const char *show_date_relative(unsigned long time, int tz,
+			       const struct timeval *now,
+			       char *timebuf,
+			       size_t timebuf_size);
 int parse_date(const char *date, char *buf, int bufsize);
 void datestamp(char *buf, int bufsize);
 unsigned long approxidate(const char *);
+unsigned long approxidate_relative(const char *date, const struct timeval *now);
 enum date_mode parse_date_format(const char *format);
 
 #define IDENT_WARN_ON_NO_NAME  1
diff --git a/date.c b/date.c
index e848d96..8e57e5e 100644
--- a/date.c
+++ b/date.c
@@ -86,6 +86,67 @@ static int local_tzoffset(unsigned long time)
 	return offset * eastwest;
 }
 
+const char *show_date_relative(unsigned long time, int tz,
+			       const struct timeval *now,
+			       char *timebuf,
+			       size_t timebuf_size)
+{
+	unsigned long diff;
+	if (now->tv_sec < time)
+		return "in the future";
+	diff = now->tv_sec - time;
+	if (diff < 90) {
+		snprintf(timebuf, timebuf_size, "%lu seconds ago", diff);
+		return timebuf;
+	}
+	/* Turn it into minutes */
+	diff = (diff + 30) / 60;
+	if (diff < 90) {
+		snprintf(timebuf, timebuf_size, "%lu minutes ago", diff);
+		return timebuf;
+	}
+	/* Turn it into hours */
+	diff = (diff + 30) / 60;
+	if (diff < 36) {
+		snprintf(timebuf, timebuf_size, "%lu hours ago", diff);
+		return timebuf;
+	}
+	/* We deal with number of days from here on */
+	diff = (diff + 12) / 24;
+	if (diff < 14) {
+		snprintf(timebuf, timebuf_size, "%lu days ago", diff);
+		return timebuf;
+	}
+	/* Say weeks for the past 10 weeks or so */
+	if (diff < 70) {
+		snprintf(timebuf, timebuf_size, "%lu weeks ago", (diff + 3) / 7);
+		return timebuf;
+	}
+	/* Say months for the past 12 months or so */
+	if (diff < 360) {
+		snprintf(timebuf, timebuf_size, "%lu months ago", (diff + 15) / 30);
+		return timebuf;
+	}
+	/* Give years and months for 5 years or so */
+	if (diff < 1825) {
+		unsigned long years = diff / 365;
+		unsigned long months = (diff % 365 + 15) / 30;
+		int n;
+		n = snprintf(timebuf, timebuf_size, "%lu year%s",
+				years, (years > 1 ? "s" : ""));
+		if (months)
+			snprintf(timebuf + n, timebuf_size - n,
+					", %lu month%s ago",
+					months, (months > 1 ? "s" : ""));
+		else
+			snprintf(timebuf + n, timebuf_size - n, " ago");
+		return timebuf;
+	}
+	/* Otherwise, just years. Centuries is probably overkill. */
+	snprintf(timebuf, timebuf_size, "%lu years ago", (diff + 183) / 365);
+	return timebuf;
+}
+
 const char *show_date(unsigned long time, int tz, enum date_mode mode)
 {
 	struct tm *tm;
@@ -97,63 +158,10 @@ const char *show_date(unsigned long time, int tz, enum date_mode mode)
 	}
 
 	if (mode == DATE_RELATIVE) {
-		unsigned long diff;
 		struct timeval now;
 		gettimeofday(&now, NULL);
-		if (now.tv_sec < time)
-			return "in the future";
-		diff = now.tv_sec - time;
-		if (diff < 90) {
-			snprintf(timebuf, sizeof(timebuf), "%lu seconds ago", diff);
-			return timebuf;
-		}
-		/* Turn it into minutes */
-		diff = (diff + 30) / 60;
-		if (diff < 90) {
-			snprintf(timebuf, sizeof(timebuf), "%lu minutes ago", diff);
-			return timebuf;
-		}
-		/* Turn it into hours */
-		diff = (diff + 30) / 60;
-		if (diff < 36) {
-			snprintf(timebuf, sizeof(timebuf), "%lu hours ago", diff);
-			return timebuf;
-		}
-		/* We deal with number of days from here on */
-		diff = (diff + 12) / 24;
-		if (diff < 14) {
-			snprintf(timebuf, sizeof(timebuf), "%lu days ago", diff);
-			return timebuf;
-		}
-		/* Say weeks for the past 10 weeks or so */
-		if (diff < 70) {
-			snprintf(timebuf, sizeof(timebuf), "%lu weeks ago", (diff + 3) / 7);
-			return timebuf;
-		}
-		/* Say months for the past 12 months or so */
-		if (diff < 360) {
-			snprintf(timebuf, sizeof(timebuf), "%lu months ago", (diff + 15) / 30);
-			return timebuf;
-		}
-		/* Give years and months for 5 years or so */
-		if (diff < 1825) {
-			unsigned long years = diff / 365;
-			unsigned long months = (diff % 365 + 15) / 30;
-			int n;
-			n = snprintf(timebuf, sizeof(timebuf), "%lu year%s",
-					years, (years > 1 ? "s" : ""));
-			if (months)
-				snprintf(timebuf + n, sizeof(timebuf) - n,
-					", %lu month%s ago",
-					months, (months > 1 ? "s" : ""));
-			else
-				snprintf(timebuf + n, sizeof(timebuf) - n,
-					" ago");
-			return timebuf;
-		}
-		/* Otherwise, just years. Centuries is probably overkill. */
-		snprintf(timebuf, sizeof(timebuf), "%lu years ago", (diff + 183) / 365);
-		return timebuf;
+		return show_date_relative(time, tz, &now,
+					  timebuf, sizeof(timebuf));
 	}
 
 	if (mode == DATE_LOCAL)
@@ -918,19 +926,13 @@ static void pending_number(struct tm *tm, int *num)
 	}
 }
 
-unsigned long approxidate(const char *date)
+static unsigned long approxidate_str(const char *date, const struct timeval *tv)
 {
 	int number = 0;
 	struct tm tm, now;
-	struct timeval tv;
 	time_t time_sec;
-	char buffer[50];
 
-	if (parse_date(date, buffer, sizeof(buffer)) > 0)
-		return strtoul(buffer, NULL, 10);
-
-	gettimeofday(&tv, NULL);
-	time_sec = tv.tv_sec;
+	time_sec = tv->tv_sec;
 	localtime_r(&time_sec, &tm);
 	now = tm;
 
@@ -954,3 +956,25 @@ unsigned long approxidate(const char *date)
 	pending_number(&tm, &number);
 	return update_tm(&tm, &now, 0);
 }
+
+unsigned long approxidate_relative(const char *date, const struct timeval *tv)
+{
+	char buffer[50];
+
+	if (parse_date(date, buffer, sizeof(buffer)) > 0)
+		return strtoul(buffer, NULL, 0);
+
+	return approxidate_str(date, tv);
+}
+
+unsigned long approxidate(const char *date)
+{
+	struct timeval tv;
+	char buffer[50];
+
+	if (parse_date(date, buffer, sizeof(buffer)) > 0)
+		return strtoul(buffer, NULL, 0);
+
+	gettimeofday(&tv, NULL);
+	return approxidate_str(date, &tv);
+}
-- 
1.6.4.2.373.g5881fd

^ permalink raw reply related

* Re: [PATCH 1/3] Add date formatting and parsing functions relative to a given time
From: Jeff King @ 2009-08-31  2:22 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Alex Riesen, git
In-Reply-To: <20090830215127.GA16303@coredump.intra.peff.net>

On Sun, Aug 30, 2009 at 05:51:27PM -0400, Jeff King wrote:

> I think the most sane thing is to rebase the whole series on top of
> lt/approxidate. Let me see what I can do.

And here it is. It was a little more complex than a simple rebase
because lt/approxidate actually introduced new bugs. :) Hopefully this
will be the last re-roll required.

The new series applies on top of lt/approxidate, and contains:

  [1/4]: Add date formatting and parsing functions relative to a given time
  [2/4]: refactor test-date interface
  [3/4]: tests: add date printing and parsing tests
  [4/4]: fix approxidate parsing of relative months and years

-Peff

^ permalink raw reply

* Re: Improve on 'approxidate'
From: Jeff King @ 2009-08-31  1:58 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <20090830223558.GA29807@coredump.intra.peff.net>

On Sun, Aug 30, 2009 at 06:35:58PM -0400, Jeff King wrote:

> > +	tm.tm_year = -1;
> > +	tm.tm_mon = -1;
> > +	tm.tm_mday = -1;
> 
> This breaks relative dates like "3.months.ago", because
> approxidate_alpha needs to see the "current" date in tm (and now it sees
> -1, subtracts from it, and assumes we are just crossing a year boundary
> because of the negative).  3.years.ago is also broken, but I don't think
> 3.days.ago is.
> 
> Probably we just need to pass "now" to approxidate_alpha, and it needs
> to call update_tm under the case for "months" and "years" (and I haven't
> quite figured out why those are not part of the "tl" list).
> Unfortunately, I'm out of time to look at it more right now, but I'll
> take a look tonight or tomorrow if you don't beat me to it.

OK, I looked at it.

The fix is pretty straightforward. We _do_ already pass "now" to
approxidate_alpha, and it looks like you already fixed the "typelen"
array case (which handles seconds, minutes, hours, days, and weeks) by
calling update_tm. But all of those units are convertible to seconds,
and months and years are not, which explains why they are handled
separately.

So I think we can just "cheat" and call update_tm to fill in the fields
from "now" as we would for the other units, and then tweak the "struct
tm" as we did before. I.e.,:

diff --git a/date.c b/date.c
index 8e57e5e..e9ee4aa 100644
--- a/date.c
+++ b/date.c
@@ -857,7 +857,9 @@ static const char *approxidate_alpha(const char *date, struct tm *tm, struct tm
 	}
 
 	if (match_string(date, "months") >= 5) {
-		int n = tm->tm_mon - *num;
+		int n;
+		update_tm(tm, now, 0); /* fill in date fields if needed */
+		n = tm->tm_mon - *num;
 		*num = 0;
 		while (n < 0) {
 			n += 12;
@@ -868,6 +870,7 @@ static const char *approxidate_alpha(const char *date, struct tm *tm, struct tm
 	}
 
 	if (match_string(date, "years") >= 4) {
+		update_tm(tm, now, 0); /* fill in date fields if needed */
 		tm->tm_year -= *num;
 		*num = 0;
 		return end;

I'll wrap this fix up in a commit message with tests and add it to the
"test approxidate" series I'm brewing.

-Peff

^ permalink raw reply related

* Re: git-svn messing up merge commits on dcommit
From: Eric Wong @ 2009-08-31  1:37 UTC (permalink / raw)
  To: Björn Steinbrink; +Cc: git
In-Reply-To: <20090830013225.GA6475@atjola.homenet>

Björn Steinbrink <B.Steinbrink@gmx.de> wrote:
> Hi Eric,
> 
> I have two "test cases" here (attached), which I actually wrote months
> ago, but forgot to sent, as I wanted to clean them up/turn them into
> something suitable for the test suite, but honestly, I'll probably never
> get around to do that, so here they are, as they are. They show git-svn
> messing up merge commits when dcommitting a branch that is not
> up-to-date WRT the svn repo.
> 
> The basic history for both cases (before dcommit) is:
> 
>         C---D (master)
>        /   /
>       /---E (side)
>      /
> A---B (trunk)
>      \
>       X (revision in SVN, not yet fetched)
> 
> So the dcommit (which would send C and D to the svn repo) needs to
> "rebase" C and D.
> 
> In the first test case, this rebasing causes conflicts, and leads to a
> linearized history:
> 
>       E (side)
>      /
> A---B---X---C' (trunk)
>              \
>               D (master)
> 
> The merge is broken apart. This is probably expected, but I thought I'd
> tell anyway.

As noted in the CAVEATS section of the manpage, pull/merge isn't
recommended with dcommit because it generates non-linear history.

Keep in mind this was written before SVN got merge tracking support, but
I haven't been particularly motivated to revisit the topic since then...

> I hope that makes any sense to you (or you can figure it out from the
> testing scripts).

Thanks for posting these, though.  They do seem to be good test cases
for developing on and hopefully somebody with more interest in
this topic can pick up where we leave off.

On a side note, I have been meaning to refactor git svn and break it
apart into separate Perl modules for easier hacking for the longest time
now...

-- 
Eric Wong

^ permalink raw reply

* What your score?
From: Frederik Briggs @ 2009-08-31  2:18 UTC (permalink / raw)
  To: git

You will never find such cheap pils anywhere! http://tsokq.naifliorh.com/

^ permalink raw reply

* Re: how to add an empty initial commit
From: bill lam @ 2009-08-31  0:06 UTC (permalink / raw)
  To: Sean Estabrooks; +Cc: git
In-Reply-To: <BLU0-SMTP13B5A682834BA2E926F610AEF30@phx.gbl>

On Sat, 29 Aug 2009, Sean Estabrooks wrote:
> This sounds like a hard way to go about things.  Instead, you can edit
> the files as you wish, "git add" the new edits, and then use
> "git commit --amend" to alter the initial commit.   Don't think rebase
> would help in the situation you describe.

Sean,

Yes, you are correct. git-rebase does not help much.  It has to initialise
another empty git repo and copy all commits to there.

-- 
regards,
====================================================
GPG key 1024D/4434BAB3 2008-08-24
gpg --keyserver subkeys.pgp.net --recv-keys 4434BAB3

^ permalink raw reply

* Re: [PATCH] Documentation/git-add.txt: Explain --patch option in layman terms
From: Junio C Hamano @ 2009-08-30 23:31 UTC (permalink / raw)
  To: Jari Aalto; +Cc: git
In-Reply-To: <87ab1gaol2.fsf@jondo.cante.net>

Jari Aalto <jari.aalto@cante.net> writes:

> I think the convention used in git's manual pages deviate from the
> standard practise. We could make the git manual pages into line of:
>
> - write all the first level headings in all caps: "HEADING LIKE THIS"
> - write second level heading: start Upper-lower: "Heading like this"
>
> Cf. rsync(1), ssh(1) etc. many pages prior git's existense.

Having seen that nothing happened after a separate thread that was also on
the documentation consistency:

    http://thread.gmane.org/gmane.comp.version-control.git/72163/focus=72213

I am having a hard time to decide how seriously I should take the above
comment from you.

Are you volunteering to coordinate such a change (in other words, you do
not necessarily have to do _all_ the work yourself, alone), or is it just
an idle speculation?

^ permalink raw reply

* Re: [PATCH] Documentation/git-add.txt: Explain --patch option in layman terms
From: Junio C Hamano @ 2009-08-30 23:20 UTC (permalink / raw)
  To: Jari Aalto; +Cc: git, Junio C Hamano
In-Reply-To: <87ab1gaol2.fsf@jondo.cante.net>

Jari Aalto <jari.aalto@cante.net> writes:

> Your proposal that starts:
>
>     ...but bypass the initial command menu

No, it doesn't..

Go re-read the message you are responding to, paying extra attention to
the parts you snipped from your quote, which was the important part you
should have read before you responded.

    If you want to start the description with "What it does/what it is used
    for", I think it is a good idea.  I already made a suggestion for such an
    improvement in my message you are responding to.

Now, what was that suggestion?

It is in the message your first response was a follow-up to.  Again you
didn't quote the relevant part in that response, and perhaps that was
because you did not even read it before responding.

    If you assume that the reader is not familiar with "add -i", then the
    above is not descriptive enough, but "Run interactive patch command" is
    not an improvement either.  We would need a description of "what it is
    used for" before "how it would look to you" (i.e.. my rewrite shown
    above).

    "What it is used for" would perhaps read like this.

            Review the difference between the index and the work tree, and add
            modified contents to the index interactively by choosing which
            patch hunks to use.

This time I re-quoted things for you because your responses obviously were
written without reading or understanding them, but please be careful not
to make me do this.  I do not have infinite time.

^ permalink raw reply

* Re: [PATCH] Documentation/git-add.txt: Explain --patch option in layman terms
From: Jari Aalto @ 2009-08-30 23:06 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, jari.aalto
In-Reply-To: <7vskf954sr.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> Sections that are common in all manual pages (e.g. NAME, SYNOPSIS,
> DESCRIPTION, EXAMPLES, SEE ALSO) are often spelled in and referred to in
> caps. 

Not just common ones. All sections that are top level heading are best
spelled out consistently. Examples can be found from the URL to
POSIX/Susv in my other post.

[I'll get back to the CAPS patch in anaother post if we can sort this out]

> See http://www.kernel.org/pub/software/scm/git/docs/git-add.html#_interactive_mode
> for what I mean.

I think the convention used in git's manual pages deviate from the
standard practise. We could make the git manual pages into line of:

- write all the first level headings in all caps: "HEADING LIKE THIS"
- write second level heading: start Upper-lower: "Heading like this"

Cf. rsync(1), ssh(1) etc. many pages prior git's existense.

>>> I personally think fixing misworded phrase "initial command loop" would be
>>> sufficient.  It should read "initial command menu".  Perhaps like this.
>>>
>>> 	Run ``add --interactive``, but bypass the initial command menu and
>>> 	directly jump to `patch` subcommand.  See ``Interactive mode'' for
>>> 	details.
>>
>> It's still too technical. The 1st line should go right into business:
>>
>>  	Patch each file on command line interactively. This is this is
>>  	the same as ``add --interactive``, but bypass the initial
>>  	command menu and directly jump to `patch` subcommand. See
>>  	``Interactive mode'' for details.
>
> I do not think it is better than the original.

Your proposal that starts:

    ...but bypass the initial command menu

Mine:

    Patch each file on command line interactively

The first line should somehow strike immediately what the command does.
I would like to see a suggestion that has 'patch(ing)' somewhere at the
very first row. I hope we can find compromise.

Jari

^ permalink raw reply

* Re: Improve on 'approxidate'
From: Jeff King @ 2009-08-30 22:35 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <alpine.LFD.2.01.0908221438450.3158@localhost.localdomain>

On Sat, Aug 22, 2009 at 03:10:07PM -0700, Linus Torvalds wrote:

>  unsigned long approxidate(const char *date)
>  {
>  	int number = 0;
> @@ -881,21 +911,24 @@ unsigned long approxidate(const char *date)
>  	time_sec = tv.tv_sec;
>  	localtime_r(&time_sec, &tm);
>  	now = tm;
> +
> +	tm.tm_year = -1;
> +	tm.tm_mon = -1;
> +	tm.tm_mday = -1;

This breaks relative dates like "3.months.ago", because
approxidate_alpha needs to see the "current" date in tm (and now it sees
-1, subtracts from it, and assumes we are just crossing a year boundary
because of the negative).  3.years.ago is also broken, but I don't think
3.days.ago is.

Probably we just need to pass "now" to approxidate_alpha, and it needs
to call update_tm under the case for "months" and "years" (and I haven't
quite figured out why those are not part of the "tl" list).
Unfortunately, I'm out of time to look at it more right now, but I'll
take a look tonight or tomorrow if you don't beat me to it.

-Peff

^ permalink raw reply

* A note from the maintainer: Follow-up questions (MaintNotes)
From: David Chanters @ 2009-08-30 22:19 UTC (permalink / raw)
  To: git; +Cc: David Chanters

Hi,

[Please retain a Cc back to me as I am not currently subscribed to the
mailing list.]

I've recently been intrigued with workflows, and have read quite a bit
about them, including various references on git-scm.com,
gitworkflows(7), and the email "A note from the maintainer" which I
have some questions on.  I'll paste random quotes from that and just
ask my question, I think, so apologies up front of it reads a little
disjointed.

I'd often wondered when I have read various posts of the git mailing
list on gmane, just how it is I am supposed to track:

dc/some-topic-feature

... Junio, are these topic branches ones you actively have somewhere
in your own private checkout?  Yes, I appreciate that when I read a
given post to the mailing list, you or other people will sometimes
make reference to these topic branches, but what do I do if I am
interested in finding out about one of them?  Indeed, perhaps even
before getting to that question, how do you go about creating and
maintaining these topic branches -- are you making heavy use of "git
am"?

I ask because of the following snippet from "MaintNotes":

    The two branches "master" and "maint" are never rewound, and
    "next" usually will not be either (this automatically means the
    topics that have been merged into "next" are usually not
    rebased, and you can find the tip of topic branches you are
    interested in from the output of "git log next"). You should be
    able to safely track them.

I am not sure if there's any real use-case for this, but I will ask
anyway:  is the above saying that I am able to *checkout* one of these
topic-branches just from their presence in "next" alone?  I appreciate
that the point is somewhat moot since the topic branch has already
been merged into "next", but I can surely see this as a really useful
way for people to manage topic-branches in a shared environment:
people can simply pick a topic branch out from the integrated one --
in this case "next".  Or is this idea a complete waste of time?

To continue:

    The "pu" (proposed updates) branch bundles all the remainder of
    topic branches.  The "pu" branch, and topic branches that are
    only in "pu", are subject to rebasing in general.  By the above
    definition of how "next" works, you can tell that this branch
    will contain quite experimental and obviously broken stuff.

I'm obviously missing something here -- but why is rebasing these
existing topic branches (I assume on top of "pu") more useful than
just merging them into "pu" -- like you do with "next"?

    When a topic that was in "pu" proves to be in testable shape, it
    graduates to "next".  I do this with:

            git checkout next
            git merge that-topic-branch

    Sometimes, an idea that looked promising turns out to be not so
    good and the topic can be dropped from "pu" in such a case.

Ah -- so if I have this straight in my head -- you continually form
the local topic-branch on its own branch, and then just merge it into
"next" only when you know that topic branch is satisfactory?  That
being the case -- again, I assume the use of "git am" for the topic
branch?  If regular readers of the git mailing list wish to track this
topic branch, can they do so from you only until it's merged into
"next"?

And a related question:  If you decide a given topic in pu is declared
to "be dropped", is this done by rebasing (as you mentioned earlier)
so as to remove any trace of the topic branch ever having been in
"pu", or am I reading too much into "dropping" here?  :)

I hope these aren't too idiotic.

Thanks all in advance.

David

^ permalink raw reply

* Re: [PATCH] Documentation/git-add.txt: Explain --patch option in layman terms
From: Junio C Hamano @ 2009-08-30 22:13 UTC (permalink / raw)
  To: Jari Aalto; +Cc: git
In-Reply-To: <87tyzp9da4.fsf@jondo.cante.net>

Jari Aalto <jari.aalto@cante.net> writes:

> It is not shouting, but standard practise to refer to manual page
> section in ALL CAPS, when they are top level headings, like in this
> case.

Why are you making excuses, ignoring the fact that you didn't have a
matching update to make the section also in caps in the patch?

Sections that are common in all manual pages (e.g. NAME, SYNOPSIS,
DESCRIPTION, EXAMPLES, SEE ALSO) are often spelled in and referred to in
caps.  You do not have to explain that to me ;-)

If you wanted to add "Interactive mode" to that set of "common sections"
and spell it in caps, do so consistently.

See http://www.kernel.org/pub/software/scm/git/docs/git-add.html#_interactive_mode
for what I mean.

>> I personally think fixing misworded phrase "initial command loop" would be
>> sufficient.  It should read "initial command menu".  Perhaps like this.
>>
>> 	Run ``add --interactive``, but bypass the initial command menu and
>> 	directly jump to `patch` subcommand.  See ``Interactive mode'' for
>> 	details.
>
> It's still too technical. The 1st line should go right into business:
>
>  	Patch each file on command line interactively. This is this is
>  	the same as ``add --interactive``, but bypass the initial
>  	command menu and directly jump to `patch` subcommand. See
>  	``Interactive mode'' for details.

Even if we ignore the double "this is this is", I do not think it is
better than the original.

What does "Patch each file" mean?  When read naively (and that is the
whole point of your "too technical" comment), a reader would expect there
will be changes made _to_ the work tree files.

If you want to start the description with "What it does/what it is used
for", I think it is a good idea.  I already made a suggestion for such an
improvement in my message you are responding to.

If you want to make a counterproposal, at least please do that with a
counter-proposal that is better.

^ permalink raw reply

* Re: [PATCH] Documentation/git-add.txt: Explain --patch option in layman terms
From: Jari Aalto @ 2009-08-30 22:00 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vab1hdppb.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

>> +	operation to a subset of the working tree. See section
>> +	``INTERACTIVE MODE'' for details.
>
> It is not justified with your commit log message, I do not see why you
> have to shout in all CAPS, 

There are plenty of examples, that it's standard practise to refer top
level headings, in all caps, from:

    POSIX/SusV guides for manual pages: "1.11 Utility Description Defaults"
    http://www.opengroup.org/onlinepubs/009695399/utilities/xcu_chap01.html#tag_01_11

Jari

^ permalink raw reply

* Re: [PATCH] Documentation/git-add.txt: Explain --patch option in layman terms
From: Jari Aalto @ 2009-08-30 21:56 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, jari.aalto
In-Reply-To: <7vab1hdppb.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> Jari Aalto <jari.aalto@cante.net> writes:
>
>> Signed-off-by: Jari Aalto <jari.aalto@cante.net>
>> ---
>>  Documentation/git-add.txt |   10 +++++-----
>>  1 files changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
>> index e67b7e8..71990c2 100644
>> --- a/Documentation/git-add.txt
>> +++ b/Documentation/git-add.txt
>> @@ -67,14 +67,14 @@ OPTIONS
>>  --interactive::
>>  	Add modified contents in the working tree interactively to
>>  	the index. Optional path arguments may be supplied to limit
>> -	operation to a subset of the working tree. See ``Interactive
>> -	mode'' for details.
>> +	operation to a subset of the working tree. See section
>> +	``INTERACTIVE MODE'' for details.
>
> It is not justified with your commit log message, I do not see why you
> have to shout in all CAPS, and there is no such section in the
> documentation.  But the "Interactive mode" section exists and is referred
> to by the original.

It is not shouting, but standard practise to refer to manual page
section in ALL CAPS, when they are top level headings, like in this
case.

>>  -p::
>>  --patch::
>> -	Similar to Interactive mode but the initial command loop is
>> -	bypassed and the 'patch' subcommand is invoked using each of
>> -	the specified filepatterns before exiting.
>> +	Run interactive patch command for each file on command line.
>> +	See section INTERACTIVE MODE and patch subcommand for more
>> +	information.
>
> I personally think fixing misworded phrase "initial command loop" would be
> sufficient.  It should read "initial command menu".  Perhaps like this.
>
> 	Run ``add --interactive``, but bypass the initial command menu and
> 	directly jump to `patch` subcommand.  See ``Interactive mode'' for
> 	details.

It's still too technical. The 1st line should go right into business:

 	Patch each file on command line interactively. This is this is
 	the same as ``add --interactive``, but bypass the initial
 	command menu and directly jump to `patch` subcommand. See
 	``Interactive mode'' for details.

Jari

^ permalink raw reply

* Re: [PATCH 1/3] Add date formatting and parsing functions relative to a given time
From: Jeff King @ 2009-08-30 21:51 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Alex Riesen, git
In-Reply-To: <20090830214309.GA16119@coredump.intra.peff.net>

On Sun, Aug 30, 2009 at 05:43:09PM -0400, Jeff King wrote:

> From: Alex Riesen <raa.lkml@gmail.com>
> 
> The main purpose is to allow predictable testing of the code.
> 
> Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
> Signed-off-by: Jeff King <peff@peff.net>
> ---

Bleh. I just started working on a 4/3 that would test Linus' recent
approxidate changes, but then I realized that this massive date.c
refactoring conflicts with his changes.

I think the most sane thing is to rebase the whole series on top of
lt/approxidate. Let me see what I can do.

-Peff

^ 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