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

gherkin: Leading and trailing "\n" are trimmed in DataTable cells #290

Closed
ncsibra opened this issue Oct 20, 2017 · 9 comments · Fixed by #891
Closed

gherkin: Leading and trailing "\n" are trimmed in DataTable cells #290

ncsibra opened this issue Oct 20, 2017 · 9 comments · Fixed by #891
Labels
🐛 bug Defect / Bug library: gherkin 🧷 pinned Tells Stalebot not to close this issue

Comments

@ncsibra
Copy link

ncsibra commented Oct 20, 2017

After upgrading from cucumber-jvm 1.2.4 to 2.0.1, some of our tests failed.
We have steps like this:

And the text should be
|something\n|

The important thing here is the "\n" at the end.
With the older version, the "\n" was retained, but the new one trims it.
I tracked down the issue and I think it's caused by an old change here: https://github.com/cucumber/gherkin-java/blob/v4.1.3/src/main/java/gherkin/GherkinLine.java#L67
It's parsing the line from the feature file and this part of the code handle the new line character perfectly: https://github.com/cucumber/gherkin-java/blob/v4.1.3/src/main/java/gherkin/GherkinLine.java#L90
But at the end of the line(at the "|" char) it trims the string: https://github.com/cucumber/gherkin-java/blob/v4.1.3/src/main/java/gherkin/GherkinLine.java#L86
It doesn't make too much sense to me, because the original string as the variable name suggests(trimmedLineText) is already trimmed, so I think everything inside the "|" characters should remain the same, with all of the white space.

@aslakhellesoy
Copy link
Contributor

I thought we had always been trimming leading and trailing whitespace - I must be wrong.

Maybe related: cucumber-attic/gherkin#114

@ncsibra
Copy link
Author

ncsibra commented Oct 20, 2017

Before this commit, it does not trimmed the whitespaces, if I understand correctly:
cucumber/gherkin-java@9b314c3
The issue you linked not related to this, because that handle the cases when the \\ not followed by an "n" character.

If you can give me a solution how to keep the new line at the end, that's fine with me. I already tried \\s and \\\s, but none of them worked.
The hackish solution is to add it in the java step definition, before comparing the actual and the expected result, but the intention won't be as clear as with the "\n" in the feature file.

@brasmusson
Copy link
Contributor

@ncsibra Cucumber-JVM v1.2.4 uses Gherkin v2.12.2, that is https://github.com/cucumber-attic/gherkin2 and not https://github.com/cucumber/gherkin-java, so tracking down this issue to somewhere in https://github.com/cucumber/gherkin-java is not really possible.

@ncsibra
Copy link
Author

ncsibra commented Oct 20, 2017

You are right, I missed that, sorry, but tracking down the issue was for the current version, which is using https://github.com/cucumber/gherkin-java.
So my question is still stands, how can I solve that? Is it a feature or a bug? Is there a way to use special characters in data table in the feature file?

@brasmusson
Copy link
Contributor

I thought we had always been trimming leading and trailing whitespace - I must be wrong.

No, we have always been trimming leading and trailing whitespace in table cells. The caveat here is that the leading and trailing "\n" is not whitespace until they are converted to a newline. Conceptually Gherkin v2 did trim the whitespace before converting "\n" to newline which means the the leading and trailing newlines are retained. Newer Gherkin versions trim the whitespace after converting "\n" to newline, so also the leading and trailing newlines (which started out as "\n") are trimmed.

@brasmusson brasmusson changed the title [gherkin-java] White space characters are trimmed in DataTable gherkin: Leading and trailing "\n" are trimmed in DataTable cells Oct 23, 2017
@ncsibra
Copy link
Author

ncsibra commented Oct 23, 2017

Interesting, so this the expected behavior?

@brasmusson
Copy link
Contributor

I think it is a regression that should be fixed.

@ncsibra
Copy link
Author

ncsibra commented Oct 26, 2017

Thanks for your help!

@stale
Copy link

stale bot commented Dec 25, 2017

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 stale bot added the ⌛ stale Will soon be closed by stalebot unless there is activity label Dec 25, 2017
@brasmusson brasmusson added the 🧷 pinned Tells Stalebot not to close this issue label Dec 26, 2017
@stale stale bot removed the ⌛ stale Will soon be closed by stalebot unless there is activity label Dec 26, 2017
@aslakhellesoy aslakhellesoy added the 🐛 bug Defect / Bug label Jun 27, 2019
@mpkorstanje mpkorstanje added this to To do in Ongoing maintenance via automation Oct 20, 2019
Ongoing maintenance automation moved this from To do to Done Feb 7, 2020
mpkorstanje added a commit that referenced this issue Feb 7, 2020
mpkorstanje added a commit that referenced this issue May 22, 2020
* [Gherkin / ruby] Do not trim spaces inside a table cell content (see #290)

* [Gherkin / Javascript] Add test to ensure #290 is not present (and will not be)

* [Gherkin / Java] Add test to ensure #290 is not present (and will not be)

* [gherkin / javascript] Split tests to make them more explicit.

Also wrap tests about getTableCells in its own 'describe'

* [gherkin / ruby] Split tests to make them more explicit

* Fix merge

* Fix merge

* Update changelog

* Update changelog

* Update changelog

Co-authored-by: M.P. Korstanje <rien.korstanje@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Defect / Bug library: gherkin 🧷 pinned Tells Stalebot not to close this issue
Projects
No open projects
3 participants