Skip to content

Conversation

@pull
Copy link

@pull pull bot commented Jan 21, 2026

See Commits and Changes for more details.


Created by pull[bot] (v2.0.0-alpha.4)

Can you help keep this open source service alive? 💖 Please sponsor : )

eregon and others added 6 commits January 21, 2026 13:21
…nts in Ripper translator

* We don't know what `on_*` events might return so we cannot assume it's an Array.
* See ruby/prism#3838 (comment)

ruby/prism@bed4271ce2
We should touch these as little as possible and just pass them along

ruby/prism@52c4fa785e
The reason this logic for different methods branches in the class instead of internally was to be eagerly aggressive about runtime performance. This code is currently only used once for the document where it's invoked ~N times (where N is number of lines):

```ruby
module SyntaxSuggest
  class CleanDocument
    # ...
    def join_trailing_slash!
      trailing_groups = @document.select(&:trailing_slash?).map do |code_line|
        take_while_including(code_line.index..) { |x| x.trailing_slash? }
      end
      join_groups(trailing_groups)
      self
    end
```

Since this is not currently a hot-spot I think merging the branches and using a case statement is a reasonable tradeoff and avoids the need to do specific version testing.

An alternative idea was presented in #241 of behavior-based testing for branch logic (which I would prefer), however, calling the code triggered requiring a `DelegateClass` when the `syntax_suggest/api` is being required.

ruby/syntax_suggest@ab122c455f
@pull pull bot locked and limited conversation to collaborators Jan 21, 2026
@pull pull bot added the ⤵️ pull label Jan 21, 2026
@pull pull bot merged commit f3a5b0c into turkdevops:master Jan 21, 2026
1 of 2 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants