[PATCH] Fix wrong xhtml option to highlight

[PATCH] Fix wrong xhtml option to highlight

[Jochen Schmitt]

---
Hallo,

because I'm the maintainer of the highlight package in the
Fedora Project I was notified, that highlight doesn't works
properly with gitweb in BZ #672293.

So I have create the following simple patch to solve the 
reported issue.

Best Regards

Jochen Schmitt

 gitweb/gitweb.perl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 1025c2f..b662420 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 |"
+	          " -xhtml --fragment --syntax $syntax |"
 		or die_error(500, "Couldn't open file or run syntax highlighter");
 	return $fd;
 }
-- 
1.7.3.4

Re: [PATCH] Fix wrong xhtml option to highlight

[Junio C Hamano]

Jochen Schmitt <Jochen@herr-schmitt.de> writes:

> ---
> Hallo,
>
> because I'm the maintainer of the highlight package in the
> Fedora Project I was notified, that highlight doesn't works
> properly with gitweb in BZ #672293.
>
> So I have create the following simple patch to solve the 
> reported issue.
>
> Best Regards
>
> Jochen Schmitt
>
>  gitweb/gitweb.perl |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 1025c2f..b662420 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 |"
> +	          " -xhtml --fragment --syntax $syntax |"

Curious.

Does the command take double-dash for the fragment and syntax options but
a single dash for the xhtml option?  Really...

A few top hits returned by Google for "highlight manual page" tells me
otherwise.

Re: [PATCH] Fix wrong xhtml option to highlight

[Drew Northup]

On Mon, 2011-01-24 at 14:48 -0800, Junio C Hamano wrote:
> Jochen Schmitt <Jochen@herr-schmitt.de> writes:
> 
> > ---
> > Hallo,
> >
> > because I'm the maintainer of the highlight package in the
> > Fedora Project I was notified, that highlight doesn't works
> > properly with gitweb in BZ #672293.
> >
> > So I have create the following simple patch to solve the 
> > reported issue.
> >
> > Best Regards
> >
> > Jochen Schmitt
> >
> >  gitweb/gitweb.perl |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> > index 1025c2f..b662420 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 |"
> > +	          " -xhtml --fragment --syntax $syntax |"
> 
> Curious.
> 
> Does the command take double-dash for the fragment and syntax options but
> a single dash for the xhtml option?  Really...
> 
> A few top hits returned by Google for "highlight manual page" tells me
> otherwise.

Certainly appears to be the case that "--xhtml" is the option in Ubuntu
10.04.1 LTS. 

Jochen,
Did you mean "-X" (which sets the same option)?

-- 
-Drew Northup
________________________________________________
"As opposed to vegetable or mineral error?"
-John Pescatore, SANS NewsBites Vol. 12 Num. 59

Re: [PATCH] Fix wrong xhtml option to highlight

[Junio C Hamano]

Drew Northup <drew.northup@maine.edu> writes:

>> > diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
>> > index 1025c2f..b662420 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 |"
>> > +	          " -xhtml --fragment --syntax $syntax |"
>> 
>> Curious.
>> 
>> Does the command take double-dash for the fragment and syntax options but
>> a single dash for the xhtml option?  Really...
>> 
>> A few top hits returned by Google for "highlight manual page" tells me
>> otherwise.
>
> Certainly appears to be the case that "--xhtml" is the option in Ubuntu
> 10.04.1 LTS. 
>
> Jochen,
> Did you mean "-X" (which sets the same option)?

The current proposal is to drop --xhtml and let highlight default to HTML.

Honestly speaking, I don't like the approach very much; it would have been
much better if highlight had a single way that is supported throughout its
versions to specify the output format.  But it appears that there isn't,
and relying on and hoping for its default to stay HTML is the best we
could do, if we plan to support highlight 2.4.something or older.

The copy of U10.04 I have has highlight 2.12, and according to its manual
pages, -X, --xhtml, and --out-format=xhtml mean the same thing.  HTML is
the default.

The change-log at www.andre-simon.de indicates that --out-format has
become the preferred method and the short options like -X and -H are not
supported in recent versions (3.0 beta and newer).

But as Jakub mentioned, 2.4.5 did not have --output-format; it was only in
3.0 beta that -O was redefined to mean --output-format and in old versions
the short option meant something else.

What a mess...

The next time we introduce a new dependency, we really should try hard to
assess the stability and maturity of that dependency.  In hindsight, I
think "highlight" was probably a bit too premature to be depended upon.

Re: [PATCH] Fix wrong xhtml option to highlight

[Jakub Narebski]

On Thu, 27 Jan 2011, Junio C Hamano wrote:
> Drew Northup <drew.northup@maine.edu> writes:
> 
>>>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
>>>> index 1025c2f..b662420 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 |"
>>>> +	          " -xhtml --fragment --syntax $syntax |"
>>> 
>>> Curious.
>>> 
>>> Does the command take double-dash for the fragment and syntax options but
>>> a single dash for the xhtml option?  Really...
>>> 
>>> A few top hits returned by Google for "highlight manual page" tells me
>>> otherwise.
>>
>> Certainly appears to be the case that "--xhtml" is the option in Ubuntu
>> 10.04.1 LTS. 
>>
>> Jochen,
>> Did you mean "-X" (which sets the same option)?
> 
> The current proposal is to drop --xhtml and let highlight default to HTML.
> 
> Honestly speaking, I don't like the approach very much; it would have been
> much better if highlight had a single way that is supported throughout its
> versions to specify the output format.  But it appears that there isn't,
> and relying on and hoping for its default to stay HTML is the best we
> could do, if we plan to support highlight 2.4.something or older.
> 
> The copy of U10.04 I have has highlight 2.12, and according to its manual
> pages, -X, --xhtml, and --out-format=xhtml mean the same thing.  HTML is
> the default.
> 
> The change-log at www.andre-simon.de indicates that --out-format has
> become the preferred method and the short options like -X and -H are not
> supported in recent versions (3.0 beta and newer).
> 
> But as Jakub mentioned, 2.4.5 did not have --output-format; it was only in
> 3.0 beta that -O was redefined to mean --output-format and in old versions
> the short option meant something else.

Well, we can always require highlight >= 2.12, or whatever version
introduced --out-format option.

> 
> What a mess...
> 
> The next time we introduce a new dependency, we really should try hard to
> assess the stability and maturity of that dependency.  In hindsight, I
> think "highlight" was probably a bit too premature to be depended upon.

By the way, the idea was to make it possible to configure other highlighter,
but I went with what I known to work, i.e. with Andre Simon's "highlight". 
I think it could be fairly easy to make it configurable via existing
$highlight_bin and to be introduced @highlight_args gitweb configuration
variables.

There are three possible ways to do syntax highlighting in gitweb:
filter, Perl module, or via JavaScript.  An alternative to "highlight"
as a filter could be GNU source-highlight... if not for the fact that
it doesn't seem to support equivalent of "highlight" --fragment option,
i.e. exclude prolog and <pre><tt> wrappers.

-- 
Jakub Narebski
Poland