[4.0] Backport syntax_suggest 2.0.3#15922
Conversation
It used to not emit this token type, but now it does. So when a newer version of prism is present, we can fall back to the same code that ripper uses. Ref: * ruby#15914 * ruby/prism#3859 ruby/syntax_suggest@42a3b8f6cb
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 ruby#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
k0kubun
left a comment
There was a problem hiding this comment.
Thanks. I just realized they released Ruby 4.0.0 with syntax_suggest v2.0.2 + ruby/syntax_suggest@54bb8ab. So this diff seems like a correct sync.
|
Can you add That works for me too, but given that you formatted commits to preserve history, I thought you would prefer keeping that. |
|
Ah, I didn't notice. Right, it should in theory look the same as for 3.4. BTW, how is one supposed to do this? I used And |
da6b74f to
edad27b
Compare
That is right.
I'm not sure why it doesn't work for you, but
No, these tags are for Redmine. They help me when I use |
|
Perhaps I have something weird in my git config or something. I'll try to check it out some time. Nice to know it's supposed to work at least. Should I do something with the ticket? It's closed now because this was merged with the reference (I also added the reference to the other two backport PRs) |
|
No, I'll work on the status of tickets right before I release the next version. So don't worry about it. |
|
Ah, I understand. As the creator of the ticket, I wouldn't consider it closed until all the backport PRs are merged. But it's no different from all the other tickets that are closed after the issue is fixed on main. So the status doesn't really matter. Thanks for explaining 👍 |
|
re
I guess it lost some error reporting along the way (I'll add that back) . How am I supposed to get this repo there, just clone it myself? Or do I need to run some command first. On main, it seems to expect it in |
yes |
https://bugs.ruby-lang.org/issues/21847