PygmentsRenderer produces unnecessary spans

[Tim Hatch <trac@…>]

On trunk post-r4399, it's possible (and likely) to end up with <span class=""> in the rendered source (I think the Pygments plain text one does this for everything).

I think this is due to pygments.formatters.html.HtmlFormatter._get_css_class (source:trunk/trac/mimeview/pygments_renderer.py@4399#L219) returning an empty string.

Perhaps it's returning the wrong class, but if indeed it's supposed to be the empty string, then I suggest the span not be output?

[cmlenz]

Thanks, I'll look into this.

[Tim Hatch <trac@…>]

I did some quick tests to eradicate:

• <span class="something-valid"></span> at beginning and end of line
• <span class="">some-stuff</span> -> some-stuff

The first is generated for most lines because a span isn't closed till after the \n and is thus opened and immediately closed a lot.

The generated size for a reasonably large test (source:trac/mimeview/tests/api.py@3544 as of Nov 10) rendered using a /file link on my test Trac, the filesize dropped from 31078 bytes to 22966 bytes (the plaintext source is 3.3k and the chrome overhead appears to be about 6.4k). This makes the bloat from syntax highlighting (this specific file) go from 7.4x in trunk to 5.0x with this patch.

I've confirmed that they are both using PygmentsRenderer. The only visual difference appears to be the extra blank line for files with \n at EOF (see comment:ticket:4336:6 -- it's due to other changes).

I'm attaching the patch in a second, welcoming comments. I don't see that there is a large overhead for doing this processing, and it does make the files quite a bit smaller.

P.S. the patch is on top of the one for #4336 and modifies some of its tests.

P.P.S. If you can't view older revs of api.py on t.e.o, see #4365

[cboos]

Is this still needed as of Pygments:0.6?

If I'm not mistaken, the empty spans are gone.

(btw, jonas, could you add an InterTrac prefix for pygments?)

[thatch]

Check source:trunk/trac/mimeview/tests/pygments.html on line 17 (and others). Although those tests don't appear to be passing on my system, the part about expecting silly spans is true. When that's passed through _group_lines you end up with empty spans at the beginning or end of line.

I just ran api.py through it again and it came up with this snippet:

...'text/plain'...

which actually has another issue too, closing and immediately reopening in the middle of the line.

[thatch]

sorry, that snippet is supposed to be

...<span class="s">'</span><span class="s">text/plain</span><span class="s">'</span>...<span class=""></span>

There we go again with the default rendering vs syntax coloring :)

[cboos]

Well, the highlighter to use in this case is #!xml:

...<span class="s">'</span><span class="s">text/plain</span><span class="s">'</span>...<span class=""></span>

About the pygment tests: yes, you're right, I overlooked the fact that the empty spans are still there. You can update the "expected" content, if you think that the pygmentized output is now "stable".

[thatch]

New patch, closer to the problem, as of r4575

[thatch]

Ok, I've got a new patch, see attachment:t4339b.diff

I've got the spans outputting the way I want, just by combining text nodes and actually checking for a null css class instead of Attrs... because Attrs([('class', '')]) doesn't evaluate to False.

The tests are all failing though, because whitespace is being munged. There are a couple of debug statements added to _test() to aid in this.

[cboos]

Bumping up the priority of this. Anything that can help reduce the memory usage is good :-)

Tim, if you could check/revive this patch, maybe we can get that in 0.11?

[cboos]

Updated patch committed as r7057, plus another optimization committed as r7058.

With both optimizations, the highlighted page size can be only 60% of the page size it used to be! (e.g. for some old version of trac/attachment.py, 24.2 kB of Python code, the size of the corresponding highlighted document is now 125 kB and used to be 215 kB).