language-service icon indicating copy to clipboard operation
language-service copied to clipboard

Syntax highlighting for comments

Open kieran-ryan opened this issue 1 year ago • 1 comments

🤔 What's changed?

  • Syntax highlighting for comments (#)

The Language Server Protocol semantic tokens specification accepts non-negative integers for relative text positions by default. As the gherkin-util's document walker processes table rows before comments, comments can have negative relative positions and break syntax highlighting of some tokens. As such, post-processing has been introduced to correct relative positions to always be positive.

Before

Screenshot 2024-05-19 094023

After

Screenshot 2024-05-19 094111

⚡️ What's your motivation?

  • Match other common language implementations of providing syntax highlighting for comments

  • Match common syntax highlighting for gherkin (such as the Cucumber docs - see below)

    image

🏷️ What kind of change is this?

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

📋 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.

kieran-ryan avatar May 19 '24 16:05 kieran-ryan

Nice feature, but I think you have the wrong approach with sorting the data.

As part of the walk step, you already have the absolute line number in makeLocationToken, which then gets converted to relative, to be pushed onto the array. Now you convert it back to absolute again, then back to relative.

It's like hokeypokey, back and forth. Maybe it's fine, but I worry that the performance can be better.

walkGherkinDocument already supports a custom accumulation type - it doesn't have to be number[]. I think it would be clearer to build a different kind of accumulation, then finalize that into the number[] at the end

I'll try to come up with something to show what I mean

Edit: PR for refactor: https://github.com/cucumber/language-service/pull/220

michaelboyles avatar Jun 19 '24 12:06 michaelboyles

Superseded by #245

luke-hill avatar May 15 '25 11:05 luke-hill