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

Update nimrod.py lexer #1970

Merged
merged 23 commits into from
Sep 24, 2022
Merged

Update nimrod.py lexer #1970

merged 23 commits into from
Sep 24, 2022

Conversation

matkuki
Copy link
Contributor

@matkuki matkuki commented Nov 23, 2021

Updated the Nim Lexer.
Did some basic testing and it's better than before.

@matkuki
Update nimrod.py
478c1b3 
Update the Nim Lexer.
@matkuki matkuki changed the title Update nimrod.py Update nimrod.py lexer Nov 24, 2021
@matkuki
Fixed linter error in 'casebranch'
4dea86a
@matkuki
Update example.nim.output
f2dafcb 
Updated example.nim.output with --update-goldens
@matkuki
Update test.nim.output
73ba943 
Updated test.nim.output with --update-goldens
Anteru
Anteru requested changes Dec 4, 2021
View reviewed changes
Copy link
Collaborator

@Anteru Anteru left a comment

Choose a reason for hiding this comment

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

Thanks for the update; there are a few things I'd like to see addressed before merging this. While you're at it: Would you mind changing the whitespace tokens to Text.Whitespace?

tests/examplefiles/nim/example.nim.output Outdated Show resolved Hide resolved
tests/examplefiles/nim/example.nim.output Outdated Show resolved Hide resolved
pygments/lexers/nimrod.py Outdated Show resolved Hide resolved
@matkuki
Update nimrod.py
7c95f8e 
Made changes requested by Anteru.
@matkuki
Update example.nim.output
8a7f2ea 
Combined some token streams as requested by Anteru
@matkuki
Update test.nim.output
50cc2da 
Updated for changes requested by Anteru
@matkuki
Copy link
Contributor Author

matkuki commented Dec 4, 2021

Thanks for the update; there are a few things I'd like to see addressed before merging this. While you're at it: Would you mind changing the whitespace tokens to Text.Whitespace?

Done.

@matkuki
Update example.nim.output
00446a9 
Re-ran the `--update-goldens`
@matkuki
Copy link
Contributor Author

matkuki commented Dec 4, 2021

Had a bug in the test .output file. Please re-run checks.

@matkuki matkuki requested a review from Anteru December 8, 2021 10:27
@Anteru Anteru added the update needed Waiting for an update from the PR/issue creator label Dec 28, 2021
@matkuki
Copy link
Contributor Author

matkuki commented May 12, 2022

Added requested fixes, can you please review? Thanks

@matkuki matkuki requested a review from Anteru May 12, 2022 10:06
jeanas
jeanas reviewed May 12, 2022
View reviewed changes
Copy link
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.

I believe @Anteru might not be able to work on Pygments at the moment; I can handle this, here's some review (again :-).

pygments/lexers/nimrod.py Outdated Show resolved Hide resolved
pygments/lexers/nimrod.py Outdated Show resolved Hide resolved
pygments/lexers/nimrod.py Outdated Show resolved Hide resolved
pygments/lexers/nimrod.py Show resolved Hide resolved
pygments/lexers/nimrod.py Outdated Show resolved Hide resolved
pygments/lexers/nimrod.py Show resolved Hide resolved
tests/examplefiles/nim/example.nim.output Outdated Show resolved Hide resolved
@matkuki
Copy link
Contributor Author

matkuki commented Sep 21, 2022

Can someone please recheck this?

@jeanas
Copy link
Contributor

jeanas commented Sep 21, 2022

Needs solving merge conflicts.

@matkuki
Copy link
Contributor Author

matkuki commented Sep 22, 2022

Needs solving merge conflicts.

Done, can you please recheck? Thanks

jeanas
jeanas reviewed Sep 22, 2022
View reviewed changes
pygments/lexers/nimrod.py Outdated Show resolved Hide resolved
@matkuki @jeanas
Update pygments/lexers/nimrod.py
9eb2db4 
Co-authored-by: Jean Abou-Samra <jean@abou-samra.fr>
@matkuki
Copy link
Contributor Author

matkuki commented Sep 23, 2022

Is there anything else needed?

@jeanas
Copy link
Contributor

jeanas commented Sep 23, 2022

CI is failing since you need to update the token output.

@matkuki
Copy link
Contributor Author

matkuki commented Sep 23, 2022

Done, can you please rerun? Thanks

@jeanas
Copy link
Contributor

jeanas commented Sep 24, 2022

Done, but it still fails, how did you do the update? It's missing lots of changes. Normally, pytest --update-goldens tests/examplefiles/nim should do the right thing.

@matkuki
Copy link
Contributor Author

matkuki commented Sep 24, 2022

Re-ran the test as you specified and uploaded all test and token files, just in case.

@jeanas
Copy link
Contributor

jeanas commented Sep 24, 2022

It still fails because your latest push introduces CRLF line endings.

I am going to do this update myself and push the result to your branch.

@jeanas jeanas merged commit 202426c into pygments:master Sep 24, 2022
@matkuki
Copy link
Contributor Author

matkuki commented Sep 24, 2022

Thank you 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeanas jeanas jeanas left review comments

@Anteru Anteru Awaiting requested review from Anteru

Assignees
No one assigned
Labels
update needed Waiting for an update from the PR/issue creator
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@matkuki @jeanas @Anteru