Skip to content

Fix trimming of padding in table cells#405

Merged
acoulton merged 1 commit intoBehat:masterfrom
stof:fix_cell_trimming
Dec 5, 2025
Merged

Fix trimming of padding in table cells#405
acoulton merged 1 commit intoBehat:masterfrom
stof:fix_cell_trimming

Conversation

@stof
Copy link
Member

@stof stof commented Dec 4, 2025

This trims Unicode spaces (as done upstream) without removing newlines produces by escape sequences.

Closes #395

This does not change the legacy mode (the parseTableCell private method is not used by the legacy parsing mode).

@codecov
Copy link

codecov bot commented Dec 4, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.67%. Comparing base (dc8d60f) to head (c5d218f).
⚠️ Report is 2 commits behind head on master.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff            @@
##             master     #405   +/-   ##
=========================================
  Coverage     95.67%   95.67%           
  Complexity      682      682           
=========================================
  Files            45       45           
  Lines          2010     2012    +2     
=========================================
+ Hits           1923     1925    +2     
  Misses           87       87           
Flag Coverage Δ
php8.1 95.67% <100.00%> (+<0.01%) ⬆️
php8.1--with=symfony/yaml:^5.4 95.67% <100.00%> (+<0.01%) ⬆️
php8.1--with=symfony/yaml:^6.4 95.67% <100.00%> (+<0.01%) ⬆️
php8.2 95.67% <100.00%> (+<0.01%) ⬆️
php8.3 95.67% <100.00%> (+<0.01%) ⬆️
php8.4 95.67% <100.00%> (+<0.01%) ⬆️
php8.5 95.67% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

src/Lexer.php Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we rename the shouldSupportNewlineEscapeSequenceInTableCell flag, since that's now covering two slightly different variances?

Copy link
Member Author

Choose a reason for hiding this comment

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

shouldSupportNewlineEscapeSequenceAndUnicodePaddingInTableCell would be the name covering all differences with the legacy mode, but this is crazy long.

Copy link
Contributor

Choose a reason for hiding this comment

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

It is indeed. Maybe we go more vague - shouldUseNewTableCellParsing() or something? It's not a huge deal, the methods are only really for our benefit, but it might be easier to understand in future with a name that's not explicit than one that is explicit but incomplete?

Copy link
Member Author

Choose a reason for hiding this comment

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

I renamed the method with your suggestion.

@stof stof force-pushed the fix_cell_trimming branch from 81bb295 to c8399ab Compare December 5, 2025 14:27
This trims Unicode spaces (as done upstream) without removing newlines
produces by escape sequences.
@stof stof force-pushed the fix_cell_trimming branch from c8399ab to c5d218f Compare December 5, 2025 16:09
Copy link
Contributor

@acoulton acoulton left a comment

Choose a reason for hiding this comment

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

Thanks very much @stof

@acoulton acoulton merged commit 64e7bbb into Behat:master Dec 5, 2025
11 checks passed
acoulton added a commit to acoulton/Gherkin that referenced this pull request Dec 5, 2025
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.

Table cell padding is not trimmed as aggressively as cucumber

2 participants