-
Notifications
You must be signed in to change notification settings - Fork 55
Decide on guiding principles #4
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
On that last point: a fmt RFC may sometimes require an analogous change to rustdoc to keep formatting consistent. |
Another related item:
|
It seems like settling on how to weigh the "ease of manual formatting" is a key question that will affect a lot of things. I know that my own style has been heavily influenced by the fact that emacs basically does it all for me automatically (and, in general, if emacs doesn't do what I like, then I decide to like what emacs does, or else tweak things a bit to get it to do something I like more). When for a brief period I I guess that even though I mostly prefer visual indent, I am forced to admit that ease of manual formatting may be better overall, since there are so many editors out there which people like to use, and most of them don't have sophisticated support (plus we sometimes have to edit code in annoying places). That said, since we have rustfmt, it can be the "tool to rule them all" to some extent and correct for the shortcomings of most editors. |
@nikomatsakis If rustfmt produces output that everyone can accept with approximately zero exceptions, and people format their code with rustfmt as standard practice, then that will address any issues of formatting inconsistency. That said, I do think that the preferred style should come naturally, and should not prove difficult to manually format while entering code. It's reasonable to expect people writing a non-trivial amount of code to have editor support, but it's not reasonable to expect people to follow a style that nobody would come up with on their own without reference to the style guide. The job of the style guide is to make the call on various individual formatting bikeshed decisions that could easily go either way, not to define an elaborate non-trivial style that nobody would have guessed naturally. That raises one key question about rustfmt, though: do we expect that rustfmt will always ignore all existing whitespace (including newlines) and re-break all lines as it sees fit, or do we expect that it will take existing newlines and alignment into account? If the former, rustfmt will break code that has carefully inserted line-breaks and visual alignment. If the latter, then rustfmt can produce different results for the same series of tokens depending on its existing indentation. Neither one of those seems like an entirely satisfying result. |
We experimented quite a bit with this, my conclusion was that Rustfmt should basically ignore any existing formatting - it is simply too difficult to integrate existing formatting with new formatting, so either we do no formatting or all formatting. Two caveats: we should avoid formatting comments and string literals too much because of this problem. It would be nice to word wrap comments, but doing this without ruining existing formatting proved very hard. There should be an opt-out - e.g., |
Agreed. We might be able to handle comments, if we assumed that alignment of items within comments would only occur within a Markdown code block or otherwise managed by Markdown formatting (such as a list). I don't know that we can assume that, though.
That seems reasonable, assuming we can scope it to apply as narrowly as necessary. (And we should take any occurrence of it as a challenge: "could rustfmt be smarter?".) |
One more guiding principle:
|
I disagree, I think there should at least be an option for this, for considering existing newlines. Imagine for example you have a function like this:
And you want to keep Formatting and ignoring existing newlines is likely to result in a more disjointed grouping of parameters:
Assuming the tool is not using ubsan's style that is. But even formatting under ubsan's style (one parameter per line) might not as good as the original code. |
@bruno-medeiros If rustfmt can't even rearrange arguments, that really limits its usefulness as a tool. Personally I don't think it's worth having an arbitrary escape hatch in the style rules for when a programmer feels things should be grouped in a non-standard way. It's more predictable if it's always standard. |
At first, I wanted to suggest counterexamples, where grouping arguments makes sense: coordinates of a point, dimensions of a rectangle, pointer and length of an array...until I realized that every one of those examples either wants a tuple or a dedicated data type. I can't actually think of a single good example where it makes sense to group options without creating an appropriate data type or using a tuple. |
From #2, @nikomatsakis:
|
I would like you to consider accessibility as a key principle. Specifically, I am not even considered "low vision" by any standard but in order to sit at my laptop all day long I have to have my font size set such that GitHub's side-by-side diffs barely fit two 80-column panes next to each other and such that my 3-way-merge tool doesn't even get close to showing 80 columns of all three panes. I've tried to make 100-character columns work in my projects since that is what people seem to prefer, but I physically cannot do it in a reasonably way, which is why I use an atypical-for-Rust 80-character max in my projects. |
@briansmith First, I'd like to completely agree with your suggestion of accessibility as a key principle. Rust's coding standards should not cater primarily to people with great vision, small fonts, or large/multiple monitors. FWIW, I don't think the majority of users have fonts or terminals that allow three 80-column windows next to each other, and many don't even have setups that allow two 80-column windows. Some users might, but most don't. I have a 2560x1440 monitor, and I still only have 196 columns in my terminal; if I use the next font size up, I have 159 columns. Many users will code on 1920x1080 or 1366x768 monitors. I don't think a three-way merge with three vertical columns is a reasonable argument for either approach, as only people with huge high-resolution monitors or tiny fonts will be able to fit 242 columns, let alone 302 columns. Personally, I do a three-way merge with old and new side-by-side above the working/editing window. I also don't tend to find it problematic when a line exceeds the width of the window; I find my editor's presentation of such a line reasonable. I also consider smaller screens a good argument for 4-space indents: I find that the wider indent makes it much easier to keep track of indentation levels when paging through a block of code that you can't see all at once. |
@briansmith I'd argue that 80-characters is not atypical for Rust, it's just not standard. |
I think this is an unnecessary goal that has harmful consequences. A good diff tool can highlight the diff in the "right" way that allows one to see differences caused by (re)formatting vs. other differences. Google's gerrit code review tool seems to do this, for example, and so does SourceGear DiffMerge. GitHub and other tools can be improved in this area; it's not worth optimizing for their current deficiencies at the cost of other factors, in particular the other principle of avoiding wasting vertical space. |
@briansmith I'd like to say "git diff" made smarter, sure. But as long as it isn't, it's worth at least considering what it doesn't handle. That doesn't mean that consideration will always win, just that it shouldn't be dismissed out of hand as a useful criteria. |
Keep in mind that the limitations of |
This also affects merge conflicts. @briansmith are the existing good merging tools capable of ignoring whitespace changes so e.g. a single-line conflict does not get amplified due to formatting differences?
At a point when a bigger number of tools are good and make that kind of reformatting transparent, maybe? Along the lines of other considerations
I'd find it nice if the style did not leave users with less advanced/popular/perfectly configured tools in the cold. |
That's a good question and I don't know the answer. I know that SourceGear DiffMerge can automatically resolve more merges than |
Changing more unrelated lines of code does tend to break merges, rebases, patch application, and various other tools. Some cleverer tools might manage to avoid that, but I still think "don't cause rippling changes outward from a single modification" seems like a reasonable consideration to keep in mind. |
I have attempted to condense the list of possible principles from throughout this thread, ordering is my personal opinion on prioritisation:
And there was one which I didn't really understand:
How do people feel about these principles? |
Some commentary on the above list: I hope it is obvious why the readability issues are most important. In particular I feel scannablility of code is important - I rapidly get frustrated with a style if it prevents me quickly navigating around code without reading too deeply. I think aesthetics are important because I spend a lot of time looking at code and I don't want to be repulsed by ugliness when I do so. I think it is easy to shy away from this because it is difficult to be objective, but just because something is subjective does not mean it should be ignored. We should, however, be aware of out prejudices, in particular, familiarity. We have strived hard in designing the syntax of Rust to be familiar to as many programmers as possible, without sacrificing functionality. I believe we should do so here too. It is good for Rust as a whole if many programmers can come to Rust code and feel familiar with the style. I realise VCS interaction is important to many people and I believe we should take it into account. But primarily, code is read by humans, not tools, and we spend a lot more time reading code than merging it or exploring VCS repos, therefore I think we should not sacrifice either readability or aesthetics for VCS compatibility. Likewise, we should minimise use of horizontal and vertical space, but not at the expense of readability. In particular, I find that styles which are quite 'spacey' vertically are much easier to read and scan than dense code. I feel it is important for the code to have a shape. Finally, I think that ease of implementation should not be taken too much into account, with the caveat that it must be possible to implement. Ideally it is easy to apply the style manually, but I find this very hard to quantify - what is manual? Every editor has different tools here, so it is hard to judge. Furthermore, if using Rustfmt at all, you can just type any old format and then apply Rustfmt. Internal consistency should be a goal where possible, but I'm happy to override wherever necessary to provide a better user experience. |
"consistency between code format and documentation format" referred to ensuring that rustdoc documentation matches the corresponding code. |
I agree completely with the above principles, and almost completely with the ranking. I'd put "simplicity of formatting rules" at the top of "application". Apart from that, 👍 |
as in the code generated by Rustdoc? |
Right, as well as code included in doc comments that gets propagated into rustdoc. |
One thing I don't think you've touched on is editability. Code should be easy to modify, while retaining the current style. A style that involves lots of precise alignment, or indenting, may be hard to modify in a simple editor (for example, a quick GitHub edit). Running rustfmt at the end to fix up small style issues is fine (comment wrapping, small indentation mistakes), but you shouldn't need to continuously run it if the style is too hard to manually write with. |
@keeperofdakeys this is part of what I meant by "ease of manual application" |
At least in Go land, and with hindent for Haskell, you just do |
This is a micro vs. macro issue. A specific line of code might be easier to read if it is split across N lines. But, a function is also easier to read if one can read it without scrolling, and a set of related functions is easier to read if they all fit on the screen together. The idea of minimizing usage of vertical space is to optimize for these macro readability concerns, at the cost of the micro concerns. |
@joshtriplett @nrc Humans just aren't built to think about the world without vision, so everyone comes up with their own coping strategies. Python is a perfect concrete example of this. I use Python fine and like it, but there are many new blind programmers who claim that Python is inaccessible to blind people because of the indent thing, and I've got at least 2 friends who don't set the screen reader to indicate indentation while coding Python because they can just infer where and what it should be (me and another 2 use indent with everything because we can't and find it helpful to match braces). Then the tabs vs. spaces argument takes on a whole new life, with some of us siding with spaces because the sighted people use it and some of us siding with tabs because that makes the indent level exactly equal to the level of the block in terms of (metaphorical or otherwise) braces. And then you get into stuff like the doc comments which I hate but can live with, etc. CC me on discussions where it comes up and I'll monitor them,. Not too sure we have many other blind programmers around here, and I wish we did for this. One person isn't consensus. |
The reason I put it at the bottom, is that for me, concrete factors that affect users directly (e.g., ease of manually applying) are more important than abstract factors that affect users indirectly (why is simplicity important? For me, because it enables ease of application and implementation. Simplicity for its own sake (i.e., not covered by the higher priority points) is relatively low priority.
SGTM, modulo @camlorn's comment |
I believe the guidelines in #4 (comment), modified with #4 (comment) are ready to go. I'm thus labelling this issue as being in FCP to give a last chance to comment on our guiding principles. |
|
I agree there is a trade-off here and I do believe in trying to minimise vertical space for this reason. However, I still think it is lower priority since I think speed of scanning is more important than speed of detailed reading (the latter is always slow) and for scanning, once you start scrolling, I think the amount you must scroll is less important. I also think that as time passes and larger, higher density displays are more common, plus tools that make side-by-side viewing easier, minimising vertical space is getting less important. |
The only time this has happened to me is when I'm building a command-line, because those frequently intermix key/value and positional arguments:
But it's a pretty niche use-case, especially for Rust, and the benefits of "just use rustfmt" would outweigh the readability improvements. |
We should decide on the principles behind the formatting guidelines and their relative priority.
Some ideas (not in order):
The text was updated successfully, but these errors were encountered: