Handle on_sp when using prism
#241
Conversation
Earlopain
force-pushed
the
prism-on-sp
branch
from
16995b1 to
04357b2
Compare
| last&.type == :on_tstring_end | ||
| return false unless last | ||
|
|
||
| last.type == :on_tstring_end || (last.type == :on_sp && last.token == TRAILING_SLASH) |
There was a problem hiding this comment.
The (last.type == :on_sp && last.token == TRAILING_SLASH) part is to handle the fact there is no Prism 1.8.1/1.9.0 yet and so no easy way to detect Prism has :on_sp vs not, right?
There was a problem hiding this comment.
Yeah, that's right. I tried feature detection (Prism.lex_compat(" ").value.dig(0, 1) == :on_sp) but then some other test here fails because prism loads delegate and that adds a method to Kernel which it didn't like
|
Some CI jobs are failing, the 4.0 job failures seem unrelated & pre-existing but https://github.com/ruby/syntax_suggest/actions/runs/21170190500/job/60887146934?pr=241 needs a changelog entry or so |
Earlopain
force-pushed
the
prism-on-sp
branch
from
04357b2 to
d34cef4
Compare
|
CI should be green with #242. I added a changelog entry |
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/ruby#15914 * ruby/prism#3859
Earlopain
force-pushed
the
prism-on-sp
branch
from
d34cef4 to
94d27c4
Compare
|
What's the order here, I suppose we could merge this PR, and that will then auto-push/sync a commit to ruby/ruby and then we only need the Reapply commit from ruby/ruby#15914? Also not sure how urgent it is, I suppose it can get messy if other PRs are merged in ruby/prism as then that sync might not work either. If it's not urgent I would probably wait for @schneems' review. |
|
I think we can merge the ruby PR, leave this open, and then apply any desired changes on top of this PR so that those get synced over. We should not merge much in prism related to ripper until the commit is reapplied. Since we know, we can wait a bit until we get a review here. |
|
Makes sense, I merged the ruby/ruby PR. |
2 participants
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:
on_spinsyntax_suggestruby#15914