Thread (9 messages) flat view 9 messages, 2 authors, 2016-06-15

Re: [PATCH] Colorize commit decorations

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:48:59

Nazri Ramliy [off-list ref] writes:
Use different color for each type of refs (local, remote, tags, HEAD,
and stash). This makes the decorations and their type stand out more
and easier to distinguish in 'git log --decorate'.

Currently all the different types of decorations are shown in the same
color as the commit id, which is not that easy to spot.

The color applied for each type of refs are customizable via
color.log.decorate.<slot> config entry, as documented in
Documentation/config.txt.
---
Sign-off?
quoted hunk ↗ jump to hunk
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 95cf73c..afa4f5a 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -725,6 +725,11 @@ color.interactive.<slot>::
 	commands.  The values of these variables may be specified as
 	in color.branch.<slot>.
 
+color.log.decorate.<slot>::
+	Use customized color for 'git log --decorate' output.
+	`<slot>` is one of `local`, `remote`, `tag`, `stash` or `head`
+	for local refs, remote refs, tags, stash and HEAD, respectively.
Configuration variable names are either 2-level or 3-level.  There are
existing examples for <slot> (e.g. color.branch.<slot>).
quoted hunk ↗ jump to hunk
diff --git a/decorate.c b/decorate.c
index 2f8a63e..cbef2b4 100644
--- a/decorate.c
+++ b/decorate.c
@@ -5,6 +5,48 @@
 #include "cache.h"
 #include "object.h"
 #include "decorate.h"
+#include "color.h"
...
+int parse_decorate_color_config(const char *var, const int ofs, const char *value) {
+	int slot = parse_decorate_color_slot(var + ofs);
+	if (slot < 0)
+		return 0;
+	if (!value)
+		return config_error_nonbool(var);
+	color_parse(value, var, decoration_colors[slot]);
+	return 0;
+}
I don't think any of the above belongs to "decorate.c", which is the
generic mechanism to annotate commits with arbitrary data.  The Porcelain
feature "log --decorate" is just one user that happens to use refname as
that "arbitrary data", and that is what you are coloring.  The code for
that is in log-tree.c, I think.
quoted hunk ↗ jump to hunk
diff --git a/decorate.h b/decorate.h
index e732804..d593d32 100644
So is any change to this file.
quoted hunk ↗ jump to hunk
diff --git a/log-tree.c b/log-tree.c
index d3ae969..95ebf1a 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -10,29 +10,41 @@
 
 struct decoration name_decoration = { "object names" };
 
-static void add_name_decoration(const char *prefix, const char *name, struct object *obj)
+static void add_name_decoration(enum decoration_type type, const char *name, struct object *obj)
 {
-	int plen = strlen(prefix);
 	int nlen = strlen(name);
-	struct name_decoration *res = xmalloc(sizeof(struct name_decoration) + plen + nlen);
-	memcpy(res->name, prefix, plen);
-	memcpy(res->name + plen, name, nlen + 1);
+	struct name_decoration *res = xmalloc(sizeof(struct name_decoration) + nlen);
+	memcpy(res->name, name, nlen + 1);
+	res->type = type;
 	res->next = add_decoration(&name_decoration, obj, res);
 }
When color is not in use (e.g. output is not going to the terminal), you
would lose "tag: " prefix, wouldn't you?

Even when color _is_ in use, people may want to see familiar "tag: "
prefix in the output---I personally do not think that is necessary,
though.
 static int add_ref_decoration(const char *refname, const unsigned char *sha1, int flags, void *cb_data)
 {
 	struct object *obj = parse_object(sha1);
+	enum decoration_type type = DECORATION_NONE;
 	if (!obj)
 		return 0;
+
+	if (!prefixcmp(refname, "refs/heads"))
+		type = DECORATION_REF_LOCAL;
+	else if (!prefixcmp(refname, "refs/remotes"))
+		type = DECORATION_REF_REMOTE;
+	else if (!prefixcmp(refname, "refs/tags"))
+		type = DECORATION_TAG;
+	else if (!prefixcmp(refname, "refs/stash"))
+		type = DECORATION_STASH;
+	else if (!prefixcmp(refname, "HEAD"))
+		type = DECORATION_HEAD;
I suspect that users would expect DECORATION_HEAD to be used to highlight
the object at the tip of the current branch, but I also suspect this code
would not do so.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help