gherkin/java:Support unicode colon as separator. #223
Conversation
|
Thanks! Can you please add tests using the new separator? |
|
@aslakhellesoy Is this mergeable? It looks cool. |
|
All other implementations must be updated with the same change before this is mergeable. In order to reduce duplication in tests, please delete the unit test and add an acceptance test in |
|
Done. |
| public interface GherkinLanguageConstants { | ||
| String TAG_PREFIX = "@"; | ||
| String COMMENT_PREFIX = "#"; | ||
| String TITLE_KEYWORD_SEPARATOR = ":"; | ||
| Pattern TITLE_KEYWORD_SEPARATOR2 = Pattern.compile(":|："); |
There was a problem hiding this comment.
Please remove the old TITLE_KEYWORD_SEPARATOR and rename TITLE_KEYWORD_SEPARATOR2 to TITLE_KEYWORD_SEPARATOR
| public interface GherkinLanguageConstants { | ||
| String TAG_PREFIX = "@"; | ||
| String COMMENT_PREFIX = "#"; | ||
| String TITLE_KEYWORD_SEPARATOR = ":"; | ||
| Pattern TITLE_KEYWORD_SEPARATOR2 = Pattern.compile(":|："); |
There was a problem hiding this comment.
This change must be ported to all other Gherkin implementations (Go, C, Ruby, JavaScript etc) before it can be merged.
There was a problem hiding this comment.
should we introduce a constants TITLE_KEYWORD_SEPARATOR_LENGTH? since pattern have no length.
|
matchTitleLine or startsWithTitleKeyword need refactor , because matchTitleLine use separator.length() |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed in a week if no further activity occurs. |
stale
bot
added
the
⌛ stale
stale
bot
removed
the
⌛ stale
|
Should I make changes in all other language implementations? |
|
This PR has gone stale in the light of #425. If you still want this, please make a PR against the |
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary
Support unicode fullwidth colon as title separator.
Details
Motivation and Context
i18n not only keyword translate,but also punctuate。
Why only colon?because it is title line separator, no syntax meaning.
Original issue post here :cucumber/gherkin#220
How Has This Been Tested?
The change not break exists test.
New test case add in ParserTest.java
Types of changes
Checklist: