threads / patch / 26339

patchPass "-O xhtml" param to highlight instead of "-xhtml"

Subject: [PATCH] Pass "-O xhtml" param to highlight instead of "-xhtml"

## tl;dr

5 messages between Jan 26, 2011 and Jan 27, 2011. Diffs are folded; open one to read it.

replies: 4people: 3as markdown or json

Adam Tkac· Jan 26, 2011, 17:11 UTC · lore

Hello,

attached patch fixes the gitweb.perl script. Current highlight utility doesn't recognize --xhtml parameter, it recognizes only "-O xhtml" parameter.

Reference: https://bugzilla.redhat.com/show_bug.cgi?id=672293

Regards, Adam

-- 
Adam Tkac, Red Hat, Inc.


>From ec8043a730c165d8b438ccaba7c3cf5dc5fa65bc Mon Sep 17 00:00:00 2001
From: Adam Tkac <atkac@redhat.com>
Date: Wed, 26 Jan 2011 17:46:29 +0100
Subject: [PATCH] Pass "-O xhtml" parameter instead of "--xhtml" to hightlight in gitweb.

Signed-off-by: Jochen Schmitt <jochen@herr-schmitt.de>
Signed-off-by: Adam Tkac <atkac@redhat.com>
---
 gitweb/gitweb.perl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 1025c2f..88556f4 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3468,7 +3468,7 @@ sub run_highlighter {
 	close $fd;
 	open $fd, quote_command(git_cmd(), "cat-file", "blob", $hash)." | ".
 	          quote_command($highlight_bin).
-	          " --xhtml --fragment --syntax $syntax |"
+	          " -O xhtml --fragment --syntax $syntax |"
 		or die_error(500, "Couldn't open file or run syntax highlighter");
 	return $fd;
 }
-- 
1.7.3.5

Jakub Narebski· Jan 26, 2011, 21:34 UTC · re: Adam Tkac · lore

Re: [PATCH] Pass "-O xhtml" param to highlight instead of "-xhtml"

Adam Tkac <atkac@redhat.com> writes:

> Hello,
> 
> attached patch fixes the gitweb.perl script.

Please don't send patches as attachements, but inline. See Documentation/SubmittingPatches

> Current highlight utility doesn't recognize "--xhtml" parameter, it
> recognizes only "-O xhtml" parameter.
> 
> Reference: https://bugzilla.redhat.com/show_bug.cgi?id=672293

Shouldn't the above be in commit message?

> Subject: [PATCH] Pass "-O xhtml" parameter instead of "--xhtml" to hightlight in gitweb.

Why the mismatch between subject of email, and subject in attached patch?

Show 19 quoted lines

> Signed-off-by: Jochen Schmitt <jochen@herr-schmitt.de>
> Signed-off-by: Adam Tkac <atkac@redhat.com>
> ---
>  gitweb/gitweb.perl |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 1025c2f..88556f4 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -3468,7 +3468,7 @@ sub run_highlighter {
>  	close $fd;
>  	open $fd, quote_command(git_cmd(), "cat-file", "blob", $hash)." | ".
>  	          quote_command($highlight_bin).
> -	          " --xhtml --fragment --syntax $syntax |"
> +	          " -O xhtml --fragment --syntax $syntax |"
>  		or die_error(500, "Couldn't open file or run syntax highlighter");
>  	return $fd;
>  }

In highlight 2.4.5 '-O' means "name of output directory", i.e. --outdir. There is no --out-format either.

WTF this backward incompatibile change in highlight... the only solution that would make it work both for old and for new versions is to rely on the fact that HTML is default output format, i.e.

  	open $fd, quote_command(git_cmd(), "cat-file", "blob", $hash)." | ".
  	          quote_command($highlight_bin).
 -	          " --xhtml --fragment --syntax $syntax |"
 +	          " --fragment --syntax $syntax |"
  		or die_error(500, "Couldn't open file or run syntax highlighter");

-- 
Jakub Narebski
Poland
ShadeHawk on #git

Junio C Hamano· Jan 26, 2011, 21:43 UTC · re: Jakub Narebski · lore

Re: [PATCH] Pass "-O xhtml" param to highlight instead of "-xhtml"

Jakub Narebski <jnareb@gmail.com> writes:

Show 6 quoted lines

>> Current highlight utility doesn't recognize "--xhtml" parameter, it
>> recognizes only "-O xhtml" parameter.
>> 
>> Reference: https://bugzilla.redhat.com/show_bug.cgi?id=672293
>
> Shouldn't the above be in commit message?

Actually, I prefer not having that "Reference" in the commit message myself, but I do want to have some relevant details missing from the proposed commit message but is in that ticket (especially the comment #5) in the commit message.

> In highlight 2.4.5 '-O' means "name of output directory", i.e. --outdir.
> There is no --out-format either.

Thanks for digging about 2.4.5; in that case, unlike the redhat ticket hinted, this change may not be backward compatible enough, as it seems that the compatibility goes only back to 3.0.something.

Show 10 quoted lines

>
> WTF this backward incompatibile change in highlight... the only
> solution that would make it work both for old and for new versions is
> to rely on the fact that HTML is default output format, i.e.
>
>   	open $fd, quote_command(git_cmd(), "cat-file", "blob", $hash)." | ".
>   	          quote_command($highlight_bin).
>  -	          " --xhtml --fragment --syntax $syntax |"
>  +	          " --fragment --syntax $syntax |"
>   		or die_error(500, "Couldn't open file or run syntax highlighter");

Adam, Jochen?  How does the counterproposal look to you?

Without knowing much about highlight nor using gitweb in general myself, the above looks a bit more reasonable fix to the issue, if the default format has been and will stay to be HTML.

Adam Tkac· Jan 27, 2011, 12:03 UTC · re: Junio C Hamano · lore

Re: [PATCH] Pass "-O xhtml" param to highlight instead of "-xhtml"

On Wed, Jan 26, 2011 at 01:43:48PM -0800, Junio C Hamano wrote:

Show 20 quoted lines

> Jakub Narebski <jnareb@gmail.com> writes:
> 
> >> Current highlight utility doesn't recognize "--xhtml" parameter, it
> >> recognizes only "-O xhtml" parameter.
> >> 
> >> Reference: https://bugzilla.redhat.com/show_bug.cgi?id=672293
> >
> > Shouldn't the above be in commit message?
> 
> Actually, I prefer not having that "Reference" in the commit message
> myself, but I do want to have some relevant details
> missing from the proposed commit message but is in
> that ticket (especially the comment #5) in the commit message.
> 
> > In highlight 2.4.5 '-O' means "name of output directory", i.e. --outdir.
> > There is no --out-format either.
> 
> Thanks for digging about 2.4.5; in that case, unlike the redhat ticket
> hinted, this change may not be backward compatible enough, as it seems
> that the compatibility goes only back to 3.0.something.

Right you are, I digged too shallowly, thanks for your inspection.

Show 17 quoted lines

> 
> >
> > WTF this backward incompatibile change in highlight... the only
> > solution that would make it work both for old and for new versions is
> > to rely on the fact that HTML is default output format, i.e.
> >
> >   	open $fd, quote_command(git_cmd(), "cat-file", "blob", $hash)." | ".
> >   	          quote_command($highlight_bin).
> >  -	          " --xhtml --fragment --syntax $syntax |"
> >  +	          " --fragment --syntax $syntax |"
> >   		or die_error(500, "Couldn't open file or run syntax highlighter");
> 
> Adam, Jochen?  How does the counterproposal look to you?
> 
> Without knowing much about highlight nor using gitweb in general myself,
> the above looks a bit more reasonable fix to the issue, if the default
> format has been and will stay to be HTML.

The proposal looks like the best (and the only one) correct solution. I will send updated patch.

Regards, Adam

-- 
Adam Tkac, Red Hat, Inc.

Adam Tkac· Jan 27, 2011, 12:51 UTC · re: Adam Tkac · lore

[PATCH] Don't pass "--xhtml" to hightlight in gitweb.perl script.

The "--xhtml" option is supported only in highlight < 3.0. There is no option to enforce (X)HTML output format compatible with both highlight < 3.0 and highlight >= 3.0. However default output format is HTML so we don't need to explicitly specify it.

Signed-off-by: Adam Tkac <atkac@redhat.com>
---
 gitweb/gitweb.perl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Show changes to gitweb/gitweb.perl +1 −1

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 1025c2f..0779f12 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3468,7 +3468,7 @@ sub run_highlighter {
 	close $fd;
 	open $fd, quote_command(git_cmd(), "cat-file", "blob", $hash)." | ".
 	          quote_command($highlight_bin).
-	          " --xhtml --fragment --syntax $syntax |"
+	          " --fragment --syntax $syntax |"
 		or die_error(500, "Couldn't open file or run syntax highlighter");
 	return $fd;
 }

-- 
1.7.3.5