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

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

[Adam Tkac]

[-- Attachment #1: Type: text/plain, Size: 265 bytes --]

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.

[-- Attachment #2: 0001-Pass-O-xhtml-parameter-instead-of-xhtml-to-hightligh.patch --]
[-- Type: text/plain, Size: 930 bytes --]

>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

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

[Jakub Narebski]

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?

> 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

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

[Junio C Hamano]

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.

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

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

[Adam Tkac]

On Wed, Jan 26, 2011 at 01:43:48PM -0800, Junio C Hamano wrote:
> 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.

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

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

[Adam Tkac]

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

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