Fix rule-based formatting for non-zero offset selections#1218
Fix rule-based formatting for non-zero offset selections#1218somiljain2006 wants to merge 3 commits into
Conversation
somiljain2006
requested review from
allevato,
hamishknight,
hborla and
rintaro
as code owners
|
@allevato Can you review this pr? |
|
I'm not sure this is the right approach. What if a rule only operated on the doc comments of a node (and we do have a couple of those)? Since this change skips leading trivia, if the user only selected the doc comment lines of the node, it would be skipped. Given the way trivia is represented in swift-syntax, I think we might need a way for a rule to indicate whether it operates on just the leading trivia of a node, or just the non-trivia content of the node, or both, and have the |
|
@allevato Thanks for pointing that out. I've pushed an update that adds a rule-level targetScope so rules can specify whether they operate on content, leading/trailing trivia, trivia, or the entire node, and Context.shouldFormat now checks selection containment against the appropriate region. I also updated trivia-based rules such as UseTripleSlashForDocumentationComments and NoBlockComments, and added tests for leading/trailing trivia selections. |
| // SwiftSyntax to continue with the standard dispatch. | ||
| guard context.shouldFormat(type(of: self), node: node) else { return node } | ||
| guard context.shouldFormat(type(of: self), node: node) else { | ||
| if type(of: self).targetScope != .content && context.selectionOverlaps(node) { |
There was a problem hiding this comment.
Why is this additional check necessary; what is it handling that shouldFormat doesn't?
There was a problem hiding this comment.
This was meant to avoid stopping traversal too early for trivia-based rules. If shouldFormat returned false, SwiftSyntax would skip descendants, which could prevent rules like NoBlockComments from reaching trivia in the selected region. The overlap check keeps traversal going in that case.
2 participants
Fixes #1114
Rule-based formatting was skipped when a selection started inside a node's leading trivia. Use positionAfterSkippingLeadingTrivia when checking whether a rule should be applied. Adds a regression test covering formatting with a non-zero offset range.