Skip to content

Commit

Permalink
Fix line length check for call chain breakables
Browse files Browse the repository at this point in the history
  • Loading branch information
@reese
reese authored Aug 18, 2023
1 parent 087a994 commit 5653f0f
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 78 deletions.
5 changes: 3 additions & 2 deletions fixtures/large/rspec_mocks_proxy_expected.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -393,8 +393,9 @@ def initialize(source_space, *args)
# That's what this method (together with `original_unbound_method_handle_from_ancestor_for`)
# does.
def original_method_handle_for(message)
unbound_method = superclass_proxy && superclass_proxy
.original_unbound_method_handle_from_ancestor_for(message.to_sym)
unbound_method = superclass_proxy && superclass_proxy.original_unbound_method_handle_from_ancestor_for(
message.to_sym
)

return super unless unbound_method
unbound_method.bind(object)
Expand Down
19 changes: 7 additions & 12 deletions librubyfmt/src/parser_state.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -333,11 +333,7 @@ impl ConcreteParserState for BaseParserState {

fn breakable_of<'a>(&mut self, delims: BreakableDelims, f: RenderFunc) {
self.shift_comments();
let mut be = BreakableEntry::new(
self.current_spaces(),
delims,
self.current_formatting_context(),
);
let mut be = BreakableEntry::new(delims, self.current_formatting_context());
be.push_line_number(self.current_orig_line_number);
self.breakable_entry_stack.push(Box::new(be));

Expand Down Expand Up @@ -369,11 +365,7 @@ impl ConcreteParserState for BaseParserState {
/// At the moment, this is only for conditions in a `when` clause
fn inline_breakable_of<'a>(&mut self, delims: BreakableDelims, f: RenderFunc) {
self.shift_comments();
let mut be = BreakableEntry::new(
self.current_spaces(),
delims,
self.current_formatting_context(),
);
let mut be = BreakableEntry::new(delims, self.current_formatting_context());
be.push_line_number(self.current_orig_line_number);
self.breakable_entry_stack.push(Box::new(be));

Expand Down Expand Up @@ -401,8 +393,11 @@ impl ConcreteParserState for BaseParserState {
f: RenderFunc,
) {
self.shift_comments();
let mut be =
BreakableCallChainEntry::new(self.current_formatting_context(), call_chain_elements);
let mut be = BreakableCallChainEntry::new(
self.current_formatting_context(),
call_chain_elements,
self.current_spaces(),
);
be.push_line_number(self.current_orig_line_number);
self.breakable_entry_stack.push(Box::new(be));

Expand Down
5 changes: 3 additions & 2 deletions librubyfmt/src/render_queue_writer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,8 +213,9 @@ impl RenderQueueWriter {
accum: &mut Intermediary,
mut bcce: BreakableCallChainEntry,
) {
let single_line_string_length = bcce.longest_multiline_string_length();
let length = accum.current_line_length() + single_line_string_length;
// N.B. longest_multiline_string_length will include the additional indentation
// while rendering, so we don't add the accum.current_line_length() here
let length = bcce.longest_multiline_string_length();
if (length > MAX_LINE_LENGTH || bcce.is_multiline())
&& bcce.entry_formatting_context() != FormattingContext::StringEmbexpr
{
Expand Down
111 changes: 49 additions & 62 deletions librubyfmt/src/render_targets.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use crate::parser_state::{will_render_as_multiline, BaseParserState, FormattingC
use crate::ripper_tree_types::{Block, CallChainElement, Expression, StringLiteral};
use crate::types::{ColNumber, LineNumber};
use std::collections::HashSet;
use std::iter;

fn insert_at<T>(idx: usize, target: &mut Vec<T>, input: &mut Vec<T>) {
let mut tail = target.split_off(idx);
Expand Down Expand Up @@ -54,8 +55,6 @@ pub trait AbstractTokenTarget: std::fmt::Debug {
fn into_tokens(self, ct: ConvertType) -> Vec<ConcreteLineTokenAndTargets>;
fn is_multiline(&self) -> bool;
fn push_line_number(&mut self, number: LineNumber);
fn increment_additional_indent(&mut self);
fn additional_indent(&self) -> u32;
fn single_line_string_length(&self) -> usize;
fn index_of_prev_newline(&self) -> Option<usize>;
fn to_breakable_entry(self: Box<Self>) -> BreakableEntry;
Expand Down Expand Up @@ -84,7 +83,6 @@ pub trait AbstractTokenTarget: std::fmt::Debug {

#[derive(Debug, Clone)]
pub struct BreakableEntry {
additional_indent: ColNumber,
tokens: Vec<AbstractLineToken>,
line_numbers: HashSet<LineNumber>,
delims: BreakableDelims,
Expand All @@ -96,14 +94,6 @@ impl AbstractTokenTarget for BreakableEntry {
*self
}

fn increment_additional_indent(&mut self) {
self.additional_indent += 1;
}

fn additional_indent(&self) -> u32 {
self.additional_indent
}

fn to_breakable_call_chain(self: Box<Self>) -> BreakableCallChainEntry {
unreachable!()
}
Expand Down Expand Up @@ -189,13 +179,8 @@ impl AbstractTokenTarget for BreakableEntry {
}

impl BreakableEntry {
pub fn new(
additional_indent: ColNumber,
delims: BreakableDelims,
context: FormattingContext,
) -> Self {
pub fn new(delims: BreakableDelims, context: FormattingContext) -> Self {
BreakableEntry {
additional_indent,
tokens: Vec::new(),
line_numbers: HashSet::new(),
delims,
Expand All @@ -219,7 +204,7 @@ impl BreakableEntry {

#[derive(Debug, Clone)]
pub struct BreakableCallChainEntry {
additional_indent: u32,
pub starting_indentation_depth: u32,
tokens: Vec<AbstractLineToken>,
line_numbers: HashSet<LineNumber>,
call_chain: Vec<CallChainElement>,
Expand All @@ -231,14 +216,6 @@ impl AbstractTokenTarget for BreakableCallChainEntry {
unreachable!()
}

fn increment_additional_indent(&mut self) {
self.additional_indent += 1;
}

fn additional_indent(&self) -> u32 {
self.additional_indent
}

fn tokens(&self) -> &Vec<AbstractLineToken> {
&self.tokens
}
Expand Down Expand Up @@ -398,11 +375,15 @@ impl AbstractTokenTarget for BreakableCallChainEntry {
}

impl BreakableCallChainEntry {
pub fn new(context: FormattingContext, call_chain: Vec<CallChainElement>) -> Self {
pub fn new(
context: FormattingContext,
call_chain: Vec<CallChainElement>,
starting_indentation_depth: u32,
) -> Self {
BreakableCallChainEntry {
additional_indent: 0,
tokens: Vec::new(),
line_numbers: HashSet::new(),
starting_indentation_depth,
context,
call_chain,
}
Expand All @@ -428,41 +409,47 @@ impl BreakableCallChainEntry {
// have multiline blocks (which will often be quite long vertically, even if
// they're under 120 characters horizontally). In this case, look for the longest
// individual line and get _that_ max length
self.tokens
.iter()
.map(|tok| {
let forced_multiline = match tok {
AbstractLineToken::BreakableCallChainEntry(bcce) => bcce.is_multiline(),
AbstractLineToken::BreakableEntry(be) => be.is_multiline(),
_ => false,
};
if forced_multiline {
RenderItem {
tokens: tok.clone().into_multi_line(),
convert_type: ConvertType::MultiLine,
}
} else {
RenderItem {
tokens: tok.clone().into_single_line(),
convert_type: ConvertType::SingleLine,
}
iter::once(&AbstractLineToken::ConcreteLineToken(
// Push the starting indentation for the first line -- other
// lines will already have this indentation
ConcreteLineToken::Indent {
depth: self.starting_indentation_depth,
},
))
.chain(&self.tokens)
.map(|tok| {
let forced_multiline = match tok {
AbstractLineToken::BreakableCallChainEntry(bcce) => bcce.is_multiline(),
AbstractLineToken::BreakableEntry(be) => be.is_multiline(),
_ => false,
};
if forced_multiline {
RenderItem {
tokens: tok.clone().into_multi_line(),
convert_type: ConvertType::MultiLine,
}
})
.flat_map(
|RenderItem {
tokens,
convert_type,
}| {
tokens
.into_iter()
.map(move |tok| tok.into_ruby(convert_type))
},
)
.collect::<String>()
.split('\n')
.map(|st| st.len())
.max()
.unwrap()
} else {
RenderItem {
tokens: tok.clone().into_single_line(),
convert_type: ConvertType::SingleLine,
}
}
})
.flat_map(
|RenderItem {
tokens,
convert_type,
}| {
tokens
.into_iter()
.map(move |tok| tok.into_ruby(convert_type))
},
)
.collect::<String>()
.split('\n')
.map(|st| st.len())
.max()
.unwrap()
}

/// In practice, this generally means something like the call chain having something
Expand Down

0 comments on commit 5653f0f

Please sign in to comment.