Skip to content

Draft: Refactor PythonConsoleLexer as a DelegatingLexer #2412

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

Merged
merged 1 commit into from
Apr 17, 2023

Conversation

jeanas
Copy link
Contributor

@jeanas jeanas commented Apr 12, 2023

This is simpler and more reliable than hand-coding the state machine.

Fixes #2411

I need to split this into more commits and check a few things before it can be 'undrafted'.

Also, while this should IMHO be the way going forward, I'm not sure about including a full rewrite of a lexer in a bugfix release. On the other hand, #2411 is not a regression compared to 2.14 as far as I can see.

This is simpler and more reliable than hand-coding the state machine.

Fixes pygments#2411
@jeanas jeanas requested review from birkenfeld and Anteru April 12, 2023 20:19
@jeanas jeanas marked this pull request as draft April 12, 2023 20:19
@Anteru
Copy link
Collaborator

Anteru commented Apr 15, 2023

I like that solution better than the current workarounds. I just checked, the new lexer doesn't appear in the documentation, so that's good. I don't mind shipping a lexer rewrite given it passes all existing tests, and the current test coverage is better than what we shipped the last major update with (hence the regressions!) Unless @birkenfeld has any reservations, my vote is on merging this and getting a 2.15.1 release out of the door as soon as we can.

@Anteru Anteru added the A-lexing area: changes to individual lexers label Apr 15, 2023
@Anteru Anteru added this to the 2.15.1 milestone Apr 15, 2023
@birkenfeld
Copy link
Member

birkenfeld commented Apr 15, 2023

I'm on holiday, my only reservations are in hotels. Go ahead :)

@Anteru
Copy link
Collaborator

Anteru commented Apr 15, 2023

@jeanas Good to go or do you want to do anything else before merging? It's currently in draft so I cannot trivially merge this.

@jeanas jeanas marked this pull request as ready for review April 17, 2023 16:40
@jeanas
Copy link
Contributor Author

jeanas commented Apr 17, 2023

Coming back to this, I didn't really find value in making more commits. Let's merge it.

I agree on releasing a bugfix version soon. Maybe also include #2404 in it? (It fixes a case of catastrophic backtracking.)

@jeanas jeanas merged commit c977624 into pygments:master Apr 17, 2023
@jeanas jeanas deleted the pycon branch April 17, 2023 16:42
@Anteru
Copy link
Collaborator

Anteru commented Apr 17, 2023

Thanks! Yes -- that's enough for a point release. I'll get #2404 included as well. I should get to it tomorrow. Good work on both!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lexing area: changes to individual lexers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pycon lexer still reorders input
3 participants