Add support for new Sorbet configs (Cache, Prism, and RBS)#888
Add support for new Sorbet configs (Cache, Prism, and RBS)#888amomchilov wants to merge 3 commits intomainfrom
Conversation
| opts << "--no-stdlib" if @no_stdlib | ||
| opts << "--cache-dir='#{@cache_dir}'" if @cache_dir | ||
| opts << "--parser=#{@parser}" if @parser | ||
| opts.join(" ") |
There was a problem hiding this comment.
use_rbs is missing from options_string. Both cache_dir and parser are included, but use_rbs is not, so a parse → options_string round-trip silently drops the RBS config.
Should we add something like:
| opts.join(" ") | |
| opts << "--parser=#{@parser}" if @parser | |
| opts << "--enable-experimental-rbs-comments" if @use_rbs |
(or whichever RBS flag makes the most sense as canonical)
There was a problem hiding this comment.
We may need to map Sorbet version <:> feature like we do in Tapioca
| config.parser = parse_option(line).to_sym | ||
| next | ||
| when /^--enable-experimental-rbs-(comments|signatures|assertions)(=|$)/ | ||
| config.use_rbs = parse_bool_option(line) |
There was a problem hiding this comment.
I'm wondering about the three separate RBS flags (comments, signatures, assertions) collapsing into a single use_rbs boolean. If a config has:
--enable-experimental-rbs-comments
--enable-experimental-rbs-signatures=false
The last one wins and use_rbs becomes false. Is that intentional? If we only need a single boolean for Spoom's purposes, maybe worth a comment explaining the simplification.
There was a problem hiding this comment.
Yeah I was trying to keep it simple, since the whole point of the deprecation was to not have to worry about the distinction (like signatures=true and assetions=false).
I think I'll change this to use the ORing of values, and raise if there's a mix of trues/falses
2 participants
No description provided.