Pretty comments

[kieran-ryan]

🤔 What's changed?

• Trim trailing whitespace from comments
• Introduce a space between the hashtag of comments and text if one does not exist (#content -> # content)

⚡️ What's your motivation?

• Consistent formatting
  • Removes redundant trailing whitespace, matching behaviour of all lines
  • Aligns with formatting of language key comment #language: <key> -> # language: <key>
  • Aligns with formatting configuration of the VSCode extension for line comments - which introduces a space after hashtags by default
  • Similar approach taken with code formatting tools such as black
  • Further decreases an element of misalignment across authored specifications i.e. comments will be visually more consistent

🏷️ What kind of change is this?

• :zap: New feature (non-breaking change which adds new behaviour)

♻️ Anything particular you want feedback on?

• How are comments handled in gherkin markdown?
• Should be considered Changed in changelog?
• Comments are not currently indented in line with their scope. Would be great to resolve, though hard to discern whether there's a consistent indentation/formatting that can be applied e.g. if I comment out an entire scenario, how should its indentation be handled. Perhaps there are partial occurrences we can categorically format.

📋 Checklist:

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

---

This text was originally generated from a template, then edited by hand. You can modify the template here.

[mpkorstanje]

Didn't look at the code (yet).

How are comments handled in gherkin markdown?

No clue unfortunately. Markdown was an experimental feature. I don't think it is used much. I would leave it out of scope.

Should be considered Changed in changelog?

Good question. If the API didn't change, it would be an added feature going by Semver.

Comments are not currently indented in line with their scope. Would be great to resolve, though hard to discern whether there's a consistent indentation/formatting that can be applied e.g. if I comment out an entire scenario, how should its indentation be handled.

I think the following would be a reasonable guideline:

Comments should be indented at the level of the next named element in the AST. If there is no next element, then no indentation. This keeps the left margin consistently clear, so visually everything looks nice.

Any space extra whitespace after the comment symbol should be preserved. So we don't destroy any ascii art.

[mpkorstanje]

It might also be good to check how comments around tags are handled. I think this is one of the few places where comments can be trailing.

[kieran-ryan]

It might also be good to check how comments around tags are handled. I think this is one of the few places where comments can be trailing.

Interesting! Hadn't considered. Currently trailing comments on tags are stripped - they are 'lost' by the parser, before reaching the formatter. Is this something we should be looking to preserve? We have Comments are only permitted at the start of a new line in the gherkin reference - though trailing comments on tags work with test runners - suppose not really relevant for them.

I think the following would be a reasonable guideline:

Comments should be indented at the level of the next named element in the AST. If there is no next element, then no indentation. This keeps the left margin consistently clear, so visually everything looks nice.

Awesome, sounds good - will see if we can apply this in all or some cases. Initial attempt looked promising, though not yet sure how to deal with the following case where commenting a scenario would be scoped to the previous scenario/background.

Will take a look at the Java implementation once we've defined how this should be handled.

[mpkorstanje]

Initial attempt looked promising, though not yet sure how to deal with the following case where commenting a scenario would be scoped to the previous scenario/background.

Mmh. I'm not sure I understand. What is the diff comparing?

[mpkorstanje]

For the comments on tags, looks like it's still an open issue https://github.com/cucumber/gherkin/issues/43. Never got around to fixing it.

[kieran-ryan]

Initial attempt looked promising, though not yet sure how to deal with the following case where commenting a scenario would be scoped to the previous scenario/background.

Mmh. I'm not sure I understand. What is the diff comparing?

With the following gherkin:

Feature: Levers

  Background:
    Given a lever long enough
    And a fulcrum on which to place it

  Scenario:
    When I apply force to the lever
    Then I shall move the world

If I was to comment-out the scenario (perhaps I temporarily wish to avoid running it), the commented scenario would be treated as trailing comments of the background (which a user could otherwise have). This occurs for comments as the formatter inherits the indentation from the previous header and adds 1 to it - which is the 'Background' header in this case - thus following the same formatting as its steps.

Feature: Levers

  Background:
    Given a lever long enough
    And a fulcrum on which to place it
    #  Scenario:
    #    When I apply force to the lever
    #    Then I shall move the world

Possibly falls into the territory of #40?

[mpkorstanje]

I'll come back with a reply. This isn't trivial.

[mpkorstanje]

There are, that I can think of, three uses for comments:

1. To explain and clarify what a certain section does or means
2. To temporarily disable a section
3. For use in meta-programming, pragmas, and other sorts of directives.

But aside from our own language and encoding pragmas, I don't want to give any consideration to the third. We may as well not consider it Gherkin.

So the first and second have different irreconcilable requirements. For example:

Feature: minimal
  Scenario: example
    Given a condition
    # When an action
    # Then an effect
  # This is a scenario comment
  Scenario: other example
    Given a condition
    When an action
    Then an effect

The commented out steps can either be aligned to the next named element. Or the scenario comment can be aligned with the previous named element. And neither is quite right. The "correct" action depends on the context of the comment. Which we can't access with the parser. So fundamentally anything we pick will be imperfect.

So given that we must make a trade off, I think it would be best to support comments as an explanation rather than comments as a means to disable parts of the scenario.

If I was to comment-out the scenario (perhaps I temporarily wish to avoid running it), the commented scenario would be treated as trailing comments of the background (which a user could otherwise have). This occurs for comments as the formatter inherits the indentation from the previous header and adds 1 to it - which is the 'Background' header in this case - thus following the same formatting as its steps.

Okay. Gotcha. So this should change from using the indentation of the previous named element, to using the indentation of the next element. Or 0 if there is no next element. So your example should look like this:

Feature: Levers
  Background:
    Given a lever long enough
    And a fulcrum on which to place it
#  Scenario:
#    When I apply force to the lever
#    Then I shall move the world

But feel free to differ if you find a case where this doesn't work.