[PATCH] ui-repolist: return HTTP 404 if no repositories found

[John Keeping]

On Mon, Dec 07, 2015 at 02:28:09PM -0500, Peter Colberg wrote:
> ---

This is missing your sign-off.  See [1] for what this means.  It would
also be useful to include some of the rationale from your cover letter
in the patch itself, particularly the motivation for the change.

I would normally expect this to be split into two patches; one to
extract the new "is_visible()" function and the other to add the new
feature.  This makes reviewers' lives easier.

The patch itself looks good; I would probably have extracted the for
loop into a "any_repos_visible()" function, which would avoid polluting
the entire cgit_print_repolist() with two new variables that are only
used in the first few lines, but the patch in its current form is also
OK.

[1] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches?h=v4.3#n409

>  ui-repolist.c | 32 +++++++++++++++++++++++++-------
>  1 file changed, 25 insertions(+), 7 deletions(-)
> 
> diff --git a/ui-repolist.c b/ui-repolist.c
> index a2e9e07..f86c6b7 100644
> --- a/ui-repolist.c
> +++ b/ui-repolist.c
> @@ -106,6 +106,15 @@ static int is_in_url(struct cgit_repo *repo)
>  	return 0;
>  }
>  
> +static int is_visible(struct cgit_repo *repo)
> +{
> +	if (repo->hide || repo->ignore)
> +		return 0;
> +	if (!(is_match(repo) && is_in_url(repo)))
> +		return 0;
> +	return 1;
> +}
> +
>  static void print_sort_header(const char *title, const char *sort)
>  {
>  	char *currenturl = cgit_currenturl();
> @@ -252,10 +261,23 @@ static int sort_repolist(char *field)
>  
>  void cgit_print_repolist(void)
>  {
> -	int i, columns = 3, hits = 0, header = 0;
> +	int i, columns = 3, found = 0, hits = 0, header = 0;
>  	char *last_section = NULL;
>  	char *section;
>  	int sorted = 0;
> +	struct cgit_repo *repo;
> +
> +	for (i = 0; i < cgit_repolist.count; i++) {
> +		repo = &cgit_repolist.repos[i];
> +		if (is_visible(repo)) {
> +			found = 1;
> +			break;
> +		}
> +	}
> +	if (!found) {
> +		cgit_print_error_page(404, "Not found", "No repositories found");
> +		return;
> +	}
>  
>  	if (ctx.cfg.enable_index_links)
>  		++columns;
> @@ -278,9 +300,7 @@ void cgit_print_repolist(void)
>  	html("<table summary='repository list' class='list nowrap'>");
>  	for (i = 0; i < cgit_repolist.count; i++) {
>  		ctx.repo = &cgit_repolist.repos[i];
> -		if (ctx.repo->hide || ctx.repo->ignore)
> -			continue;
> -		if (!(is_match(ctx.repo) && is_in_url(ctx.repo)))
> +		if (!is_visible(ctx.repo))
>  			continue;
>  		hits++;
>  		if (hits <= ctx.qry.ofs)
> @@ -340,9 +360,7 @@ void cgit_print_repolist(void)
>  		html("</tr>\n");
>  	}
>  	html("</table>");
> -	if (!hits)
> -		cgit_print_error("No repositories found");
> -	else if (hits > ctx.cfg.max_repo_count)
> +	if (hits > ctx.cfg.max_repo_count)
>  		print_pager(hits, ctx.cfg.max_repo_count, ctx.qry.search, ctx.qry.sort);
>  	cgit_print_docend();
>  }
> -- 
> 2.6.2
> 
> _______________________________________________
> CGit mailing list
> CGit at lists.zx2c4.com
> http://lists.zx2c4.com/mailman/listinfo/cgit