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

[Core] Delegate encoding and BOM handling to gherkin #2624

Merged
merged 1 commit into from
Oct 12, 2022

Conversation

mpkorstanje
Copy link
Contributor

@mpkorstanje mpkorstanje commented Oct 11, 2022

🤔 What's changed?

Previously before handing over a feature file to the Gherkin parser, Cucumber would remove any Byte Order Markers (BOM) and determine the encoding of the feature file based on the #encoding: <encoding> comment.

With cucumber/common#2018 this can now be handled by Gherkin. Removing it simplifies Cucumber a little. Unfortunately the FeatureParser doesn't take a InputStream as an argument. So we need to add a default interface to avoid breaking semver.

🏷️ What kind of change is this?

  • 🏦 Refactoring/debt/DX (improvement to code design, tooling, documentation etc. without changing behaviour)

📋 Checklist:

  • I agree to respect and uphold the Cucumber Community Code of Conduct
  • I've changed the behaviour of the code
    • I have added/updated tests to cover my changes.
  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.
  • Users should know about my change
    • I have added an entry to the "Unreleased" section of the CHANGELOG, linking to this pull request.

@mpkorstanje mpkorstanje force-pushed the delegate-encoding-and-bom-handling-to-gherkin branch 2 times, most recently from d0e3440 to f783b18 Compare October 11, 2022 13:52
@codecov
Copy link

codecov bot commented Oct 11, 2022

Codecov Report

Merging #2624 (c71b10d) into main (43dec6e) will decrease coverage by 0.07%.
The diff coverage is 81.81%.

❗ Current head c71b10d differs from pull request most recent head e9cd3fa. Consider uploading reports for the commit e9cd3fa to get more accurate results

@@             Coverage Diff              @@
##               main    #2624      +/-   ##
============================================
- Coverage     84.87%   84.79%   -0.08%     
+ Complexity     2699     2693       -6     
============================================
  Files           320      322       +2     
  Lines          9547     9544       -3     
  Branches        913      908       -5     
============================================
- Hits           8103     8093      -10     
- Misses         1111     1120       +9     
+ Partials        333      331       -2     
Impacted Files Coverage Δ
...n/java/io/cucumber/core/feature/FeatureParser.java 87.50% <50.00%> (-12.50%) ⬇️
...gherkin/messages/GherkinMessagesFeatureParser.java 94.11% <83.33%> (-3.39%) ⬇️
...n/java/io/cucumber/core/gherkin/FeatureParser.java 100.00% <100.00%> (ø)
...c/main/java/io/cucumber/core/gherkin/StepType.java 0.00% <0.00%> (ø)
.../cucumber/core/gherkin/FeatureParserException.java 0.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@mpkorstanje mpkorstanje force-pushed the delegate-encoding-and-bom-handling-to-gherkin branch from f783b18 to c71b10d Compare October 12, 2022 13:43
@mpkorstanje mpkorstanje marked this pull request as ready for review October 12, 2022 13:52
Previously before handing over a feature file to the Gherkin parser, Cucumber
would remove any Byte Order Markers (BOM) and determine the encoding of the
feature file based on the `#encoding: <encoding>` comment.

With cucumber/common#2018 this can now be handled by
Gherkin. Removing it simplifies Cucumber a little. Unfortunately the
`FeatureParser` doesn't take a `InputStream` as an argument. So we need to add
a default interface to avoid breaking semver.
@mpkorstanje mpkorstanje force-pushed the delegate-encoding-and-bom-handling-to-gherkin branch from c71b10d to e9cd3fa Compare October 12, 2022 13:54
@mpkorstanje mpkorstanje merged commit e1495b8 into main Oct 12, 2022
@mpkorstanje mpkorstanje deleted the delegate-encoding-and-bom-handling-to-gherkin branch October 12, 2022 13:55
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

1 participant