From mboxrd@z Thu Jan  1 00:00:00 1970
From: john at keeping.me.uk (John Keeping)
Date: Fri, 29 Apr 2016 23:16:48 +0100
Subject: [PATCH v3] ui-shared: allow to split the repository link
In-Reply-To: <1461962807-8863-1-git-send-email-petr.vorel@gmail.com>
References: <1461962807-8863-1-git-send-email-petr.vorel@gmail.com>
Message-ID: <20160429221648.GE14612@serenity.lan>

On Fri, Apr 29, 2016 at 10:46:47PM +0200, Petr Vorel wrote:
> Teach cgit split the repository link in the top of repository "summary"
> view. This emulates the same behaviour as it's in gitweb.
> 
> This behaviour is not implemented for repositories which have
> "repo.name" set different than "repo.url".
> 
> This feature is controlled by a new config variable:
> "summary-enable-split-repo-link" (disabled by default).
> 
> Signed-off-by: Petr Vorel <petr.vorel at gmail.com>
> ---
> v3: New config variable "summary-enable-split-repo-link", minor cleanup.
> v2: Minor cleanup.
> ---
>  cgit.c         |  3 +++
>  cgit.h         |  1 +
>  cgitrc.5.txt   |  6 ++++++
>  tests/setup.sh |  1 +
>  ui-shared.c    | 20 +++++++++++++++++++-
>  5 files changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/cgit.c b/cgit.c
> index fc482be..d9050f4 100644
> --- a/cgit.c
> +++ b/cgit.c
> @@ -250,6 +250,8 @@ static void config_cb(const char *name, const char *value)
>  		ctx.cfg.summary_log = atoi(value);
>  	else if (!strcmp(name, "summary-branches"))
>  		ctx.cfg.summary_branches = atoi(value);
> +	else if (!strcmp(name, "summary-enable-split-repo-link"))
> +		ctx.cfg.summary_enable_split_repo_link = atoi(value);
>  	else if (!strcmp(name, "summary-tags"))
>  		ctx.cfg.summary_tags = atoi(value);
>  	else if (!strcmp(name, "side-by-side-diffs"))
> @@ -389,6 +391,7 @@ static void prepare_context(void)
>  	ctx.cfg.repository_sort = "name";
>  	ctx.cfg.section_sort = 1;
>  	ctx.cfg.summary_branches = 10;
> +	ctx.cfg.summary_enable_split_repo_link = 0;
>  	ctx.cfg.summary_log = 10;
>  	ctx.cfg.summary_tags = 10;
>  	ctx.cfg.max_atom_items = 10;
> diff --git a/cgit.h b/cgit.h
> index 325432b..fec5b7e 100644
> --- a/cgit.h
> +++ b/cgit.h
> @@ -255,6 +255,7 @@ struct cgit_config {
>  	int snapshots;
>  	int section_sort;
>  	int summary_branches;
> +	int summary_enable_split_repo_link;
>  	int summary_log;
>  	int summary_tags;
>  	diff_type difftype;
> diff --git a/cgitrc.5.txt b/cgitrc.5.txt
> index 2e1912d..be58857 100644
> --- a/cgitrc.5.txt
> +++ b/cgitrc.5.txt
> @@ -438,6 +438,12 @@ summary-branches::
>  	Specifies the number of branches to display in the repository "summary"
>  	view. Default value: "10".
>  
> +summary-enable-split-repo-link::
> +	Flag which, when set to "1", will make cgit split the repository link in
> +	the top of repository "summary" view. This emulates the same behaviour as
> +	it's in gitweb. This behaviour is not implemented for repositories which
> +	have "repo.name" set different than "repo.url". Default value: "0".
> +
>  summary-log::
>  	Specifies the number of log entries to display in the repository
>  	"summary" view. Default value: "10".
> diff --git a/tests/setup.sh b/tests/setup.sh
> index 7590f04..14e4f93 100755
> --- a/tests/setup.sh
> +++ b/tests/setup.sh
> @@ -109,6 +109,7 @@ enable-log-filecount=1
>  enable-log-linecount=1
>  summary-log=5
>  summary-branches=5
> +summary-enable-split-repo-link=0
>  summary-tags=5
>  clone-url=git://example.org/\$CGIT_REPO_URL.git
>  enable-filter-overrides=1
> diff --git a/ui-shared.c b/ui-shared.c
> index 9a38aa9..ec7e8c9 100644
> --- a/ui-shared.c
> +++ b/ui-shared.c
> @@ -937,7 +937,25 @@ static void print_header(void)
>  	if (ctx.repo) {
>  		cgit_index_link("index", NULL, NULL, NULL, NULL, 0, 1);
>  		html(" : ");
> -		cgit_summary_link(ctx.repo->name, ctx.repo->name, NULL, NULL);
> +
> +		if (ctx.cfg.summary_enable_split_repo_link &&
> +			!(strcmp(ctx.repo->name, ctx.repo->url))) {

This deserves a comment: why is the relationship between "repo->name"
and "repo->url" important?  Future readers will want to know why these
two variables are related.

> +			char *token, *link, *delim = "/";
> +			char dir[strlen(ctx.repo->name)];
> +			strcpy(dir, "");
> +			link = ctx.repo->name;
> +			while ((token = strtok_r(link, delim, &link))) {
> +				if (strcmp(dir, ""))
> +					strcat(dir, delim);
> +				strcat(dir, token);
> +				strcpy(ctx.repo->name, dir);
> +				cgit_summary_link(token, ctx.repo->name, NULL, NULL);
> +				if (strcmp(link, ""))
> +					html(delim);
> +			}

We compile as C89, so we shouldn't be using variadic arrays.  I think
this could be simpler without using strtok_r if we use strchr to search
for delimiters (untested):

	char *name = ctx.repo->name;
	char *start = name;
	for (;;) {
		char *delim = strchr(start, '/');
		if (delim)
			*delim = '\0';
		cgit_summary_link(start, name, NULL, NULL);
		if (!delim)
			break;
		*delim = '/';
		start = delim + 1;
	}

although I still don't think cgit_summary_link() is right here.

> +		} else
> +			cgit_summary_link(ctx.repo->name, ctx.repo->name, NULL, NULL);
> +
>  		if (ctx.env.authenticated) {
>  			html("</td><td class='form'>");
>  			html("<form method='get' action=''>\n");
> -- 
> 2.8.1