Conversation
I'm not sure if I understand this. I don't think Markdown supports comments. What would a comment in Markdown look like?
Would you be able to add the functionality to generate tokens to the JS implementation? |
mpkorstanje
left a comment
There was a problem hiding this comment.
I've given this a once over. Other than my remark on the AST builder I don't see anything too odd. But this PR also exceeds my current capacity for in-depth review.
@ehuelsmann would you be so kind?
sorry, I can't. |
|
rebased, and the commit "[Perl] don't populate comments in AST when MDG" was replaced by "[Perl] ignore Comment in AST" |
There was a problem hiding this comment.
Yes. This is now the equivalent of the JS code. Very good.
While you were heads down implementing this MD support, the project received and fixed #400. Based on my review of the JS code and this PR both are lacking the required fix. Please add the fix and I'll PR for JS. [Edit: I reviewed main and came to the conclusion that it's actually there, but apparently not where I was looking for it.]
see `escapeRegExp` in GherkinInMarkdownTokenMatcher.ts
follow Javascript
Addresses my comment to have consistency between the regular and Markdown token parser implementations.
ehuelsmann
left a comment
There was a problem hiding this comment.
I approve of the current state.
|
@fperrad out of curiosity, where does your interest in contributing GherkinInMarkdown from? Are you planning to use this Perl implementation yourself? |
|
@mpkorstanje lets wait a few days for the Python implementation to be ready for merging too. Then we can release both in a single release. |
|
I would say that more than a few days have passed. 😉 |
🤔 What's changed?
Adding Markdown with Gherkin (MDG) support for Perl.
⚡️ What's your motivation?
🏷️ What kind of change is this?
♻️ Anything particular you want feedback on?
Javascript is the reference implementation for MDG, but it lacks the
gherkin-generate-tokens(and the corresponding tests).So, I am not confident with the test suite.
Some points need clarification:
with MDG, the messages(feature)Commentare skipped when building the AST : bug or feature ?testdata/good/*.feature.md.tokensfrom Markdown support for java #377, but there need some changes. So, these files need a review / validation.📋 Checklist:
This text was originally generated from a template, then edited by hand. You can modify the template here.