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.

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

Comments with /* */ have whitespace issues when formatting #5298

Open
brandonsurh opened this issue Nov 22, 2023 · 2 comments
Open

Comments with /* */ have whitespace issues when formatting #5298

brandonsurh opened this issue Nov 22, 2023 · 2 comments

Comments

@brandonsurh
Copy link
Contributor

brandonsurh commented Nov 22, 2023

This was discovered during the progress of #5297.

So it seems that formatting comments such as /* inline test */ is a little finnicky within swayfmt. The comments seem to get shifted around in various ways depending on the category that they are identified as. This issue persists across all the items I have tested so far.

It is possible that the whitespace formatting I see is desired, though I wouldn't expect it to be as the items treat the same comments differently.

Useful commit to pull for testing here.

Example:

Behavior for item_abi and item_impl:
Before:

script;

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

abi MyContract {
    // double slash comment
    /// triple slash comment
    /* inline comment */
    /*
        multiline comment
    */
    fn test_function() -> bool;
}

impl MyContract for Contract {
    // double slash comment
    /// triple slash comment
    /* inline comment */
    /*
        multiline comment
    */

    fn test_function() -> bool {
        true
    }
}

After:

script;

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

abi MyContract {
    // double slash comment
    /// triple slash comment /* inline comment */
    /*
        multiline comment
    */
    fn test_function() -> bool;
}

impl MyContract for Contract {
    // double slash comment
    /// triple slash comment /* inline comment */
    /*
        multiline comment
    */
    fn test_function() -> bool {
        true
    }
}

Behavior for item_enum and item_configurable:
Before:

enum Color {
    // double slash comment
    /// triple slash comment
    /* inline comment */
    /*
        multiline comment
    */
    blue: (),
    red: ()
}

configurable {
    // double slash comment
    /// triple slash comment
    /* inline comment */
    /*
        multiline comment
    */
    SIGNER: EvmAddress = EvmAddress {
        value: ZERO_B256,
    },
}

After:

enum Color {
    // double slash comment
    /// triple slash comment
/* inline comment */    /*
        multiline comment
    */    blue: (),
    red: (),
}

configurable {
    // double slash comment
    /// triple slash comment
/* inline comment */    /*
        multiline comment
    */    SIGNER: EvmAddress = EvmAddress {
        value: ZERO_B256,
    },
}

The implementation for item_enum and item_configurable are the same. I'm assuming this is why they have the same behavior.

Also, if I try to place the same comments inside of fn test_function() then the test fails with:

running 1 test
thread 'configurable_comments' panicked at 'called `Result::unwrap()` on an `Err` value: ParseFileError(ParseFileError([Parse { error: ParseError { span: Span { src (ptr): 0x7f929c002290, source_id: None, start: 478, end: 502, as_str(): "/// triple slash comment" }, kind: ExpectedExpression } }]))', swayfmt/tests/mod.rs:8:86
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
test configurable_comments ... FAILED

failures:

failures:
    configurable_comments

This is possibly a duplicate as we fix formatting issues regarding comments, but I couldn't find anything about this in particular.

@sdankel
Copy link
Member

sdankel commented Nov 27, 2023

Thanks for digging in! How does this behavior compare to cargo fmt's handling of comments?

@brandonsurh
Copy link
Contributor Author

Just did some testing and those whitespace changes don't exist with cargo fmt. At least not when formatting enum and impl blocks. To save you the trouble, the before and after blocks below are exactly the same.

Before:

enum IpAddrKind {
    // double slash comment
    /// triple slash comment
    /* inline comment */
    /*
        multiline comment
    */
    V4,
    V6,
}

impl Rectangle {
    // double slash comment
    /// triple slash comment
    /* inline comment */
    /*
        multiline comment
    */
    fn test(&self) -> u32 {
        // double slash comment
        /// triple slash comment
        /* inline comment */
        /*
            multiline comment
        */
        10
    }
}

fn main() {}

After:

enum IpAddrKind {
    // double slash comment
    /// triple slash comment
    /* inline comment */
    /*
        multiline comment
    */
    V4,
    V6,
}

impl Rectangle {
    // double slash comment
    /// triple slash comment
    /* inline comment */
    /*
        multiline comment
    */
    fn test(&self) -> u32 {
        // double slash comment
        /// triple slash comment
        /* inline comment */
        /*
            multiline comment
        */
        10
    }
}

fn main() {}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants