Regression: wrap_comments with a markdown list

[whatisaphone]

I have this list in a module doc comment:

//! * [`examples/simple`] – Demonstrates use of the [`init`] API with plain
//!   structs.
//! * [`examples/simple_flatbuffer`] – Demonstrates use of the [`init`] API with
//!   FlatBuffers.
//! * [`examples/gravity`] – Demonstrates use of the [`RLBot::set_game_state`]
//!   API

Recently, this diff appeared in CI:

 //! * [`examples/simple`] – Demonstrates use of the [`init`] API with plain
 //!   structs.
 //! * [`examples/simple_flatbuffer`] – Demonstrates use of the [`init`] API with
-//!   FlatBuffers.
+//! FlatBuffers.
 //! * [`examples/gravity`] – Demonstrates use of the [`RLBot::set_game_state`]
 //!   API

For some reason rustfmt decided that the middle item, and only the middle item, should not indent past the bullet point.

Unfortunately I don't work on this crate too often so I don't have a good idea of when it regressed. The last successful build used the nightly from Nov 3. The failure used today's nightly.

[scampi]

Do you have some options set in your rustfmt.toml ?

[whatisaphone]

Yes. wrap_comments = true is probably the only relevant one. I can reproduce the issue with just these two files:

rustfmt.toml

wrap_comments = true # Commenting this solves the issue

lib.rs

//! * [`examples/simple`] – Demonstrates use of the [`init`] API with plain
//!   structs.
//! * [`examples/simple_flatbuffer`] – Demonstrates use of the [`init`] API with
//!   FlatBuffers.
//! * [`examples/gravity`] – Demonstrates use of the [`RLBot::set_game_state`]
//!   API

And here's my version which should have gone in the top post. I think I did all the right steps to update fully, but I'm sitting at ~2 weeks ago:

$ rustup update nightly
  nightly-x86_64-pc-windows-msvc updated - rustc 1.32.0-nightly (d09466ceb 2018-11-30)

$ rustup component add rustfmt-preview --toolchain nightly
info: component 'rustfmt-preview' for target 'x86_64-pc-windows-msvc' is up to date

$ rustup run nightly rustfmt --version
rustfmt 1.0.0-nightly (1cc61cfc 2018-11-19)

[scampi]

@whatisaphone thanks for the extra information. I wrote that comment while asleep because I didn't try the obvious option! If you want there's an open PR with a fix you can try out.

With your example, something funny came out: the dash in the flatbuffer line is not a dash: see on the playground. A "normal" dash is 1-byte, however yours is 3-bytes. It does look kinda longer than what is in --version but it didn't strike me at first.

@nrc this difference in bytes length had a negative impact in the wrap_str method: the extra 2 bytes made the line go over the threshold, hence nullifying what rewrite_string did. The rewrite_string method is based on graphemes: a grapheme is closer to what a person may expect a character to be, regardless of the number of bytes it takes.

I'm wondering if we should use graphemes throughout the project, in order to avoid problems like above ? That way each rewrite operation would be more "realistic".
I think it makes sense for graphemes to be only used in rewrite_string: one wouldn't expect some fancy characters used in, e.g., a variable. However, other parts like comments could have such characters, especially I guess for non-european languages.

[whatisaphone]

Sorry for waking you up! :) Yep, it's an en-dash – gotta keep the docs pretty.

I tested your PR and it works, thanks for the fix!

I think I'm with you on graphemes. As an lowly user, my expectation is that the characters wrap based on whatever my editor displays. I just tried a few editors and they all display both these lines identically:

fn main() {
    println!("{}", "é".len());
    println!("{}", "é".len());
}

(although it breaks editing on play.rust-lang.org, that was an interesting bug)

[scampi]

@whatisaphone I don't know what's that character but it's messing my editor too!

[nrc]

I'm wondering if we should use graphemes throughout the project

We absolutely should! We tried to move off bytes years ago and anywhere they're are left is a bug (though there are rather a lot of such places).