Re: [PATCH] Support specific color for a specific remote branches

Aviv Eyal [off-list ref] writes:

+remote.<name>.color:
+	If set, the `branch` command will use the colors specified here
+	for the names of all branches tracking this remote. Colors
+	specified here take precedent over `color.branch.remote` config.

+static char *strclone(const char *start, int len)
+{

Hmm, don't we have xmemdupz() for this?

+	char *result = malloc((len + 1) * sizeof(char));
+	if (!result)
+		return NULL;
+	strncpy(result, start, len);
+	result[len] = '\0';
+	return result;
+}
+
+static int git_branch_config_custom_color_remote(const char *var, const char *value)
+{
+	struct string_list_item *item;
+	char *name, *color;
+	name = strclone(var + 7, strlen(var) - 13); // "remote."=7, "remote..color"=13.

No // comments please.

+	if (!name)
+		return 0;
+	color =  malloc(COLOR_MAXLEN * sizeof(char));

At least use xmalloc(); also isn't sizeof(char) by definition 1?

+	if (!color) {
+		free(name);
+		return 0;
+	}
+	item = string_list_insert(&custom_colors, name);
+	color_parse(value, var, color);
+	item->util = color;

It may be just a style thing, but I'd rather see this caller call
color_parse() first, and after letting that function validate the end-user
input, call string_list_insert().

Also what happens when there are two entries that talk about the same
remote branch in the configuration? Such a configuration is not an error;
your ~/.gitconfig may say one thing for remote.origin.master and your
repository specific .git/config may override it for a particular
repository.

+	return 0;
+}
+
+static char *git_branch_get_custom_color_remote(const char *name)
+{
+	int name_len;
+	char* repo_name;
+	struct string_list_item *custom;
+	name_len = strchr(name, '/') - name;

Who said a remote name is terminated with (and cannot contain) a slash?

Shouldn't this code be consulting the configuration file to learn the
remote mapping, e.g.

    [remote "frotz"]
        fetch = +refs/heads/*:refs/remotes/nitfol/*

so that remote branches from "frotz" remote, that happen to be stored
under refs/remotes/nitfol/ hierarchy, are painted in the correct color?

On Mon, Aug 08, 2011 at 11:08:26AM -0700, Junio C Hamano wrote:

quoted

+	return 0;
+}
+
+static char *git_branch_get_custom_color_remote(const char *name)
+{
+	int name_len;
+	char* repo_name;
+	struct string_list_item *custom;
+	name_len = strchr(name, '/') - name;

Who said a remote name is terminated with (and cannot contain) a slash?

Shouldn't this code be consulting the configuration file to learn the
remote mapping, e.g.

    [remote "frotz"]
        fetch = +refs/heads/*:refs/remotes/nitfol/*

so that remote branches from "frotz" remote, that happen to be stored
under refs/remotes/nitfol/ hierarchy, are painted in the correct color?

This seems related to the recent thread about showing branches only for
a specific remote:

  http://article.gmane.org/gmane.comp.version-control.git/178668

Maybe the two should share code.

Right now, "git branch -r" means "show everything under refs/remotes
instead of refs/heads". This would be easy to implement if it instead
meant "show all refs created by the RHS of a fetch refspec in a
configured remote". The two are equivalent in the default config, but
the latter may make more sense in a complex case.

-Peff

On 8 August 2011 21:08, Junio C Hamano [off-list ref] wrote:

Who said a remote name is terminated with (and cannot contain) a slash?

...

Shouldn't this code be consulting the configuration file to learn the

...

so that remote branches from "frotz" remote, that happen to be stored
under refs/remotes/nitfol/ hierarchy, are painted in the correct color?

Sorry, that's too dip & complex for me... Consider this patch 'withdrawn' then.

-- Aviv