threads / patch / 26157

v3, 3 partsminor gitweb modifications

Subject: [PATCH 0/3 v3] minor gitweb modifications

## tl;dr

6 messages between Dec 29, 2010 and Dec 29, 2010. Diffs are folded; open one to read it.

replies: 5people: 2as markdown or json

Sylvain Rabot· Dec 29, 2010, 19:33 UTC · lore
here a three patch serie with minor updates for gitweb based on master.
This serie has been improved regarding the comments of :
 - Jakub Narebski <jnareb@gmail.com>
 - Jonathan Nieder <jrnieder@gmail.com>
Regards.
Sylvain Rabot (3):
  gitweb: add extensions to highlight feature
  gitweb: remove test when closing file descriptor
  gitweb: add css class to remote url titles
 gitweb/gitweb.perl       |   18 +++++++++---------
 gitweb/static/gitweb.css |    5 +++++
 2 files changed, 14 insertions(+), 9 deletions(-)
-- 
1.7.3.4.523.g72f0d.dirty
Sylvain Rabot· Dec 29, 2010, 19:33 UTC · re: Sylvain Rabot · lore

[PATCH 1/3] gitweb: add extensions to highlight feature

added: sql, php5, phps, bash, zsh, ksh, mk, make
Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr>
---
 gitweb/gitweb.perl |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)
Show changes to gitweb/gitweb.perl +4 −3
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 4779618..ea984b9 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -250,13 +250,14 @@ our %highlight_ext = (
 	# main extensions, defining name of syntax;
 	# see files in /usr/share/highlight/langDefs/ directory
 	map { $_ => $_ }
-		qw(py c cpp rb java css php sh pl js tex bib xml awk bat ini spec tcl),
+		qw(py c cpp rb java css php sh pl js tex bib xml awk bat ini spec tcl sql make),
 	# alternate extensions, see /etc/highlight/filetypes.conf
 	'h' => 'c',
+	map { $_ => 'sh'  } qw(bash zsh ksh),
 	map { $_ => 'cpp' } qw(cxx c++ cc),
-	map { $_ => 'php' } qw(php3 php4),
+	map { $_ => 'php' } qw(php3 php4 php5 phps),
 	map { $_ => 'pl'  } qw(perl pm), # perhaps also 'cgi'
-	'mak' => 'make',
+	map { $_ => 'make'} qw(mak mk),
 	map { $_ => 'xml' } qw(xhtml html htm),
 );
 

-- 
1.7.3.4.523.g72f0d.dirty
Sylvain Rabot· Dec 29, 2010, 19:33 UTC · re: Sylvain Rabot · lore

[PATCH 2/3] gitweb: remove test when closing file descriptor

it happens that closing file descriptor fails whereas the blob is perfectly readable. According to perlman the reasons could be:

   If the file handle came from a piped open, "close" will additionally
   return false if one of the other system calls involved fails, or if the
   program exits with non-zero status.  (If the only problem was that the
   program exited non-zero, $! will be set to 0.)  Closing a pipe also waits
   for the process executing on the pipe to complete, in case you want to
   look at the output of the pipe afterwards, and implicitly puts the exit
   status value of that command into $?.
   Prematurely closing the read end of a pipe (i.e. before the process writ-
   ing to it at the other end has closed it) will result in a SIGPIPE being
   delivered to the writer.  If the other end can't handle that, be sure to
   read all the data before closing the pipe.
In this case we don't mind that close fails.
Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr>
---
 gitweb/gitweb.perl |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)
Show changes to gitweb/gitweb.perl +1 −2
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index ea984b9..eae75ac 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3465,8 +3465,7 @@ sub run_highlighter {
 	my ($fd, $highlight, $syntax) = @_;
 	return $fd unless ($highlight && defined $syntax);
 
-	close $fd
-		or die_error(404, "Reading blob failed");
+	close $fd;
 	open $fd, quote_command(git_cmd(), "cat-file", "blob", $hash)." | ".
 	          quote_command($highlight_bin).
 	          " --xhtml --fragment --syntax $syntax |"

-- 
1.7.3.4.523.g72f0d.dirty
Sylvain Rabot· Dec 29, 2010, 19:33 UTC · re: Sylvain Rabot · lore

[PATCH 3/3] gitweb: add css class to remote url titles

Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr>
---
 gitweb/gitweb.perl       |    8 ++++----
 gitweb/static/gitweb.css |    5 +++++
 2 files changed, 9 insertions(+), 4 deletions(-)
Show changes to 2 files +9 −4

gitweb/gitweb.perl, gitweb/static/gitweb.css

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index eae75ac..cb169c7 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -5146,13 +5146,13 @@ sub git_remote_block {
 
 	if (defined $fetch) {
 		if ($fetch eq $push) {
-			$urls_table .= format_repo_url("URL", $fetch);
+			$urls_table .= format_repo_url("<span class=\"metadata_remote_fetch_url\">URL</span>", $fetch);
 		} else {
-			$urls_table .= format_repo_url("Fetch URL", $fetch);
-			$urls_table .= format_repo_url("Push URL", $push) if defined $push;
+			$urls_table .= format_repo_url("<span class=\"metadata_remote_fetch_url\">Fetch URL</span>", $fetch);
+			$urls_table .= format_repo_url("<span class=\"metadata_remote_push_url\">Push URL</span>", $push) if defined $push;
 		}
 	} elsif (defined $push) {
-		$urls_table .= format_repo_url("Push URL", $push);
+		$urls_table .= format_repo_url("<span class=\"metadata_remote_push_url\">Push URL</span>", $push);
 	} else {
 		$urls_table .= format_repo_url("", "No remote URL");
 	}
diff --git a/gitweb/static/gitweb.css b/gitweb/static/gitweb.css
index 79d7eeb..631b20d 100644
--- a/gitweb/static/gitweb.css
+++ b/gitweb/static/gitweb.css
@@ -579,6 +579,11 @@ div.remote {
 	display: inline-block;
 }
 
+.metadata_remote_fetch_url,
+.metadata_remote_push_url {
+	font-weight: bold;
+}
+
 /* Style definition generated by highlight 2.4.5, http://www.andre-simon.de/ */
 
 /* Highlighting theme definition: */

-- 
1.7.3.4.523.g72f0d.dirty
Jakub Narebski· Dec 29, 2010, 22:44 UTC · re: Sylvain Rabot · lore

Re: [PATCH 3/3] gitweb: add css class to remote url titles

Sylvain Rabot <sylvain@abstraction.fr> writes:
Show 28 quoted lines
> Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr>
> ---
>  gitweb/gitweb.perl       |    8 ++++----
>  gitweb/static/gitweb.css |    5 +++++
>  2 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index eae75ac..cb169c7 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -5146,13 +5146,13 @@ sub git_remote_block {
>  
>  	if (defined $fetch) {
>  		if ($fetch eq $push) {
> -			$urls_table .= format_repo_url("URL", $fetch);
> +			$urls_table .= format_repo_url("<span class=\"metadata_remote_fetch_url\">URL</span>", $fetch);
>  		} else {
> -			$urls_table .= format_repo_url("Fetch URL", $fetch);
> -			$urls_table .= format_repo_url("Push URL", $push) if defined $push;
> +			$urls_table .= format_repo_url("<span class=\"metadata_remote_fetch_url\">Fetch URL</span>", $fetch);
> +			$urls_table .= format_repo_url("<span class=\"metadata_remote_push_url\">Push URL</span>", $push) if defined $push;
>  		}
>  	} elsif (defined $push) {
> -		$urls_table .= format_repo_url("Push URL", $push);
> +		$urls_table .= format_repo_url("<span class=\"metadata_remote_push_url\">Push URL</span>", $push);
>  	} else {
>  		$urls_table .= format_repo_url("", "No remote URL");
>  	}

I'm not sure if in this situation if it would not be better to extend format_repo_url subroutine to take additional parameter describing _type_ of repo URL; it then would do styling internally. Which means moving wrapping 'URL', 'Fetch URL' etc. in span element to format_repo_url from the caller.

Show 13 quoted lines
> diff --git a/gitweb/static/gitweb.css b/gitweb/static/gitweb.css
> index 79d7eeb..631b20d 100644
> --- a/gitweb/static/gitweb.css
> +++ b/gitweb/static/gitweb.css
> @@ -579,6 +579,11 @@ div.remote {
>  	display: inline-block;
>  }
>  
> +.metadata_remote_fetch_url,
> +.metadata_remote_push_url {
> +	font-weight: bold;
> +}
> +
Good!
-- 
Jakub Narebski
Poland
ShadeHawk on #git
Jakub Narebski· Dec 29, 2010, 22:41 UTC · re: Sylvain Rabot · lore

Re: [PATCH 0/3 v3] minor gitweb modifications

Sylvain Rabot <sylvain@abstraction.fr> writes:
Show 8 quoted lines
> here a three patch serie with minor updates for gitweb based on master.
> 
> This serie has been improved regarding the comments of :
> 
>  - Jakub Narebski <jnareb@gmail.com>
>  - Jonathan Nieder <jrnieder@gmail.com>
> 
> Regards.
A few comments, in this version about commit messages, not the code:
 
> Sylvain Rabot (3):
>   gitweb: add extensions to highlight feature

It is about adding support for supporting additional file extensions for syntax highlighting, not about extending the highlight feature or adding some plugin thingy to it, as one can think on first read.

>   gitweb: remove test when closing file descriptor

It is about closing open pipe (file descriptor) without checking for error in the case where we don't care about errors in the caller; in short about not testing status of closing filehandle in git_highlight

>   gitweb: add css class to remote url titles

It is about distinguishing visually type of link using styles; adding CSS class is just a way to do it.

This actually has few comments about code.
Show 8 quoted lines
> 
>  gitweb/gitweb.perl       |   18 +++++++++---------
>  gitweb/static/gitweb.css |    5 +++++
>  2 files changed, 14 insertions(+), 9 deletions(-)
> 
> -- 
> 1.7.3.4.523.g72f0d.dirty
> 
-- 
Jakub Narebski
Poland
ShadeHawk on #git

← back to recent threads