common
common copied to clipboard
Description retains original indentation.
👓 What did you see?
In reviewing https://github.com/cucumber/cucumber-react/pull/97 it was observed that the description text retains the whitespace indentation from the original feature file.
✅ What did you expect to see?
Description text should have indentation whitespace removed.
Unlike DocString, where the indentation has to be consistent or it's an error, there's no constraint on indentation in a description
Scenario: Whatever
This
Is
fine
I
believe
Given foo
When bar
Then bam
It's not 100% clear what the rule would be for left-trimming the description lines - assume the least-indented one sets a baseline?
IMO it should ideally error if the subsequent lines don't match the first one, and use that first line as the indent margin for the rest.
e.g.
Feature: Good indentation
This
is
fine
=>
This
is
fine
But
Feature: Bad indentation
This
is not OK
=>
Parse error at line 3: Indentation of description is inconsistent.
IMO both of your representations are valid. I think one of the "bigger" issues at the moment is newlines are being absorbed.
So something like
As a foo
I want to bar
So I can baz
Should look "something like"
As a foo\nI want to bar\nSo I can baz
Currently it looks like
As a foo I want to bar So I can baz
@mattwynne
IMO it should ideally error if the subsequent lines don't match the first one, and use that first line as the indent margin for the rest.
I think we talked IRL but here's an example that I think we'd want to support:
Feature: Password hashing
* In 1999 we had a data breach that cost us $4bn
* In 2018 we covered one up by the skin of our teeth
As a consequence we ensure we do not store passwords in plaintext
my preference would be a 'trim the smallest indent' also which would result in
* In 1999 we had a data breach that cost us $4bn
* In 2018 we covered one up by the skin of our teeth
As a consequence we ensure we do not store passwords in plaintext
And I'd suggest it'd be a good idea to store indent: 2
on the relevant Message
(I also want to be able to do this)
Feature: Da Boss
______ _ ___ ___
/ ___(_)__ ________ ____ (_)__ / _ \___ _ / _ )___ ___ ___
/ /__/ / _ `/ __/ _ `/ _ \ / (_-< / // / _ `/ / _ / _ \(_-<(_-<
\___/_/\_,_/_/ \_,_/_//_/ /_/___/ /____/\_,_/ /____/\___/___/___/
Note from meeting: Also consider how markdown styling affects sanitization (If it does).
EDIT: markdown requires 2 new lines to actually show a new line visually.
So, regarding the indentation, and indentation only, at the community meeting, we suggest to retain the indentation using HEREDOC rule - we pick the less-indented line and remove that indentation from all the lines - and eventually keeping in the messages the indentation level that has been taken down
Any objection into doing that way?
If you have some objection, could you explain those and maybe suggest something else?
IMO both of your representations are valid. I think one of the "bigger" issues at the moment is newlines are being absorbed.
So something like
As a foo I want to bar So I can baz
Should look "something like"
As a foo\nI want to bar\nSo I can baz
Currently it looks like
As a foo I want to bar So I can baz
@luke-hill I think this is a separate issue: it's purely to do with the choice that we've made in the HTML formatter to use Markdown to render the description
field from the Gherkin. This is expected behaviour in Markdown, but I agree it can be surprising.
In order to keep this discussion focussed on the whitespace indentation, let's move that discussion about newlines over to https://github.com/cucumber/react-components/issues/225
To throw out some other options, the Behat/Gherkin parser's rule is:
remove whitespace up to 2 chars more than the indent of the preceding keyword
Which I never noticed until today
That idea of 'looking at the keyword indent' may be a useful one?
Although I am a great fusspot about lining things up in source code files, I think being opinionated about 2-space indentation in .feature
files is a lost battle. .Net developers, for example, always insist on using tabs. Ugly as anything IMO, but it happens enough in real life that I think we could not start imposing any kind of standard over it.