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

Correctly highlight placeholders in Gherkin syntax #952

Merged
merged 5 commits into from
Jul 18, 2019

Conversation

jamis
Copy link
Contributor

@jamis jamis commented Jun 27, 2018

While placeholders (variables surrounded by angle brackets, like <this>) are mostly highlighted correctly, there are two cases that are not:

  1. When the angle bracket is immediately preceded by text, like something (<foo>), and
  2. When the angle bracket is the first non-whitespace text following a step keyword, like When <foo>.

This PR addresses these two cases. Some things to note about this PR.

  1. The Text rule in the step state now stops as soon as it encounters either whitespace, OR a left angle bracket. This fixes case number 1.
  2. Rouge::Lexers::Gherkin.step_regex has been modified so that it does not added a \b to the end of the regex, if the pattern ends with a whitespace character. In fact, I wonder if there is a disconnect between the step_regex function and the keyword configs, since the comment mentions patterns that end with a <, but there are no patterns in the config that do so. This change, though, fixes case number 2.

@pyrmont pyrmont force-pushed the placeholder-following-text branch from 425c7ee to 83ae4b1 Compare July 18, 2019 20:25
@pyrmont pyrmont merged commit 0280ed7 into rouge-ruby:master Jul 18, 2019
@pyrmont
Copy link
Contributor

pyrmont commented Jul 18, 2019

@jamis Two apologies:

  1. I'm sorry it's taken so long to get around to this PR. Rouge had been lying fallow for a while and while I came onto the project in May, I've been working my way through the PR backlog in reverse chronological order. I'm afraid that meant it took a while to get to this one.

  2. I'm sorry for force pushing the changes onto your branch. Since you submitted the PR, we've moved to using regex literals that avoid the ambiguity warning Ruby will generate when run with verbose warnings. This meant changing the form of expressions from rule /.../ to rule %r/.../ in a couple of cases. Since I was already making some changes, I also made two other small adjustments.

We're on a two-week cadence for Rouge updates as we clear things up. It's been so long it may not be relevant to you any more but this fix will land in version 3.7.0 of the gem next Tuesday. Thanks for taking the time to submit the fix. Although very belated, it is nevertheless very much appreciated!

@pyrmont pyrmont removed the needs-review The PR needs to be reviewed label Jul 18, 2019
@ghost
Copy link

ghost commented Jul 18, 2019

Thanks for the follow-up! I appreciate your hard work, and your continued efforts on Rouge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants