Skip to content

// Reformated to // even with normalize_comments = true #3209

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

Closed
D4nte opened this issue Nov 18, 2018 · 8 comments
Closed

// Reformated to // even with normalize_comments = true #3209

D4nte opened this issue Nov 18, 2018 · 8 comments
Labels
only-with-option requires a non-default option value to reproduce poor-formatting

Comments

@D4nte
Copy link

D4nte commented Nov 18, 2018

Hi,

When activating wrap_comments = true, whether or not normalize_comments is set to true, the following happens:

Before:

    // Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.

After:

    /* Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt
     * ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation
     * ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in
     * reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur
     * sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id
     * est laborum. */

What I expected:

    // Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt
    // ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation
    // ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in
    // reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur
    // sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id
    // est laborum. */

Version:

cargo fmt --version
rustfmt 0.99.6-nightly (750b2526 2018-10-18)
@D4nte
Copy link
Author

D4nte commented Nov 18, 2018

It also happens when format_doc_comments = true. Is that expected? It does not see to be when checking the doc.

@nrc nrc added poor-formatting only-with-option requires a non-default option value to reproduce labels Nov 19, 2018
@scampi
Copy link
Contributor

scampi commented Nov 22, 2018

@D4nte I cannot reproduce this on master, whatever is the value of normalize_comments I get comments with // as the sigil.
Can you share the options you have activated ? Maybe there is a clash with other things that trigger what you have.

@D4nte
Copy link
Author

D4nte commented Nov 26, 2018

I just install cargo fmt from master and ran it.

    // Do we want to remember already generated addresses or regenerate them?
    // Memory vs CPU -> could be a switch/option
    // Common practice for wallets is to pre-generate some addresses, hence:
    // TODO: manage a key pool
    // - key ready for use (pool)
    // - key already used

Changed to

    /* Do we want to remember already generated addresses or regenerate them?
     * Memory vs CPU -> could be a switch/option
     * Common practice for wallets is to pre-generate some addresses, hence:
     * TODO: manage a key pool
     * - key ready for use (pool)
     * - key already used */

I use a rustfmt.toml for options, here is the full content:

condense_wildcard_suffixes = true
format_macro_matchers = true
merge_imports = true
use_field_init_shorthand = true
format_doc_comments = true
normalize_comments = true
wrap_comments = true

Thank you for your help!

@scampi
Copy link
Contributor

scampi commented Nov 26, 2018

I am still unable to reproduce what you have. Could you share the file in which that happens ?

To test, I do the following:

  • run cargo run --bin rustfmt -- --check 3209.rs from master (4609252)

  • the file 3209.rs is as follows:

    // Do we want to remember already generated addresses or regenerate them?
    // Memory vs CPU -> could be a switch/option
    // Common practice for wallets is to pre-generate some addresses, hence:
    // TODO: manage a key pool
    // - key ready for use (pool)
    // - key already used
    fn foo() {}
  • nothing gets diffed, and removing the --check option does not have any effect on the file either.

@D4nte
Copy link
Author

D4nte commented Nov 28, 2018

Hi,

I was able to reproduce with a single file. See below. Thank you for investigating this!

✔ ~/src/rustfmt [master|✚ 1…1]
08:18 $ git log |head -n9
commit bbc380b1e6b0e41042c8c259dfad49cea7af0b35
Merge: 70fe8ee3 7fb5c0b0
Author: Nick Cameron <[email protected]>
Date:   Wed Nov 28 15:29:18 2018 +1300

    Merge pull request #3221 from alexreg/cosmetic-1

    Cosmetic improvements

✔ ~/src/rustfmt [master|✚ 1…1]
08:18 $ cat 3209.rs
#[derive(Debug)]
pub struct KeyStore {
    _master_privkey: String,
    transient_root_privkey: String,
    internal_root_privkey: String,
    // TODO: replace with AtomicU32 once stable https://doc.rust-lang.org/std/sync/atomic/struct.AtomicU32.html
    next_internal_index: Mutex<u32>,
    // Do we want to remember already generated addresses or regenerate them?
    // Memory vs CPU -> could be a switch/option
    // Common practice for wallets is to pre-generate some addresses, hence:
    // TODO: manage a key pool
    // - key ready for use (pool)
    // - key already used
}

fn main() {
    println!("Hello, world!");
}
✔ ~/src/rustfmt [master|✚ 1…1]
08:18 $ cat rustfmt.toml
condense_wildcard_suffixes = true
format_macro_matchers = true
merge_imports = true
use_field_init_shorthand = true
format_doc_comments = true
normalize_comments = true
wrap_comments = true
✔ ~/src/rustfmt [master|✚ 1…1]
08:18 $ cargo +nightly run --bin rustfmt -- 3209.rs --check
    Finished dev [unoptimized + debuginfo] target(s) in 0.21s
     Running `target/debug/rustfmt 3209.rs --check`
Diff in /Users/froyer/src/rustfmt/3209.rs at line 5:
     internal_root_privkey: String,
     // TODO: replace with AtomicU32 once stable https://doc.rust-lang.org/std/sync/atomic/struct.AtomicU32.html
     next_internal_index: Mutex<u32>,
-    // Do we want to remember already generated addresses or regenerate them?
-    // Memory vs CPU -> could be a switch/option
-    // Common practice for wallets is to pre-generate some addresses, hence:
-    // TODO: manage a key pool
-    // - key ready for use (pool)
-    // - key already used
+    /* Do we want to remember already generated addresses or regenerate them?
+     * Memory vs CPU -> could be a switch/option
+     * Common practice for wallets is to pre-generate some addresses, hence:
+     * TODO: manage a key pool
+     * - key ready for use (pool)
+     * - key already used */
 }

 fn main() {

@scampi
Copy link
Contributor

scampi commented Nov 28, 2018

I didn't try with the block comment after the struct's fields in my tries! thanks for providing a reproducible example ;o)

@scampi
Copy link
Contributor

scampi commented Nov 28, 2018

@D4nte That is actually done on purpose, see #93 for some more context.

@D4nte
Copy link
Author

D4nte commented Dec 11, 2018

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
only-with-option requires a non-default option value to reproduce poor-formatting
Projects
None yet
Development

No branches or pull requests

3 participants