* repo.desc as raw html? @ 2018-06-20 9:04 andy 2018-06-20 13:17 ` [PATCH] noheader: place branch combo on tabs if no header andy ` (2 more replies) 0 siblings, 3 replies; 10+ messages in thread From: andy @ 2018-06-20 9:04 UTC (permalink / raw) Hi - I want to put a repo-specific link to my project mailing list in the header area. But there doesn't seem to be a way to do it. noheader=1 also removes functionality for branch switching UI, which is not desirable. There's a "homepage" link concept but a) that's positioned in the wrong place logically, it's in the vertical context of the repo display area not the header, and b) just adding a few more inflexible individual settings doesn't sound a good way. What's the feeling about just changing ui-shared.c where it prints it from html_txt(ctx.repo->desc); to html(ctx.repo->desc); ? As html I can put what I want in there... homepage and mailing list links, text, inline images etc, and because it's in the header, it doesn't need to know any detail about what's going on in the repo view. -Andy ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] noheader: place branch combo on tabs if no header 2018-06-20 9:04 repo.desc as raw html? andy @ 2018-06-20 13:17 ` andy 2018-06-23 10:50 ` john 2018-06-21 6:46 ` [PATCH 1/2] config: add root-desc-html and repo.desc-html andy 2018-06-21 6:46 ` [PATCH 2/2] ui-shared: emit root-desc-html and repo.desc-html after their text counterparts andy 2 siblings, 1 reply; 10+ messages in thread From: andy @ 2018-06-20 13:17 UTC (permalink / raw) noheader=1 stops the static page header from being emitted by cgit, but along with that, the small form that lets the user change branch context on the page is also lost. This isn't actually static since it contains a dynamic list of branches; it can't be reproduced on the user's external header static html. So it seems it doesn't belong to the set of header things that should be disabled. This patch relocates the branch selection combo and form on the left of the tabs line if noheader=1. It doesn't change anything if noheader is not set. Signed-off-by: Andy Green <andy at warmcat.com> --- ui-shared.c | 34 ++++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/ui-shared.c b/ui-shared.c index c9a34fb..082a6f1 100644 --- a/ui-shared.c +++ b/ui-shared.c @@ -958,6 +958,19 @@ static void cgit_print_path_crumbs(char *path) ctx.qry.path = old_path; } +static void print_branch_combo_form(void) +{ + html("<form method='get'>\n"); + cgit_add_hidden_formfields(0, 1, ctx.qry.page); + html("<select name='h' onchange='this.form.submit();'>\n"); + for_each_branch_ref(print_branch_option, ctx.qry.head); + if (ctx.repo->enable_remote_branches) + for_each_remote_ref(print_branch_option, ctx.qry.head); + html("</select> "); + html("<input type='submit' value='switch'/>"); + html("</form>"); +} + static void print_header(void) { char *logo = NULL, *logo_link = NULL; @@ -991,15 +1004,7 @@ static void print_header(void) 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"); - cgit_add_hidden_formfields(0, 1, ctx.qry.page); - html("<select name='h' onchange='this.form.submit();'>\n"); - for_each_branch_ref(print_branch_option, ctx.qry.head); - if (ctx.repo->enable_remote_branches) - for_each_remote_ref(print_branch_option, ctx.qry.head); - html("</select> "); - html("<input type='submit' value='switch'/>"); - html("</form>"); + print_branch_combo_form(); } } else html_txt(ctx.cfg.root_title); @@ -1023,8 +1028,15 @@ void cgit_print_pageheader(void) if (!ctx.env.authenticated || !ctx.cfg.noheader) print_header(); - html("<table class='tabs'><tr><td>\n"); + html("<table class='tabs'><tr>\n"); if (ctx.env.authenticated && ctx.repo) { + if (ctx.cfg.noheader) { + html("<td class='form' style='text-align:left'>"); + print_branch_combo_form(); + html("</td><td style='text-align:center'>"); + } + html("<td>"); + if (ctx.repo->readme.nr) reporevlink("about", "about", NULL, hc("about"), ctx.qry.head, NULL, @@ -1081,6 +1093,8 @@ void cgit_print_pageheader(void) html("</form>\n"); } else if (ctx.env.authenticated) { char *currenturl = cgit_currenturl(); + + html("<td>"); site_link(NULL, "index", NULL, hc("repolist"), NULL, NULL, 0, 1); if (ctx.cfg.root_readme) site_link("about", "about", NULL, hc("about"), ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] noheader: place branch combo on tabs if no header 2018-06-20 13:17 ` [PATCH] noheader: place branch combo on tabs if no header andy @ 2018-06-23 10:50 ` john 0 siblings, 0 replies; 10+ messages in thread From: john @ 2018-06-23 10:50 UTC (permalink / raw) On Wed, Jun 20, 2018 at 09:17:00PM +0800, Andy Green wrote: > noheader=1 stops the static page header from being emitted by > cgit, but along with that, the small form that lets the user > change branch context on the page is also lost. > > This isn't actually static since it contains a dynamic list of > branches; it can't be reproduced on the user's external header > static html. So it seems it doesn't belong to the set of header > things that should be disabled. > > This patch relocates the branch selection combo and > form on the left of the tabs line if noheader=1. It doesn't > change anything if noheader is not set. > > Signed-off-by: Andy Green <andy at warmcat.com> This makes noheader=1 a lot more usable! I wonder if the branch combo should be somewhere to the right of the main tabs, but I don't feel strongly either way. Reviewed-by: John Keeping <john at keeping.me.uk> > --- > ui-shared.c | 34 ++++++++++++++++++++++++---------- > 1 file changed, 24 insertions(+), 10 deletions(-) > > diff --git a/ui-shared.c b/ui-shared.c > index c9a34fb..082a6f1 100644 > --- a/ui-shared.c > +++ b/ui-shared.c > @@ -958,6 +958,19 @@ static void cgit_print_path_crumbs(char *path) > ctx.qry.path = old_path; > } > > +static void print_branch_combo_form(void) > +{ > + html("<form method='get'>\n"); > + cgit_add_hidden_formfields(0, 1, ctx.qry.page); > + html("<select name='h' onchange='this.form.submit();'>\n"); > + for_each_branch_ref(print_branch_option, ctx.qry.head); > + if (ctx.repo->enable_remote_branches) > + for_each_remote_ref(print_branch_option, ctx.qry.head); > + html("</select> "); > + html("<input type='submit' value='switch'/>"); > + html("</form>"); > +} > + > static void print_header(void) > { > char *logo = NULL, *logo_link = NULL; > @@ -991,15 +1004,7 @@ static void print_header(void) > 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"); > - cgit_add_hidden_formfields(0, 1, ctx.qry.page); > - html("<select name='h' onchange='this.form.submit();'>\n"); > - for_each_branch_ref(print_branch_option, ctx.qry.head); > - if (ctx.repo->enable_remote_branches) > - for_each_remote_ref(print_branch_option, ctx.qry.head); > - html("</select> "); > - html("<input type='submit' value='switch'/>"); > - html("</form>"); > + print_branch_combo_form(); > } > } else > html_txt(ctx.cfg.root_title); > @@ -1023,8 +1028,15 @@ void cgit_print_pageheader(void) > if (!ctx.env.authenticated || !ctx.cfg.noheader) > print_header(); > > - html("<table class='tabs'><tr><td>\n"); > + html("<table class='tabs'><tr>\n"); > if (ctx.env.authenticated && ctx.repo) { > + if (ctx.cfg.noheader) { > + html("<td class='form' style='text-align:left'>"); > + print_branch_combo_form(); > + html("</td><td style='text-align:center'>"); > + } > + html("<td>"); > + > if (ctx.repo->readme.nr) > reporevlink("about", "about", NULL, > hc("about"), ctx.qry.head, NULL, > @@ -1081,6 +1093,8 @@ void cgit_print_pageheader(void) > html("</form>\n"); > } else if (ctx.env.authenticated) { > char *currenturl = cgit_currenturl(); > + > + html("<td>"); > site_link(NULL, "index", NULL, hc("repolist"), NULL, NULL, 0, 1); > if (ctx.cfg.root_readme) > site_link("about", "about", NULL, hc("about"), ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] config: add root-desc-html and repo.desc-html 2018-06-20 9:04 repo.desc as raw html? andy 2018-06-20 13:17 ` [PATCH] noheader: place branch combo on tabs if no header andy @ 2018-06-21 6:46 ` andy 2018-06-21 6:46 ` [PATCH 2/2] ui-shared: emit root-desc-html and repo.desc-html after their text counterparts andy 2 siblings, 0 replies; 10+ messages in thread From: andy @ 2018-06-21 6:46 UTC (permalink / raw) These are optional, default-empty raw html strings that are emitted after the corresponding text-only versions root-desc and repo.desc when they are used in the header area. This provides a flexible way to place buttons or links in the header region for both the repo index page and for repos individually. The existing root-desc and repo.desc keep their original meaning as a shortish text-only repo description. Signed-off-by: Andy Green <andy at warmcat.com> --- cgit.c | 4 ++++ cgit.h | 2 ++ cgitrc.5.txt | 8 ++++++++ 3 files changed, 14 insertions(+) diff --git a/cgit.c b/cgit.c index bdb2fad..1b819e7 100644 --- a/cgit.c +++ b/cgit.c @@ -52,6 +52,8 @@ static void repo_config(struct cgit_repo *repo, const char *name, const char *va repo->clone_url = xstrdup(value); else if (!strcmp(name, "desc")) repo->desc = xstrdup(value); + else if (!strcmp(name, "desc-html")) + repo->desc_html = xstrdup(value); else if (!strcmp(name, "owner")) repo->owner = xstrdup(value); else if (!strcmp(name, "homepage")) @@ -142,6 +144,8 @@ static void config_cb(const char *name, const char *value) ctx.cfg.root_title = xstrdup(value); else if (!strcmp(name, "root-desc")) ctx.cfg.root_desc = xstrdup(value); + else if (!strcmp(name, "root-desc-html")) + ctx.cfg.root_desc_html = xstrdup(value); else if (!strcmp(name, "root-readme")) ctx.cfg.root_readme = xstrdup(value); else if (!strcmp(name, "css")) diff --git a/cgit.h b/cgit.h index 99ea7a2..1094062 100644 --- a/cgit.h +++ b/cgit.h @@ -81,6 +81,7 @@ struct cgit_repo { char *name; char *path; char *desc; + char *desc_html; char *owner; char *homepage; char *defbranch; @@ -207,6 +208,7 @@ struct cgit_config { char *robots; char *root_title; char *root_desc; + char *root_desc_html; char *root_readme; char *script_name; char *section; diff --git a/cgitrc.5.txt b/cgitrc.5.txt index 99fc799..4ddb51e 100644 --- a/cgitrc.5.txt +++ b/cgitrc.5.txt @@ -378,6 +378,10 @@ root-desc:: Text printed below the heading on the repository index page. Default value: "a fast webinterface for the git dscm". +root-desc-html:: + Optional additional raw html to show in the header after root-desc. + Default value: none. + root-readme:: The content of the file specified with this option will be included verbatim below the "about" link on the repository index page. Default @@ -503,6 +507,10 @@ repo.defbranch:: repo.desc:: The value to show as repository description. Default value: none. +repo.desc-html:: + Optional additional raw html to show in the header after repo.desc. + Default value: none. + repo.email-filter:: Override the default email-filter. Default value: none. See also: "enable-filter-overrides". See also: "FILTER API". ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] ui-shared: emit root-desc-html and repo.desc-html after their text counterparts 2018-06-20 9:04 repo.desc as raw html? andy 2018-06-20 13:17 ` [PATCH] noheader: place branch combo on tabs if no header andy 2018-06-21 6:46 ` [PATCH 1/2] config: add root-desc-html and repo.desc-html andy @ 2018-06-21 6:46 ` andy 2018-06-23 10:28 ` john 2 siblings, 1 reply; 10+ messages in thread From: andy @ 2018-06-21 6:46 UTC (permalink / raw) Where root-desc and repo.desc are used in the header region, also emit their html counterparts afterwards if they are defined. Where root-desc are repo.desc are used outside the header, eg in the repo list, leave it as it is without adding any related html. Signed-off-by: Andy Green <andy at warmcat.com> --- ui-shared.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ui-shared.c b/ui-shared.c index c8f4d8f..a9ec430 100644 --- a/ui-shared.c +++ b/ui-shared.c @@ -1013,11 +1013,15 @@ static void print_header(void) html("<tr><td class='sub'>"); if (ctx.repo) { html_txt(ctx.repo->desc); + if (ctx.repo->desc_html) + html(ctx.repo->desc_html); html("</td><td class='sub right'>"); html_txt(ctx.repo->owner); } else { if (ctx.cfg.root_desc) html_txt(ctx.cfg.root_desc); + if (ctx.cfg.root_desc_html) + html(ctx.cfg.root_desc_html); } html("</td></tr></table>\n"); } ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] ui-shared: emit root-desc-html and repo.desc-html after their text counterparts 2018-06-21 6:46 ` [PATCH 2/2] ui-shared: emit root-desc-html and repo.desc-html after their text counterparts andy @ 2018-06-23 10:28 ` john 2018-06-23 10:33 ` andy 0 siblings, 1 reply; 10+ messages in thread From: john @ 2018-06-23 10:28 UTC (permalink / raw) On Thu, Jun 21, 2018 at 02:46:47PM +0800, Andy Green wrote: > Where root-desc and repo.desc are used in the header region, also > emit their html counterparts afterwards if they are defined. > > Where root-desc are repo.desc are used outside the header, > eg in the repo list, leave it as it is without adding any > related html. > > Signed-off-by: Andy Green <andy at warmcat.com> > --- I think this should be squashed with the previous patch since it makes it easier to see what's going on. When I read your initial email on this, I thought we could introduce a new HTML version of the description and use that *instead of* the plain text one if the HTML variant is available. Having looked at the current implementation of repo->desc, I think that's desirable because the reason we don't have a null-check for that in the context below is that it will be set to "[no description]" if no other value is provided. If a user has set repo->desc_html, I don't think we want to print "[no description]" before showing the HTML description! > ui-shared.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/ui-shared.c b/ui-shared.c > index c8f4d8f..a9ec430 100644 > --- a/ui-shared.c > +++ b/ui-shared.c > @@ -1013,11 +1013,15 @@ static void print_header(void) > html("<tr><td class='sub'>"); > if (ctx.repo) { > html_txt(ctx.repo->desc); > + if (ctx.repo->desc_html) > + html(ctx.repo->desc_html); > html("</td><td class='sub right'>"); > html_txt(ctx.repo->owner); > } else { > if (ctx.cfg.root_desc) > html_txt(ctx.cfg.root_desc); > + if (ctx.cfg.root_desc_html) > + html(ctx.cfg.root_desc_html); > } > html("</td></tr></table>\n"); > } ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] ui-shared: emit root-desc-html and repo.desc-html after their text counterparts 2018-06-23 10:28 ` john @ 2018-06-23 10:33 ` andy 2018-06-23 10:53 ` john 0 siblings, 1 reply; 10+ messages in thread From: andy @ 2018-06-23 10:33 UTC (permalink / raw) On 06/23/2018 06:28 PM, John Keeping wrote: > On Thu, Jun 21, 2018 at 02:46:47PM +0800, Andy Green wrote: >> Where root-desc and repo.desc are used in the header region, also >> emit their html counterparts afterwards if they are defined. >> >> Where root-desc are repo.desc are used outside the header, >> eg in the repo list, leave it as it is without adding any >> related html. >> >> Signed-off-by: Andy Green <andy at warmcat.com> >> --- > > I think this should be squashed with the previous patch since it makes > it easier to see what's going on. > > When I read your initial email on this, I thought we could introduce a > new HTML version of the description and use that *instead of* the plain > text one if the HTML variant is available. I actually first implemented just rendering what we have as raw html... > Having looked at the current implementation of repo->desc, I think > that's desirable because the reason we don't have a null-check for that > in the context below is that it will be set to "[no description]" if no > other value is provided. If a user has set repo->desc_html, I don't > think we want to print "[no description]" before showing the HTML > description! I take the point, but it turned out there are two separate kinds of description here... the text-only, existing one that is used, eg, in the list of repos. And a "functional" HTML part that has buttons or whatever specific to the repo and used on the header part. With just treating them as one, the repo list gained meaningless HTML buttons or pictures or whatever decoration was put there. The repo list just wants a short textual description that already exists. So it arrived at this, leave that be, and add an optional HTML decoration part. -Andy ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] ui-shared: emit root-desc-html and repo.desc-html after their text counterparts 2018-06-23 10:33 ` andy @ 2018-06-23 10:53 ` john 2018-06-23 11:08 ` andy 0 siblings, 1 reply; 10+ messages in thread From: john @ 2018-06-23 10:53 UTC (permalink / raw) On Sat, Jun 23, 2018 at 06:33:38PM +0800, Andy Green wrote: > > > On 06/23/2018 06:28 PM, John Keeping wrote: > > On Thu, Jun 21, 2018 at 02:46:47PM +0800, Andy Green wrote: > >> Where root-desc and repo.desc are used in the header region, also > >> emit their html counterparts afterwards if they are defined. > >> > >> Where root-desc are repo.desc are used outside the header, > >> eg in the repo list, leave it as it is without adding any > >> related html. > >> > >> Signed-off-by: Andy Green <andy at warmcat.com> > >> --- > > > > I think this should be squashed with the previous patch since it makes > > it easier to see what's going on. > > > > When I read your initial email on this, I thought we could introduce a > > new HTML version of the description and use that *instead of* the plain > > text one if the HTML variant is available. > > I actually first implemented just rendering what we have as raw html... I don't think we can do that without introducing an HTML injection risk in configurations that are currently safe. > > Having looked at the current implementation of repo->desc, I think > > that's desirable because the reason we don't have a null-check for that > > in the context below is that it will be set to "[no description]" if no > > other value is provided. If a user has set repo->desc_html, I don't > > think we want to print "[no description]" before showing the HTML > > description! > > I take the point, but it turned out there are two separate kinds of > description here... the text-only, existing one that is used, eg, in the > list of repos. And a "functional" HTML part that has buttons or > whatever specific to the repo and used on the header part. > > With just treating them as one, the repo list gained meaningless HTML > buttons or pictures or whatever decoration was put there. The repo list > just wants a short textual description that already exists. So it > arrived at this, leave that be, and add an optional HTML decoration part. OK, that makes sense. Maybe we need the following check, but it is quite ugly! if (ctx.repo->desc && (ctx.repo->desc != cgit_default_repo_desc || !ctx.repo->html_desc)) that is, show the plain text description only if it has been customised or if there is no HTML description. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] ui-shared: emit root-desc-html and repo.desc-html after their text counterparts 2018-06-23 10:53 ` john @ 2018-06-23 11:08 ` andy 2018-06-23 16:33 ` john 0 siblings, 1 reply; 10+ messages in thread From: andy @ 2018-06-23 11:08 UTC (permalink / raw) On 06/23/2018 06:53 PM, John Keeping wrote: > On Sat, Jun 23, 2018 at 06:33:38PM +0800, Andy Green wrote: >> >> >> On 06/23/2018 06:28 PM, John Keeping wrote: >>> On Thu, Jun 21, 2018 at 02:46:47PM +0800, Andy Green wrote: >>>> Where root-desc and repo.desc are used in the header region, also >>>> emit their html counterparts afterwards if they are defined. >>>> >>>> Where root-desc are repo.desc are used outside the header, >>>> eg in the repo list, leave it as it is without adding any >>>> related html. >>>> >>>> Signed-off-by: Andy Green <andy at warmcat.com> >>>> --- >>> >>> I think this should be squashed with the previous patch since it makes >>> it easier to see what's going on. >>> >>> When I read your initial email on this, I thought we could introduce a >>> new HTML version of the description and use that *instead of* the plain >>> text one if the HTML variant is available. >> >> I actually first implemented just rendering what we have as raw html... > > I don't think we can do that without introducing an HTML injection risk > in configurations that are currently safe. If someone else controls the repo.desc, yes it's not nice. Isn't it the same problem if someone else controls the repo.desc_html? >>> Having looked at the current implementation of repo->desc, I think >>> that's desirable because the reason we don't have a null-check for that >>> in the context below is that it will be set to "[no description]" if no >>> other value is provided. If a user has set repo->desc_html, I don't >>> think we want to print "[no description]" before showing the HTML >>> description! >> >> I take the point, but it turned out there are two separate kinds of >> description here... the text-only, existing one that is used, eg, in the >> list of repos. And a "functional" HTML part that has buttons or >> whatever specific to the repo and used on the header part. >> >> With just treating them as one, the repo list gained meaningless HTML >> buttons or pictures or whatever decoration was put there. The repo list >> just wants a short textual description that already exists. So it >> arrived at this, leave that be, and add an optional HTML decoration part. > > OK, that makes sense. Maybe we need the following check, but it is > quite ugly! > > if (ctx.repo->desc && > (ctx.repo->desc != cgit_default_repo_desc || > !ctx.repo->html_desc)) > > that is, show the plain text description only if it has been customised > or if there is no HTML description. I'm used to looking in the mirror, so it's OK for me :-) It could also be done as a filter script that purifies strings given to it and wraps them in canned html. Basically the cgit repo probably isn't existing on its own. It's part of a project that has links relevant to someone who, eg, stumbled on the cgit repo. In cgit's own case, the cgit header has the author name but no link to its mailing list... that's not right actually. So somehow there should be a way to integrate the cgit url with other urls strongly likely to be of interest to someone who is interested in the cgit. -Andy ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] ui-shared: emit root-desc-html and repo.desc-html after their text counterparts 2018-06-23 11:08 ` andy @ 2018-06-23 16:33 ` john 0 siblings, 0 replies; 10+ messages in thread From: john @ 2018-06-23 16:33 UTC (permalink / raw) On Sat, Jun 23, 2018 at 07:08:08PM +0800, Andy Green wrote: > > > On 06/23/2018 06:53 PM, John Keeping wrote: > > On Sat, Jun 23, 2018 at 06:33:38PM +0800, Andy Green wrote: > >> > >> > >> On 06/23/2018 06:28 PM, John Keeping wrote: > >>> On Thu, Jun 21, 2018 at 02:46:47PM +0800, Andy Green wrote: > >>>> Where root-desc and repo.desc are used in the header region, also > >>>> emit their html counterparts afterwards if they are defined. > >>>> > >>>> Where root-desc are repo.desc are used outside the header, > >>>> eg in the repo list, leave it as it is without adding any > >>>> related html. > >>>> > >>>> Signed-off-by: Andy Green <andy at warmcat.com> > >>>> --- > >>> > >>> I think this should be squashed with the previous patch since it makes > >>> it easier to see what's going on. > >>> > >>> When I read your initial email on this, I thought we could introduce a > >>> new HTML version of the description and use that *instead of* the plain > >>> text one if the HTML variant is available. > >> > >> I actually first implemented just rendering what we have as raw html... > > > > I don't think we can do that without introducing an HTML injection risk > > in configurations that are currently safe. > > If someone else controls the repo.desc, yes it's not nice. > > Isn't it the same problem if someone else controls the repo.desc_html? No, because repo.desc_html will always be treated as HTML. If we change the behaviour of CGit for the _existing_ repo.desc then a configuration which is currently serving plain text will suddenly start interpreting that as HTML. Even if we make it very clear in the release notes that this is happening, it's not great for a simple software update to change this. > >>> Having looked at the current implementation of repo->desc, I think > >>> that's desirable because the reason we don't have a null-check for that > >>> in the context below is that it will be set to "[no description]" if no > >>> other value is provided. If a user has set repo->desc_html, I don't > >>> think we want to print "[no description]" before showing the HTML > >>> description! > >> > >> I take the point, but it turned out there are two separate kinds of > >> description here... the text-only, existing one that is used, eg, in the > >> list of repos. And a "functional" HTML part that has buttons or > >> whatever specific to the repo and used on the header part. > >> > >> With just treating them as one, the repo list gained meaningless HTML > >> buttons or pictures or whatever decoration was put there. The repo list > >> just wants a short textual description that already exists. So it > >> arrived at this, leave that be, and add an optional HTML decoration part. > > > > OK, that makes sense. Maybe we need the following check, but it is > > quite ugly! > > > > if (ctx.repo->desc && > > (ctx.repo->desc != cgit_default_repo_desc || > > !ctx.repo->html_desc)) > > > > that is, show the plain text description only if it has been customised > > or if there is no HTML description. > > I'm used to looking in the mirror, so it's OK for me :-) > > It could also be done as a filter script that purifies strings given to > it and wraps them in canned html. > > Basically the cgit repo probably isn't existing on its own. It's part > of a project that has links relevant to someone who, eg, stumbled on the > cgit repo. In cgit's own case, the cgit header has the author name but > no link to its mailing list... that's not right actually. So somehow > there should be a way to integrate the cgit url with other urls strongly > likely to be of interest to someone who is interested in the cgit. I had a look at libwebsockets.org and the additional links look really nice. I think the implementation here is fine (maybe with the explanation of "two kinds of description" in the commit message), but I can see some people wanting to disable the plain text description and use the HTML one for everything. To make that work, we need to use the ugly if condition above so that cgit_default_repo_desc is hidden (even though it's ugly, it works and with your explanation above I agree that the simple if(repo_desc_html) else if(repo_desc) version isn't sufficient). ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-06-23 16:33 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-06-20 9:04 repo.desc as raw html? andy 2018-06-20 13:17 ` [PATCH] noheader: place branch combo on tabs if no header andy 2018-06-23 10:50 ` john 2018-06-21 6:46 ` [PATCH 1/2] config: add root-desc-html and repo.desc-html andy 2018-06-21 6:46 ` [PATCH 2/2] ui-shared: emit root-desc-html and repo.desc-html after their text counterparts andy 2018-06-23 10:28 ` john 2018-06-23 10:33 ` andy 2018-06-23 10:53 ` john 2018-06-23 11:08 ` andy 2018-06-23 16:33 ` john
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).