Skip to content

Conversation

@alexanderglueck
Copy link
Contributor

Description

This pull request makes commonmark-highlighter compatible with league/commonmark v2. By upgrading to v2, support for v1 is dropped. This is a breaking change, and therefor I marked the release in the changelog as 3.0.

Changes:

Changes to tests:

highlight.php produces different output depending on the installed version:

  • Without my changes to the fixtures of the tests: If you run the tests with highlight.php locked at exactly 9.18.1.4 all the tests pass.
  • Using the latest version (9.18.1.7) the generated output differs which causes the tests to fail.
    I updated the tests to use the newly generated output. However, now testing for "prefer-lowest" fails because .4 is installed instead of .7.

I am looking forward to suggestions on how I can fix this tricky situation (fixtures).

TLDR on tests: highlight.php 9.18.1.4 generates different output than 9.18.1.7. I updated the fixtures to work with the latest version. Tests fail because of prefer-lowest.

Closing

Let me know if there is anything else you need.

@freekmurze
Copy link
Member

This is awesome, I'll review it soon.

I you feel up to it, you could send a similar PR to our commonmark-shiki-highlighter package, which provides superior highlighting.

@freekmurze
Copy link
Member

@alexanderglueck could you make those failing tests pass by updating the versions of requirements of the package?

@alexanderglueck
Copy link
Contributor Author

I increased the minimum required version of highlight.php to 9.18.1.7 and now the tests are only failing (on my branch) due to not being able to push to Scrutinizer.
So it should probably work on your account.

@freekmurze
Copy link
Member

@alexanderglueck running the tests... 🤞

@freekmurze
Copy link
Member

All good 🙌

@freekmurze freekmurze merged commit 83f7751 into spatie:main Aug 4, 2021
@freekmurze
Copy link
Member

Thank you!

@alexanderglueck
Copy link
Contributor Author

You're welcome. Just one more quick question, when can I expect a release so I can change my project to pull in the new version? 😄

@freekmurze
Copy link
Member

You can expect it in... one minute.

@alexanderglueck
Copy link
Contributor Author

Thank you!

@alexanderglueck alexanderglueck mentioned this pull request Aug 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants