Skip to content

Reduce duplication between code and snippet generation#3127

Merged
mpkorstanje merged 4 commits into
cucumber:mainfrom
ielatif:gh-3063
Dec 18, 2025
Merged

Reduce duplication between code and snippet generation#3127
mpkorstanje merged 4 commits into
cucumber:mainfrom
ielatif:gh-3063

Conversation

@ielatif
Copy link
Copy Markdown
Contributor

@ielatif ielatif commented Dec 4, 2025

Fixes #3063

🤔 What's changed?

⚡️ What's your motivation?

As seen in #3062 currently boths GenerateI18n classes for cucumber-java[8] and SnippetGenerator turn Gherkin keywords into code. This code is now duplicated 3 times over. Each with a special exception for Emoji.

🏷️ What kind of change is this?

@ielatif
Copy link
Copy Markdown
Contributor Author

ielatif commented Dec 5, 2025

Build fail with error :

[ERROR] java.class.externalClassExposedInAPI: class io.cucumber.gherkin.GherkinDialect: A class from supplementary archives is used in a public capacity in the API. [io.cucumber:gherkin:jar:36.1.0]

@mpkorstanje can you advice in which module should i put GherkinKeywordNormalizer class ?

@mpkorstanje
Copy link
Copy Markdown
Member

I'll have to think about that for a minute. It might be that we'd have to move this to the gherkin library instead.

@ielatif
Copy link
Copy Markdown
Contributor Author

ielatif commented Dec 8, 2025

@mpkorstanje that makes perfect sense. Shifting this to the Gherkin library would improve separation of concerns, as this logic naturally fits there. We can keep specific logic in cucumber-jvm.

@mpkorstanje
Copy link
Copy Markdown
Member

I thought about it for a while. And normalization happens in a context. In this case we want identifiers that are safe to use in Java class and package names. That context is lost if this were to be moved to the Gherkin library.

Copy link
Copy Markdown
Member

@mpkorstanje mpkorstanje left a comment

Choose a reason for hiding this comment

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

Looks good. I've pushed some small changes while reviewing. Please do:

  • Remove capitalize from the public API
  • Add a few test for both remaining public methods.

return language.replaceAll("[\\s-]", "_").toLowerCase();
}

public static String capitalize(String str) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This doesn't quite belong. Please make private and duplicate as needed.

@ielatif
Copy link
Copy Markdown
Contributor Author

ielatif commented Dec 8, 2025

@mpkorstanje done. Thanks for your review!

@ielatif ielatif requested a review from mpkorstanje December 9, 2025 10:30
@mpkorstanje mpkorstanje self-assigned this Dec 18, 2025
@mpkorstanje mpkorstanje merged commit b2bed6a into cucumber:main Dec 18, 2025
6 checks passed
@mpkorstanje
Copy link
Copy Markdown
Member

Cheers!

@ielatif
Copy link
Copy Markdown
Contributor Author

ielatif commented Dec 18, 2025

Thank you for your valuable feedback and review

@ielatif ielatif deleted the gh-3063 branch December 18, 2025 14:43
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.

Reduce duplication between code and snippet generation

2 participants