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 support for BDD features / stories #1803

Merged
merged 50 commits into from
Nov 27, 2021
Merged

Conversation

leexuan0128
Copy link
Contributor

@leexuan0128 leexuan0128 commented May 13, 2021

Hi there,

This pull request adds the support for BDD (behavior-driven development) features/stories discussed in issue #668.
We added keywords, punctuations, comments and numbers and etc for lexers to parse. It now should highlight those different parts correctly. Let us know if there are any bugs that need to be fixed or somewhere that needs to be changed.

Test sample

Hongyuan Yan and others added 30 commits April 29, 2021 18:07
Initialize the lexer file of BDD
Add keywords of BDD
Add regular expression for tokens
Add regular expression for tokens
Assign different colors to keywords, punctuation, numbers, and variables.
Update the BddLexer file from Jessie
Fix the file name.
Co-Authored-By: OMGJL <8707895+OMGJL@users.noreply.github.com>
Add detection of double quotes.

Co-Authored-By: OMGJL <8707895+OMGJL@users.noreply.github.com>
@leexuan0128
Copy link
Contributor Author

Hi, thanks for preparing this. A few notes:

  • Please add a test file with a known-good output.
  • Don't change the .gitignore
  • Please remove the .ds_store files which are part of this commit

Hi, thanks for your reminder, already add test cases. let me know if there are any changes that need to make :)

pygments/lexers/bdd.py Show resolved Hide resolved
pygments/lexers/bdd.py Outdated Show resolved Hide resolved
pygments/lexers/bdd.py Outdated Show resolved Hide resolved
pygments/lexers/bdd.py Outdated Show resolved Hide resolved
pygments/lexers/bdd.py Outdated Show resolved Hide resolved
pygments/lexers/bdd.py Outdated Show resolved Hide resolved
pygments/lexers/bdd.py Outdated Show resolved Hide resolved
pygments/lexers/bdd.py Outdated Show resolved Hide resolved
Copy link
Contributor Author

@leexuan0128 leexuan0128 left a comment

Choose a reason for hiding this comment

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

add versionadded: 2.10 comment and the explanation

@Anteru
Copy link
Collaborator

Anteru commented May 14, 2021

Changes look mostly good, but you do highlight whitespace as Text. Please assign whitespace to things like and \n as some style might highlight whitespace differently from text. Check the golden test output.

Copy link
Contributor

@amitkummer amitkummer left a comment

Choose a reason for hiding this comment

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

Thanks a lot for your work! Some of the things I commented about appear on multiple places, so please, if you think my comment/suggestion is justified, then change it in all other places as well.

Also, check your test output, it still has some issues (I see spaces as Text a lot).

pygments/lexers/bdd.py Outdated Show resolved Hide resolved
pygments/lexers/bdd.py Outdated Show resolved Hide resolved
tests/examplefiles/bdd/example.feature.output Outdated Show resolved Hide resolved
tests/examplefiles/bdd/example.feature.output Outdated Show resolved Hide resolved
pygments/lexers/bdd.py Outdated Show resolved Hide resolved
pygments/lexers/bdd.py Outdated Show resolved Hide resolved
pygments/lexers/bdd.py Outdated Show resolved Hide resolved
pygments/lexers/bdd.py Outdated Show resolved Hide resolved
pygments/lexers/bdd.py Outdated Show resolved Hide resolved
pygments/lexers/bdd.py Outdated Show resolved Hide resolved
include('comments'),
include('miscellaneous'),
include('numbers'),
(r'.', Text),
Copy link
Collaborator

Choose a reason for hiding this comment

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

This rule produces tons of tokens each containing a single character. I think you want \S+ here, which should also dramatically cut down the size of your test output.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you so much for your advice. We have changed the bdd.py according to your help. Now the token size was greatly reduced.

Change "." to the "\S+",
Which reduce the test output file size.

Co-Authored-By: OMGJL <8707895+OMGJL@users.noreply.github.com>
(r'(\d+\.?\d*|\d*\.\d+)([eE][+-]?[0-9]+)?', Number),
],
'root': [
(r'\n|\s', Whitespace),
Copy link
Collaborator

Choose a reason for hiding this comment

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

That should be \s+ so you don't produce a new token for each space (sorry I missed this previously.)

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for your correction. Now, bdd.py won't generate the new token for each space.

@Anteru Anteru self-assigned this May 21, 2021
Reduce the new token for each space
@leexuan0128 leexuan0128 requested a review from Anteru May 26, 2021 04:48
@Anteru Anteru merged commit 97e5c04 into pygments:master Nov 27, 2021
@Anteru
Copy link
Collaborator

Anteru commented Nov 27, 2021

Merged, sorry it took so long.

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

6 participants