Michael Haggerty <mhag...@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)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
