Model call chains as an AbstractTokenTarget
#443
Conversation
reese
force-pushed
the
reese-call-chain-breakable
branch
from
f6208b6
to
4cb900c
Compare
| # some stuff | ||
| } | ||
| .each do | ||
| <<~MYHEREDOC |
There was a problem hiding this comment.
on the downside, i'm currently still debugging even more crimes
reese
force-pushed
the
reese-call-chain-breakable
branch
from
00da814
to
087a994
Compare
reese
force-pushed
the
reese-call-chain-breakable
branch
from
8ff1e2b
to
5653f0f
Compare
reese
force-pushed
the
reese-call-chain-breakable
branch
from
b6b6103
to
64d273a
Compare
reese
changed the title
AbstractTokenTarget
| } else { | ||
| format_ident(ps, ident); | ||
| } | ||
| ps.shift_comments(); | ||
| } | ||
| CallChainElement::Block(b) => { | ||
| ps.emit_space(); | ||
| format_block(ps, b) |
There was a problem hiding this comment.
are we missing a shift_comments call here?
There was a problem hiding this comment.
I don't think we need to because format_{do,brace}_block should handle comment shifting internally (since they'll want to render the comments inside the block delimiters), but I'll add a comment here to make that explicit
| .any(|fc| fc == &FormattingContext::StringEmbexpr) | ||
| } | ||
|
|
||
| fn contains_hard_newline(&self) -> bool { |
There was a problem hiding this comment.
does this need to iter any nested breakable entries?
There was a problem hiding this comment.
I don't think so, at least not in the way it's currently used, because when we check this we also check whether self.line_numbers has multiple lines, which I think would handle having a multiline nested breakable. (FWIW this method is unchanged from before)
reese
force-pushed
the
reese-call-chain-breakable
branch
from
8282aa6
to
cf581b7
Compare
reese
force-pushed
the
reese-call-chain-breakable
branch
from
cf581b7
to
24f4b27
Compare
Resolves #397
(Apologies in advance for the large PR, but this a pretty foundational change that required quite a bit of boilerplate!)
This PR implements call chains as a new
AbstractTokenTarget:BreakableCallChainEntry. This means that multilining for call chains -- just like for existing breakables -- will be decided on-the-fly during rendering, where we have more contextual information and can better enforce line length.This is done with a combination of the normal breakable machinery and some additional "magic tokens." Specifically, this adds tokens at the beginning and end of the indent-able portion of the call chain, and then during rendering, we maintain a count of how many nested call chains we're in and adjust indentation accordingly. This isn't exactly the cleanest implementation, but given the complexity in use-cases for call chains, it was the best I could muster in a way that functioned how users would generally expect.
This is a fairly large PR, but there's primarily three main portions which each are about a third of the total diff each: fixtures, the boilerplate in
ripper_tree_typesto support getting expression starting lines, and finally the main formatting changes. I've done my best to comment these areas appropriately, but it's helpful to add further explanation there, I'm more than happy to add more.