New style guidelines

announcements
21

The same isn't true for me. I've found block indent to be quite popular.

But like I said, this was never a standard style. It was chosen quite arbitrarily. The whole purpose of the formatting RFCs process is to more carefully design a better default style for rustfmt with input from the community.

In general, the new style will lean towards matching existing code as much as possible, but existing code isn't always consistent and I think it was well argued in the block indent issue that there are strong reasons to make it the default.

5 Likes
22

All in all I like the direction of the guidelines rework but jwilm is completely right imho.
The where body is part of the function and thus should be indented in a way that makes this visually obvious.

2 Likes
23

I love this style. It is the one I have always used, except for the "where" clause.
I hate the following style that I find sometimes:

fn very_very_very_very_long_function_name<T1, T2>(
                                                  _first:           T1,
                                                  _second_argument: T2) {
}
very_very_very_very_long_function_name(
                                       34,
                                       3.4);
if 3 < 4 && 4 < 5 && 5 < 6 && 6 < 7 && 7 < 8 && 8 < 9 {
                                                       print!("A");
                                                       print!("B");
}
struct VeryVeryVeryVeryVeryLongStructName<T1, T2> {
                                                   _first:        T1,
                                                   _second_field: T2,
}
let _s = VeryVeryVeryVeryVeryLongStructName {
                                             _first:        34,
                                             _second_field: 3.4,
};

The proposed style has two objective advantages:

  • It minimizes screen space usage, while keeping blocks clear.
  • It minimizes the lines to change when a single line is to be changed. And that is useful when comparing versions, in which only the real differences are highlighted.
2 Likes
24

Why not move where up a line? I usually block format like this:

fn long_function_name(
    some: Arguments,
) -> ReturnType where
    Bounds: AreUpheld,
{
    code()
}

That said, other than where being on its own line striking me as odd, I really like the formatting of the example code overall. Does rustfmt support this style already?

6 Likes
25

I'm happy with the proposed style, and don't mind too much about slight variations to where clauses, etc.

Visual indent would be completely unacceptable to me though, there's no way I'd be able to follow that style were it chosen. I don't think the move away from visual indent was sudden, there was a long thread on the formatting issue, and block indent was favoured by the majority there.

To reiterate a few points against it:

  • Complete lack of good editor support for visual indent, outside of vim
  • Inability to fully specify how visual indent should be applied in practice
  • Unwanted rightward drift caused by visual indent
  • Attempting to solve the above results in a very complex set of rules, which are impossible to follow manually, and still result in nasty edge cases
  • Inconsistent indentation levels
  • Difficulty of refactoring code, cannot simply select a block and tab/untab as desired

Unless the style guidelines can be followed easily without requiring careful consideration, they're not doing their job.

5 Likes
26

Generally :thumbsup: from me. The where is OK. It has to be somewhere, and there's no perfect place.

One question from me. Am I the only one that would change this:

#[derive(Eq, PartialEq, Debug, Copy, Clone)]
/// Formatting tactic for lists. This will be cast down to a
/// DefinitiveListTactic depending on the number and length of the items and
/// their comments.
pub enum ListTactic {

into:

/// Formatting tactic for lists. This will be cast down to a
/// DefinitiveListTactic depending on the number and length of the items and
/// their comments.
#[derive(Eq, PartialEq, Debug, Copy, Clone)]
pub enum ListTactic {

The proper documentation can span for tens of lines and is secondary data (I read it in the browser most of the time), while derive is important and usually much shorter.

16 Likes
27

I do it the second way too, but the first way makes sense to me (the documentation applies to the item, not the meta tags).

28

+1 for putting attributes after docs

One thing noone has commented on:

fn as_ref(&self) -> &ListItem {
    self
}

I really wish there were a way to put simple code blocks on a single line. I really hate how rustfmt turns even the most trivial trait impl into a dozen line monstrosity. In fact, that's one of the main reasons I avoid it.

2 Likes
29

We did consider that style. We agreed that putting where on the same line as a close paren worked fine, but we didn't want to do so when the function had a return type, for two reasons: the where gets lost off to the right (with a more complex return type), and the subsequent bounds then don't have any good separation from the return value so they all blend together.

30

I'd personally prefer having the the where on the same line as the return type. Even if it isn't the default, at least give me an option for it.

pub fn itemize_list<'a, T, I, F1, F2, F3>(
    codemap: &'a CodeMap,
    inner: I,
    terminator: &'a str,
    get_lo: F1,
    get_hi: F2,
    get_item_string: F3,
    prev_span_end: BytePos,
    next_span_start: BytePos,
) -> ListItems<'a, I, F1, F2, F3> where
    I: Iterator<Item = T>,
    F1: Fn(&T) -> BytePos,
    F2: Fn(&T) -> BytePos,
    F3: Fn(&T) -> Option<String>,
{
    ListItems {
        codemap: codemap,
        inner: inner.peekable(),
        get_lo: get_lo,
        get_hi: get_hi,
        get_item_string: get_item_string,
        prev_span_end: prev_span_end,
        next_span_start: next_span_start,
        terminator: terminator,
    }
}

Also, the example doesn't have any multiline imports. How would you format this for example?

use um::d3dcommon::{
    D3D_CBUFFER_TYPE, D3D_FEATURE_LEVEL, D3D_INTERPOLATION_MODE, D3D_MIN_PRECISION, D3D_NAME,
    D3D_PARAMETER_FLAGS, D3D_PRIMITIVE, D3D_PRIMITIVE_TOPOLOGY, D3D_REGISTER_COMPONENT_TYPE,
    D3D_RESOURCE_RETURN_TYPE, D3D_SHADER_INPUT_TYPE, D3D_SHADER_VARIABLE_CLASS,
    D3D_SHADER_VARIABLE_TYPE, D3D_SRV_DIMENSION, D3D_TESSELLATOR_DOMAIN,
    D3D_TESSELLATOR_OUTPUT_PRIMITIVE, D3D_TESSELLATOR_PARTITIONING, ID3DBlob,
};
5 Likes
31

Looks good. I personally prefer:

fn needs_trailing_separator(separator_tactic: SeparatorTactic,
                            list_tactic: DefinitiveListTactic,
) -> bool {
    // code
}

over

fn needs_trailing_separator(
    separator_tactic: SeparatorTactic,
    list_tactic: DefinitiveListTactic,
) -> bool {
    // code
}

because it's easier to visually separate arguments from a function body. But the second one is still easier to edit and it will lock better in diff.

Also, attributes should be after comments.

32

I'm also in favor of the "indented where" style mentioned by @stebalien.
To me it seems a lot easier to scan the signature when where isn't at the same indentation level as fn.

1 Like
33

FWIW, this is pretty close to my current style, so my heavily biased opinion is that I like it. As a broad default, I'm happy that it doesn't involve having to use a limited set of editors, doesn't require constant alignment fiddling, and looks diff-friendly. As a personal style, I'll probably still do things a little differently.

(Also, I hope attributes-before-docs is an oversight, because whilst I have no technical objection, it looks bonkers.)

3 Likes
34

I see a lot of negative reaction about it in this thread, but I do like the where clause, and more generally (consistent) use of block indentation. For me it is a huge improvement over the previous style.

5 Likes
35

Good improvement in my opinion, I still dislike the following style (too much right drift) but I can live with it.

        let context = Arc::new(GlobalContext {
                                   node: node,
                                   addr: addr,
                                   loop_remote: remote,
                                   nodes_addr: Default::default(),
                                   msg_handlers: Default::default(),
                                   writer_chans: Default::default(),
                                   chan_id_gen: AtomicUsize::new(0),
                               });
3 Likes
36

I had someone write this function for me and I like its style

fn apply<A, B, C, F, G>(mut f: F, a: A) 
-> impl FnMut(&B) -> C // must still be `for<'r> impl FnMut(&'r B) -> C`, because that’s what filter requires
        where F: FnMut(B) -> G, // must not be `for<'r> FnMut(&'r B) -> G`, because regular functions do not implement it
             G: FnMut(A) -> C,
             B: Copy, // for dereferencing
             A: Clone {
    move |b| f(*b)(a.clone()) // this must do any bridging necessary to satisfy the requirements
}

I see the style guide differs greatly from it, especially regarding where clauses

37

My first two expectations are neat and easy to read, and it meets those, so +1.

But flexibility is very important in achieving a neat style, e.g. sometimes inline functions are the best option:

impl ... {
    fn name(&self) -> &str { &self.name }
    fn number(&self) -> u32 { self.number }
}

Recently a colleague complained that he wanted a visitor instead of a full-blown for loop. I pointed out that the latter is just as simple, if inlining is allowed:

my_vec.iter().visit(|x| println!("{}", x));
for x in my_vec { println!("{}", x); }

Sometimes just a few extra/less spaces make code look much neater:

v[i  ] = f(x, i);
v[i+1] = g(x, i);
v[i+2] = h(x, i);

Finally, wouldn't it be much more useful to implement a semantic diff tool before tying down "best" formatting practices? I'm so tired of diffs like this:

1,4c1
< fn needs_trailing_separator(
<     separator_tactic: SeparatorTactic,
<     list_tactic: DefinitiveListTactic,
< ) -> bool {
---
> fn needs_trailing_separator(separator_tactic: SeparatorTactic, is_vert: bool) -> bool {
7c4
<         SeparatorTactic::Vertical => list_tactic == DefinitiveListTactic::Vertical,
---
>         SeparatorTactic::Vertical => is_vert,
4 Likes
38

In the same vein of avoiding the blend between argument/return type and where clause, I've started to add more one line to my functions declarations when they are already on multiple lines:

// Inline arguments, but not enough space for return type!
fn do_something_smart(arg1: Arg1, arg2: Arg2)
    -> ReturnType
{
}

// Not enough space for inline arguments!
fn do_something_clever<T, U>(
    arg1: T,
    arg2: U
)
    -> ReturnType
where
    T: Arg1,
    U: Arg2
{
}

I find that when return type get a bit more complex, having a clear delineation between arguments and return type made easier to spot it, whereas when it is "stuck" to the argument list, it doesn't stand out as much.

I'll admit that I find the where clause jarring, but once I get past the skin-deep reaction, I like that the indentation is consistent with the rest of the language instead of being ad-hoc. I regularly use "dumb" text editors for quick fixes and having to spam "space" to fiddle out with the alignment is annoying, with this block indent style I just have to set up "tab" to put 4 spaces and in a single press of a key, done.

39

I like the proposed style, but I would like to have some specific rules on when to use multi-line args. Is it simply when the line exceeds or is it optional etc.?

40

Also, I wonder how you break long strings. This is not shown in the example file.