-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix handling of hugging calls #228
Conversation
|
||
Ok(matches!( | ||
arg, | ||
AnyRExpression::RCall(_) | AnyRExpression::RSubset(_) | AnyRExpression::RSubset2(_) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we sure we want this hugging behavior with subset and subset2?
abort(the_thing[
an_argument,
another_argument
])
And the reverse? (where the outer call is [
and the inner is a standard function call)
the_thing[paste0(
"This is a section",
and,
"this is another section",
"and this is a final section"
)]
If so, I think we need to add more tests with the combinations of call, subset, and subset2 here
I think i like it? Definitely want lots of tests though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep no reason to treat these differently I think
1, | ||
2 | ||
))) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's one more
# Call: Recursive hugging case, persistent line breaks stop hugging
c(list(
foobarbafoobarbafoobarbafoobarbafoobarbafoobarbafoobarbafoobarbafoobarbazzzzzzzzzfoobarbaz(1, 2)))
} | ||
|
||
// Unwrap the value to get the `AnyRExpression` | ||
let Some(arg) = item.element().node.clone()?.value() else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let Some(arg) = item.element().node.clone()?.value() else { | |
let Some(arg) = item.element().node()?.value() else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The clone is necessary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can make it work with:
let Some(arg) = item.element().node.as_ref().ok() else {
return Ok(false);
};
let Some(arg) = arg.value() else {
return Ok(false);
};
but I'm not sure that's a win
oh I just noticed that these test cases are actually different, and the second one currently fails! # Trailing comment of inner paren
c(list(
1
) #foo
)
# Leading comment of outer paren
c(list(
1
)
#foo
) |
I had to refactor things a bit to give the formatting method access to the syntax tree so I could check for comments a bit more flexibly. See last commit, should hopefully be reasonable. |
@@ -76,6 +76,326 @@ impl RCallLikeArguments { | |||
Self::Subset2(node) => node.syntax(), | |||
} | |||
} | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is now a method on RCallLikeArguments
, hence the big diff as the whole thing moved. It feels right and this gives access to self
and the syntax elements.
write!( | ||
buffer, | ||
[FormatAllArgsBrokenOut { | ||
call: self, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can then pass self
here.
@@ -919,6 +932,7 @@ impl Format<RFormatContext> for FormatGroupedArgument { | |||
} | |||
|
|||
struct FormatAllArgsBrokenOut<'a> { | |||
call: &'a RCallLikeArguments, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The whole call is now part of this struct. This way we can peek at anything we want if needed.
@@ -945,7 +959,7 @@ impl Format<RFormatContext> for FormatAllArgsBrokenOut<'_> { | |||
Ok(()) | |||
}); | |||
|
|||
let args = if !self.expand && is_hugging_call(self.args)? { | |||
let args = if !self.expand && is_hugging_call(self.args, self.call.r_token())? { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Taking a peek at the right delimiter so we can check for comments.
Branched from #227 (for access to
Either
type).Closes #21.