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.

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

#2268: Fix test code to support Pygments 2.0 #3486

Closed

Conversation

shibukawa
Copy link
Contributor

Subject:

Feature or Bugfix

  • Bugfix

Purpose

  • Fix test code to support Pygments 2.0

Relates

@shibukawa
Copy link
Contributor Author

I tested with Pygments 2.0, 2.1, 2.2.

@shibukawa shibukawa force-pushed the fix/pygments_compatibility branch from aa90c99 to e92150f Compare March 2, 2017 08:14
@tk0miya
Copy link
Member

tk0miya commented Mar 2, 2017

Thank you for PR, but it's not good way. What I'd like to say in #2268 is not support to old pygments on our tests.
Even if this merged, it might be broken in case of pygments will change the structure of HTML.
To be robust, we should not inspect the results of pygments. This is testcases for Sphinx, not for pygments. So we should not be care of the highlighted codes in our tests.
What do you think?

@shibukawa
Copy link
Contributor Author

Pygments' releases are not so fast:

  • 2.2: Jan 22, 2017
  • 2.1: Jan 17, 2016
  • 2.0: Nov 9, 2014
  • 1.9: Feb 3, 2013

So I think the risk of adding some Pygments specific test cases is not so high. Resulting of Pygments is just HTML. Matching with a regular expression is enough to support multiple version of Pygments.

@tk0miya
Copy link
Member

tk0miya commented Mar 3, 2017

Sure, it's not hurried.

Anyway supporting 2.0 is not important. So I close this.
Thanks,

@tk0miya tk0miya closed this Mar 3, 2017
@shibukawa shibukawa deleted the fix/pygments_compatibility branch March 3, 2017 15:56
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants