Don't create tokens by hand by Earlopain · Pull Request #1696 · ruby/rdoc

Earlopain · 2026-05-06T07:02:08Z

There is some awkward code that dances around the fact that the tokens for a method actually contain a 3 extra tokens that don't exist in the source code.

Now RipperStateLex is only referenced to actually parse, rest is kept internal

Earlopain · 2026-05-06T07:04:56Z


  def test_markup_code
-    tokens = [
-      { :line_no => 0, :char_no => 0, :kind => :on_const, :text => 'CONSTANT' },


This testcase didn't make much sense since only methods have their source code shown. I simply removed it.

matzbot · 2026-05-06T07:06:15Z

🚀 Preview deployment available at: https://0206fae0.rdoc-6cd.pages.dev (commit: 01677d0)

Copilot

Pull request overview

This PR removes “synthetic” tokens (file/line comment, newline, indent) from Ruby parser token streams and moves location/line-number decoration into RDoc::MethodAttr#markup_code, with tests updated to build token streams via RipperStateLex instead of hand-crafted hashes.

Changes:

Stop prepending non-source tokens in RDoc::Parser::Ruby#visible_tokens_from_location and related paths.
Rework RDoc::MethodAttr#markup_code to dedent, optionally add line numbers, and prepend a generated location comment.
Update any_method_test to use RipperStateLex.parse and assert the new HTML output format.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.

File	Description
`test/rdoc/code_object/any_method_test.rb`	Updates expectations and replaces hand-built token hashes with `RipperStateLex.parse` output.
`lib/rdoc/parser/ruby.rb`	Removes construction of extra non-source tokens from token streams; returns only sliced source tokens.
`lib/rdoc/generator/markup.rb`	Moves location comment and line-number generation into `markup_code`, and adjusts dedent logic.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Earlopain · 2026-05-06T08:55:04Z

    meth.start_collecting_tokens(:ruby)
    node = @line_nodes[line_no]
-    tokens = node ? visible_tokens_from_location(node.location) : [file_line_comment_token(start_line)]
+    tokens = node ? visible_tokens_from_location(node.location) : []
    tokens.each { |token| meth.token_stream << token }



Seems correct. Previously they were not empty because it contained the comment

Earlopain · 2026-05-06T08:55:17Z

        line_no = node.location.start_line
      else
-        tokens = [file_line_comment_token(line_no)]
+        tokens = []


tompng · 2026-05-06T18:46:58Z

    meth.start_collecting_tokens(:ruby)
    node = @line_nodes[line_no]
-    tokens = node ? visible_tokens_from_location(node.location) : [file_line_comment_token(start_line)]
+    tokens = node ? visible_tokens_from_location(node.location) : []


If a method doesn't have tokens, it used to have a line number information but it's gone.

## # :method: ghost_method ## # :method: # :call-seq: ghost_method2() -> Integer

I think the line number is important to know why and where the ghost-method is defined.

How about adding a new attribute that represents source location and use it instead of token_stream.first[:line_no]

Makes sense, I'll try to fix that. It already exists as line on CodeObject and the parser sets it as far as I can tell so it shouldn't be too involved.

Should be done. I moved the tests to the ruby parser, since that is where the line number is actually being set. That said, I only ran rdoc against ruby/prism to check for differences (there are none).

Is there an easy way to run it on ruby/ruby with a local checkout? I couldn't figure it out.

I usually use bundle exec rdoc path/to/ruby/ruby --op ruby_doc for quick check (though it may not be as same as document deployed to https://docs.ruby-lang.org/en/master/)

Comparison without --line-numbers option

Only Delegator.html and Singleton.html had a small diff that we can ignore 👍

# Before <pre class="ruby"># File lib/delegate.rb, line 67</pre> # After <pre class="ruby"># File lib/delegate.rb, line 67 </pre>

Comparison with --line-numbers option

Removing spaces before # File file.rb seems a great improvement 👍
Most files doesn't have a diff after normalizing: compare after removing that space with gsub

Delegator.html and Singleton.html had a diff #1696 (comment)

# Before <pre class="ruby"> # File ../ruby/lib/delegate.rb</pre> # After <pre class="ruby"># File ../ruby/lib/delegate.rb 67 </pre>

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Earlopain · 2026-05-07T10:12:47Z

+  def add_location_comment(src)
+    path = CGI.escapeHTML(file.relative_name)
+    if options.line_numbers
+      src.prepend("<span class=\"ruby-comment\"># File #{path}</span>\n")
+    else
+      src.prepend("<span class=\"ruby-comment\"># File #{path}, line #{line}</span>\n")
+    end


Is this necessary?

No I think it's not. Feel free to ignore it

There is some awkward code that dances around the fact that the tokens for a method actually contain a 3 extra tokens that don't exist in the source code. Now `RipperStateLex` is only referenced to actually parse, rest is kept internal

tompng · 2026-05-09T16:01:38Z

-    first = $3.to_i - 1
-    last  = first + src.count("\n")
-    size = last.to_s.length
+    start_line = line or return


I think we can add one more early return when src is empty.

bundle exec rdoc path/to/ruby/ruby/lib/delegate.rb --line-numbers --op outputdir

Master branch

This branch

If src is empty, line number wasn't and won't be displayed. So perhaps we can change def add_location_comment such as:

if options.line_numbers && !src.empty? # Add empty check here src.prepend("# File #{path}\n") else src.prepend("# File #{path}, line #{line}\n") end

Copilot AI review requested due to automatic review settings May 6, 2026 07:02

Earlopain temporarily deployed to fork-preview-protection May 6, 2026 07:02 — with GitHub Actions Inactive

Copilot started reviewing on behalf of Earlopain May 6, 2026 07:02 View session

Earlopain commented May 6, 2026

View reviewed changes

Copilot AI reviewed May 6, 2026

View reviewed changes

Earlopain force-pushed the no-manual-token-creation branch from 45fa797 to 187755a Compare May 6, 2026 08:57

Earlopain requested a deployment to fork-preview-protection May 6, 2026 08:57 — with GitHub Actions Waiting

tompng reviewed May 6, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings May 7, 2026 10:04

Earlopain force-pushed the no-manual-token-creation branch from 187755a to 7e0269b Compare May 7, 2026 10:04

Earlopain requested a deployment to fork-preview-protection May 7, 2026 10:04 — with GitHub Actions Waiting

Copilot started reviewing on behalf of Earlopain May 7, 2026 10:05 View session

Copilot AI reviewed May 7, 2026

View reviewed changes

Don't create tokens by hand

01677d0

There is some awkward code that dances around the fact that the tokens for a method actually contain a 3 extra tokens that don't exist in the source code. Now `RipperStateLex` is only referenced to actually parse, rest is kept internal

Earlopain force-pushed the no-manual-token-creation branch from 7e0269b to 01677d0 Compare May 7, 2026 10:14

Earlopain deployed to fork-preview-protection May 7, 2026 10:14 — with GitHub Actions Active

tompng reviewed May 9, 2026

View reviewed changes

Conversation

Earlopain commented May 6, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matzbot commented May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Earlopain May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tompng May 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Comparison without --line-numbers option

Comparison with --line-numbers option

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

matzbot commented May 6, 2026 •

edited

Loading

Earlopain May 6, 2026 •

edited

Loading

tompng May 9, 2026 •

edited

Loading

Comparison without `--line-numbers` option

Comparison with `--line-numbers` option