From: Junio C Hamano <gitster@pobox•com>
To: Michael Haggerty <mhagger@alum•mit.edu>
Cc: git discussion list <git@vger•kernel.org>
Subject: Re: "HEAD -> branch" decoration doesn't work with "--decorate=full"
Date: Wed, 13 May 2015 10:11:58 -0700 [thread overview]
Message-ID: <xmqqwq0c9zc1.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <55534D95.60609@alum.mit.edu> (Michael Haggerty's message of "Wed, 13 May 2015 15:11:49 +0200")
Michael Haggerty <mhagger@alum•mit.edu> writes:
> The new-style "HEAD -> branch" style decoration doesn't work when
> "--decorate=full" is used:
>
>> $ bin-wrappers/git show --oneline --decorate
>> c518059 (HEAD -> master, gitster/master) Merge branch 'maint'
>>
>> $ bin-wrappers/git show --oneline --decorate=full
>> c518059 (HEAD, refs/remotes/gitster/master, refs/heads/master) Merge branch 'maint'
>
> I would have expected the second invocation to show "HEAD ->
> refs/heads/master".
>
> Was that an oversight or a conscious decision?
I actually think this ultimately comes from a poor design of the
name-decorations infrastructure. The program is expected to call
load_ref_decorations() only once and make the choice between the
full/short at that point, which is passed to add_ref_decoration() to
record either 'refs/heads/master' or 'master' in the singleton
name_decoration decoration. But it does not store which one was
chosen by the caller of load_ref_decorations() anywhere in the
subsystem.
When current_pointed_by_HEAD() wants to see if decorations on an
object, e.g. 'master', matches what 'HEAD' resolves to, it cannot
tell if the original set-up was done for the full decoration, and
the current code just assumes (without even realizing that it is
making that assumption) the decoration must have been set up for the
short ones.
Perhaps something like this, but I am not committing it without
tests or a proper log messge.
I moved "static loaded" outside as it is in the same category as the
global name-decoration and decoration-flags, i.e. to be initialised
once at the beginning to a fixed setting and then be used with that
setting.
log-tree.c | 23 ++++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/log-tree.c b/log-tree.c
index 2c1ed0f..92259bc 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -13,6 +13,8 @@
#include "line-log.h"
static struct decoration name_decoration = { "object names" };
+static int decoration_loaded;
+static int decoration_flags;
static char decoration_colors[][COLOR_MAXLEN] = {
GIT_COLOR_RESET,
@@ -146,9 +148,9 @@ static int add_graft_decoration(const struct commit_graft *graft, void *cb_data)
void load_ref_decorations(int flags)
{
- static int loaded;
- if (!loaded) {
- loaded = 1;
+ if (!decoration_loaded) {
+ decoration_loaded = 1;
+ decoration_flags = flags;
for_each_ref(add_ref_decoration, &flags);
head_ref(add_ref_decoration, &flags);
for_each_commit_graft(add_graft_decoration, NULL);
@@ -196,8 +198,19 @@ static const struct name_decoration *current_pointed_by_HEAD(const struct name_d
branch_name = resolve_ref_unsafe("HEAD", 0, unused, &rru_flags);
if (!(rru_flags & REF_ISSYMREF))
return NULL;
- if (!skip_prefix(branch_name, "refs/heads/", &branch_name))
- return NULL;
+
+ if ((decoration_flags == DECORATE_SHORT_REFS)) {
+ if (!skip_prefix(branch_name, "refs/heads/", &branch_name))
+ return NULL;
+ } else {
+ /*
+ * Each decoration has a refname in full; keep
+ * branch_name also in full, but still make sure
+ * HEAD is a reasonable ref.
+ */
+ if (!starts_with(branch_name, "refs/"))
+ return NULL;
+ }
/* OK, do we have that ref in the list? */
for (list = decoration; list; list = list->next)
next prev parent reply other threads:[~2015-05-13 17:12 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-05-13 13:11 "HEAD -> branch" decoration doesn't work with "--decorate=full" Michael Haggerty
2015-05-13 14:51 ` Junio C Hamano
2015-05-13 15:26 ` Michael J Gruber
2015-05-13 17:11 ` Junio C Hamano [this message]
2015-05-13 17:13 ` Junio C Hamano
2015-05-13 19:40 ` [PATCH 2/2] log: do not shorten decoration names too early Junio C Hamano
2015-05-14 6:33 ` Jeff King
2015-05-14 17:37 ` Junio C Hamano
2015-05-14 17:49 ` Jeff King
2015-05-14 18:01 ` Junio C Hamano
2015-05-14 18:10 ` Jeff King
2015-05-14 21:49 ` Junio C Hamano
2015-05-14 21:54 ` Jeff King
2015-05-14 22:25 ` Junio C Hamano
2015-05-14 22:33 ` Jeff King
2015-05-22 21:21 ` Junio C Hamano
2015-05-22 21:38 ` Jeff King
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqwq0c9zc1.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=mhagger@alum$(echo .)mit.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox