Add means of comment formatting to sway-fmt-v2
#2229
Conversation
sway-fmt-v2/src/utils/comments.rs
Outdated
|
|
||
| impl Ord for CommentSpan { | ||
| fn cmp(&self, other: &Self) -> Ordering { | ||
| self.start.cmp(&other.start) |
There was a problem hiding this comment.
When it comes to ordering spans, I think the correct approach might be to reflect not just the order in which the start occurs, but also the order in which the spans would be visited during traversal (e.g. larger enclosing spans preceding inner spans.)
E.g. when comparing the spans 2..6 and 2..4, the 2..6 span should come first as it encloses the 2..4 span. I think this would look something like:
match self.start.cmp(&other.start) {
Ordering::Equal => other.end.cmp(&self.end),
ord => ord,
}Anyway, this is more relevant to the ordering of AST spans generally, and shouldn't be relevant to our case as I suppose comments cannot overlap 😂
There was a problem hiding this comment.
I was actually thinking about this while writing that part but later on couldn't think of an example where comments would be overlapping as you pointed out 😄. But I will fix the ordering like this. Although it seems like overlapping won't be happening, if somehow it happens I feel like this will be really hard to debug.
|
This needs couple more touches but at the current status it can format the following code piece: contract;
// This is a comment.
struct Foo {
barasdd: u64,
baz: bool,
}
// This is the second comment.
struct Foo2 {
barasdd: u64,
baz: bool,
}to contract;
// This is a comment.
struct Foo {
barasdd: u64,
baz: bool,
}
// This is the second comment.
struct Foo2 {
barasdd: u64,
baz: bool,
}Currently, it cannot handle comments that are inside a code block correctly (although they are correctly placed in the comment map). I should have that shortly and after some final clean-ups, this will be ready to go. |
sway-fmt-v2/src/utils/comments.rs
Outdated
| // If the starting position is the same encapsulatig span (i.e, wider one) should come | ||
| // first | ||
| match self.start.cmp(&other.start) { | ||
| Ordering::Equal => self.end.cmp(&other.end), |
There was a problem hiding this comment.
| Ordering::Equal => self.end.cmp(&other.end), | |
| Ordering::Equal => other.end.cmp(&self.end), |
^ I think it should be this
There was a problem hiding this comment.
Hmm while sorting 2..4 and 2..6, should 2..6 come first? I think if we do it as you are suggesting 2..4 is coming first. I added a test for ensuring the ordering is working as we intended but I might have misunderstood the order we should have for the spans (I understand as the encapsulating span should be coming first, i.e 2..6).
|
For now, we might want to preserve the whitespace "gap" between comments and their preceding/following items too. We might be better off thinking about how to format comments and the whitespace gaps between comments and surrounding items in future PRs, as there is probably a lot to consider when trying to re-arrange or format them. People do all kinds of stuff in comments like diagrams, post-fix on the same line, prefix to the line above, etc. Could be good to keep some gross examples in mind, or even some as tests to check that we don't accidentally remove them or break them. I'm imagining stuff like: // The following items are all related to the foo bar baz. Blah blah blah.
// This is a Foobar.
struct Foobar {
a: A, // A is a A.
b: B, // TODO: We should change this to a C.
/// D is important and pub.
/// blah blah.
/// -----
/// | D |
/// -----
pub d: D,
}
// This is a foo function.
fn foo(
bar: Bar, // this is a bar
baz: Baz, // this is a baz
// Qux is a little more complicated.
// So I'm writing multiline on top to describe why.
// Blah blah blah
qux: Qux,
) -> /* TODO: Result<Foobar> */ Result<Barfoo> {
} |
|
As I started to explore how I can insert the comments to relevant places without formatting them, I realized that it is going to be somewhat tricky. I am thinking to have a separate PR for the actual comment insert operation and keep this one for only providing the means of comment formatting which will complete these 2 todos in #2122:
Would that make sense @mitchmindtree or should I keep going on this one until we have the complete comment stuff implemented? Also once you are available another brainstorming session about actually inserting the comments would be great for me because as I started to dive deeper into actually inserting them, I started to have some concerns regarding the comments inside a code block. (I feel like I need some sort of mapping between the spans of items before formatting to after formatting, I can do that by re-parsing the formatted code and adding comments in a separate pass after while traversing the AST (that contains formatted code spans) but I feel like there could be a better approach for this) |
kayagokalp
changed the title
sway-fmt-v2sway-fmt-v2
…stream, rename: construct_comment_map to comment_map_from_src
sway-fmt-v2/src/utils/comments.rs
Outdated
| #[test] | ||
| fn test_comment_span_ordering() { | ||
| let first_span = CommentSpan { start: 2, end: 4 }; | ||
| let second_span = CommentSpan { start: 2, end: 6 }; |
There was a problem hiding this comment.
Oh, I just saw your response to my Ordering suggestion. It looks like this test is incorrect - I think the start: 2, end: 6 span should occur before the start: 2, end: 4 span, as I think we want the "visit" order where the enclosing spans precede the enclosed spans. Does that make sense?
Opening up for visibility. This is highly WIP.If we decide to have the actual formatting in a separate PR, this is ready to review.
Related to #2224 and #2122.
To Do