Skip to content

TableCell with unescaped backslash is escaped in the AST value #2115

Closed

Description

@jdeniau

👓 What did you see?

While creating a prettier plugin for gherkin file, I found out that the TableCell with unescaped backslash is escaped in the AST.

Example from escaped_pipes.feature :

| \o\no\ |

is reprensented in the AST like this:

{
  "location": { "column": 22, "line": 10 },
  "value": "\\o\no\\"
}

So it's impossible to reconstitute the original string.

✅ What did you expect to see?

An unescaped backslash in the node value for unescaped backslash

📦 Which tool/library version are you using?

Javascript librairies :

  • @cucumber/gherkin: 24.1.0"
  • @cucumber/messages: 19.1.4

🔬 How could we reproduce it?

Probably run the cucumber test stack.

📚 Any additional context?

If this is intended, then how can we regenerate the original node value ?
According to https://cucumber.io/docs/gherkin/reference/#table-cell-escaping a backslash can not be alone (while not really explicitly said).

If the cucumber test data allow parsing this un-escaped backslash, is it a bug or a feature ? Some tool allow this comportment, like PHP library behat.

Thanks

Activity

  1. mpkorstanje commented on Nov 3, 2022

    @mpkorstanje
    Member

    Unfortunately this looks like a feature to me:

    if (escape) {
    switch (c) {
    case 'n':
    cellBuilder.append('\n');
    break;
    case '\\':
    cellBuilder.append('\\');
    break;
    case '|':
    cellBuilder.append('|');
    break;
    default:
    // Invalid escape. We'll just ignore it.
    cellBuilder.append("\\");
    cellBuilder.appendCodePoint(c);
    break;
    }

    Though since the escape is invalid, I think you might be okay mapping every \ to \\. I don't immediately see any clashes.

    So reformatting:

    | \o\no\ |
    

    Should yield

    | \\o\no\\ |
    
  2. jdeniau commented on Nov 3, 2022

    @jdeniau
    Author

    OK thank you for your fast response.

    One think I find weird then (but that possibly because I do not know the implementation details here) is the fact that there is a difference between \| and the rest : \| are converted to |. But after reading my phrase several times, it seems logical as you want the cell value here.

    About mapping \ to \\, that's what I did, but I think that Behat does not escape the same way as cucumber handles it, so changing from \ to \\ in gherkin files does crash on the Behat side (I might open an issue about this when I do investigate a little further on their side).

    For the moment I'm still stucked on the prettier plugin, and the hack I found is not pretty 😰

    https://github.com/mapado/prettier-plugin-gherkin/blob/main/src/GherkinAST/index.ts#L46-L59

    https://github.com/mapado/prettier-plugin-gherkin#escapebackslashes

    But it does work for the moment, so that's fine 👍

    As an evolution, what do you think about adding the initial data in the AST ? or at least the a endColumn information in the Location of every nodes ? This way we might get the original data when we want (I can not actually do that because the size of the TableCell is lost as the value is transformed).

    Thanks again

  3. mpkorstanje commented on Nov 3, 2022

    @mpkorstanje
    Member

    About mapping \ to \, that's what I did, but I think that Behat does not escape the same way as cucumber handles it, so changing from \ to \ in gherkin files does crash on the Behat side (I might open an issue about this when I do investigate a little further on their side).

    I think that would be the way forward. I know that @ciaranmcnulty contributes to both Cucumber Gherkin and Behat Gherkin. Perhaps he can help out.

  4. ciaranmcnulty commented on Nov 4, 2022

    @ciaranmcnulty
    Contributor

    Behat currently uses its own parser which isn't 100% compatible with the Cucumber one. We do attempt to test against the same testdata but exclude some of the example features . The data table examples don't seem to be in that exclusion list, which indicates the Behat parser produces the same result as Cucumber for those testdata.

    Therefore a good step would be to add an example with this problematic case to the testdata in this repo and canonicalise the correct parsing. This would help us ensure that other implementations are parsing the source in the same way too.

    The good news is that Behat should use the new parser at some point which will mean the parsing will be much more compatible, and maintained in this organisation

  5. jdeniau commented on Nov 7, 2022

    @jdeniau
    Author

    @ciaranmcnulty thank you for your response.
    The file I did work with is escaped_pipes.feature that is in your list. I did not remember all other files.

    I did add a file with lots of \ in my test stack and the actual output of the prettier plugin is visible here (but with backslashes escaped, which is nearly unreadable 😄 ).

    I think the fix for the prettier plugin should work for now. I will watch carefully the fact that behat will use the cucumber parser and test it once released.

    I'm closing this as the option in the prettier plugin should handle most cases.

    Thanks @mpkorstanje too for your time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions