From 31be72477a25af3a0e83eea4df10705672eb24fa Mon Sep 17 00:00:00 2001 From: Aaron Lieberman Date: Sun, 20 Jul 2025 22:24:02 -0700 Subject: [PATCH] fix: Improve contextual spacing for if statements when ifStatement.spaceAround: true When ifStatement.spaceAround is true, make sure the spaces around parens are right based on the context of where the paren lies. The change is to only add the space if the open paren is not at the end of a line. We do that on the opening paren by adding a Signal::SpaceOrNewLine which allows the system to add the space if it needs to for purposes of word wrap, otherwise ommitting it. For the close paren, we add the space if we're not the first token on the line. The impetus for this was the case where an opening paren for an if statement control block was the last character on a line, or a closing paren was the first. For example: (where lineWidth: 40) ``` if ( somereallylongidentifier <= anotherreallylongidentifier ) { } ``` The above case was getting formatted with an extra space at the end of the if statement line and an extra space at the start of the close paren line. --- src/generation/generate.rs | 13 ++++++------- .../ifStatement/IfStatement_SpaceAround_True.txt | 13 ++++++++++++- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/generation/generate.rs b/src/generation/generate.rs index f978a04b..dfb095b2 100644 --- a/src/generation/generate.rs +++ b/src/generation/generate.rs @@ -9538,16 +9538,15 @@ fn gen_surrounded_by_tokens<'a>( // generate let open_token_start_line = open_token_end.start_line_fast(context.program); - let is_single_line; items.extend(gen_leading_comments(&range, context)); items.push_sc(opts.open_token); if let Some(first_member) = opts.first_member { let first_member_start_line = first_member.start_line_fast(context.program); - is_single_line = open_token_start_line == first_member_start_line; + let is_single_line = open_token_start_line == first_member_start_line; if is_single_line && opts.single_line_space_around { - items.push_space(); + items.push_signal(Signal::SpaceOrNewLine); } if opts.allow_open_token_trailing_comments && !is_single_line { @@ -9577,10 +9576,10 @@ fn gen_surrounded_by_tokens<'a>( )); } else { let comments = open_token_end.trailing_comments_fast(context.program); - is_single_line = open_token_start_line == close_token_start.start_line_fast(context.program); + let is_single_line = open_token_start_line == close_token_start.start_line_fast(context.program); if is_single_line && opts.single_line_space_around { - items.push_space(); + items.push_signal(Signal::SpaceOrNewLine); } if !comments.is_empty() { @@ -9638,8 +9637,8 @@ fn gen_surrounded_by_tokens<'a>( } } - if is_single_line && opts.single_line_space_around { - items.push_space(); + if opts.single_line_space_around { + items.push_condition(if_false("addSpaceIfNotStartOfLine", condition_resolvers::is_start_of_line(), " ".into())); } } else { // todo: have a warning here when this happens diff --git a/tests/specs/statements/ifStatement/IfStatement_SpaceAround_True.txt b/tests/specs/statements/ifStatement/IfStatement_SpaceAround_True.txt index b64e5889..9541e4f8 100644 --- a/tests/specs/statements/ifStatement/IfStatement_SpaceAround_True.txt +++ b/tests/specs/statements/ifStatement/IfStatement_SpaceAround_True.txt @@ -1,4 +1,4 @@ -~~ ifStatement.spaceAround: true ~~ +~~ ifStatement.spaceAround: true, lineWidth: 40 ~~ == should format with space around the condition == if (i <= arr.length) { } @@ -6,3 +6,14 @@ if (i <= arr.length) { [expect] if ( i <= arr.length ) { } + +== shouldn't add the space around the condition if multi-line == +if (somereallylongidentifier <= anotherreallylongidentifier) { +} + +[expect] +if ( + somereallylongidentifier + <= anotherreallylongidentifier +) { +}