Skip to content

java multiline highlighting based on issue 1858#2132

Merged
jeanas merged 2 commits intopygments:masterfrom
VishalN7:1858/VishalN7/jvm_multiline
May 10, 2022
Merged

java multiline highlighting based on issue 1858#2132
jeanas merged 2 commits intopygments:masterfrom
VishalN7:1858/VishalN7/jvm_multiline

Conversation

@VishalN7
Copy link
Copy Markdown
Contributor

@VishalN7 VishalN7 commented May 9, 2022

We have implemented what we thought to be multiline support, and would appreciate any feedback regarding implementation/commenting and any supporting documentation you would like from us.

Copy link
Copy Markdown
Contributor

@jeanas jeanas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the code level this looks good to me, but you need to write a test. Please add a file under tests/snippets/java/ containing a small sample of Java code with a text block. Then use pytest --update-goldens tests/snippets/java/ in order to generate the output appended to the file. After that, reopen the file and read the token output to check it's correct. If all's well, commit it and run pytest to perform a full round of testing. Thanks.

@jeanas jeanas added the update needed Waiting for an update from the PR/issue creator label May 10, 2022
@jeanas
Copy link
Copy Markdown
Contributor

jeanas commented May 10, 2022

Thanks! I love this test :-)

@jeanas jeanas merged commit 4d2a285 into pygments:master May 10, 2022
@jeanas jeanas added changelog-update Items which need to get mentioned in the changelog and removed update needed Waiting for an update from the PR/issue creator labels May 10, 2022
@jeanas jeanas removed the changelog-update Items which need to get mentioned in the changelog label May 20, 2022
@jeanas jeanas added this to the 2.13.0 milestone May 20, 2022
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.

3 participants