Git development
 help / color / mirror / Atom feed
* [PATCH v3.1 3/5] Introduce new pretty formats %g[sdD] for reflog information
From: Thomas Rast @ 2009-10-17 14:39 UTC (permalink / raw)
  To: Jeff King, Junio C Hamano; +Cc: Jef Driesen, Nanako Shiraishi, git
In-Reply-To: <cover.1255789678.git.trast@student.ethz.ch>

Add three new --pretty=format escapes:

  %gD  long  reflog descriptor (e.g. refs/stash@{0})
  %gd  short reflog descriptor (e.g. stash@{0})
  %gs  reflog message

This is achieved by passing down the reflog info, if any, inside the
pretty_print_context struct.

We use the newly refactored get_reflog_selector(), and give it some
extra functionality to extract a shortened ref.  The shortening is
cached inside the commit_reflogs struct; the only allocation of it
happens in read_complete_reflog(), where it is initialised to 0.  Also
add another helper get_reflog_message() for the message extraction.

Note that the --format="%h %gD: %gs" tests may not work in real
repositories, as the --pretty formatter doesn't know to leave away the
": " on the last commit in an incomplete (because git-gc removed the
old part) reflog.  This equivalence is nevertheless the main goal of
this patch.

Thanks to Jeff King for reviews, the %gd testcase and documentation.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

[Tacking this on the corresponding v3 patch, but the text below is
from <20091017011801.GA31443@coredump.intra.peff.net>.]

Jeff King wrote:
> I am tempted to do a note like the one below, but maybe the behavior is
> obvious enough to users that it isn't necessary.
[...]
> +NOTE: Some placeholders may depend on other options given to the
> +revision traversal engine. For example, the `%g*` reflog options will
> +insert an empty string unless we are traversing reflog entries (e.g., by
> +`git log -g`). The `%d` placeholder will use the "short" decoration
> +format if `--decorate` was not already provided on the command line.

Sure.  I figured it was obvious enough, but since you already went to
the lengths of documenting the exact behaviour of %d, I squashed it
and adjusted the acknowledgement in the commit message accordingly.


 Documentation/pretty-formats.txt |    9 +++++++++
 commit.h                         |    1 +
 log-tree.c                       |    1 +
 pretty.c                         |   17 +++++++++++++++++
 reflog-walk.c                    |   35 ++++++++++++++++++++++++++++++++---
 reflog-walk.h                    |    8 ++++++++
 t/t6006-rev-list-format.sh       |   18 ++++++++++++++++++
 7 files changed, 86 insertions(+), 3 deletions(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 2a845b1..38b9904 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -123,6 +123,9 @@ The placeholders are:
 - '%s': subject
 - '%f': sanitized subject line, suitable for a filename
 - '%b': body
+- '%gD': reflog selector, e.g., `refs/stash@\{1\}`
+- '%gd': shortened reflog selector, e.g., `stash@\{1\}`
+- '%gs': reflog subject
 - '%Cred': switch color to red
 - '%Cgreen': switch color to green
 - '%Cblue': switch color to blue
@@ -132,6 +135,12 @@ The placeholders are:
 - '%n': newline
 - '%x00': print a byte from a hex code
 
+NOTE: Some placeholders may depend on other options given to the
+revision traversal engine. For example, the `%g*` reflog options will
+insert an empty string unless we are traversing reflog entries (e.g., by
+`git log -g`). The `%d` placeholder will use the "short" decoration
+format if `--decorate` was not already provided on the command line.
+
 * 'tformat:'
 +
 The 'tformat:' format works exactly like 'format:', except that it
diff --git a/commit.h b/commit.h
index 011766d..15cb649 100644
--- a/commit.h
+++ b/commit.h
@@ -70,6 +70,7 @@ struct pretty_print_context
 	const char *after_subject;
 	enum date_mode date_mode;
 	int need_8bit_cte;
+	struct reflog_walk_info *reflog_info;
 };
 
 extern int non_ascii(int);
diff --git a/log-tree.c b/log-tree.c
index f57487f..8e782fc 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -409,6 +409,7 @@ void show_log(struct rev_info *opt)
 	ctx.date_mode = opt->date_mode;
 	ctx.abbrev = opt->diffopt.abbrev;
 	ctx.after_subject = extra_headers;
+	ctx.reflog_info = opt->reflog_info;
 	pretty_print_commit(opt->commit_format, commit, &msgbuf, &ctx);
 
 	if (opt->add_signoff)
diff --git a/pretty.c b/pretty.c
index d6d57eb..fc65fca 100644
--- a/pretty.c
+++ b/pretty.c
@@ -7,6 +7,7 @@
 #include "mailmap.h"
 #include "log-tree.h"
 #include "color.h"
+#include "reflog-walk.h"
 
 static char *user_format;
 
@@ -701,6 +702,22 @@ static size_t format_commit_item(struct strbuf *sb, const char *placeholder,
 	case 'd':
 		format_decoration(sb, commit);
 		return 1;
+	case 'g':		/* reflog info */
+		switch(placeholder[1]) {
+		case 'd':	/* reflog selector */
+		case 'D':
+			if (c->pretty_ctx->reflog_info)
+				get_reflog_selector(sb,
+						    c->pretty_ctx->reflog_info,
+						    c->pretty_ctx->date_mode,
+						    (placeholder[1] == 'd'));
+			return 2;
+		case 's':	/* reflog message */
+			if (c->pretty_ctx->reflog_info)
+				get_reflog_message(sb, c->pretty_ctx->reflog_info);
+			return 2;
+		}
+		return 0;	/* unknown %g placeholder */
 	}
 
 	/* For the rest we have to parse the commit header. */
diff --git a/reflog-walk.c b/reflog-walk.c
index 596bafe..caba4f7 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -8,6 +8,7 @@
 
 struct complete_reflogs {
 	char *ref;
+	const char *short_ref;
 	struct reflog_info {
 		unsigned char osha1[20], nsha1[20];
 		char *email;
@@ -243,15 +244,26 @@ void fake_reflog_parent(struct reflog_walk_info *info, struct commit *commit)
 
 void get_reflog_selector(struct strbuf *sb,
 			 struct reflog_walk_info *reflog_info,
-			 enum date_mode dmode)
+			 enum date_mode dmode,
+			 int shorten)
 {
 	struct commit_reflog *commit_reflog = reflog_info->last_commit_reflog;
 	struct reflog_info *info;
+	const char *printed_ref;
 
 	if (!commit_reflog)
 		return;
 
-	strbuf_addf(sb, "%s@{", commit_reflog->reflogs->ref);
+	if (shorten) {
+		if (!commit_reflog->reflogs->short_ref)
+			commit_reflog->reflogs->short_ref
+				= shorten_unambiguous_ref(commit_reflog->reflogs->ref, 0);
+		printed_ref = commit_reflog->reflogs->short_ref;
+	} else {
+		printed_ref = commit_reflog->reflogs->ref;
+	}
+
+	strbuf_addf(sb, "%s@{", printed_ref);
 	if (commit_reflog->flag || dmode) {
 		info = &commit_reflog->reflogs->items[commit_reflog->recno+1];
 		strbuf_addstr(sb, show_date(info->timestamp, info->tz, dmode));
@@ -263,6 +275,23 @@ void get_reflog_selector(struct strbuf *sb,
 	strbuf_addch(sb, '}');
 }
 
+void get_reflog_message(struct strbuf *sb,
+			struct reflog_walk_info *reflog_info)
+{
+	struct commit_reflog *commit_reflog = reflog_info->last_commit_reflog;
+	struct reflog_info *info;
+	size_t len;
+
+	if (!commit_reflog)
+		return;
+
+	info = &commit_reflog->reflogs->items[commit_reflog->recno+1];
+	len = strlen(info->message);
+	if (len > 0)
+		len--; /* strip away trailing newline */
+	strbuf_add(sb, info->message, len);
+}
+
 void show_reflog_message(struct reflog_walk_info *reflog_info, int oneline,
 	enum date_mode dmode)
 {
@@ -272,7 +301,7 @@ void show_reflog_message(struct reflog_walk_info *reflog_info, int oneline,
 		struct strbuf selector = STRBUF_INIT;
 
 		info = &commit_reflog->reflogs->items[commit_reflog->recno+1];
-		get_reflog_selector(&selector, reflog_info, dmode);
+		get_reflog_selector(&selector, reflog_info, dmode, 0);
 		if (oneline) {
 			printf("%s: %s", selector.buf, info->message);
 		}
diff --git a/reflog-walk.h b/reflog-walk.h
index 74c9096..7bd2cd4 100644
--- a/reflog-walk.h
+++ b/reflog-walk.h
@@ -3,6 +3,8 @@
 
 #include "cache.h"
 
+struct reflog_walk_info;
+
 extern void init_reflog_walk(struct reflog_walk_info** info);
 extern int add_reflog_for_walk(struct reflog_walk_info *info,
 		struct commit *commit, const char *name);
@@ -10,5 +12,11 @@ extern void fake_reflog_parent(struct reflog_walk_info *info,
 		struct commit *commit);
 extern void show_reflog_message(struct reflog_walk_info *info, int,
 		enum date_mode);
+extern void get_reflog_message(struct strbuf *sb,
+		struct reflog_walk_info *reflog_info);
+extern void get_reflog_selector(struct strbuf *sb,
+		struct reflog_walk_info *reflog_info,
+		enum date_mode dmode,
+		int shorten);
 
 #endif
diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index 59d1f62..7f61ab0 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -162,4 +162,22 @@ test_expect_success 'empty email' '
 	}
 '
 
+test_expect_success '"%h %gD: %gs" is same as git-reflog' '
+	git reflog >expect &&
+	git log -g --format="%h %gD: %gs" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success '"%h %gD: %gs" is same as git-reflog (with date)' '
+	git reflog --date=raw >expect &&
+	git log -g --format="%h %gD: %gs" --date=raw >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success '%gd shortens ref name' '
+	echo "master@{0}" >expect.gd-short &&
+	git log -g -1 --format=%gd refs/heads/master >actual.gd-short &&
+	test_cmp expect.gd-short actual.gd-short
+'
+
 test_done
-- 
1.6.5.116.g4aaa3

^ permalink raw reply related

* [PATCH] bash: complete more options for 'git rebase'
From: Björn Gustavsson @ 2009-10-17  9:33 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git

Complete all long options for 'git rebase' except --no-verify
(probably used very seldom) and the long options corresponding
to -v, -q, and -f.

Signed-off-by: Björn Gustavsson <bgustavsson@gmail.com>
---
I am primarily interested in having the --whitespace= option completed,
but while at it I have added completion for the other potentially useful
long options.

 contrib/completion/git-completion.bash |   12 +++++++++++-
 1 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index d3fec32..7c7318c 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1323,8 +1323,18 @@ _git_rebase ()
 	fi
 	__git_complete_strategy && return
 	case "$cur" in
+	--whitespace=*)
+		__gitcomp "$__git_whitespacelist" "" "${cur##--whitespace=}"
+		return
+		;;
 	--*)
-		__gitcomp "--onto --merge --strategy --interactive"
+		__gitcomp "
+			--onto --merge --strategy --interactive
+			--preserve-merges --stat --no-stat
+			--committer-date-is-author-date --ignore-date
+			--ignore-whitespace --whitespace=
+			"
+
 		return
 	esac
 	__gitcomp "$(__git_refs)"
-- 
1.6.5.2.gd6127

^ permalink raw reply related

* Re: [PATCH] Proof-of-concept patch to remember what the detached HEAD was
From: Junio C Hamano @ 2009-10-17  9:04 UTC (permalink / raw)
  To: Björn Steinbrink
  Cc: Julian Phillips, Daniel Barkalow, James Pickens, Jeff King,
	Nicolas Pitre, Jay Soffian, git
In-Reply-To: <20091017084025.GC5474@atjola.homenet>

Björn Steinbrink <B.Steinbrink@gmx.de> writes:

> Interestingly, I irregularly give advice to use "git stash; git
> checkout; git stash apply" instead of "git checkout -m" on #git, as that
> allows you to try again if you messed up the conflict resolution, and
> even allows you to completely go back to the initial state. Maybe that
> would be an option?

As I already said many times (here and elsewhere), "up" is an inferior and
more dangerous model we would be better off not following if we can.

It is not even entirely CVS/SVN's fault that their "scm up" is an error
prone operation.  They use a centralized model, in which there is _no_ way
to record your local modification and run a retryable merge, so they have
an excuse to force users to do things in "work, then update without
recording wip anywhere" order.

You can afford to (and it is even more natural to) do things in "work,
save and then merge" order with git.  I simply cannot believe people who
advocate for helping uninitiated would even think of modelling any "user
friendliness" features around "up" model.

The "save" part of the work-save-then-merge sequence should be made very
visible to help people get used to the "not up, but work-save-then-merge"
mental model.  I do not think it would help people in the long run to make
the "save" step less visible by wrapping the sequence into an unreliable
"up" script, especially because the script would sometimes work but other
times *has to* force users to know that what is happening behind the scene
is work-save-then-merge in order to resolve and recover from conflicts
anyway.

> OTOH, it might be easier to just tell the user to do the stash thing
> himself. But I wonder how many users would really know how to get back
> to the initial state then.

I agree with the first sentence, but I do not understand what "the initial
state" you talk about here in the second sentence, sorry.

^ permalink raw reply

* Re: [PATCH] Proof-of-concept patch to remember what the detached HEAD was
From: Björn Steinbrink @ 2009-10-17  8:40 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Julian Phillips, Daniel Barkalow, James Pickens, Jeff King,
	Nicolas Pitre, Jay Soffian, git
In-Reply-To: <7vws2ue8yc.fsf@alter.siamese.dyndns.org>

On 2009.10.17 01:11:23 -0700, Junio C Hamano wrote:
> Björn Steinbrink <B.Steinbrink@gmx.de> writes:
> 
> > ... If so, it does a "git checkout --merge
> > <upstream>" (possibly leaving conflicts for the uncommitted changes,
> > just like "svn update").
> 
> Up to this point I was reading with quite a lot of interest.  But here I
> strongly disagree to the point of getting actually disgusted.
> 
> "svn up" is one of the areas Subversion folks failed to make their system
> a better CVS.  It has the same "local changes are lost in the merge
> conflict mess in an irreversible way" failure mode, and we shouldn't be
> making it easy to new people.  It is not something we should emulate.
> 
> You can and should instead refuse the update, and suggest committing first
> so that the user has a safe record of what he has done and the merge with
> upstream can be retried if necessary.  As you need to have that "refuse
> but guide the lost soul by telling what to do" mode anyway when...

Hm, probably I (mentally) focussed to much on "people that just want to
look" and didn't really think that they'd have much to lose. Not sure.
Over the time, I got so used to do "git checkout --merge <something>"
with some I-don't-care-much debugging or whatever change, that I lost
track of how bad that was with svn. If I have just I care about, I
commit, and then "git update" would no longer be an option anyway. But
yeah, not a good option to give to the uninitiated user...

Interestingly, I irregularly give advice to use "git stash; git
checkout; git stash apply" instead of "git checkout -m" on #git, as that
allows you to try again if you messed up the conflict resolution, and
even allows you to completely go back to the initial state. Maybe that
would be an option? But I fail to come up with a convenient and at the
same time error safe way for such a "stash wrap". The only thing that
comes to mind would be to have "git update" completely wrapping the
thing:

git update
 - Stashes uncommitted changes
 - fetches
 - checks out
 - "stash apply --index", if that fails, just "stash apply"

If there are conflicts, show options (similar to what rebase does):

git update --retry # Start over
 - reset --hard
 - "stash apply --index", if that fails, just "stash apply"

git update --abort
 - git reset --hard HEAD@{1}
 - "stash apply --index"
 - "stash drop stash@{0}"

git update --done
 - Check whether there are still unmerged files, if so: complain
 - otherwise: "stash drop stash@{0}"


OTOH, it might be easier to just tell the user to do the stash thing
himself. But I wonder how many users would really know how to get back
to the initial state then.

Björn

^ permalink raw reply

* Re: [PATCH] Proof-of-concept patch to remember what the detached HEAD was
From: Björn Steinbrink @ 2009-10-17  8:19 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Christoph Bartoschek, git, Daniel Barkalow
In-Reply-To: <7vr5t2h3do.fsf@alter.siamese.dyndns.org>

On 2009.10.17 00:43:31 -0700, Junio C Hamano wrote:
> Christoph Bartoschek <bartoschek@gmx.de> writes:
> > Daniel Barkalow wrote:
> >
> >> The upshot of the messages should be:
> >> 
> >>  $ git checkout origin/master
> >>  Since you can't actually change "origin/master" yourself, you'll just
> >>  be sightseeing unless you create a local branch to hold new local work.
> >> 
> >>  $ git branch
> >>  * (not a local branch, but "origin/master")
> >> 
> >>  $ git commit
> >>  You've been sightseeing "origin/master". The commit can't change that
> >>  value, so your commit isn't held in any branch. If you want to create
> >>  a branch to hold it, here's how.

[...]

> The second item in the Daniel's transcript above may be an improvement but
> I think it is a wrong economy to record and show 'but "origin/master"'
> (which cannot be correct forever and has to be invalidated once the user
> starts committing or resetting) in the message.

I don't think it's entirely wrong to record that information, git just
has to know when to invalidate it, possibly requiring the user to really
detach HEAD.

git checkout origin/master
git checkout origin/master~3
git checkout HEAD^2~5
git reset --hard HEAD~2

Those commands are all about walking the ancestry of origin/master in
some way. So it seems reasonable to assume that HEAD is still weakly
bound to origin/master. And based upon that, there could be something
like "git update", and things like "git status" could show that you're
browsing through the ancestry of origin/master, and that "git commit"
message could maybe say "You've been sightseeing 'origin/master'
[currently at 'origin/master~3^2~7'] ...".

> I am wondering if a similar effect to help new users can be had by
> rewording the message to:
> 
>     $ git branch
>     * (not a local branch; see "git reflog" to learn how you got here)
> 
> The user can see how he got there even after doing something else after
> the checkout (see Nico's write-up in $gmane/130527).  The difference is
> between giving fish and teaching how to catch one himself.

That could be used when the user actively detached HEAD, invalidating
the "weak binding". Maybe implicitly by "git commit" or 
"git reset <something_we_can't_keep_track_of>", or maybe explicitly,
if committing on a "semi-detached HEAD" becomes forbidden.

Or maybe it could always be shown, in addition to "You are here", you
also get told "Do this to find out how you got there". Does seem like a
good idea.

Björn

^ permalink raw reply

* Re: [PATCH] Proof-of-concept patch to remember what the detached HEAD was
From: Junio C Hamano @ 2009-10-17  8:11 UTC (permalink / raw)
  To: Björn Steinbrink
  Cc: Julian Phillips, Daniel Barkalow, James Pickens, Jeff King,
	Nicolas Pitre, Jay Soffian, git
In-Reply-To: <20091017075551.GA5474@atjola.homenet>

Björn Steinbrink <B.Steinbrink@gmx.de> writes:

> ... If so, it does a "git checkout --merge
> <upstream>" (possibly leaving conflicts for the uncommitted changes,
> just like "svn update").

Up to this point I was reading with quite a lot of interest.  But here I
strongly disagree to the point of getting actually disgusted.

"svn up" is one of the areas Subversion folks failed to make their system
a better CVS.  It has the same "local changes are lost in the merge
conflict mess in an irreversible way" failure mode, and we shouldn't be
making it easy to new people.  It is not something we should emulate.

You can and should instead refuse the update, and suggest committing first
so that the user has a safe record of what he has done and the merge with
upstream can be retried if necessary.  As you need to have that "refuse
but guide the lost soul by telling what to do" mode anyway when...

> ... If a fast-forward is not possible, it
> complains, telling the user that he needs to use "git merge/rebase/pull"
> instead, and might want to create a branch head, in case of a detached
> HEAD.

^ permalink raw reply

* Re: [PATCH] Proof-of-concept patch to remember what the detached HEAD was
From: Björn Steinbrink @ 2009-10-17  7:55 UTC (permalink / raw)
  To: Julian Phillips
  Cc: Daniel Barkalow, James Pickens, Jeff King, Junio C Hamano,
	Nicolas Pitre, Jay Soffian, git
In-Reply-To: <alpine.LNX.2.00.0910161821230.30589@reaper.quantumfyre.co.uk>

On 2009.10.16 18:31:23 +0100, Julian Phillips wrote:
> On Fri, 16 Oct 2009, Bj?rn Steinbrink wrote:
> >On 2009.10.16 13:15:35 +0100, Julian Phillips wrote:
> >>How about:
> >>
> >>$ git checkout origin/master
> >>$ git fetch
> >>Refusing to fetch, as it would update a checkedout branch
> >>"git fetch -f" will force the update, but you will need to run "git
> >>reset --hard HEAD" to update your checkout to match.
> >
> >That would redefine -f (currently means "allow non-fast-forward
> >updates"), the flag that allows the checked out branch head to be
> >updated is -u, --update-head-ok, and is for internal use only.
> >
> >And suggesting "reset --hard" seems wrong, that just kills any
> >uncommitted changes.
> 
> Ok, so the commands were wrong.  Not important.
> 
> It was the approach that I was trying to suggest rather than the
> actual commands.  The point I was trying to make was how, as a user,
> I would be happy to git behave.

Your approach explicitly included "mess up the index/worktree state",
otherwise, "git fetch" would not have to tell the user that he has to do
a "git reset --hard HEAD". I honestly can't believe that you would be
happy with git messing up your work.

> So, I try to run fetch, git says "ooh, now that would be dangerous -
> you can force it happen by running "git foo", but you will then be
> in situation X, which you can then recover from by running "git
> bar", though you may need to run "git stash" to save any edits you
> have made" or something similar.

But why make "git fetch" with non-"obscure" refspecs dangerous to begin
with? If we detach but keep some extra information, there's no need to
make "git fetch" dangerous, _and_ we can still provide a command that
just fetches the most recent version of the "checked out" remote
tracking branch and checks that out. May it be another mode of operation
for "git pull" or some "git up" command or whatever.

> >And such uncommitted changes would be lost in the big "undo the fetch
> >update" diff. So you'd have to do:
> >git reset --soft HEAD@{1}
> >git checkout --merge HEAD@{1}
> >
> >to keep them, while updating to the new state of the remote tracking
> >branch. Not quite intuitive, is it?
> 
> I don't care what git has to do, I'm talking about the user
> experience - if we have to write some new code to support it, that
> really isn't a terribly hard thing to do.  UIs should be driven down
> from the user interaction not up from the implementation.

Those commands are those that git would have to show to you, instead of
"git reset --hard HEAD", i.e. they're what you, as the user, have to do.
And while "git reset --hard HEAD" might be remotely understandable to
the user, that command sequence is very unlikely to be understood by
most. And providing a command that does just this sequence is insane,
it's just a bandaid for git doing crap to your worktree/index state. So
let's better not start with git doing that crap at all.

My current "idea" (I don't think I'll have time to implement that any
time soon) is:

Keep some extra information in HEAD (or somewhere else) when HEAD is
detached about the ref that HEAD is "weakly" bound to. For example, "git
checkout origin/master" might create a weak binding to
refs/remotes/origin/master (for other [corner] cases, see the other
mail I wrote, in which I outlined some cases I considered interesting).

"git update" can use the branch.<name>.{remote,merge} setup, or
alternatively the "weak binding" information, to fetch from the right
remote, and checks whether a fast-forward of HEAD to the according
upstream branch head is possible. If so, it does a "git checkout --merge
<upstream>" (possibly leaving conflicts for the uncommitted changes,
just like "svn update"). If a fast-forward is not possible, it
complains, telling the user that he needs to use "git merge/rebase/pull"
instead, and might want to create a branch head, in case of a detached
HEAD.

If there's also going to be a rule that forbids commits on some kind of
detached HEAD, than the command could also tell the user (when a
fast-forward is not possible) that upstream possibly rewrote history,
and that the user might want to use "git checkout --merge <upstream>"
(or maybe "git update -f"?) to just go to the new upstream version (not
sure if that hint should be shown in addition to or instead of the "git
merge/rebase/pull" hint).

Björn

^ permalink raw reply

* Re: [PATCH] grep: do not segfault when -f is used
From: Junio C Hamano @ 2009-10-17  7:44 UTC (permalink / raw)
  To: Matt Kraai; +Cc: git, gitster, Johannes Sixt
In-Reply-To: <1255702405-7050-1-git-send-email-kraai@ftbfs.org>

Matt Kraai <kraai@ftbfs.org> writes:

> "git grep" would segfault if its -f option was used because it would
> try to use an uninitialized strbuf, so initialize the strbuf.
>
> Thanks to Johannes Sixt <j.sixt@viscovery.net> for the help with the
> test cases.
>
> Signed-off-by: Matt Kraai <kraai@ftbfs.org>

Thanks, both of you.

^ permalink raw reply

* Re: [PATCH] Proof-of-concept patch to remember what the detached HEAD was
From: Junio C Hamano @ 2009-10-17  7:43 UTC (permalink / raw)
  To: Christoph Bartoschek; +Cc: git, Daniel Barkalow
In-Reply-To: <m49nq6-uk5.ln1@burns.bruehl.pontohonk.de>

Christoph Bartoschek <bartoschek@gmx.de> writes:

[jc: added Daniel back to cc list; please do not cull the cc list without
good reason]

> Daniel Barkalow wrote:
>
>> The upshot of the messages should be:
>> 
>>  $ git checkout origin/master
>>  Since you can't actually change "origin/master" yourself, you'll just
>>  be sightseeing unless you create a local branch to hold new local work.
>> 
>>  $ git branch
>>  * (not a local branch, but "origin/master")
>> 
>>  $ git commit
>>  You've been sightseeing "origin/master". The commit can't change that
>>  value, so your commit isn't held in any branch. If you want to create
>>  a branch to hold it, here's how.
> ...
> But then I was not able to verify that the checkout indeed matched the 
> 1.3.0-beta.  "git status" and "git branch" did not help here. 

This is not going to help you, but "git reflog" would have helped here.

The reason my suggesting "git reflog" now won't help you is because the
word "reflog" does not connect the question "how did I get here" unless
and until you know git already; in other words, it is not your fault that
you got lost, but it is showing a wart in the UI.

If the question you were asking was "does the files I have in my work tree
after issuing that scary checkout actually match origin/1.3.0-beta?", you
could have asked that question in a more direct way, and the command to do
so is "git diff origin/1.3.0-beta".  I do not think this would be asking
the user to be doing something unreasonably unintuitive.

If the question you were asking was (and it was not, from the description
of your experience, but you could be in that situation when you "return
some weeks later") "how does the checked out history relate to 1.3.0-beta?",
then there is a way to ask the question in a very direct way, and the
command to do so is "git show-branch HEAD origin/1.3.0-beta" (or give the
same argument to "gitk").

Although it is not _so_ unreasonable to expect "git status" to show the
information, I suspect it would not be practical.  After all, whenever
somebody is lost, everything is "status".  For a person who is lost and
does not know where in the history he is, it might be reasonable to expect
"status" to give the relationship between your HEAD and some branch/tag,
while for another person who was hit by "git gui" complaining that he has
too many loose objects, it might be reasonable for him to expect "status"
to give the number of loose objects in the repository.  IOW, "status" is
too broad a word and following the path to cram everything into "status"
so that any new person who gets lost can get necessary infor from the
command will unfortunately lead to insanity.

The second item in the Daniel's transcript above may be an improvement but
I think it is a wrong economy to record and show 'but "origin/master"'
(which cannot be correct forever and has to be invalidated once the user
starts committing or resetting) in the message.  I am wondering if a
similar effect to help new users can be had by rewording the message to:

    $ git branch
    * (not a local branch; see "git reflog" to learn how you got here)

The user can see how he got there even after doing something else after
the checkout (see Nico's write-up in $gmane/130527).  The difference is
between giving fish and teaching how to catch one himself.

^ permalink raw reply

* Re: [PATCH] Proof-of-concept patch to remember what the detached HEAD was
From: Sean Estabrooks @ 2009-10-17  7:24 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Nicolas Pitre, Jeff King, Junio C Hamano, Daniel Barkalow,
	Jay Soffian, git
In-Reply-To: <alpine.DEB.1.00.0910160357370.4985@pacific.mpi-cbg.de>

On Fri, 16 Oct 2009 04:07:23 +0200 (CEST)
Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

> Just recently, I had a user request (a very valid one, mind you) where the 
> user does not want to provide a commit message, and wants to just commit 
> all the current changes.  In that particular case, it is very sensible to 
> ask for these things.  It is something utterly simple to ask for. Yet, it 
> is utterly hard with Git, especially if I have to explain it.

Hey Johannes,

It's actually easy, but maybe hard to find:

	$ git commit --cleanup=verbatim -m ""

Cheers,
Sean

^ permalink raw reply

* Re: [PATCH] Proof-of-concept patch to remember what the detached HEAD was
From: Christoph Bartoschek @ 2009-10-16 22:36 UTC (permalink / raw)
  To: git
In-Reply-To: <alpine.LNX.2.00.0910140037570.32515@iabervon.org>

Daniel Barkalow wrote:

> The upshot of the messages should be:
> 
>  $ git checkout origin/master
>  Since you can't actually change "origin/master" yourself, you'll just
>  be sightseeing unless you create a local branch to hold new local work.
> 
>  $ git branch
>  * (not a local branch, but "origin/master")
> 
>  $ git commit
>  You've been sightseeing "origin/master". The commit can't change that
>  value, so your commit isn't held in any branch. If you want to create
>  a branch to hold it, here's how.


I appreciate such a message for git branch as a user. Today I had the 
following problem:

I wanted to compile Qt Creator 1.3 from their repository. I cloned it with 
git clone and then issued git checkout origin/1.3.0-beta. First there came a 
the warning and I was not sure whether the checkout succeeded at all. I had 
to ask in the IRC channel whether the checkout was ok.

But then I was not able to verify that the checkout indeed matched the 
1.3.0-beta.  "git status" and "git branch" did not help here. 

Having an improved "git branch" would help a lot, especially if one returns 
some weeks later to the directory and wants to know what the checkout is.

Christoph

^ permalink raw reply

* Re: Introduction and Wikipedia and Git Blame
From: jamesmikedupont @ 2009-10-17  6:50 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <7v3a5irkel.fsf@alter.siamese.dyndns.org>

Thank you very much for your input and advice,
I have a lot of learn about this great tool.
I am working on learning how the existing blame tool runs now.
Will report back when I have some code.
mike

On Sat, Oct 17, 2009 at 1:25 AM, Junio C Hamano <gitster@pobox.com> wrote:
> "jamesmikedupont@googlemail.com" <jamesmikedupont@googlemail.com> writes:
>
>> What do you think of my idea to create blames along a specific user
>> defined byte positions ?
>
> Overly complicated and not enough time for _review_.  If you are blaming
> one-byte (or one-char) per line, wouldn't it be enough to consider the
> line number in the output as byte (or char) position when reconstituting
> the original text?
>

^ permalink raw reply

* Re: [PATCH v3 0/5] Pretty formats for reflog data
From: Jeff King @ 2009-10-17  1:18 UTC (permalink / raw)
  To: Thomas Rast; +Cc: Junio C Hamano, Jef Driesen, Nanako Shiraishi, git
In-Reply-To: <cover.1255701207.git.trast@student.ethz.ch>

On Fri, Oct 16, 2009 at 04:20:32PM +0200, Thomas Rast wrote:

> I tried for some time, but all attempts at interrupting the lists
> ended up terminating it again, so that the %g family list would not
> line up with the rest of the parameters.  Having the note there would
> be nice, but I think keeping the list together optically is more
> important.  However, AFAICS it really is the first character that only
> works with certain options (%m makes little sense without A...B, but
> still expands to >).

Another example is "%d", but there we load decorations on the fly the
first time it is used. I don't think that's a good idea here, since
using "%gd" shouldn't automatically imply "-g", which drastically
changes the nature of the command.

The "%m" behavior does what I would expect. If you haven't done any
limiting, then there can be no "left" or "boundary" entries.

I am tempted to do a note like the one below, but maybe the behavior is
obvious enough to users that it isn't necessary.

> Looking at it did make me notice that @{1} is invalid asciidoc and
> needs to be spelled @\{1\} though :-)

Thanks. I am often very guilty of not actually building documentation
when reviewing patches, because it is so tedious to build and examine.

---
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 6359272..43192af 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -135,6 +135,12 @@ The placeholders are:
 - '%n': newline
 - '%x00': print a byte from a hex code
 
+NOTE: Some placeholders may depend on other options given to the
+revision traversal engine. For example, the `%g*` reflog options will
+insert an empty string unless we are traversing reflog entries (e.g., by
+`git log -g`). The `%d` placeholder will use the "short" decoration
+format if `--decorate` was not already provided on the command line.
+
 * 'tformat:'
 +
 The 'tformat:' format works exactly like 'format:', except that it

^ permalink raw reply related

* Re: [PATCH v3 0/5] Pretty formats for reflog data
From: Junio C Hamano @ 2009-10-17  0:50 UTC (permalink / raw)
  To: Thomas Rast; +Cc: Jeff King, Jef Driesen, Nanako Shiraishi, git
In-Reply-To: <cover.1255701207.git.trast@student.ethz.ch>

Thomas Rast <trast@student.ethz.ch> writes:

> Next round :-)

Very nicely done; I like it.

Thanks.

^ permalink raw reply

* Re: [PATCH] am: allow some defaults to be specified via git-config
From: Junio C Hamano @ 2009-10-17  0:50 UTC (permalink / raw)
  To: Sam Vilain; +Cc: git, Nigel McNie
In-Reply-To: <1255650627-17576-1-git-send-email-sam.vilain@catalyst.net.nz>

Sam Vilain <sam.vilain@catalyst.net.nz> writes:

> Some users prefer in particular '3way' to be the default, let them
> specify it via the config file - and some other boolean settings while
> we're at it.

I have to wonder how this will interact with the internal call rebase
makes into am.  Would there be unexpected fallouts?

^ permalink raw reply

* Re: [PATCH RFC] git describe without refs distinguishes dirty working tree
From: Nicolas Pitre @ 2009-10-17  0:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jean Privat, Shawn O. Pearce, git
In-Reply-To: <7vvdifq6vw.fsf@alter.siamese.dyndns.org>

On Fri, 16 Oct 2009, Junio C Hamano wrote:

> But the thing is, "git describe" without arguments already works on HEAD
> and describes it, and people depend on the behaviour.
> 
> I originally thought this _might_ break and hoped it won't be a big issue,
> but now I've seen that even the kernel would break (it runs the command
> without saying "HEAD"), I do not want to risk breaking other projects I
> may not even heard of.  Some people might have copied our GIT-VERSION-GEN
> (that says "HEAD"), but I would not bet against that many many more people
> would have copied the use of "git describe" from the kernel build tree
> than from us.

OpenOCD is another project that just started using Git and copied 
describe usage from Linux.


Nicolas

^ permalink raw reply

* Re: [PATCH 3/3] Add proof-of-concept %[w(width,in1,in2)<<any-string>>%] implementation
From: Junio C Hamano @ 2009-10-17  0:18 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Johannes Schindelin, git
In-Reply-To: <200910170200.03681.jnareb@gmail.com>

Jakub Narebski <jnareb@gmail.com> writes:

> Why not
>
>     %[w(-1,4,4)%s%+b]

Yeah.  As you said "subject unwrapped and body indented" or something like
that, I excluded that part outside the indentation, but log output indents
everything, so placing both inside %[w] would be correct.

> (i.e. %+ is this empty line between subject and body, if it exists).

As %+ is to add LF iff the next expansion is non-empty, it must always be
followed by an expansion, i.e. %something.  That means "%+%something" is
unnecessary verbose and "%+something" is enough.  "%+anything" syntax,
just like "%[func()anything]" syntax, is not limited to any particular
expansion.

> The %+x seems a bit strange... but I guess implementing conditional
> expansion a la shell or rpn spec/queryformat would be out of question
> (i.e. %?s:+ )...

Why not?

I can see us doing something like %[conditional%|iftrue%|iffalse%] quite
easily, now we have the "nested" variant of the strbuf_expand()
infrastructure.

But that is not the primary focus of the two patch series I've
demonstrated so far.

^ permalink raw reply

* Re: [PATCH 3/3] Add proof-of-concept %[w(width,in1,in2)<<any-string>>%] implementation
From: Jakub Narebski @ 2009-10-17  0:00 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <7v7huurkif.fsf@alter.siamese.dyndns.org>

On Sat, 17 Oct 2009, Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
> > But even with original w(<width>,<indent1>,<indent2>) we can get output
> > of bare "git log" using pretty format... well, almost; it would be the
> > same if there was ability to put infinite width, and there doesn't seem
> > to be specifier for the whole, unchanged commit message (subject,
> > unwrapped + separating lines + body).
> 
> I think I already discussed this when I sent out %s%+b patch.  You would
> need to adjust and apply both series, but essentially it would become
> something like:
> 
>     %s%+[w(-1,4,4)%b]
> 
> I.e. a single subject line, potentially followed by a LF and body indented
> by 4-place, but the LF will be there only when the body is not empty.

Why not

    %[w(-1,4,4)%s%+b]

Or is it

    %[w(-1,4,4)%s%+%b]

(i.e. %+ is this empty line between subject and body, if it exists).

The %+x seems a bit strange... but I guess implementing conditional
expansion a la shell or rpn spec/queryformat would be out of question
(i.e. %?s:+ )...
-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: Introduction and Wikipedia and Git Blame
From: Junio C Hamano @ 2009-10-16 23:25 UTC (permalink / raw)
  To: jamesmikedupont@googlemail.com; +Cc: Junio C Hamano, Johannes Schindelin, git
In-Reply-To: <ee9cc730910161419x608f5972x705ce8088d72c94a@mail.gmail.com>

"jamesmikedupont@googlemail.com" <jamesmikedupont@googlemail.com> writes:

> What do you think of my idea to create blames along a specific user
> defined byte positions ?

Overly complicated and not enough time for _review_.  If you are blaming
one-byte (or one-char) per line, wouldn't it be enough to consider the
line number in the output as byte (or char) position when reconstituting
the original text?

^ permalink raw reply

* Re: [PATCH 3/3] Add proof-of-concept %[w(width,in1,in2)<<any-string>>%] implementation
From: Junio C Hamano @ 2009-10-16 23:23 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Junio C Hamano, Johannes Schindelin, git
In-Reply-To: <200910170020.01756.jnareb@gmail.com>

Jakub Narebski <jnareb@gmail.com> writes:

> I think better solution might be to give _string_ to use for initial and
> subsequent indent rather than number of spaces... well, at least more
> generic one, allowing one to use e.g. "\t" (TAB) character to indent,
> or indent in the following way:

I do not see there is anything _better_ or _worse_.  It may simply be a
useful _addition_.  I'd usually say "care to send in a patch?" here, but
given that the general framework and the "wrap like the shortlog" hasn't
been plugged-in yet, there is no need to rush.

> But even with original w(<width>,<indent1>,<indent2>) we can get output
> of bare "git log" using pretty format... well, almost; it would be the
> same if there was ability to put infinite width, and there doesn't seem
> to be specifier for the whole, unchanged commit message (subject,
> unwrapped + separating lines + body).

I think I already discussed this when I sent out %s%+b patch.  You would
need to adjust and apply both series, but essentially it would become
something like:

    %s%+[w(-1,4,4)%b]

I.e. a single subject line, potentially followed by a LF and body indented
by 4-place, but the LF will be there only when the body is not empty.

^ permalink raw reply

* Re: [RFC PATCH v3 00/17] Return of smart HTTP
From: Shawn O. Pearce @ 2009-10-16 23:16 UTC (permalink / raw)
  To: Mark Lodato; +Cc: git
In-Reply-To: <ca433830910161604g5a6bde76n26eb2b1e8155fb36@mail.gmail.com>

Mark Lodato <lodatom@gmail.com> wrote:
> The gitweb part is just bonus.  (The only thing for gitweb is the one
> ScriptAlias line.)  The real challenge is getting git-http-backend to
> serve repositories out of something other than DocumentRoot - say
> DocumentRoot is /var/www/htdocs but your git repositories are in
> /pub/git - which is why I posted the configuration.  If you'd like, I
> can send you a patch to add this to the documentation.

Yes, a patch would be wonderful.
 
> One idea to improve the situation is to first try
> $GIT_PROJECT_ROOT$PATH_INFO, falling back to $PATH_TRANSLATED if
> $GIT_PROJECT_ROOT is empty.  This would make the configuration simple:
> 
> -- 8< --
> SetEnv GIT_PROJECT_ROOT /pub/git
> ScriptAlias /git /usr/libexec/git-core/git-http-backend
> -- >8 --

Oooh.  Perhaps a good idea.  :-)

-- 
Shawn.

^ permalink raw reply

* Re: [RFC PATCH v3 00/17] Return of smart HTTP
From: Mark Lodato @ 2009-10-16 23:04 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20091016143154.GS10505@spearce.org>

I just realized I forgot to say something in my last email: THANK
YOU!!!  I have been looking forward to this for a long time.  I was
planning to one day to sit down and start thinking about how to
implement a smart protocol, and then I see a post saying that not only
has someone figured out the protocol, but he has implemented it!
Amazing!  This is really crucial for corporate adoption, at least at
my job.  We need to have strong authentication for many users, and the
SSH key management is just a nightmare, not to mention that not all
users can SSH due to firewalls.  This will save me so much time and
frustration.  Thanks!

On Fri, Oct 16, 2009 at 10:31 AM, Shawn O. Pearce <spearce@spearce.org> wrote:
> If users don't want to upgrade, or can't upgrade, then you can't
> push over HTTP.  Simple.

Yeah, I realized after I wrote my previous email that it probably
doesn't matter - the current state of HTTP push is so awful that no
one probably uses it!

> Really, what it comes down to is, I don't think it matters that
> we don't have backwards compatiblity for pushing through WebDAV.
> If you think it matters, you are free to write a patch series on
> top of mine which adds the functionality.  But don't wait for me
> to do it, it won't happen.

Agreed.  I was just wondering if it was on the to-do list for the far
off future.  Either way, it should probably be noted in the
documentation that dumb push is not supported.

>> Also, your examples use "DocumentRoot /pub/git", but I think most
>> people would want to have their main website as the DocumentRoot, have
>> the URL "/git" serve the repositories through gitweb, and have that
>> same "/git" URL be `git clone'-able.
>
> Why not have git-http-backend exec gitweb when it gets a request
> for the repository itself?  Why do you have to go through such
> contortions in Apache for this?  The two CGIs are shipped in the
> same software package, surely one could actually invoke the other.
>
> [snip]
>
> Yes, I'd like to have examples in the git-http-backend manpage.
> I put a couple in there already, but they don't consider gitweb
> because I assumed we'd find a way to have gitweb be invoked out
> of git-http-backend.  Unfortunately that hasn't happened yet.

The gitweb part is just bonus.  (The only thing for gitweb is the one
ScriptAlias line.)  The real challenge is getting git-http-backend to
serve repositories out of something other than DocumentRoot - say
DocumentRoot is /var/www/htdocs but your git repositories are in
/pub/git - which is why I posted the configuration.  If you'd like, I
can send you a patch to add this to the documentation.

One idea to improve the situation is to first try
$GIT_PROJECT_ROOT$PATH_INFO, falling back to $PATH_TRANSLATED if
$GIT_PROJECT_ROOT is empty.  This would make the configuration simple:

-- 8< --
SetEnv GIT_PROJECT_ROOT /pub/git
ScriptAlias /git /usr/libexec/git-core/git-http-backend
-- >8 --

As far as having git-http-backend launch an external process such as
gitweb, I personally don't think this is important enough to block
this from continuing.  One could configure the webserver as I have in
my previous email until such a feature is implemented.  For cgit, the
solution is simple - just integrate git-http-backend into the
executable.

Regards,
Mark

^ permalink raw reply

* Re: [PATCH RFC] git describe without refs distinguishes dirty working  tree
From: Junio C Hamano @ 2009-10-16 23:02 UTC (permalink / raw)
  To: Jean Privat; +Cc: Shawn O. Pearce, git
In-Reply-To: <dffdbd190910161452o4ac0b426i7c48649eafa0d53@mail.gmail.com>

Jean Privat <jean@pryen.org> writes:

>> I still haven't heard anything that helps me to decide which way the
>> default should be.  The only concrete thing I have heard against the
>> change of the default is that it will break existing setup, but I haven't
>> heard anything concrete for the change yet.
>
> As a said in the comment part of the initial message, I initially
> planed to add a --worktree option that means "I want you to describe
> my working tree". Knowing that from my naive point of view, the
> description of the working tree is what build script wanted :
> description of HEAD (on which is based the working tree) + saying that
> if the working tree is dirty or not (done manually by scripts).
> Moreover, in my naive view with the "--worktree" option, no refs where
> allowed (i.e. describe the working tree xor describe some commit
> references).

I do not think it is naive at all.  "git describe --worktree HEAD^" won't
make any sense in that world view; by definition the work tree is not
derived from anything but HEAD.

> Then, I realized that for some other git commands that can work both
> on the working tree and on an arbitrary commit reference, the default
> was to work on the working tree and require an explicit HEAD to work
> on the HEAD commit.

But the thing is, "git describe" without arguments already works on HEAD
and describes it, and people depend on the behaviour.

I originally thought this _might_ break and hoped it won't be a big issue,
but now I've seen that even the kernel would break (it runs the command
without saying "HEAD"), I do not want to risk breaking other projects I
may not even heard of.  Some people might have copied our GIT-VERSION-GEN
(that says "HEAD"), but I would not bet against that many many more people
would have copied the use of "git describe" from the kernel build tree
than from us.  After all, they are more famous and established than we
are.  [*1*]

One line of argument that I would have found reasonable to defend the
change of the default is that "The --worktree (or --dirty) option is
cumbersome to type.  And in _this_ workflow (that I haven't imagined, but
still would be *very* valid and useful one---so anybody who argues along
this line needs to fill in the blank here), it is the best solution to ask
the "git describe" command about the state of the work tree from the
command line very often, and here is why (here is another blank to fill
in), because there is no other way to get at the information, and/or the
info given by other existing commands are suboptimal for these reasons
(here is another blank to fill in)."

Then we would need to weigh benefits (for the interactive use of a very
useful and often used form of the command) against downsides (for people
having to update their existing scripts).

For use by scripts, the argument against having to give an extra option to
get the new behaviour becomes much weaker than for interactive use, even
if the new behaviour may be more common.  The point of scripting is that
you can write it once and forget about it; if it requires "--worktree" in
order to omit an extra "diff --index HEAD", it is not such a big deal.

And it is important to realize that "can write it once and forget about
it" cuts both ways.  It should apply not only to the people who write
their scripts after this patch of yours is applied to git, but should
equally apply to the people who wrote their scripts long time ago,
expecting that they can safely forget about them once they wrote them,
relying on the existing behaviour of the command.  Changing the defaults
for them means we lied to them and they have to update their scripts.  In
other words, they cannot script and forget about it anymore.

An attitude to change the default lightly like that, be it in 1.7.0 or
not, will then later come back and haunt your scripts you write while
assuming that the behaviour your patch will bring in will stay forever.
Allowing such a casual attitude will lead to other people changing the
default equally casually in the future.

It is my job to say no, even when I 100% agree that the new behaviour
would have been the default one if we were inventing "git describe" from
scratch and there were no existing users [*2*].  We do not live in an
ideal world, and 1.7.0 is not a blank check to change everything in sight.

>> How about "describe --dirty" and "describe --dirty=-mod" (the latter
>> creates v1.6.5-15-gc274db7-mod")
>
> May be better than "--wortree" (especially because of the value part),
> but what happen with
>  $ git describe --dirty v1.2.1
> should it show an error, output "v1.2.1" anyway, or output
> "v1.2.1-dirty" if the working tree is different from v1.2.1 ?

I am actually fine with your --worktree option, especially after seeing
your much more clear (i.e. "the state of my work tree") explanation in the
beginning of your response.

But I think "git describe" working in this "describe the version in the
work tree in the point in history" mode should reject any explicit
revision argument; by definition the work tree is not derived from
anything but HEAD.


[Footnote]

*1* In addition to the "it would break the kernel" issue, there are some
projects that expect users to tweak files they ship before building
(e.g. makefiles and config.h and the like).  For these people, any and all
builds would be -dirty.  I expect these projects, when migrating to git,
would either update their build procedures (moving config.h to
config.h-sample or something), but another solution for them would be to
use "git describe" without the -dirty bits.  So they are another class of
people who may not want -dirty by default.

*2* It personally hurts in a case like this, but that is what maintainers
have to do.  The maintainer's job is not to be loved, but is largely to
protect existing users from the second system syndrome.

^ permalink raw reply

* Re: [PATCH RFC] git describe without refs distinguishes dirty working tree
From: Shawn O. Pearce @ 2009-10-16 22:37 UTC (permalink / raw)
  To: Jean Privat; +Cc: Junio C Hamano, git
In-Reply-To: <dffdbd190910161452o4ac0b426i7c48649eafa0d53@mail.gmail.com>

Jean Privat <jean@pryen.org> wrote:
> > I still haven't heard anything that helps me to decide which way the
> > default should be.  The only concrete thing I have heard against the
> > change of the default is that it will break existing setup, but I haven't
> > heard anything concrete for the change yet.
...
> Then, I realized that for some other git commands that can work both
> on the working tree and on an arbitrary commit reference, the default
> was to work on the working tree and require an explicit HEAD to work
> on the HEAD commit. Thus it makes sense to me that "git describe"
> alone should describe the working tree and that "git describe HEAD"
> should describe the HEAD commit.

Yup.  That's my take on it too.  This default of "no argument means
describe the working tree" matches with tools like `git diff`,
`git checkout`, `git status`, `git blame` with no revision arguments.

We are being blasted by users for being inconsistent in our UI in too
many places.  Here's yet another.  We need to start standardizing
on a more consistent UI model.  If that model means we need to
use a "--worktree" flag to mean "against the working tree" then
we should start doing that also to `git status`, `git checkout`,
`git blame`, and `git diff`.

>  $ git describe --dirty v1.2.1
> should it show an error, output "v1.2.1" anyway, or output
> "v1.2.1-dirty" if the working tree is different from v1.2.1 ?

IMHO, that should be a fatal usage error, if we go that approach.
I would also argue `git describe --dirty HEAD` is equally fatal.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH 3/3] Add proof-of-concept %[w(width,in1,in2)<<any-string>>%] implementation
From: Jakub Narebski @ 2009-10-16 22:19 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <7v3a5jupr7.fsf@alter.siamese.dyndns.org>

On Fri, 16 Oct 2009, Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
> > I don't remember what were original parameters to w(72,4,8) means...
> 
> "man git-shortlog" look for -w.

Thanks.  So those would be:

-w[<width>[,<indent1>[,<indent2>]]]::
        Linewrap the output by wrapping each line at `width`.  The first
        line of each entry is indented by `indent1` spaces, and the second
        and subsequent lines are indented by `indent2` spaces. `width`,
        `indent1`, and `indent2` default to 76, 6 and 9 respectively.

I think better solution might be to give _string_ to use for initial and
subsequent indent rather than number of spaces... well, at least more
generic one, allowing one to use e.g. "\t" (TAB) character to indent,
or indent in the following way:

 * Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod
   tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim
   veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea
   commodo consequat. [...]

But even with original w(<width>,<indent1>,<indent2>) we can get output
of bare "git log" using pretty format... well, almost; it would be the
same if there was ability to put infinite width, and there doesn't seem
to be specifier for the whole, unchanged commit message (subject,
unwrapped + separating lines + body).

-- 
Jakub Narebski
Poland

^ 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