style: Wrap comments at 100 characters#1268
Conversation
davidlattimore
left a comment
There was a problem hiding this comment.
Thanks. I think this is a good idea.
I recently (as in yesterday) disabled rebases on the repo. My motivation was that if we always squash, then it's easier to track commits back to their PRs, since github adds the PR number to the commit message. So perhaps we could add .git-blame-ignore-revs in a separate PR once this one is merged?
| // Because we use the index that gets pushed, we ignore the bytes of the later | ||
| // instructions, so that we support these variants. | ||
| // Note: Some variants use jmp instead of bnd jmp, then a different padding | ||
| // instruction. Because we use the index that gets pushed, we ignore |
There was a problem hiding this comment.
Some of the wrapping decisions that it makes are a bit weird. "the bytes of" could have stayed on this line. On the up-side, if I reflow this paragraph with my editor, rustfmt doesn't undo my change. Perhaps you could look for any that stand out as being weird and reflow them?
| // GCC unfortunately doesn't provide any way to use a custom linker. | ||
| // Their flag for switching linkers only accepts a hard-coded list | ||
| // GCC unfortunately doesn't provide any way to use a custom | ||
| // linker. Their flag for |
| imports_granularity = "Item" | ||
| group_imports = "One" | ||
| wrap_comments = true | ||
| comment_width = 120 |
There was a problem hiding this comment.
I've used 100 as the wrapping width for comments since the start of the project. I've mostly used the rewrap extension in vscode to do this. So I think it'd be good to stick to what has been (mostly) used to date. Also, the default code width in rustfmt is 100, so it's good to be consistent with that.
Interestingly, when I just tried different settings for this, 120 and 100 produced exactly the same result. As far as I can tell, it's capping comment_width at the value of max_width (100).
Sure, we can do that.
Ah, I normally use Rustfmt to reflow my comments, because it's very good at it, and understands markdown and code embedded in markdown. |
|
To fix the weirdly flowed cases, I put all the text onto the same line and then re-ran Rustfmt. It then produced sensible wrapping (diff) - including putting "the bytes of" in the right place. I don't know why this is. |
This is a small stylistic change. I normally rely on Rustfmt's comment wrapping feature. Even though it's not a stable feature, it has worked well and stably for the last few years. Also, I added a second commit so that the changes affecting these 28 files can be excluded from
git blame. See also: https://www.stefanjudis.com/today-i-learned/how-to-exclude-commits-from-git-blame/GitHub respects this file by default:
