Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions fixtures/small/begin_ensure_rescue_actual.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
class ForIndents
def func
ants
rescue Bees => e
a
# too tired to do more
end

def func2
Expand All @@ -12,8 +14,11 @@ def func2
end

def func3
bees
ensure
# TODO
a
# more TODO
end


Expand Down
5 changes: 5 additions & 0 deletions fixtures/small/begin_ensure_rescue_expected.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
class ForIndents
def func
ants
rescue Bees => e
a
# too tired to do more
end

def func2
Expand All @@ -12,8 +14,11 @@ def func2
end

def func3
bees
ensure
# TODO
a
# more TODO
end

def func4
Expand Down
18 changes: 18 additions & 0 deletions fixtures/small/block_ensure_actual.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
it "calls the function" do
# temporarily set this
ENV["CALL"] = "the function"
the_function.call
ensure
# reset
ENV["CALL"] = nil
# maybe something rubocop disable
end

it "calls the function again" do
ENV["CALL"] = "the function again"
the_function.call
ensure
# reset
ENV["CALL"] = nil
# maybe something rubocop disable
end
18 changes: 18 additions & 0 deletions fixtures/small/block_ensure_expected.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
it "calls the function" do
# temporarily set this
ENV["CALL"] = "the function"
the_function.call
ensure
# reset
ENV["CALL"] = nil
# maybe something rubocop disable
end

it "calls the function again" do
ENV["CALL"] = "the function again"
the_function.call
ensure
# reset
ENV["CALL"] = nil
# maybe something rubocop disable
end
24 changes: 18 additions & 6 deletions librubyfmt/src/format_prism.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,23 @@ pub fn format_node(ps: &mut ParserState, node: prism::Node) {
use prism::Node;

ps.at_offset(node.location().start_offset());
// StatementsNode is the only real "wrapper" node, meaning it purely contains
// other statements which themselves would be at the start of a line.
// We just ignore it here -- the alternative would be callers might need to have
// `ps.with_start_of_line(false, ...` for statements, which is semantically confusing
if ps.at_start_of_line() && !matches!(node, Node::StatementsNode { .. }) {

// StatementsNode is a "wrapper" node, meaning it purely contains other statements
// which themselves would be at the start of a line. We just ignore it here -- the
// alternative would be callers might need to have `ps.with_start_of_line(false, ...`
// for statements, which is semantically confusing.
//
// BeginNode without a location for `begin` occurs in `def f; ... rescue; ... end` or
// similar and also -- at least for its `statements` field -- serves a similar
// wrapping function.
let needs_handling = ps.at_start_of_line()
&& !(matches!(node, Node::StatementsNode { .. })
|| node
.as_begin_node()
.map(|b| b.begin_keyword_loc().is_none())
Comment on lines +30 to +31
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be worth unifying all the places we do this in one function or something, since we also have pretty similar cases handled in various other places, not sure if it's better to unify them in format_node or leave them handled in callers though 🤷

.unwrap_or(false));

if needs_handling {
ps.emit_indent();
}

Expand Down Expand Up @@ -409,7 +421,7 @@ pub fn format_node(ps: &mut ParserState, node: prism::Node) {
}

ps.at_offset(node.location().end_offset());
if ps.at_start_of_line() && !matches!(node, Node::StatementsNode { .. }) {
if needs_handling {
ps.emit_newline();
}
}
Expand Down