Syntax highlighting broken.

Syntax highlighting broken.

[smithj4]

Hi,

After updating to cgit-0.9.2 I noticed that the syntax highlighting 
broke. It looks like a recent commit:

http://git.zx2c4.com/cgit/commit/cgit.css?id=8149be213f1c8f52b0dbe6c213f6073af57fa954

removed style sheet elements that are required for the highlight command 
to properly display its added syntax highlighting, as mentioned in the 
comments here:

http://git.zx2c4.com/cgit/tree/filters/syntax-highlighting.sh

Can this commit be reversed to re-enable the syntax highlighting?

Thanks,
~Jason

Syntax highlighting broken.

[Jason]

This CSS was removed so that cgit could support multiple highlighting
engines without preferring one over another. Either include this CSS
along with the highlighted output in <style> tags (as in the case of
[1]) or add it back to the global CSS yourself.

[1] http://git.zx2c4.com/cgit/tree/filters/syntax-highlighting.py

Syntax highlighting broken.

[mailings]

We include scripting to use the highlight tool.
IMHO we therefore should also ship the corresponding css.
Removal is very unfortunate and makes the lives of users and packagers
hard without a good reason. I frowned when I saw the patch but didn't
want to comment until now.

Either do none of it, or do all of it. I vote for all of it.

Please resurrect the css

On 12/08/13 21:07, Jason A. Donenfeld wrote:
> This CSS was removed so that cgit could support multiple highlighting
> engines without preferring one over another. Either include this CSS
> along with the highlighted output in <style> tags (as in the case of
> [1]) or add it back to the global CSS yourself.
> 
> [1] http://git.zx2c4.com/cgit/tree/filters/syntax-highlighting.py
> _______________________________________________
> CGit mailing list
> CGit at lists.zx2c4.com
> http://lists.zx2c4.com/mailman/listinfo/cgit
> 

-- 
Ferry Huberts

Syntax highlighting broken.

[smithj4]

I understand the desire to move to a newer and possibly better syntax 
highlighting tool like that python script, but since that requires 
python version 3, it is not possible to use on a RHEL5/6 server, which 
is a show-stopper for us. RHEL7 will probably have it, but we need to 
use the current Enterprise OSes here and RHEL7 probably won't be out for 
close to another year.

~Jason


On 08/12/2013 03:23 PM, Ferry Huberts wrote:
> We include scripting to use the highlight tool.
> IMHO we therefore should also ship the corresponding css.
> Removal is very unfortunate and makes the lives of users and packagers
> hard without a good reason. I frowned when I saw the patch but didn't
> want to comment until now.
>
> Either do none of it, or do all of it. I vote for all of it.
>
> Please resurrect the css
>
> On 12/08/13 21:07, Jason A. Donenfeld wrote:
>> This CSS was removed so that cgit could support multiple highlighting
>> engines without preferring one over another. Either include this CSS
>> along with the highlighted output in <style> tags (as in the case of
>> [1]) or add it back to the global CSS yourself.
>>
>> [1] http://git.zx2c4.com/cgit/tree/filters/syntax-highlighting.py
>> _______________________________________________
>> CGit mailing list
>> CGit at lists.zx2c4.com
>> http://lists.zx2c4.com/mailman/listinfo/cgit
>>
>

Syntax highlighting broken.

[mailings]

On 12/08/13 21:29, Jason A. Smith wrote:
> I understand the desire to move to a newer and possibly better syntax
> highlighting tool like that python script, but since that requires
> python version 3, it is not possible to use on a RHEL5/6 server, which
> is a show-stopper for us. RHEL7 will probably have it, but we need to
> use the current Enterprise OSes here and RHEL7 probably won't be out for
> close to another year.

I fully agree.

Although RHEL7 will probably be out a bit sooner than that ;-)


> 
> ~Jason
> 
> 
> On 08/12/2013 03:23 PM, Ferry Huberts wrote:
>> We include scripting to use the highlight tool.
>> IMHO we therefore should also ship the corresponding css.
>> Removal is very unfortunate and makes the lives of users and packagers
>> hard without a good reason. I frowned when I saw the patch but didn't
>> want to comment until now.
>>
>> Either do none of it, or do all of it. I vote for all of it.
>>
>> Please resurrect the css
>>
>> On 12/08/13 21:07, Jason A. Donenfeld wrote:
>>> This CSS was removed so that cgit could support multiple highlighting
>>> engines without preferring one over another. Either include this CSS
>>> along with the highlighted output in <style> tags (as in the case of
>>> [1]) or add it back to the global CSS yourself.
>>>
>>> [1] http://git.zx2c4.com/cgit/tree/filters/syntax-highlighting.py
>>> _______________________________________________
>>> CGit mailing list
>>> CGit at lists.zx2c4.com
>>> http://lists.zx2c4.com/mailman/listinfo/cgit
>>>
>>
> 

-- 
Ferry Huberts

Syntax highlighting broken.

[Jason]

On Mon, Aug 12, 2013 at 1:29 PM, Jason A. Smith <smithj4 at bnl.gov> wrote:
> I understand the desire to move to a newer and possibly better syntax
> highlighting tool like that python script, but since that requires python
> version 3, it is not possible to use on a RHEL5/6 server, which is a
> show-stopper for us. RHEL7 will probably have it, but we need to use the
> current Enterprise OSes here and RHEL7 probably won't be out for close to
> another year.

Pygments works well with python2 as well. It's easily rewritten to work for it.

Syntax highlighting broken.

[Jason]

On Mon, Aug 12, 2013 at 1:23 PM, Ferry Huberts <mailings at hupie.com> wrote:
> Either do none of it, or do all of it. I vote for all of it.
>
> Please resurrect the css

Right now we don't ship any highlighting css by default. It's up to
the admin on how the css should be integrated, and what highlighting
engine is in use. Perhaps different highlighted files will want
different css. It may be more efficient to spit out a <link> to a
style sheet or the actual styles in <style> from the actual
highlighting engine rather than ship them globally on each in every
page in the global styles document.

Syntax highlighting broken.

[mailings]

We ship highlight in the example script. Without the corresponding css.
FAIL.

Why on earth would we want to do that?
Put the css back!

On 13/08/13 21:32, Jason A. Donenfeld wrote:
> On Mon, Aug 12, 2013 at 1:23 PM, Ferry Huberts <mailings at hupie.com> wrote:
>> Either do none of it, or do all of it. I vote for all of it.
>>
>> Please resurrect the css
> 
> Right now we don't ship any highlighting css by default. It's up to
> the admin on how the css should be integrated, and what highlighting
> engine is in use. Perhaps different highlighted files will want
> different css. It may be more efficient to spit out a <link> to a
> style sheet or the actual styles in <style> from the actual
> highlighting engine rather than ship them globally on each in every
> page in the global styles document.
> 

-- 
Ferry Huberts

Syntax highlighting broken.

[cgit]

On Wed, Aug 14, 2013 at 07:40:21AM +0200, Ferry Huberts wrote:
> We ship highlight in the example script. Without the corresponding css.
> FAIL.
> 
> Why on earth would we want to do that?
> Put the css back!

As you already said, it is an *example* script. It even includes a
comment mentioning that the CSS must be adapted when using that script:

    # Note: the highlight command (http://www.andre-simon.de/) uses css for syntax
    # highlighting, so you'll probably want something like the following included
    # in your css file (generated by highlight 2.4.8 and adapted for cgit):
    #
    # table.blob .num  { color:#2928ff; }
    # table.blob .esc  { color:#ff00ff; }
    # table.blob .str  { color:#ff0000; }
    # table.blob .dstr { color:#818100; }
    # table.blob .slc  { color:#838183; font-style:italic; }
    # table.blob .com  { color:#838183; font-style:italic; }
    # table.blob .dir  { color:#008200; }
    # table.blob .sym  { color:#000000; }
    # table.blob .kwa  { color:#000000; font-weight:bold; }
    # table.blob .kwb  { color:#830000; }
    # table.blob .kwc  { color:#000000; font-weight:bold; }
    # table.blob .kwd  { color:#010181; }

I don't know what all the fuss is about.

What we could do is add the style sheet back as a separate file and add
a commented out "@import" of that file to the main CSS. This might make
it a bit more convenient for administrators to reintroduce the colors. I
don't think it is absolutely needed, though, so this gets a +/-0 from
me.

> 
> On 13/08/13 21:32, Jason A. Donenfeld wrote:
> > On Mon, Aug 12, 2013 at 1:23 PM, Ferry Huberts <mailings at hupie.com> wrote:
> >> Either do none of it, or do all of it. I vote for all of it.
> >>
> >> Please resurrect the css
> > 
> > Right now we don't ship any highlighting css by default. It's up to
> > the admin on how the css should be integrated, and what highlighting
> > engine is in use. Perhaps different highlighted files will want
> > different css. It may be more efficient to spit out a <link> to a
> > style sheet or the actual styles in <style> from the actual
> > highlighting engine rather than ship them globally on each in every
> > page in the global styles document.
> > 
> 
> -- 
> Ferry Huberts
> _______________________________________________
> CGit mailing list
> CGit at lists.zx2c4.com
> http://lists.zx2c4.com/mailman/listinfo/cgit

Syntax highlighting broken.

[smithj4]

On Wed, 2013-08-14 at 12:53 +0200, Lukas Fleischer wrote:
> As you already said, it is an *example* script. It even includes a
> comment mentioning that the CSS must be adapted when using that script:
> 
>     # Note: the highlight command (http://www.andre-simon.de/) uses css for syntax
>     # highlighting, so you'll probably want something like the following included
>     # in your css file (generated by highlight 2.4.8 and adapted for cgit):
>     #
>     # table.blob .num  { color:#2928ff; }
>     # table.blob .esc  { color:#ff00ff; }
>     # table.blob .str  { color:#ff0000; }
>     # table.blob .dstr { color:#818100; }
>     # table.blob .slc  { color:#838183; font-style:italic; }
>     # table.blob .com  { color:#838183; font-style:italic; }
>     # table.blob .dir  { color:#008200; }
>     # table.blob .sym  { color:#000000; }
>     # table.blob .kwa  { color:#000000; font-weight:bold; }
>     # table.blob .kwb  { color:#830000; }
>     # table.blob .kwc  { color:#000000; font-weight:bold; }
>     # table.blob .kwd  { color:#010181; }
> 
> I don't know what all the fuss is about.

The fuss is because it was removed which broke what is currently the
only was to achieve syntax highlighting for some. An easy way to restore
the old working behavior would be preferable, without having to manually
hack the default installed css. Manually editing a non-config file
installed by a package is not the best way to go about doing things.
Another config option, which could be used to select an additional css
to be used for syntax highlighting would be more useful, and could be
used by any of the highlighting methods now supported.

> What we could do is add the style sheet back as a separate file and add
> a commented out "@import" of that file to the main CSS. This might make
> it a bit more convenient for administrators to reintroduce the colors. I
> don't think it is absolutely needed, though, so this gets a +/-0 from
> me.

Still would require manually hacking a non-config file from an installed
rpm package for us. The next time this package gets updated, it is
broken again unless we remember to hack the file after the package is
updated.

Syntax highlighting broken.

[mailings]

On 14/08/13 13:47, Jason A. Smith wrote:
> On Wed, 2013-08-14 at 12:53 +0200, Lukas Fleischer wrote:
>> As you already said, it is an *example* script. It even includes a
>> comment mentioning that the CSS must be adapted when using that script:
>>
>>     # Note: the highlight command (http://www.andre-simon.de/) uses css for syntax
>>     # highlighting, so you'll probably want something like the following included
>>     # in your css file (generated by highlight 2.4.8 and adapted for cgit):
>>     #
>>     # table.blob .num  { color:#2928ff; }
>>     # table.blob .esc  { color:#ff00ff; }
>>     # table.blob .str  { color:#ff0000; }
>>     # table.blob .dstr { color:#818100; }
>>     # table.blob .slc  { color:#838183; font-style:italic; }
>>     # table.blob .com  { color:#838183; font-style:italic; }
>>     # table.blob .dir  { color:#008200; }
>>     # table.blob .sym  { color:#000000; }
>>     # table.blob .kwa  { color:#000000; font-weight:bold; }
>>     # table.blob .kwb  { color:#830000; }
>>     # table.blob .kwc  { color:#000000; font-weight:bold; }
>>     # table.blob .kwd  { color:#010181; }
>>
>> I don't know what all the fuss is about.
> 
> The fuss is because it was removed which broke what is currently the
> only was to achieve syntax highlighting for some. An easy way to restore
> the old working behavior would be preferable, without having to manually
> hack the default installed css. Manually editing a non-config file
> installed by a package is not the best way to go about doing things.
> Another config option, which could be used to select an additional css
> to be used for syntax highlighting would be more useful, and could be
> used by any of the highlighting methods now supported.
> 


Since we're sort of fighting about it, I'm working on a patch to show
the different css-es for highlight 2.4, 2.6 and 3.8.
These are different, so maybe we should instead start assembling a
README.PACKAGERS that mentiones this difference and let them select the
correct css for their distribution.

Or we need to come up with a way to detect the highlight version and use
that to include the correct css.

patch will follow shortly


>> What we could do is add the style sheet back as a separate file and add
>> a commented out "@import" of that file to the main CSS. This might make
>> it a bit more convenient for administrators to reintroduce the colors. I
>> don't think it is absolutely needed, though, so this gets a +/-0 from
>> me.
> 
> Still would require manually hacking a non-config file from an installed
> rpm package for us. The next time this package gets updated, it is
> broken again unless we remember to hack the file after the package is
> updated.
> 
> 
> _______________________________________________
> CGit mailing list
> CGit at lists.zx2c4.com
> http://lists.zx2c4.com/mailman/listinfo/cgit
> 

-- 
Ferry Huberts

Syntax highlighting broken.

[cgit]

On Wed, Aug 14, 2013 at 07:47:55AM -0400, Jason A. Smith wrote:
> On Wed, 2013-08-14 at 12:53 +0200, Lukas Fleischer wrote:
> > As you already said, it is an *example* script. It even includes a
> > comment mentioning that the CSS must be adapted when using that script:
> > 
> >     # Note: the highlight command (http://www.andre-simon.de/) uses css for syntax
> >     # highlighting, so you'll probably want something like the following included
> >     # in your css file (generated by highlight 2.4.8 and adapted for cgit):
> >     #
> >     # table.blob .num  { color:#2928ff; }
> >     # table.blob .esc  { color:#ff00ff; }
> >     # table.blob .str  { color:#ff0000; }
> >     # table.blob .dstr { color:#818100; }
> >     # table.blob .slc  { color:#838183; font-style:italic; }
> >     # table.blob .com  { color:#838183; font-style:italic; }
> >     # table.blob .dir  { color:#008200; }
> >     # table.blob .sym  { color:#000000; }
> >     # table.blob .kwa  { color:#000000; font-weight:bold; }
> >     # table.blob .kwb  { color:#830000; }
> >     # table.blob .kwc  { color:#000000; font-weight:bold; }
> >     # table.blob .kwd  { color:#010181; }
> > 
> > I don't know what all the fuss is about.
> 
> The fuss is because it was removed which broke what is currently the
> only was to achieve syntax highlighting for some. An easy way to restore
> the old working behavior would be preferable, without having to manually
> hack the default installed css. Manually editing a non-config file
> installed by a package is not the best way to go about doing things.

If the style sheet is a non-config file, how do you change the
appearance of the cgit site? Change the CSS in the source tree, commit
on a separate branch, recompile and reinstall?

To me, the cgit.css that gets installed when running `make install` is
just a default config file that needs to be adjusted depending on the
layout requirements and copied to a proper place. Hence, it is fine to
have a CSS that works with the default cgit and can easily be adjusted
to work with any non-default website layout or any non-default cgit
filters. If the default cgit.css ever gets updated, you can use a
default config merge tool like you do for config files of other software
packages.

> Another config option, which could be used to select an additional css
> to be used for syntax highlighting would be more useful, and could be
> used by any of the highlighting methods now supported.
> 
> > What we could do is add the style sheet back as a separate file and add
> > a commented out "@import" of that file to the main CSS. This might make
> > it a bit more convenient for administrators to reintroduce the colors. I
> > don't think it is absolutely needed, though, so this gets a +/-0 from
> > me.
> 
> Still would require manually hacking a non-config file from an installed
> rpm package for us. The next time this package gets updated, it is
> broken again unless we remember to hack the file after the package is
> updated.
> 
> 
> _______________________________________________
> CGit mailing list
> CGit at lists.zx2c4.com
> http://lists.zx2c4.com/mailman/listinfo/cgit

Syntax highlighting broken.

[smithj4]

Maybe there should be another config option, if set would include the
requested additional style sheet in the html, and the css needed by the
script can be included as one possibility.

On Wed, 2013-08-14 at 07:40 +0200, Ferry Huberts wrote:
> We ship highlight in the example script. Without the corresponding css.
> FAIL.
> 
> Why on earth would we want to do that?
> Put the css back!
> 
> On 13/08/13 21:32, Jason A. Donenfeld wrote:
> > On Mon, Aug 12, 2013 at 1:23 PM, Ferry Huberts <mailings at hupie.com> wrote:
> >> Either do none of it, or do all of it. I vote for all of it.
> >>
> >> Please resurrect the css
> > 
> > Right now we don't ship any highlighting css by default. It's up to
> > the admin on how the css should be integrated, and what highlighting
> > engine is in use. Perhaps different highlighted files will want
> > different css. It may be more efficient to spit out a <link> to a
> > style sheet or the actual styles in <style> from the actual
> > highlighting engine rather than ship them globally on each in every
> > page in the global styles document.
> > 
> 

-- 
/------------------------------------------------------------------\
|  Jason A. Smith                          Email:  smithj4 at bnl.gov |
|  Atlas Computing Facility, Bldg. 510M    Phone: +1-631-344-4226  |
|  Brookhaven National Lab, P.O. Box 5000  Fax:   +1-631-344-7616  |
|  Upton, NY 11973-5000,  U.S.A.                                   |
\------------------------------------------------------------------/