Skip to content

Lexer options don't get passed to lexer #92

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.

Sign up for GitHub

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

Jump to bottom
Merged
markets merged 5 commits into from
Jul 23, 2025
Merged

Lexer options don't get passed to lexer #92

markets merged 5 commits into from
Jul 23, 2025

Conversation

Copy link
Contributor

Copilot AI commented Jul 22, 2025 •
edited
Loading

  • Explore repository structure and understand the issue
  • Set up development environment and run existing tests (all 15 scenarios passing)
  • Analyze the problem: lexer_options are not being passed to Rouge::Lexer.find_fancy()
  • Verify Rouge API supports lexer_options as third parameter to find_fancy()
  • Create focused test case for lexer_options functionality
  • Make minimal code change to pass lexer_options to find_fancy()
  • Verify tests pass and lexer options work correctly (all 16 scenarios now passing)
  • Manual testing confirms the fix works as expected

Changes Made:

  1. Fixed the core issue in /lib/middleman-syntax/highlighter.rb:

    • Extract lexer_options from highlighter_options before calling find_fancy()
    • Pass lexer_options as the third parameter to Rouge::Lexer.find_fancy()
    • Remove lexer_options parameter from lexer.lex() call (which was causing deprecation warnings)

  2. Added comprehensive test in /features/lexer_options.feature:

    • Tests that custom prompt patterns work correctly with console lexer
    • Verifies myshell>>> is parsed as single prompt instead of myshell> + >> operator

  3. Updated step definitions in /features/support/step_definitions.rb:

    • Added support for new Cucumber step format

  4. Updated .gitignore:

    • Added vendor/bundle to exclude bundled dependencies

Technical Details:
The issue was that lexer_options were being extracted from the configuration but only passed to lexer.lex(), which doesn't accept options (causing deprecation warnings). The Rouge API requires lexer options to be passed to Rouge::Lexer.find_fancy() as the third parameter to properly configure the lexer instance.

Test Results:

  • All existing 15 tests continue to pass
  • New lexer_options test passes, confirming custom prompts work correctly
  • No deprecation warnings from Rouge
  • Minimal, surgical code change that doesn't break existing functionality

Fixes #72.

💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

@markets
Copy link
Member

markets commented Jul 22, 2025

Please remove the files under ./vendor/*

Copilot AI requested a review from markets July 22, 2025 13:14
Copy link
Contributor Author

Copilot AI commented Jul 22, 2025

Please remove the files under ./vendor/*

Removed all files under ./vendor/* and .bundle/* from git tracking in commit 5a4fcb6. Also updated .gitignore to prevent these dependency files from being committed again in the future.

@markets markets marked this pull request as ready for review July 23, 2025 08:26
@markets markets changed the title [WIP] Lexer options don't get passed to lexer Lexer options don't get passed to lexer Jul 23, 2025
@markets markets merged commit e150dba into master Jul 23, 2025
6 checks passed
@markets markets deleted the copilot/fix-72 branch July 23, 2025 08:31
huwd added a commit to alphagov/tech-docs-gem that referenced this pull request Jul 31, 2025
@huwd
Fix syntax highlighting by locking middleman-syntax to 3.5.0
341356e 
Starting around July 29th 2025 tests started failing between ruby 3.1,
3.2 and 3.3 versions.

The two failures were:

```
1) GovukTechDocs::TechDocsHTMLRenderer#render a code block with syntax highlighting sets tab index to 0
     Failure/Error: fragment = Nokogiri::HTML::DocumentFragment.parse(super)

     NoMethodError:
       undefined method `each' for nil:NilClass

             opts.each { |k, v| @options[k.to_s] = v }
                 ^^^^^
     # ./vendor/bundle/ruby/3.1.0/gems/rouge-3.30.0/lib/rouge/lexer.rb:325:in `initialize'
     # ./vendor/bundle/ruby/3.1.0/gems/rouge-3.30.0/lib/rouge/lexer.rb:97:in `new'
     # ./vendor/bundle/ruby/3.1.0/gems/rouge-3.30.0/lib/rouge/lexer.rb:97:in `find_fancy'
     # ./vendor/bundle/ruby/3.1.0/gems/middleman-syntax-3.6.0/lib/middleman-syntax/highlighter.rb:13:in `highlight'
     # ./vendor/bundle/ruby/3.1.0/gems/middleman-syntax-3.6.0/lib/middleman-syntax/redcarpet_code_renderer.rb:10:in `block_code'
     # ./lib/govuk_tech_docs/tech_docs_html_renderer.rb:90:in `block_code'
     # ./spec/govuk_tech_docs/tech_docs_html_renderer_spec.rb:49:in `render'
     # ./spec/govuk_tech_docs/tech_docs_html_renderer_spec.rb:49:in `block (3 levels) in <top (required)>'
     # ./spec/govuk_tech_docs/tech_docs_html_renderer_spec.rb:85:in `block (4 levels) in <top (required)>'

   2) GovukTechDocs::TechDocsHTMLRenderer#render a code block with syntax highlighting renders the code with syntax highlighting
     Failure/Error: fragment = Nokogiri::HTML::DocumentFragment.parse(super)

     NoMethodError:
       undefined method `each' for nil:NilClass

             opts.each { |k, v| @options[k.to_s] = v }
                 ^^^^^
     # ./vendor/bundle/ruby/3.1.0/gems/rouge-3.30.0/lib/rouge/lexer.rb:325:in `initialize'
     # ./vendor/bundle/ruby/3.1.0/gems/rouge-3.30.0/lib/rouge/lexer.rb:97:in `new'
     # ./vendor/bundle/ruby/3.1.0/gems/rouge-3.30.0/lib/rouge/lexer.rb:97:in `find_fancy'
     # ./vendor/bundle/ruby/3.1.0/gems/middleman-syntax-3.6.0/lib/middleman-syntax/highlighter.rb:13:in `highlight'
     # ./vendor/bundle/ruby/3.1.0/gems/middleman-syntax-3.6.0/lib/middleman-syntax/redcarpet_code_renderer.rb:10:in `block_code'
     # ./lib/govuk_tech_docs/tech_docs_html_renderer.rb:90:in `block_code'
     # ./spec/govuk_tech_docs/tech_docs_html_renderer_spec.rb:49:in `render'
     # ./spec/govuk_tech_docs/tech_docs_html_renderer_spec.rb:49:in `block (3 levels) in <top (required)>'
     # ./spec/govuk_tech_docs/tech_docs_html_renderer_spec.rb:89:in `block (4 levels) in <top (required)>'
```

This seemed odd because we hadn't done anything to change this area in
the intervening time.

After some investigations of if it could be related to a ruby change
(and a slight adventure where a very old Gemfile.lock meant we couldn't
reproduce this locally :facepalm:), it was pointed out that
middleman-syntax had an update on Jule 23rd 2025 to v3.6.0:
https://github.com/middleman/middleman-syntax/releases/tag/v3.6.0

This seems to be specifically about lexer, which draws attention:
middleman/middleman-syntax#92

So what do I think is going on here?
Well, Lexer seems to be throwing the error, apparently because the `opts`
parameter which should be a hash, is being passed as nil.

We can see why that would cause problems when calling the enumerator
here:
https://github.com/rouge-ruby/rouge/blob/3b461b1ffe5fc6416373df8c3c35da83a283606d/lib/rouge/lexer.rb#L323

This gets called from lexer's find_fancy class, both nothing has changed
there nor is there anything that I can see that mutates opts to nil in
either `lookup_fancy` or `find_fancy`:
https://github.com/rouge-ruby/rouge/blob/3b461b1ffe5fc6416373df8c3c35da83a283606d/lib/rouge/lexer.rb#L46
https://github.com/rouge-ruby/rouge/blob/3b461b1ffe5fc6416373df8c3c35da83a283606d/lib/rouge/lexer.rb#L94

So we're back in our trace into middleman-syntax, here's where
find_fancy gets called:
https://github.com/middleman/middleman-syntax/blob/d5042d6a583494aad3ceb0517685e945aa093d9b/lib/middleman-syntax/highlighter.rb#L13
Line 13 must make it through lexer without error before
Rogue::Lexer::PLainText can be offered as a fallback. And at this point
in a debugger i'm seeing that `lexer_options` has become nil!

So I think this line is sus:
https://github.com/middleman/middleman-syntax/blob/d5042d6a583494aad3ceb0517685e945aa093d9b/lib/middleman-syntax/highlighter.rb#L11
on line 11, the helper is attempting to delete lexer_options but when
there is no key of `lexer_options` then it's returning nil, which is
expected behaviour from delete.
https://ruby-doc.org/3.4.1/Hash.html#method-i-delete

You can test this with:
```
lexer_options = highlighter_options.delete(:lexer_options)
lexer_options
```

When I think what they want is:

```
lexer_options = {}.delete_if {|k,v| k == :lexer_options}
```

I'll open an issue over there, but for the time being for syntax
highlighting to continue to work we want to stay locked to v3.5.0.

This commit can be disregarded and the lock removed once we think this
problem has been resolved and the existing test suite passes.
@huwd huwd mentioned this pull request Jul 31, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@markets markets markets approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@markets markets

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Lexer options don't get passed to lexer

2 participants

@markets