[PATCH] gitweb: bugfix: list regression

[PATCH] gitweb: bugfix: list regression

[Luben Tuikov]

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(-)

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

Re: [PATCH] gitweb: bugfix: list regression

[Junio C Hamano]

Luben Tuikov <ltuikov@yahoo.com> writes:

> 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?

Re: [PATCH] gitweb: bugfix: list regression

[Jakub Narebski]

Luben Tuikov wrote:

> 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

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

[Jakub Narebski]

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(-)

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

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

[Junio C Hamano]

Jakub Narebski <jnareb@git.vger.kernel.org> writes:

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

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

[Jakub Narebski]

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.
[...]
> 
> 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

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

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

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

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

[Luben Tuikov]

--- Jakub Narebski <jnareb@git.vger.kernel.org> wrote:

> 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


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

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

[Luben Tuikov]

--- Jakub Narebski <jnareb@git.vger.kernel.org> wrote:

> 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


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

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

[Junio C Hamano]

Luben Tuikov <ltuikov@yahoo.com> writes:

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