Skip to content

Commit 2a55f0c

Browse files
reesereese
committed
[prism] Model inline conditionals as breakables
1 parent 2d44be3 commit 2a55f0c

File tree

5 files changed

+324
-44
lines changed

5 files changed

+324
-44
lines changed

‎librubyfmt/src/format_prism.rs‎

Lines changed: 21 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -3666,47 +3666,29 @@ fn format_conditional_node<'src>(
36663666
// For while/until, always use the inline format for modifiers, since
36673667
// some transformations are unsafe.
36683668
// For if/unless, check if we should convert to block form.
3669-
let should_convert_to_block = match conditional {
3670-
Conditional::While(_) | Conditional::Until(_) => false,
3669+
match conditional {
3670+
Conditional::While(_) | Conditional::Until(_) => {
3671+
// Always use inline format for while/until modifiers
3672+
format_inline_conditional(
3673+
ps,
3674+
conditional.predicate(),
3675+
conditional.statements(),
3676+
conditional_keyword,
3677+
);
3678+
}
36713679
Conditional::If(_) | Conditional::Unless(_) => {
3672-
let predicate = conditional.predicate();
3673-
let statements = conditional.statements();
3674-
let predicate_start_line =
3675-
ps.get_line_number_for_offset(predicate.location().start_offset());
3676-
let predicate_end_line =
3677-
ps.get_line_number_for_offset(predicate.location().end_offset());
3678-
if predicate_start_line != predicate_end_line {
3679-
true
3680-
} else {
3681-
// Check if it renders multiline due to length
3682-
ps.will_render_as_multiline(|next_ps| {
3683-
format_inline_conditional(
3684-
next_ps,
3685-
predicate,
3686-
statements,
3687-
conditional_keyword,
3688-
)
3689-
})
3690-
}
3680+
ps.conditional_layout_of(
3681+
conditional_keyword,
3682+
|ps| format_node(ps, conditional.predicate()),
3683+
|ps| {
3684+
if let Some(statements) = conditional.statements()
3685+
&& let Some(first_statement) = statements.body().iter().next()
3686+
{
3687+
format_node(ps, first_statement);
3688+
}
3689+
},
3690+
);
36913691
}
3692-
};
3693-
3694-
if should_convert_to_block {
3695-
format_conditional_block_form(
3696-
ps,
3697-
conditional_keyword,
3698-
conditional.predicate(),
3699-
conditional.statements(),
3700-
conditional.subsequent_or_else(),
3701-
requires_end_keyword,
3702-
);
3703-
} else {
3704-
format_inline_conditional(
3705-
ps,
3706-
conditional.predicate(),
3707-
conditional.statements(),
3708-
conditional_keyword,
3709-
);
37103692
}
37113693
} else {
37123694
format_conditional_block_form(

‎librubyfmt/src/line_tokens.rs‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use crate::heredoc_string::{HeredocKind, HeredocString};
2-
use crate::render_targets::{BreakableCallChainEntry, BreakableEntry};
2+
use crate::render_targets::{BreakableCallChainEntry, BreakableEntry, ConditionalLayoutEntry};
33
use crate::types::ColNumber;
44
use crate::util::get_indent;
55
use std::borrow::Cow;
@@ -215,6 +215,9 @@ impl<'src> From<ConcreteLineTokenAndTargets<'src>> for AbstractLineToken<'src> {
215215
ConcreteLineTokenAndTargets::BreakableCallChainEntry(bcce) => {
216216
AbstractLineToken::BreakableCallChainEntry(bcce)
217217
}
218+
ConcreteLineTokenAndTargets::ConditionalLayoutEntry(cle) => {
219+
AbstractLineToken::ConditionalLayoutEntry(cle)
220+
}
218221
}
219222
}
220223
}
@@ -224,6 +227,7 @@ pub enum ConcreteLineTokenAndTargets<'src> {
224227
ConcreteLineToken(ConcreteLineToken<'src>),
225228
BreakableEntry(BreakableEntry<'src>),
226229
BreakableCallChainEntry(BreakableCallChainEntry<'src>),
230+
ConditionalLayoutEntry(ConditionalLayoutEntry<'src>),
227231
}
228232

229233
impl<'src> ConcreteLineTokenAndTargets<'src> {
@@ -251,6 +255,7 @@ pub enum AbstractLineToken<'src> {
251255
SoftIndent { depth: u32 },
252256
BreakableEntry(BreakableEntry<'src>),
253257
BreakableCallChainEntry(BreakableCallChainEntry<'src>),
258+
ConditionalLayoutEntry(ConditionalLayoutEntry<'src>),
254259
}
255260

256261
impl<'src> AbstractLineToken<'src> {
@@ -278,6 +283,9 @@ impl<'src> AbstractLineToken<'src> {
278283
Self::BreakableCallChainEntry(bcce) => {
279284
out.push(ConcreteLineTokenAndTargets::BreakableCallChainEntry(bcce));
280285
}
286+
Self::ConditionalLayoutEntry(cle) => {
287+
out.push(ConcreteLineTokenAndTargets::ConditionalLayoutEntry(cle))
288+
}
281289
}
282290
}
283291

@@ -305,6 +313,9 @@ impl<'src> AbstractLineToken<'src> {
305313
Self::BreakableCallChainEntry(bcce) => {
306314
out.push(ConcreteLineTokenAndTargets::BreakableCallChainEntry(bcce));
307315
}
316+
Self::ConditionalLayoutEntry(cle) => {
317+
out.push(ConcreteLineTokenAndTargets::ConditionalLayoutEntry(cle))
318+
}
308319
}
309320
}
310321

@@ -376,6 +387,7 @@ impl<'src> AbstractLineToken<'src> {
376387
Self::ConcreteLineToken(clt) => clt.len(),
377388
Self::BreakableEntry(be) => be.single_line_len(),
378389
Self::BreakableCallChainEntry(bcce) => bcce.single_line_len(),
390+
Self::ConditionalLayoutEntry(cle) => cle.inline_single_line_len(),
379391
}
380392
}
381393
}

‎librubyfmt/src/parser_state.rs‎

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@ use crate::heredoc_string::{HeredocKind, HeredocString};
55
use crate::line_tokens::*;
66
use crate::render_queue_writer::{MAX_LINE_LENGTH, RenderQueueWriter};
77
use crate::render_targets::{
8-
BaseQueue, Breakable, BreakableCallChainEntry, BreakableEntry, MultilineHandling,
8+
BaseQueue, Breakable, BreakableCallChainEntry, BreakableEntry, ConditionalLayoutEntry,
9+
MultilineHandling,
910
};
1011
use crate::types::{ColNumber, LineNumber, SourceOffset};
1112
use log::debug;
@@ -968,6 +969,51 @@ impl<'src> ParserState<'src> {
968969
f(&mut next_ps);
969970
next_ps
970971
}
972+
973+
/// Format a conditional modifier expression (e.g., `x if y` or `x unless y`).
974+
pub(crate) fn conditional_layout_of<FP, FS>(
975+
&mut self,
976+
keyword: &'static str,
977+
format_predicate: FP,
978+
format_statement: FS,
979+
) where
980+
FP: FnOnce(&mut ParserState<'src>),
981+
FS: FnOnce(&mut ParserState<'src>),
982+
{
983+
// Save and clear any pending comments to prevent them from being
984+
// inserted into the conditional layout
985+
let saved_comments = self.comments_to_insert.take();
986+
987+
let entry = ConditionalLayoutEntry::new(keyword, self.current_spaces());
988+
self.breakable_entry_stack
989+
.push(Breakable::InlineConditional(entry));
990+
991+
self.with_start_of_line(false, |ps| {
992+
ps.new_block(format_predicate);
993+
});
994+
995+
self.breakable_entry_stack
996+
.last_mut()
997+
.expect("just pushed InlineConditional")
998+
.as_conditional_layout_mut()
999+
.expect("just pushed InlineConditional")
1000+
.switch_to_statement();
1001+
1002+
self.with_start_of_line(false, |ps| {
1003+
ps.new_block(format_statement);
1004+
});
1005+
1006+
let cle = self
1007+
.breakable_entry_stack
1008+
.pop()
1009+
.expect("just pushed InlineConditional")
1010+
.into_conditional_layout()
1011+
.expect("InlineConditional always returns Some from into_conditional_layout");
1012+
1013+
self.comments_to_insert = saved_comments;
1014+
1015+
self.push_target(ConcreteLineTokenAndTargets::ConditionalLayoutEntry(cle));
1016+
}
9711017
}
9721018

9731019
pub fn line_difference_requires_newline(to_line: LineNumber, from_line: LineNumber) -> bool {

‎librubyfmt/src/render_queue_writer.rs‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@ use crate::heredoc_string::HeredocKind;
22
use crate::intermediary::{BlanklineReason, Intermediary};
33
use crate::line_tokens::*;
44
use crate::render_targets::{
5-
AbstractTokenTarget, BreakableCallChainEntry, BreakableEntry, ConvertType,
5+
AbstractTokenTarget, BreakableCallChainEntry, BreakableEntry, ConditionalLayoutEntry,
6+
ConvertType,
67
};
78
#[cfg(debug_assertions)]
89
use log::debug;
@@ -115,6 +116,9 @@ impl<'src> RenderQueueWriter<'src> {
115116
ConcreteLineTokenAndTargets::BreakableCallChainEntry(bcce) => {
116117
Self::format_breakable_call_chain_entry(accum, bcce)
117118
}
119+
ConcreteLineTokenAndTargets::ConditionalLayoutEntry(cle) => {
120+
Self::format_conditional_layout_entry(accum, cle)
121+
}
118122
ConcreteLineTokenAndTargets::ConcreteLineToken(x) => match x {
119123
BeginCallChainIndent => accum.additional_indent += 1,
120124
EndCallChainIndent => accum.additional_indent -= 1,
@@ -268,6 +272,14 @@ impl<'src> RenderQueueWriter<'src> {
268272
}
269273
}
270274

275+
fn format_conditional_layout_entry(accum: &mut Intermediary<'src>, cle: ConditionalLayoutEntry<'src>) {
276+
if cle.should_use_block_form(accum.current_line_length()) {
277+
Self::render_as(accum, cle.into_block_tokens());
278+
} else {
279+
Self::render_as(accum, cle.into_inline_tokens());
280+
}
281+
}
282+
271283
fn renders_over_max_line_length(
272284
accum: &Intermediary<'src>,
273285
breakable: &dyn AbstractTokenTarget<'src>,

0 commit comments

Comments
 (0)