Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fixed SwayFmt removing comments in configurable blocks #5297

Closed
wants to merge 13 commits into from
Closed

Fixed SwayFmt removing comments in configurable blocks #5297

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
6636598
Added test for config block case
brandonsurh Nov 17, 2023
8fb5a12
save debug logs
brandonsurh Nov 21, 2023
a3fd251
removed debugging logs and methods
brandonsurh Nov 21, 2023
0519bc4
cleaned up test
brandonsurh Nov 21, 2023
ed998df
Merge branch 'FuelLabs:master' into config_block_comments
brandonsurh Nov 21, 2023
c94d396
removed whitespace
brandonsurh Nov 21, 2023
f02b929
Merge branch 'config_block_comments' of https://github.com/brandonsur…
brandonsurh Nov 21, 2023
67ea4c1
removed whitespace
brandonsurh Nov 21, 2023
b8a37db
Failing testcase for testing purposes
brandonsurh Nov 22, 2023
6b2acff
Finalized test case with inline comment
brandonsurh Nov 22, 2023
d45ff0b
Merge branch 'master' into config_block_comments
brandonsurh Nov 27, 2023
3e1562d
Merge branch 'FuelLabs:master' into config_block_comments
brandonsurh Dec 19, 2023
9a2fa74
added test comments and changed names
brandonsurh Dec 19, 2023
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
11 changes: 11 additions & 0 deletions swayfmt/src/items/item_configurable/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::{
comments::rewrite_with_comments,
config::user_def::FieldAlignment,
formatter::{
shape::{ExprKind, LineStyle},
Expand Down Expand Up @@ -27,6 +28,8 @@ impl Format for ItemConfigurable {
.shape
.with_code_line_from(LineStyle::Multiline, ExprKind::default()),
|formatter| -> Result<(), FormatterError> {
// Required for comment formatting
let start_len = formatted_code.len();
// Add configurable token
write!(
formatted_code,
Expand Down Expand Up @@ -119,6 +122,14 @@ impl Format for ItemConfigurable {
// Handle closing brace
Self::close_curly_brace(formatted_code, formatter)?;

rewrite_with_comments::<ItemConfigurable>(
formatter,
self.span(),
self.leaf_spans(),
formatted_code,
start_len,
)?;

Ok(())
},
)?;
Expand Down
34 changes: 34 additions & 0 deletions swayfmt/tests/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1608,6 +1608,40 @@ impl MyContract for Contract {
);
}

#[test]
fn configurable_comments() {
check(
r#"
script;

use std::{constants::ZERO_B256, vm::evm::evm_address::EvmAddress};

configurable {
// config test
/// config test triple
SIGNER: EvmAddress = EvmAddress {
value: ZERO_B256,
Copy link
Member

Choose a reason for hiding this comment

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

it would be also useful to test adding comments inside of structs and enums in configurable blocks, and at the end of the line.

Suggested change
value: ZERO_B256,
// comment here
// multiline!
value: ZERO_B256, // end of line too

},
}

fn main() {}"#,
r#"script;

use std::{constants::ZERO_B256, vm::evm::evm_address::EvmAddress};

configurable {
// config test
/// config test triple
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for adding tests! It would be good to test inline comments as well, e.g. /* inline test */

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, good catch! That style of comment is coming out a little strange after testing it just now. Weird behaviors with whitespace. Gonna dig into this a little more 👍

Copy link
Contributor Author

@brandonsurh brandonsurh Nov 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

After digging into the behaviors a little more, I discovered that this strange whitespace formatting behavior exists across other various items. I have put detailed it in this issue: #5298. Seems like what I found may be outside of the scope of this issue.

I added the /* inline test */ comment and it still exists after the formatting. However, the whitespace around it is a different thing. I had to form the testcase around this for it to pass.

Let me know what you think!

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for opening the issue. I agree that it's out of scope for this issue, but it would be great to have that test case added as part of #5298

SIGNER: EvmAddress = EvmAddress {
value: ZERO_B256,
},
}

fn main() {}
"#,
);
}

#[test]
fn empty_fn() {
check(
Expand Down
Loading