Re: [PATCH] gitweb: allow extra breadcrumbs to prefix the trail
Jakub Narębski jna...@gmail.com wrote: On Wed, Jul 3, 2013 at 11:59 PM, Jonathan Nieder jrnie...@gmail.com wrote: Tony Finch wrote: +@extra_breadcrumbs:: + Additional links to be added to the start of the breadcrumb trail, + that are logically above the gitweb projects list. For example, + links to the organization and department which host the gitweb + server. Each element of the list is a reference to an array, + in which element 0 is the link text and element 1 is the + target URL. Is arbitrary HTML permitted in the link text? I had the same question when I was wondering about abusing $home_link_str to do this without a patch :-) I think it makes sense to permit it for consistency with $home_link_str, but it might be worth mentioning in the manpage so the administrator knows not to set it to something user-controlled --- e.g.: I left this detail out of the man page for consistency with the documentation for $home_link_str. Nb. it would be nice to have relation of @extra_breadcrumbs with $home_link_str explained. I will make that clearer. Thanks for reviewing the patch. Tony. -- f.anthony.n.finch d...@dotat.at http://dotat.at/ Forties, Cromarty: East, veering southeast, 4 or 5, occasionally 6 at first. Rough, becoming slight or moderate. Showers, rain at first. Moderate or good, occasionally poor at first.
Re: [PATCH] gitweb: allow extra breadcrumbs to prefix the trail
On Thu, Jul 4, 2013 at 10:44 AM, Tony Finch d...@dotat.at wrote: Jakub Narębski jna...@gmail.com wrote: On Wed, Jul 3, 2013 at 11:59 PM, Jonathan Nieder jrnie...@gmail.com wrote: Tony Finch wrote: +@extra_breadcrumbs:: BTW. perhaps (it is only an idea) @top_level_breadcrumbs or @home_breadcrumbs would be a better name for this variable. + Additional links to be added to the start of the breadcrumb trail, + that are logically above the gitweb projects list. For example, + links to the organization and department which host the gitweb + server. Each element of the list is a reference to an array, + in which element 0 is the link text and element 1 is the + target URL. Is arbitrary HTML permitted in the link text? I had the same question when I was wondering about abusing $home_link_str to do this without a patch :-) Not exactly arbitrary, as it is inside A element, so it cannot contain A links itself (hyperlinks should not be nested), but it is not esc_html-aped. I think it makes sense to permit it for consistency with $home_link_str, but it might be worth mentioning in the manpage so the administrator knows not to set it to something user-controlled --- e.g.: I left this detail out of the man page for consistency with the documentation for $home_link_str. It would be better to improve documentation, than follow current bad practice... ;-P Nb. it would be nice to have relation of @extra_breadcrumbs with $home_link_str explained. I will make that clearer. Perhaps even make ( [ $home_link_str, $home_link ] ) to be default value for @extra_breadcrumbs, making new feature generalization of $home_link*, similarly to how it was done for $stylesheet - @stylesheets transition. What do you think about it? -- Jakub Narębski -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gitweb: allow extra breadcrumbs to prefix the trail
Jakub Narębski jna...@gmail.com wrote: It would be better to improve documentation, than follow current bad practice... ;-P The v2 patch does just that :-) Perhaps even make ( [ $home_link_str, $home_link ] ) to be default value for @extra_breadcrumbs, making new feature generalization of $home_link*, similarly to how it was done for $stylesheet - @stylesheets transition. I don't think that's a win. There's a lot of existing gitweb.conf out there which sets $home_link_str, so the code would have to either print the last element of @extra_breadcrumbs or the $home_link variables depending on whether the variables were modified. And the documentation would have to explain this complicated arrangement. Tony. -- f.anthony.n.finch d...@dotat.at http://dotat.at/ Forties, Cromarty: East, veering southeast, 4 or 5, occasionally 6 at first. Rough, becoming slight or moderate. Showers, rain at first. Moderate or good, occasionally poor at first.
Re: [PATCH] gitweb: allow extra breadcrumbs to prefix the trail
On Thu, Jul 4, 2013 at 5:11 PM, Tony Finch d...@dotat.at wrote:
Jakub Narębski jna...@gmail.com wrote:
It would be better to improve documentation, than follow current bad
practice... ;-P
The v2 patch does just that :-)
Thanks.
Perhaps even make ( [ $home_link_str, $home_link ] ) to be default
value for @extra_breadcrumbs, making new feature generalization
of $home_link*, similarly to how it was done for $stylesheet - @stylesheets
transition.
I don't think that's a win. There's a lot of existing gitweb.conf out
there which sets $home_link_str, so the code would have to either print
the last element of @extra_breadcrumbs or the $home_link variables
depending on whether the variables were modified. And the documentation
would have to explain this complicated arrangement.
First, do I understand corrctly that @extra_breadcrumbs are rendered *after*
$home_link*, and in exactly the same manner?
Second, I misremembered how $stylesheet / @stylesheets is handled.
I was thinking more about having in gitweb.perl the following default
initialization for @extra_breadcrumbs:
our @extra_breadcrumbs = ( [ $home_link_str, $home_link ] );
Then one can add breadcrumbs with
push @extra_breadcrumbs, [ $foo_html, $foo_url ], [
esc_html($bar_txt), $bar_url ];
But now I think that we can do better, simply put $home_link_str and $home_link
in @extra_breadcrumbs / @top_level_breadcrumbs / @nav_breadcrumbs before
using it, either via
unshift @nav_breadcrumbs, [ $home_link_str, $home_link ];
or
for $breadcrumb ([ $home_link_str, $home_link ], @nav_breadcrumbs) {
... unless we treat home link in some special way (do we?).
P.S. It is a bit late, but wouldn't { name = $link_name, href = $link_url }
(like %features hash) be a better solution than [ $link_name, $link_url ],
i.e. hashref (named parameters) instead of arrayref (positional parameters).
You wouldn't have to remember which is first: text or URL.
--
Jakub Narebski
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gitweb: allow extra breadcrumbs to prefix the trail
Jakub Narębski jna...@gmail.com wrote:
First, do I understand corrctly that @extra_breadcrumbs are rendered *after*
$home_link*, and in exactly the same manner?
Before the home link, and yes, in the same manner. The extra breadcrumbs
are for links to parent pages above gitweb in some hierarchy.
But now I think that we can do better, simply put $home_link_str and
$home_link
in @extra_breadcrumbs / @top_level_breadcrumbs / @nav_breadcrumbs before
using it,
We could save a line that way:
- print $cgi-a({-href = esc_url($home_link)}, $home_link_str) . / ;
+ for my $crumb (@extra_breadcrumbs, [ $home_link_str = $home_link ]) {
+ print $cgi-a({-href = esc_url($crumb-[1])}, $crumb-[0]) .
/ ;
+ }
P.S. It is a bit late, but wouldn't { name = $link_name, href = $link_url }
(like %features hash) be a better solution than [ $link_name, $link_url ],
i.e. hashref (named parameters) instead of arrayref (positional parameters).
You wouldn't have to remember which is first: text or URL.
I thought the fat arrow would be mnemonic enough, and less verbose.
Tony.
--
f.anthony.n.finch d...@dotat.at http://dotat.at/
Forties, Cromarty: East, veering southeast, 4 or 5, occasionally 6 at first.
Rough, becoming slight or moderate. Showers, rain at first. Moderate or good,
occasionally poor at first.
Re: [PATCH] gitweb: allow extra breadcrumbs to prefix the trail
On Thu, Jul 4, 2013 at 5:56 PM, Tony Finch d...@dotat.at wrote:
Jakub Narębski jna...@gmail.com wrote:
First, do I understand correctly that @extra_breadcrumbs are rendered *after*
$home_link*, and in exactly the same manner?
Before the home link, and yes, in the same manner. The extra breadcrumbs
are for links to parent pages above gitweb in some hierarchy.
Hmmm... I would have thought that they were after home link. I wonder
if leaving it up to user to configure @extra_breadcrumbs to include $home_link
in appropriate place (the unshift / push solution to adding to
@extra_breadcrumbs,
starting with $home_link) would be good idea, or over-engineering.
In what situation do you need those extra breadcrumbs useful? What
necessity / itch to scratch is behind idea of this patch?
But now I think that we can do better, simply put $home_link_str and
$home_link
in @extra_breadcrumbs / @top_level_breadcrumbs / @nav_breadcrumbs before
using it,
We could save a line that way:
- print $cgi-a({-href = esc_url($home_link)}, $home_link_str) . / ;
+ for my $crumb (@extra_breadcrumbs, [ $home_link_str = $home_link ]) {
+ print $cgi-a({-href = esc_url($crumb-[1])}, $crumb-[0]) .
/ ;
+ }
And avoid a bit of code duplication; now we are sure that both
@extra_breadcrumbs and $home_link are rendered in the same way.
P.S. It is a bit late, but wouldn't { name = $link_name, href = $link_url }
(like %features hash) be a better solution than [ $link_name, $link_url ],
i.e. hashref (named parameters) instead of arrayref (positional parameters).
You wouldn't have to remember which is first: text or URL.
I thought the fat arrow would be mnemonic enough, and less verbose.
Yes, hashref solution is a bit verbose. I don't like abusing fat arrow notation,
but here it gives nice mnemonic (hopefully explained in documentation).
--
Jakub Narebski
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gitweb: allow extra breadcrumbs to prefix the trail
Jakub Narębski jna...@gmail.com wrote: In what situation do you need those extra breadcrumbs useful? What necessity / itch to scratch is behind idea of this patch? For an example, see https://git.csx.cam.ac.uk/x/ucs/git/git.git I have three items in @extra_breadcrumbs which point to the University home page, my department home page, and my git server's home page; there are a number of gitolite accounts on the server each of which has a project listing which is where gitweb's home link points. (I expect to change the link texts to make it less confusing when you happen to be looking at my department's account on the git server...) our @extra_breadcrumbs = ( [ cam = 'http://www.cam.ac.uk/' ], [ ucs = 'http://www.ucs.cam.ac.uk/' ], [ git = 'https://git.csx.cam.ac.uk/' ], ); This is in line with our house style (none of which I have implemented on this server yet) - there are other examples of similar breadcrumb trails at https://raven.cam.ac.uk and http://new-webmail.hermes.cam.ac.uk There is a more generic version of this description and config example in v2 of my patch. I hope it is clear enough. I'll send a v3 patch with the code tweak. Tony. -- f.anthony.n.finch d...@dotat.at http://dotat.at/ Forties, Cromarty: East, veering southeast, 4 or 5, occasionally 6 at first. Rough, becoming slight or moderate. Showers, rain at first. Moderate or good, occasionally poor at first.
Re: [PATCH] gitweb: allow extra breadcrumbs to prefix the trail
On Thu, Jul 4, 2013 at 7:08 PM, Tony Finch d...@dotat.at wrote: Jakub Narębski jna...@gmail.com wrote: In what situation do you need those extra breadcrumbs useful? What necessity / itch to scratch is behind idea of this patch? For an example, see https://git.csx.cam.ac.uk/x/ucs/git/git.git I have three items in @extra_breadcrumbs which point to the University home page, my department home page, and my git server's home page; there are a number of gitolite accounts on the server each of which has a project listing which is where gitweb's home link points. Of course those @external_breadcrumbs are really useful iff they are repeated consistently in at least similar form in all those external pages... (I expect to change the link texts to make it less confusing when you happen to be looking at my department's account on the git server...) -- Jakub Narebski -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gitweb: allow extra breadcrumbs to prefix the trail
(cc-ing Jakub, gitweb wrangler)
Tony Finch wrote:
There are often parent pages logically above the gitweb projects
list, e.g. home pages of the organization and department that host
the gitweb server. This change allows you to include links to those
pages in gitweb's breadcrumb trail.
Neat.
Signed-off-by: Tony Finch d...@dotat.at
---
Documentation/gitweb.conf.txt | 8
gitweb/gitweb.perl| 6 ++
2 files changed, 14 insertions(+)
diff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt
index ea0526e..4579578 100644
--- a/Documentation/gitweb.conf.txt
+++ b/Documentation/gitweb.conf.txt
@@ -339,6 +339,14 @@ $home_link_str::
as this link leads to the list of projects. Other popular choice it to
set it to the name of site.
+@extra_breadcrumbs::
+ Additional links to be added to the start of the breadcrumb trail,
+ that are logically above the gitweb projects list. For example,
+ links to the organization and department which host the gitweb
+ server. Each element of the list is a reference to an array,
+ in which element 0 is the link text and element 1 is the
+ target URL.
Is arbitrary HTML permitted in the link text?
I think it makes sense to permit it for consistency with $home_link_str,
but it might be worth mentioning in the manpage so the administrator
knows not to set it to something user-controlled --- e.g.:
The link text can contain arbitrary HTML --- to escape link
text generated programatically, use esc_html($text).
For what it's worth, with or without such a change,
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
(Patch left unsnipped for reference.)
+
$logo_url::
$logo_label::
URI and label (title) for the Git logo link (or your site logo,
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 8d69ada..436f17a 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -85,6 +85,9 @@ our $project_maxdepth = ++GITWEB_PROJECT_MAXDEPTH++;
# string of the home link on top of all pages
our $home_link_str = ++GITWEB_HOME_LINK_STR++;
+# extra breadcrumbs preceding the home link
+our @extra_breadcrumbs = ();
+
# name of your site or organization to appear in page titles
# replace this with something more descriptive for clearer bookmarks
our $site_name = ++GITWEB_SITENAME++
@@ -3982,6 +3985,9 @@ sub print_nav_breadcrumbs_path {
sub print_nav_breadcrumbs {
my %opts = @_;
+ for my $crumb (@extra_breadcrumbs) {
+ print $cgi-a({-href = esc_url($crumb-[1])}, $crumb-[0]) .
/ ;
+ }
print $cgi-a({-href = esc_url($home_link)}, $home_link_str) . / ;
if (defined $project) {
my @dirname = split '/', $project;
--
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gitweb: allow extra breadcrumbs to prefix the trail
On Wed, Jul 3, 2013 at 11:59 PM, Jonathan Nieder jrnie...@gmail.com wrote: Tony Finch wrote: +@extra_breadcrumbs:: + Additional links to be added to the start of the breadcrumb trail, + that are logically above the gitweb projects list. For example, + links to the organization and department which host the gitweb + server. Each element of the list is a reference to an array, + in which element 0 is the link text and element 1 is the + target URL. Is arbitrary HTML permitted in the link text? I think it makes sense to permit it for consistency with $home_link_str, but it might be worth mentioning in the manpage so the administrator knows not to set it to something user-controlled --- e.g.: The link text can contain arbitrary HTML --- to escape link text generated programatically, use esc_html($text). Nb. it would be nice to have relation of @extra_breadcrumbs with $home_link_str explained. -- Jakub Narebski -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] gitweb: allow extra breadcrumbs to prefix the trail
There are often parent pages logically above the gitweb projects
list, e.g. home pages of the organization and department that host
the gitweb server. This change allows you to include links to those
pages in gitweb's breadcrumb trail.
Signed-off-by: Tony Finch d...@dotat.at
---
Documentation/gitweb.conf.txt | 8
gitweb/gitweb.perl| 6 ++
2 files changed, 14 insertions(+)
diff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt
index ea0526e..4579578 100644
--- a/Documentation/gitweb.conf.txt
+++ b/Documentation/gitweb.conf.txt
@@ -339,6 +339,14 @@ $home_link_str::
as this link leads to the list of projects. Other popular choice it to
set it to the name of site.
+@extra_breadcrumbs::
+ Additional links to be added to the start of the breadcrumb trail,
+ that are logically above the gitweb projects list. For example,
+ links to the organization and department which host the gitweb
+ server. Each element of the list is a reference to an array,
+ in which element 0 is the link text and element 1 is the
+ target URL.
+
$logo_url::
$logo_label::
URI and label (title) for the Git logo link (or your site logo,
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 8d69ada..436f17a 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -85,6 +85,9 @@ our $project_maxdepth = ++GITWEB_PROJECT_MAXDEPTH++;
# string of the home link on top of all pages
our $home_link_str = ++GITWEB_HOME_LINK_STR++;
+# extra breadcrumbs preceding the home link
+our @extra_breadcrumbs = ();
+
# name of your site or organization to appear in page titles
# replace this with something more descriptive for clearer bookmarks
our $site_name = ++GITWEB_SITENAME++
@@ -3982,6 +3985,9 @@ sub print_nav_breadcrumbs_path {
sub print_nav_breadcrumbs {
my %opts = @_;
+ for my $crumb (@extra_breadcrumbs) {
+ print $cgi-a({-href = esc_url($crumb-[1])}, $crumb-[0]) .
/ ;
+ }
print $cgi-a({-href = esc_url($home_link)}, $home_link_str) . / ;
if (defined $project) {
my @dirname = split '/', $project;
--
1.8.3.1.605.g85318f5
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
