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

Add procfile lexer #1808

Merged
merged 10 commits into from Jun 20, 2021
Merged

Add procfile lexer #1808

merged 10 commits into from Jun 20, 2021

Conversation

sblondon
Copy link
Contributor

This PR adds Procfile lexer with very basic unit tests.

@Anteru
Copy link
Collaborator

Anteru commented May 17, 2021

Thanks for the contribution. Can you please add an example file with golden test output (see https://github.com/pygments/pygments/blob/master/Contributing.md for details)? This also seems to add a spurious Procfile which I think was for testing, can you please remove it?

@sblondon
Copy link
Contributor Author

@Anteru Thank you for the quick review!

./Procfile is removed.
./tests/examplefiles/procfile/Procfile and its .output file are updated.
The .output file is now added to the repository.

@Anteru Anteru self-assigned this May 18, 2021
(r'[0-9.]+', Number),
(r'\$[a-zA-Z_][\w]*', Name.Variable),
(r'(\w+)(=)(\w+)', bygroups(Name.Variable, Punctuation, String)),
(r'([\w\s\-\./])', Text),
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you want a + in here. Right now this matches each individual letter as a single token as you can see in the output, and I doubt that's what you're after. Additionally, this matches whitespace -- I'd suggest a separate rule for whitespace (and using the right token type for whitespace.) In that case you need to match whitespace first because you have a dot in this rule (which I hope is intentional :) )

Copy link
Contributor Author

@sblondon sblondon May 22, 2021

Choose a reason for hiding this comment

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

I modified it: there are now 2 patterns. One for Whitespace and one for Text. These two patterns end with +.
Is it good for you?

pygments/lexers/procfile.py Outdated Show resolved Hide resolved
pygments/lexers/procfile.py Show resolved Hide resolved
(r'^([a-z]+)(:)', bygroups(Name.Label, Punctuation)),
(r'"[^"]*"', String),
(r"'[^']*'", String),
(r'[0-9.]+', Number),
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will match ..9.. as a number -- is that intentional? Could you please add a number into your Procfile example?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a basic number in Procfile file and removed the dot from the regular expression so it detects Number.Integer. I think it's good enough because I doubt someone use float number in commands. It is ok for you?

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's not great but I guess it'll do for now. I assume people will try to highlight valid Procfiles with it :)

@sblondon sblondon requested a review from Anteru June 7, 2021 18:46
@Anteru Anteru added this to the 2.10 milestone Jun 20, 2021
@Anteru Anteru added the changelog-update Items which need to get mentioned in the changelog label Jun 20, 2021
@Anteru Anteru merged commit 5f976c2 into pygments:master Jun 20, 2021
@Anteru
Copy link
Collaborator

Anteru commented Jun 20, 2021

Merged, thanks!

@Anteru Anteru removed the changelog-update Items which need to get mentioned in the changelog label Aug 8, 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.

None yet

2 participants