search

trentm / python-markdown2

markdown2: A fast and complete implementation of Markdown in Python
Other
2.66k stars 433 forks source link

Update tests for new Pygments package #496

Closed nicholasserra closed 1 year ago

nicholasserra commented 1 year ago

https://pypi.org/project/Pygments/2.14.0/

nicholasserra commented 1 year ago

Tests currently failing in master

Crozzers commented 1 year ago

I checked this out and the issue seems to be that pygments has started wrapping whitespace in span tags.

- <pre><span></span><code><span class="k">def</span> <span class="nf">foo</span>
+ <pre><span></span><code><span class="k">def</span><span class="w"> </span><span class="nf">foo</span>
-     <span class="nb">puts</span> <span class="s2">&quot;hi&quot;</span>
+ <span class="w">    </span><span class="nb">puts</span><span class="w"> </span><span class="s2">&quot;hi&quot;</span>
  <span class="k">end</span>

I think this is kinda cool as we don't have to worry so much about maintaining code block whitespace.

I've got a branch that fixes up the two affected test cases but what should be done about testing? I see three solutions: bump the required pygments version up and lose Py3.5 support, cap the maximum pygments version or update the test harness to only run the affected tests on a supported pygments version.

I think the latter would be doable, could use the existing tag system with the tests to tag certain test cases as pygments<2.14 and then check the installed pygments version matches that requirement

nicholasserra commented 1 year ago

Nice, thank you. Didn't have capacity to dive into this one lately, even if it's minor.

I think your idea of only running on affected versions is good, assuming that functionality is supported and it's not too much a pain. Otherwise I say just go path of least resistance. I've been chasing the pygments bump on this repo for a decade now so it's no big deal to have to keep dealing with it from time to time.

Crozzers commented 1 year ago

@nicholasserra I forgot to link this issue to the associated PR. This one can probably be closed