threads / patch / 5351

patchgitweb: bugfix: list regression

Subject: [PATCH] gitweb: bugfix: list regression

## tl;dr

10 messages between Aug 22, 2006 and Aug 23, 2006. Diffs are folded; open one to read it.

replies: 9people: 4as markdown or json

Luben Tuikov· Aug 22, 2006, 08:55 UTC · lore

Fix regression introduced by commit 17d07443188909ef5f8b8c24043cb6d9fef51bca.

1. Commit 17d07443188909ef5f8b8c24043cb6d9fef51bca defines
"a.list" twice in gitweb.css, once with bold and once with
normal font-weight.
2. "a.list" being "bold", makes a myriad of things shown by
gitweb in bold font-weight, which is a regression from
pre-17d07443188909ef5f8b8c24043cb6d9fef51bca behavior.

The fix is to define separately "a.list_bold" and use that style in format_subject_html().

Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
---
 gitweb/gitweb.css  |    9 +++++++--
 gitweb/gitweb.perl |    4 ++--
 2 files changed, 9 insertions(+), 4 deletions(-)
Show changes to 2 files +9 −4

gitweb/gitweb.css, gitweb/gitweb.perl

diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
index 9013895..db58629 100644
--- a/gitweb/gitweb.css
+++ b/gitweb/gitweb.css
@@ -115,13 +115,18 @@ div.list_head {
 	font-style: italic;
 }
 
-a.list {
+a.list_bold {
 	text-decoration: none;
 	font-weight: bold;
 	color: #000000;
 }
 
-table.tags a.list {
+a.list {
+	text-decoration: none;
+	color: #000000;
+}
+
+table.tags {
 	font-weight: normal;
 }
 
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 063735d..5237f25 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -504,11 +504,11 @@ sub format_subject_html {
 	$extra = '' unless defined($extra);
 
 	if (length($short) < length($long)) {
-		return $cgi->a({-href => $href, -class => "list",
+		return $cgi->a({-href => $href, -class => "list_bold",
 		                -title => $long},
 		       esc_html($short) . $extra);
 	} else {
-		return $cgi->a({-href => $href, -class => "list"},
+		return $cgi->a({-href => $href, -class => "list_bold"},
 		       esc_html($long)  . $extra);
 	}
 }

-- 
1.4.2.g3851f
Junio C Hamano· Aug 22, 2006, 09:19 UTC · re: Luben Tuikov · lore

Re: [PATCH] gitweb: bugfix: list regression

Luben Tuikov <ltuikov@yahoo.com> writes:
Show 6 quoted lines
> Fix regression introduced by
> commit 17d07443188909ef5f8b8c24043cb6d9fef51bca.
>
> 1. Commit 17d07443188909ef5f8b8c24043cb6d9fef51bca defines
> "a.list" twice in gitweb.css, once with bold and once with
> normal font-weight.

The normal one is limited to a.list that is inside table.tags isn't it? I had an impression that this was done on purpose.

> 2. "a.list" being "bold", makes a myriad of things shown by
> gitweb in bold font-weight, which is a regression from
> pre-17d07443188909ef5f8b8c24043cb6d9fef51bca behavior.

The only difference I see offhand is that Jakub's version shows the tag text in normal weight in summary pane (tags section), while yours show it in bold. Also clickable object names in many views are shown in bold in Jakub's version while yours do not distinguish them from unclickable ones.

I tend to agree that bold looks too loud in tree view, but showing tree and parent diffently from commit itself in commit view may be a good thing. I am not sure bold is a good choice, though. Usually people underline clickable links not type in bold.

I do not case either way myself that much. Could you battle this out with the guilty party?

Jakub Narebski· Aug 22, 2006, 09:30 UTC · re: Luben Tuikov · lore

Re: [PATCH] gitweb: bugfix: list regression

Luben Tuikov wrote:
Show 13 quoted lines
> Fix regression introduced by
> commit 17d07443188909ef5f8b8c24043cb6d9fef51bca.
> 
> 1. Commit 17d07443188909ef5f8b8c24043cb6d9fef51bca defines
> "a.list" twice in gitweb.css, once with bold and once with
> normal font-weight.
> 
> 2. "a.list" being "bold", makes a myriad of things shown by
> gitweb in bold font-weight, which is a regression from
> pre-17d07443188909ef5f8b8c24043cb6d9fef51bca behavior.
> 
> The fix is to define separately "a.list_bold" and use
> that style in format_subject_html().

This fix is counter to what I wanted to do. I thought that "list" class is used only in "subject" links, so I defaulted it to bold (all such elements except one were inside <b>...</b>), and encoded exceptions; notice

+table.tags a.list {
+       font-weight: normal;
+}
+
We could as well define "bold" class for "a", i.e.
+a.bold {
+       font-weight: bold;
+}
but this is presentational CSS, no better than <b>...</b> element.

The correct solution would be to add "subject" class perhaps, and do exceptions for it.

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
Jakub Narebski· Aug 22, 2006, 10:02 UTC · re: Luben Tuikov · lore

[PATCH] gitweb: bugfix: a.list formatting regression

From: Jakub Narebski <jnareb@gmail.com>

Fix regression introduced by commit 17d07443188909ef5f8b8c24043cb6d9fef51bca.

"a.list" being "bold", makes a myriad of things shown by gitweb in bold font-weight, which is a regression from pre-17d07443188909ef5f8b8c24043cb6d9fef51bca behavior.

The fix is to add "subject" class and use this class to replace pre-format_subject_html formatting of subject (comment) via using (or not) <b>...</b> element. This should go back to the pre-17d0744318... style.

Regression noticed by Luben Tuikov.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
---
Luben Tuikov wrote:
> 1. Commit 17d07443188909ef5f8b8c24043cb6d9fef51bca defines
> "a.list" twice in gitweb.css, once with bold and once with
> normal font-weight.

"table.tags a.list" means "a.list" inside "table.tags", so it is not redefinition of "a.list", but override (more specific CSS rule wins). It is for subject of tags list ("summary" and "tags" actions) to be in normal weight, as in such list the tag itself (ref) is in bold, not the tag comment (first line of free form part of tag).

 gitweb/gitweb.css  |    7 +++++--
 gitweb/gitweb.perl |    4 ++--
 2 files changed, 7 insertions(+), 4 deletions(-)
Show changes to 2 files +7 −4

gitweb/gitweb.css, gitweb/gitweb.perl

diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
index 9013895..6c13d9e 100644
--- a/gitweb/gitweb.css
+++ b/gitweb/gitweb.css
@@ -117,11 +117,14 @@ div.list_head {
 
 a.list {
 	text-decoration: none;
-	font-weight: bold;
 	color: #000000;
 }
 
-table.tags a.list {
+a.subject {
+	font-weight: bold;
+}
+
+table.tags a.subject {
 	font-weight: normal;
 }
 
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 90157d5..40c5177 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -504,11 +504,11 @@ sub format_subject_html {
 	$extra = '' unless defined($extra);
 
 	if (length($short) < length($long)) {
-		return $cgi->a({-href => $href, -class => "list",
+		return $cgi->a({-href => $href, -class => "list subject",
 		                -title => $long},
 		       esc_html($short) . $extra);
 	} else {
-		return $cgi->a({-href => $href, -class => "list"},
+		return $cgi->a({-href => $href, -class => "list subject"},
 		       esc_html($long)  . $extra);
 	}
 }

-- 
1.4.1.1
Junio C Hamano· Aug 22, 2006, 22:34 UTC · re: Jakub Narebski · lore

Re: [PATCH] gitweb: bugfix: a.list formatting regression

Jakub Narebski <jnareb@git.vger.kernel.org> writes:
Show 19 quoted lines
> From: Jakub Narebski <jnareb@gmail.com>
>
> Fix regression introduced by
> commit 17d07443188909ef5f8b8c24043cb6d9fef51bca.
>
> "a.list" being "bold", makes a myriad of things shown by
> gitweb in bold font-weight, which is a regression from
> pre-17d07443188909ef5f8b8c24043cb6d9fef51bca behavior.
>
> The fix is to add "subject" class and use this class
> to replace pre-format_subject_html formatting of subject
> (comment) via using (or not) <b>...</b> element. This
> should go back to the pre-17d0744318... style.
>
> Regression noticed by Luben Tuikov.
>
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
> Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
> ---

I haven't seen an Ack from Luben for the proposed renaming of the class, although I very much like naming things for what they mean (i.e. "subject"), not what they do (i.e. "bold") myself.

It might be worthwhile to do a full sweep of the pages we generate and identify what classes we would want. I have a feeling that we overuse a.list for example -- they are all in some form of list alright but they might benefit from a bit finer logical separatoin. One list is enumaration of refs and another is a chronological sequence of commits.

Then people who would want to tweak the presentation would have an easier time changing only gitweb.css.

Jakub Narebski· Aug 22, 2006, 22:46 UTC · re: Junio C Hamano · lore

Re: [PATCH] gitweb: bugfix: a.list formatting regression

Junio C Hamano wrote:
> Jakub Narebski <jnareb@git.vger.kernel.org> writes:

(the above is result of paste in wrong place in git-send-email invocation) [...]

>> The fix is to add "subject" class and use this class
>> to replace pre-format_subject_html formatting of subject
>> (comment) via using (or not) <b>...</b> element. This
>> should go back to the pre-17d0744318... style.
[...]
Show 14 quoted lines
> 
> I haven't seen an Ack from Luben for the proposed renaming of
> the class, although I very much like naming things for what they
> mean (i.e. "subject"), not what they do (i.e. "bold") myself.
> 
> It might be worthwhile to do a full sweep of the pages we
> generate and identify what classes we would want.  I have a
> feeling that we overuse a.list for example -- they are all in
> some form of list alright but they might benefit from a bit
> finer logical separatoin.  One list is enumaration of refs and
> another is a chronological sequence of commits.
> 
> Then people who would want to tweak the presentation would have
> an easier time changing only gitweb.css.
Next patch in this series of patches 
   [PATCH] gitweb: Replace some presentational HTML by CSS
   Message-Id: <11562431392439-git-send-email-jnareb@git.vger.kernel.org>
adds "name" subclass (a.list.name) for ref names. (I think
subclasses/multiple classes is the way to do it, as all a.list
elements have something in common: use next class to divide).
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
Junio C Hamano· Aug 22, 2006, 23:14 UTC · re: Jakub Narebski · lore

Re: [PATCH] gitweb: bugfix: a.list formatting regression

Jakub Narebski <jnareb@gmail.com> writes:
Show 18 quoted lines
> Junio C Hamano wrote:
>
>> It might be worthwhile to do a full sweep of the pages we
>> generate and identify what classes we would want.  I have a
>> feeling that we overuse a.list for example -- they are all in
>> some form of list alright but they might benefit from a bit
>> finer logical separatoin.  One list is enumaration of refs and
>> another is a chronological sequence of commits.
>> 
>> Then people who would want to tweak the presentation would have
>> an easier time changing only gitweb.css.
>
> Next patch in this series of patches 
>    [PATCH] gitweb: Replace some presentational HTML by CSS
>    Message-Id: <11562431392439-git-send-email-jnareb@git.vger.kernel.org>
> adds "name" subclass (a.list.name) for ref names. (I think
> subclasses/multiple classes is the way to do it, as all a.list
> elements have something in common: use next class to divide).

I agree about the implementation aspect, such as using "list.name" as class names; I was suggesting to start from design.

Luben Tuikov· Aug 23, 2006, 04:43 UTC · re: Jakub Narebski · lore

Re: [PATCH] gitweb: bugfix: a.list formatting regression

--- Jakub Narebski <jnareb@git.vger.kernel.org> wrote:
Show 19 quoted lines
> From: Jakub Narebski <jnareb@gmail.com>
> 
> Fix regression introduced by
> commit 17d07443188909ef5f8b8c24043cb6d9fef51bca.
> 
> "a.list" being "bold", makes a myriad of things shown by
> gitweb in bold font-weight, which is a regression from
> pre-17d07443188909ef5f8b8c24043cb6d9fef51bca behavior.
> 
> The fix is to add "subject" class and use this class
> to replace pre-format_subject_html formatting of subject
> (comment) via using (or not) <b>...</b> element. This
> should go back to the pre-17d0744318... style.
> 
> Regression noticed by Luben Tuikov.
> 
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
> Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
> ---
ACK.  Looks good.
   Luben
Show 64 quoted lines
> Luben Tuikov wrote:
> 
> > 1. Commit 17d07443188909ef5f8b8c24043cb6d9fef51bca defines
> > "a.list" twice in gitweb.css, once with bold and once with
> > normal font-weight.
> 
> "table.tags a.list" means "a.list" inside "table.tags",
> so it is not redefinition of "a.list", but override
> (more specific CSS rule wins). It is for subject of tags
> list ("summary" and "tags" actions) to be in normal
> weight, as in such list the tag itself (ref) is in bold,
> not the tag comment (first line of free form part of tag).
> 
>  gitweb/gitweb.css  |    7 +++++--
>  gitweb/gitweb.perl |    4 ++--
>  2 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
> index 9013895..6c13d9e 100644
> --- a/gitweb/gitweb.css
> +++ b/gitweb/gitweb.css
> @@ -117,11 +117,14 @@ div.list_head {
>  
>  a.list {
>  	text-decoration: none;
> -	font-weight: bold;
>  	color: #000000;
>  }
>  
> -table.tags a.list {
> +a.subject {
> +	font-weight: bold;
> +}
> +
> +table.tags a.subject {
>  	font-weight: normal;
>  }
>  
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 90157d5..40c5177 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -504,11 +504,11 @@ sub format_subject_html {
>  	$extra = '' unless defined($extra);
>  
>  	if (length($short) < length($long)) {
> -		return $cgi->a({-href => $href, -class => "list",
> +		return $cgi->a({-href => $href, -class => "list subject",
>  		                -title => $long},
>  		       esc_html($short) . $extra);
>  	} else {
> -		return $cgi->a({-href => $href, -class => "list"},
> +		return $cgi->a({-href => $href, -class => "list subject"},
>  		       esc_html($long)  . $extra);
>  	}
>  }
> -- 
> 1.4.1.1
> 
> -
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
Luben Tuikov· Aug 23, 2006, 04:45 UTC · re: Jakub Narebski · lore

Re: [PATCH] gitweb: bugfix: a.list formatting regression

--- Jakub Narebski <jnareb@git.vger.kernel.org> wrote:
Show 19 quoted lines
> From: Jakub Narebski <jnareb@gmail.com>
> 
> Fix regression introduced by
> commit 17d07443188909ef5f8b8c24043cb6d9fef51bca.
> 
> "a.list" being "bold", makes a myriad of things shown by
> gitweb in bold font-weight, which is a regression from
> pre-17d07443188909ef5f8b8c24043cb6d9fef51bca behavior.
> 
> The fix is to add "subject" class and use this class
> to replace pre-format_subject_html formatting of subject
> (comment) via using (or not) <b>...</b> element. This
> should go back to the pre-17d0744318... style.
> 
> Regression noticed by Luben Tuikov.
> 
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
> Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
> ---
ACK. Looks good.
    Luben
Show 60 quoted lines
> Luben Tuikov wrote:
> 
> > 1. Commit 17d07443188909ef5f8b8c24043cb6d9fef51bca defines
> > "a.list" twice in gitweb.css, once with bold and once with
> > normal font-weight.
> 
> "table.tags a.list" means "a.list" inside "table.tags",
> so it is not redefinition of "a.list", but override
> (more specific CSS rule wins). It is for subject of tags
> list ("summary" and "tags" actions) to be in normal
> weight, as in such list the tag itself (ref) is in bold,
> not the tag comment (first line of free form part of tag).
> 
>  gitweb/gitweb.css  |    7 +++++--
>  gitweb/gitweb.perl |    4 ++--
>  2 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
> index 9013895..6c13d9e 100644
> --- a/gitweb/gitweb.css
> +++ b/gitweb/gitweb.css
> @@ -117,11 +117,14 @@ div.list_head {
>  
>  a.list {
>  	text-decoration: none;
> -	font-weight: bold;
>  	color: #000000;
>  }
>  
> -table.tags a.list {
> +a.subject {
> +	font-weight: bold;
> +}
> +
> +table.tags a.subject {
>  	font-weight: normal;
>  }
>  
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 90157d5..40c5177 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -504,11 +504,11 @@ sub format_subject_html {
>  	$extra = '' unless defined($extra);
>  
>  	if (length($short) < length($long)) {
> -		return $cgi->a({-href => $href, -class => "list",
> +		return $cgi->a({-href => $href, -class => "list subject",
>  		                -title => $long},
>  		       esc_html($short) . $extra);
>  	} else {
> -		return $cgi->a({-href => $href, -class => "list"},
> +		return $cgi->a({-href => $href, -class => "list subject"},
>  		       esc_html($long)  . $extra);
>  	}
>  }
> -- 
> 1.4.1.1
> 
>
Junio C Hamano· Aug 23, 2006, 05:23 UTC · re: Luben Tuikov · lore

Re: [PATCH] gitweb: bugfix: a.list formatting regression

Luben Tuikov <ltuikov@yahoo.com> writes:
Show 14 quoted lines
>> The fix is to add "subject" class and use this class
>> to replace pre-format_subject_html formatting of subject
>> (comment) via using (or not) <b>...</b> element. This
>> should go back to the pre-17d0744318... style.
>> 
>> Regression noticed by Luben Tuikov.
>> 
>> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
>> Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
>> ---
>
> ACK. Looks good.
>
>     Luben
Thanks, both.

← back to recent threads