threads / patch / 26328

patchFix wrong xhtml option to highlight

Subject: [PATCH] Fix wrong xhtml option to highlight

## tl;dr

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

replies: 4people: 4as markdown or json

Jochen Schmitt· Jan 24, 2011, 19:44 UTC · lore

--- 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(-)
Show changes to gitweb/gitweb.perl +1 −1
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
Junio C Hamano· Jan 24, 2011, 22:48 UTC · re: Jochen Schmitt · lore

Re: [PATCH] Fix wrong xhtml option to highlight

Jochen Schmitt <Jochen@herr-schmitt.de> writes:
Show 27 quoted lines
> ---
> 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.

Drew Northup· Jan 27, 2011, 01:44 UTC · re: Junio C Hamano · lore

Re: [PATCH] Fix wrong xhtml option to highlight

On Mon, 2011-01-24 at 14:48 -0800, Junio C Hamano wrote:
Show 37 quoted lines
> 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
Junio C Hamano· Jan 27, 2011, 19:00 UTC · re: Drew Northup · lore

Re: [PATCH] Fix wrong xhtml option to highlight

Drew Northup <drew.northup@maine.edu> writes:
Show 24 quoted lines
>> > 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.

Jakub Narebski· Jan 28, 2011, 12:35 UTC · re: Junio C Hamano · lore

Re: [PATCH] Fix wrong xhtml option to highlight

On Thu, 27 Jan 2011, Junio C Hamano wrote:
Show 46 quoted lines
> 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.

Show 6 quoted lines
> 
> 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

← back to recent threads