List for cgit developers and users
 help / color / mirror / Atom feed
* [PATCH v7 1/1] ui-shared: allow to split the repository link
@ 2017-02-20 23:36 petr.vorel
  2017-03-12 15:03 ` john
  2017-03-12 15:30 ` john
  0 siblings, 2 replies; 4+ messages in thread
From: petr.vorel @ 2017-02-20 23:36 UTC (permalink / raw)


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".

Signed-off-by: Petr Vorel <petr.vorel at gmail.com>
Cc: "Jason A. Donenfeld" <Jason at zx2c4.com>
Cc: John Keeping <john at keeping.me.uk>
Reviewed-by: John Keeping <john at keeping.me.uk>
---
Thanks for your review John. Are we there yet? :-)

Changes v6->v7:
* Remove redundant '*'
* Add Reviewed-by
---
 cgitrc.5.txt |  4 +++-
 ui-shared.c  | 27 ++++++++++++++++++++++++++-
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/cgitrc.5.txt b/cgitrc.5.txt
index 9fcf445..500b7ee 100644
--- a/cgitrc.5.txt
+++ b/cgitrc.5.txt
@@ -604,7 +604,9 @@ repo.source-filter::
 
 repo.url::
 	The relative url used to access the repository. This must be the first
-	setting specified for each repo. Default value: none.
+	setting specified for each repo. Unless repo.url is different from
+	repo.name, cgit splits the repository link in the top of repository
+	"summary" view. Default value: none.
 
 
 REPOSITORY-SPECIFIC CGITRC FILE
diff --git a/ui-shared.c b/ui-shared.c
index 2e4fcd9..e4f956d 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -936,7 +936,32 @@ 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);
+
+		/*
+		 * NOTE: If repo.name and repo.url are different, we don't split link as
+		 * it wouldn't make sense to split the path.
+		 */
+		if (!strcmp(ctx.repo->name, ctx.repo->url)) {
+			char *name = ctx.repo->name;
+			char *start = name;
+			for (;;) {
+				char *delim = strchr(start, '/');
+				if (delim)
+					*delim = '\0';
+
+				html_link_open(cgit_repourl(name), NULL, NULL);
+				html_ntxt(strlen(start), start);
+				html_link_close();
+
+				if (!delim)
+					break;
+				*delim = '/';
+				html("/");
+				start = delim + 1;
+			}
+		} 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'>\n");
-- 
2.11.0



^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v7 1/1] ui-shared: allow to split the repository link
  2017-02-20 23:36 [PATCH v7 1/1] ui-shared: allow to split the repository link petr.vorel
@ 2017-03-12 15:03 ` john
  2017-03-12 15:30 ` john
  1 sibling, 0 replies; 4+ messages in thread
From: john @ 2017-03-12 15:03 UTC (permalink / raw)


On Tue, Feb 21, 2017 at 12:36:04AM +0100, 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".
> 
> Signed-off-by: Petr Vorel <petr.vorel at gmail.com>
> Cc: "Jason A. Donenfeld" <Jason at zx2c4.com>
> Cc: John Keeping <john at keeping.me.uk>
> Reviewed-by: John Keeping <john at keeping.me.uk>
> ---
> Thanks for your review John. Are we there yet? :-)

I thought we were, but... I just applied this and it breaks t0103-log.sh
'no links with space in path'.

Having looked at it, our general printing of repo URLs falls foul of
that test - if you copy+paste it into t0101-index.sh then that fails as
well, and the tests are right to fail here.

Given that, I don't see the need to block this patch for introducing a
failure here (exactly the same failure will already exist on other
pages), but we should do something about the failing test case.

I propose squashing this in:

-- >8 --
diff --git a/tests/t0103-log.sh b/tests/t0103-log.sh
index bdf1435..2818016 100755
--- a/tests/t0103-log.sh
+++ b/tests/t0103-log.sh
@@ -17,7 +17,7 @@ test_expect_success 'generate "with%20space/log?qt=grep&q=commit+1"' '
 test_expect_success 'find commit 1' 'grep "commit 1" tmp'
 test_expect_success 'find link with %20 in path' 'grep "/with%20space/log/?qt=grep" tmp'
 test_expect_success 'find link with + in arg' 'grep "/log/?qt=grep&amp;q=commit+1" tmp'
-test_expect_success 'no links with space in path' '! grep "href=./with space/" tmp'
+test_expect_failure 'no links with space in path' '! grep "href=./with space/" tmp'
 test_expect_success 'no links with space in arg' '! grep "q=commit 1" tmp'
 test_expect_success 'commit 2 is not visible' '! grep "commit 2" tmp'
-- 8< --
 
> Changes v6->v7:
> * Remove redundant '*'
> * Add Reviewed-by
> ---
>  cgitrc.5.txt |  4 +++-
>  ui-shared.c  | 27 ++++++++++++++++++++++++++-
>  2 files changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/cgitrc.5.txt b/cgitrc.5.txt
> index 9fcf445..500b7ee 100644
> --- a/cgitrc.5.txt
> +++ b/cgitrc.5.txt
> @@ -604,7 +604,9 @@ repo.source-filter::
>  
>  repo.url::
>  	The relative url used to access the repository. This must be the first
> -	setting specified for each repo. Default value: none.
> +	setting specified for each repo. Unless repo.url is different from
> +	repo.name, cgit splits the repository link in the top of repository
> +	"summary" view. Default value: none.
>  
>  
>  REPOSITORY-SPECIFIC CGITRC FILE
> diff --git a/ui-shared.c b/ui-shared.c
> index 2e4fcd9..e4f956d 100644
> --- a/ui-shared.c
> +++ b/ui-shared.c
> @@ -936,7 +936,32 @@ 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);
> +
> +		/*
> +		 * NOTE: If repo.name and repo.url are different, we don't split link as
> +		 * it wouldn't make sense to split the path.
> +		 */
> +		if (!strcmp(ctx.repo->name, ctx.repo->url)) {
> +			char *name = ctx.repo->name;
> +			char *start = name;
> +			for (;;) {
> +				char *delim = strchr(start, '/');
> +				if (delim)
> +					*delim = '\0';
> +
> +				html_link_open(cgit_repourl(name), NULL, NULL);
> +				html_ntxt(strlen(start), start);
> +				html_link_close();
> +
> +				if (!delim)
> +					break;
> +				*delim = '/';
> +				html("/");
> +				start = delim + 1;
> +			}
> +		} 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'>\n");
> -- 
> 2.11.0
> 
> _______________________________________________
> CGit mailing list
> CGit at lists.zx2c4.com
> https://lists.zx2c4.com/mailman/listinfo/cgit


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v7 1/1] ui-shared: allow to split the repository link
  2017-02-20 23:36 [PATCH v7 1/1] ui-shared: allow to split the repository link petr.vorel
  2017-03-12 15:03 ` john
@ 2017-03-12 15:30 ` john
  2017-03-12 17:02   ` petr.vorel
  1 sibling, 1 reply; 4+ messages in thread
From: john @ 2017-03-12 15:30 UTC (permalink / raw)


On Tue, Feb 21, 2017 at 12:36:04AM +0100, 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".
> 
> Signed-off-by: Petr Vorel <petr.vorel at gmail.com>
> Cc: "Jason A. Donenfeld" <Jason at zx2c4.com>
> Cc: John Keeping <john at keeping.me.uk>
> Reviewed-by: John Keeping <john at keeping.me.uk>
> ---
> Thanks for your review John. Are we there yet? :-)

I just noticed one more thing...

> Changes v6->v7:
> * Remove redundant '*'
> * Add Reviewed-by
> ---
>  cgitrc.5.txt |  4 +++-
>  ui-shared.c  | 27 ++++++++++++++++++++++++++-
>  2 files changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/cgitrc.5.txt b/cgitrc.5.txt
> index 9fcf445..500b7ee 100644
> --- a/cgitrc.5.txt
> +++ b/cgitrc.5.txt
> @@ -604,7 +604,9 @@ repo.source-filter::
>  
>  repo.url::
>  	The relative url used to access the repository. This must be the first
> -	setting specified for each repo. Default value: none.
> +	setting specified for each repo. Unless repo.url is different from
> +	repo.name, cgit splits the repository link in the top of repository
> +	"summary" view. Default value: none.
>  
>  
>  REPOSITORY-SPECIFIC CGITRC FILE
> diff --git a/ui-shared.c b/ui-shared.c
> index 2e4fcd9..e4f956d 100644
> --- a/ui-shared.c
> +++ b/ui-shared.c
> @@ -936,7 +936,32 @@ 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);
> +
> +		/*
> +		 * NOTE: If repo.name and repo.url are different, we don't split link as
> +		 * it wouldn't make sense to split the path.
> +		 */
> +		if (!strcmp(ctx.repo->name, ctx.repo->url)) {
> +			char *name = ctx.repo->name;
> +			char *start = name;
> +			for (;;) {
> +				char *delim = strchr(start, '/');
> +				if (delim)
> +					*delim = '\0';
> +
> +				html_link_open(cgit_repourl(name), NULL, NULL);

The value returned by cgit_repourl() should be freed.

> +				html_ntxt(strlen(start), start);
> +				html_link_close();
> +
> +				if (!delim)
> +					break;
> +				*delim = '/';
> +				html("/");
> +				start = delim + 1;
> +			}
> +		} 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'>\n");
> -- 
> 2.11.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v7 1/1] ui-shared: allow to split the repository link
  2017-03-12 15:30 ` john
@ 2017-03-12 17:02   ` petr.vorel
  0 siblings, 0 replies; 4+ messages in thread
From: petr.vorel @ 2017-03-12 17:02 UTC (permalink / raw)


Hi John,
> > Thanks for your review John. Are we there yet? :-)

> I just noticed one more thing...

> > +				html_link_open(cgit_repourl(name), NULL, NULL);
> The value returned by cgit_repourl() should be freed.

Thanks for info, fixed in v8.


Kind regards,
Petr


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-03-12 17:02 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-20 23:36 [PATCH v7 1/1] ui-shared: allow to split the repository link petr.vorel
2017-03-12 15:03 ` john
2017-03-12 15:30 ` john
2017-03-12 17:02   ` petr.vorel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).