Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Open
agentydragon opened this issue Jan 10, 2020 · 6 comments · May be fixed by #20
Open

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

agentydragon opened this issue Jan 10, 2020 · 6 comments · May be fixed by #20
Labels
bug Something isn't working

Comments

@agentydragon
Copy link
Contributor

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:

image

Error message (if any)

None
@agentydragon agentydragon added the bug Something isn't working label Jan 10, 2020
@prurph
Copy link

prurph commented Feb 11, 2020 •
edited
Loading

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
Copy link
Contributor Author

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
Copy link
Owner

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
Copy link

@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
Copy link
Contributor

ijgnd commented Mar 31, 2020

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.

@gottschali gottschali linked a pull request Apr 4, 2020 that will close this issue
Open
13 tasks
@agentydragon
Copy link
Contributor Author

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 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

remove unnecessary HTML tags if linenumbering is not requred
5 participants
@agentydragon @glutanimate @prurph @gottschali @ijgnd