Code unnecessarily wrapped in <table> when line numbers are disabled

agentydragon commented 4 years ago

Problem description

This is how the HTML from Syntax Highlighting addon looks:

<table><tbody><tr><td><div class="highlight"><pre><span class="o">&gt;&gt;&gt;</span> <span class="n">df</span> <span class="o">=</span> <span class="n">pd</span><span class="o">.</span><span class="n">DataFrame</span><span class="p">(</span>
<span class="o">...</span>     <span class="n">data</span><span class="o">=</span><span class="p">{</span><span class="s1">'name'</span><span class="p">:</span> <span class="p">[</span><span class="s1">'Alice'</span><span class="p">,</span> {{c4::<span class="s1">'Bob'</span><span class="p">,</span> <span class="s1">'Charlie'</span>}}<span class="p">]},</span>
<span class="o">...</span>     <span class="n">{{c3::index}}</span><span class="o">=</span><span class="p">[</span><span class="mi">2</span><span class="p">,</span> <span class="mi">0</span><span class="p">,</span> <span class="mi">1</span><span class="p">])</span>
<span class="o">&gt;&gt;&gt;</span> <span class="k">print</span><span class="p">(</span><span class="n">df</span><span class="o">.</span><span class="n">{{c1::lo}}c</span><span class="p">[</span><span class="mi">1</span><span class="p">][</span><span class="s1">'name'</span><span class="p">],</span>
<span class="o">...</span>       <span class="n">df</span><span class="o">.</span><span class="n">{{c1::ilo}}c</span><span class="p">[</span><span class="mi">1</span><span class="p">][</span><span class="s1">'name'</span><span class="p">])</span>
<span class="n">{{c2::Charlie}}</span> <span class="n">{{c2::Bob}}</span></pre></div></td></tr></tbody></table>

The <table><tbody><tr><td> tags seem to me to be unnecessary. As far as I understand, they are there to support line numbers, but since I don't have line numbers enabled, I don't think they serve any purpose. (With my CSS, they just take up extra whitespace.)

I've set up syntax highlighting without line numbers, and style it with my own CSS.

Checklist

Please replace the space inside the brackets with an x if the following items apply:

[ x ] I've restarted Anki to see if it helps
[ ] I've verified that I use the latest version of the add-on by redownloading it from AnkiWeb
[ ] I've verified that I use the latest version of Anki by checking at https://apps.ankiweb.net#download
[ ] I've tried to disable other add-ons to see if there are any interactions present
[ ] My issue disappears when I hold shift while starting Anki.
[ ] I've checked if anyone else reported this problem before by looking through the issue reports. I also checked to see if there is a section about known issues in the add-on description, documentation, or README.

Information about your Anki set-up

Please fill out the section corresponding with your Anki version:

I don't have "copy debug info" in my Anki, it's probably an older version:

Version 2.1.8 (71e0c880)
Qt 5.12.4 PyQt 5.12.3

OS: Ubuntu 19.10

Distributor ID: Ubuntu
Description:    Ubuntu 19.10
Release:    19.10
Codename:   eoan

Please open Anki, go to Tools → Add-ons, take a screenshot of your installed add-ons, and paste it below:

Error message (if any)

None

prurph commented 4 years ago

I'd like this as well. I manually edited the code on my local install to remove the tables but it would be nice to have it in the official package also.

agentydragon commented 4 years ago

FYI, I added a $15 bounty on this: https://www.bountysource.com/issues/86739059-code-unnecessarily-wrapped-in-table-when-line-numbers-are-disabled

glutanimate commented 4 years ago

Thanks for the suggestion! This sounds reasonable. Can't think of any good reason to keep the tables when no line numbers are required (though I might be missing something).

@Aliuakbar: Am I interpreting this right that you started working on this in your fork? Are you planning on submitting a PR? If so I will hold off on this and give you the floor of course.

gottschali commented 4 years ago

@glutanimate yes I was working on this on my fork. Removing the tag was relatively easy but I stumbled upon a problem that the centering broke. You don't have to hold on this issue and may also use my code if it is any help.

ijgnd commented 4 years ago

In my fork I kept the table tag. I made the following note:

    # to show line numbers pygments uses a table. The background color for the code
    # highlighting is limited to this table
    # If pygments doesn't use linenumbers it doesn't use a table. This means
    # that the background color covers the whole width of the card.
    # I don't like this. I didn't find an easier way than reusing the existing
    # solution.
    # Glutanimate removed the table in the commit
    # https://github.com/glutanimate/syntax-highlighting/commit/afbf5b3792611ecd2207b9975309d05de3610d45
    # which hasn't been published on Ankiweb in 2019-10-02.

If I remember correctly it's useful to wrap the output of pygments in some tag so that you can set the style of the code block to left-aligned so that you can override text-align: center; from the .card class. The table tag might be repurposed for this.

General caveat also applies here: My coding skills are limited etc.

agentydragon commented 3 years ago

For anyone else: I talked with @glutanimate, turns out this was already fixed a long time back and a new version of the addon just has not been released since. A fix is to build the addon at current master.

Leaving issue open, so that it serves as a reminder for @glutanimate to release a new version at some point :)

glutanimate / syntax-highlighting