Skip to content
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

Statements #11

Closed
nrc opened this issue Sep 21, 2016 · 19 comments
Closed

Statements #11

nrc opened this issue Sep 21, 2016 · 19 comments

Comments

@nrc
Copy link
Member

nrc commented Sep 21, 2016

Decide on rules for statements (deliberately excluding expressions and items for now)

In general, statements should be on their own line and any comments should be on the previous line. Avoid comments inside statements. Attributes are left for another issue.

let declarations

The formatting of patterns, types, and expressions is left for other issues. There should be spaces after the : and on both sides of the = (if they are present). No space before the semi-colon.

// A comment.
let pattern: Type = expr;

let pattern;
let pattern: Type;
let pattern = expr;

If possible the declaration should be formatted on a single line. If this is not possible, then try splitting after the =, if the declaration can fit on two lines. The expression should be block indented.

let pattern: Type =
    expr;

If the first line does not on a single line, then split after the colon, both the type and expression are block indented (and start new block indenting contexts) If pattern spans multiple lines, then it should be visually indented:

let pattern:
    Type =
    expr;

e.g,

let Foo {
        f: abcd,
        g: qwer,
    }:
    Foo<Bar> =
    Foo { f: a, g: b };

let (abcd,
     defg):
    Baz =
{ ... }

Alternative: the pattern could be block indented or not indented at all. I think it is very rare to have multi-line patterns, so it is not so important (and might be affected by how we format patterns themselves).

If the expression covers multiple lines, if the first line of the expression fits in the remaining space, it stays on the same line as the =, the rest of the expression is not indented. If the first line does not fit, then it should start on the next lines, and should be block indented. If the expression is a block and the
type or pattern cover multiple lines, then the opening brace should be on a new line and not indented (this provides separation for the interior of the block from the type), otherwise the opening brace follows the =.

Examples:

let foo = Foo {
    f: abcd,
    g: qwer,
};

let foo =
    ALongName {
        f: abcd,
        g: qwer,
    };

let foo: Type = {
    an_expression();
    ...
};

let foo:
    ALongType =
{
    an_expression();
    ...    
};

macro in statement position

a macro use in statement position should use parentheses as delimiters and should be terminated with a semi-colon. There should be no spaces between the name, !, the delimiters, or the ;. How to format any tokens passed to the macro is left for a macro-specific issue.

// A comment.
a_macro!(...);

expressions in statement position

There should be no space between the expression and the semi-colon.

<expr>;

All expressions in statement position should be terminated with a semi-colon, unless they end with a block or are used as the value for a block.

E.g.,

{
    an_expression();
    expr_as_value()
}

return foo();

loop {
    break;
}

Note that let declarations have a semicolon of their own.

Use a semi-colon where an expression has void type, even if it could be propagated. E.g.,

fn foo() { ... }

fn bar() {
    foo();
}
@nrc nrc added the P-high label Sep 25, 2016
@joshtriplett
Copy link
Member

Most of this seems fine to me.

One nit: even when breaking in the middle of a pattern, I don't think that should use visual indentation. I'm indifferent about visual indentation for a plain tuple (since the only thing you're indenting past to align is an open parenthesis, (), but I don't think this should use visual indentation for a struct or tuple struct pattern.

@nrc
Copy link
Member Author

nrc commented Sep 26, 2016

I propose editing this paragraph:

If the first line does not on a single line, then split after the colon, both the type and expression are block indented (and start new block indenting contexts) If pattern spans multiple lines, then it should be visually indented:

To remove the final sentence. Formatting patterns will be decided later.

@joshtriplett
Copy link
Member

@nrc In that case, you should also drop the paragraph after that starting with "Alternative:", and add "patterns" to the parenthetical in the first sentence excluding things this doesn't cover (along with expressions and items).

@steveklabnik
Copy link
Member

This looks generally good to me as well; I will admit a bit of revulsion to

let Foo {
        f: abcd,
        g: qwer,
    }:
    Foo<Bar> =
    Foo { f: a, g: b };

but I'm not sure there's a good way to write this. So I'm not actually opposed.

@joshtriplett
Copy link
Member

@steveklabnik I'd probably write it either that way or with the type on the same line as the }:, in the absence of a style guide saying otherwise. Or I'd try to find a way to make the pattern or the RHS shorter so I didn't need to break it across lines. :)

@strega-nil
Copy link

@steveklabnik

let Foo {
    f: abcd,
    g: qwer,
}: Foo<Bar> =
    Foo { f: a, g: b };

is how I would write it.

@UtherII
Copy link

UtherII commented Oct 19, 2016

I would prefer split the line before the : and the =

let pattern
    : Type
    = expr;

It seems much more readable to me. You see at first sight the purpose of each line without having to look back at the end of the previous line.

@nrc
Copy link
Member Author

nrc commented Oct 25, 2016

Moving this to FCP. I propose the using the original proposal adjusted with #11 (comment), #11 (comment), and #11 (comment)

@UtherII
Copy link

UtherII commented Nov 2, 2016

In my opinion, the syntax proposed on #11 (comment) is unreadable.

I saw no return about my proposal.

let Foo {
        f: abcd,
        g: qwer,
    }
    : Foo<Bar> 
    = Foo{ f: a, g: b }; 

Seems much more readable to me. If you think this syntax is wrong, I'd like to know why.

@nielsle
Copy link

nielsle commented Nov 2, 2016

I generally like Ubsans formatting, but how about the following two situations? (just to be clear)

let Foo {
    f: abcd,
    g: qwer,
}: Foo<Bar> = 
    Foo { 
        f: blimblimblim, 
        g: blamblamblam 
    };

let Foo {
    f: abcd,
    g: qwer,
}: Foo<Bar> = 
    foo (
        blimblimblim,
        blamblamblam 
    );

@strega-nil
Copy link

strega-nil commented Nov 3, 2016

@nielsle usually, probably,

let Foo {
    f: abcd,
    g: qwer,
}: Foo<Bar> = Foo { 
    f: blimblimblim, 
    g: blamblamblam 
};

let Foo {
    f: abcd,
    g: qwer,
}: Foo<Bar> = foo (
    blimblimblim,
    blamblamblam 
);

@nrc
Copy link
Member Author

nrc commented Nov 6, 2016

@UtherII

If you think this syntax is wrong, I'd like to know why.

I generally prefer punctuation to finish a line, rather than to start it, since that is what we do in written English.

the syntax proposed on #11 (comment) is unreadable.

Please avoid this kind of hyperbole, it makes your comment read as aggressive.

@nrc
Copy link
Member Author

nrc commented Nov 6, 2016

@ubsan contrasting your two suggestions for a single line and multi-line 'rhs':

let Foo {
    f: abcd,
    g: qwer,
}: Foo<Bar> =
    Foo { f: a, g: b };
let Foo {
    f: abcd,
    g: qwer,
}: Foo<Bar> = Foo { 
    f: blimblimblim, 
    g: blamblamblam 
};

The former keeps the rhs expression on one line and splits after the =, the latter splits the rhs, but keeps parts of the expression on the = line. I find this a little inconsistent and tend to dislike the latter - when a statement is spread over multiple lines, I find that putting parts of the statement on the same line make it hard to identify the logical parts. I think therefore that I prefer @UtherII's proposal, even though the extra indent is a bit irritating to me.

@strega-nil
Copy link

strega-nil commented Nov 8, 2016

@nrc I don't mind either mine or @UtherII's, personally.

@solson
Copy link
Member

solson commented Nov 8, 2016

I prefer @ubsan's, particularly in the multi-line case. It seems to best match what block indent looks like elsewhere.

For the one-line RHS, I'm less sure. One thing I'd like to point out is that a multi-line pattern in a let is something I've probably never seen before... seems pretty rare. With that in mind, I'd probably suggest:

// Single-line RHS (when it fits):
let Foo { f, g }: Foo<Bar> = Foo { f: a, g: b };

// Multi-line RHS (when it doesn't fit):
let Foo { f, g }: Foo<Bar> = Foo {
    f: blimblimblim,
    g: blamblamblam,
};

And then technically do the same block-indent thing for multi-line patterns, though I wouldn't expect it to come up much.

@solson
Copy link
Member

solson commented Nov 8, 2016

I guess there's an open question because you could block indent either the pattern or the RHS to get under the max line width in some cases. I would always prefer to block indent the RHS first and only block indent the pattern if completely necessary.

Alternately, you could block indent whichever of the two is larger, I suppose.

@strega-nil
Copy link

@solson I prefer "whichever of the two is larger", generally, although I'm not sure about this specific case. I think it generally looks nicer.

@brson
Copy link

brson commented Nov 14, 2016

I'm fine with moving this to an RFC now. Seems like it's in the ballpark.

@nrc
Copy link
Member Author

nrc commented Mar 14, 2017

We've changed our process: rather than requiring a full RFC, a PR to close this issue only requires making the changes discussed here to the style guide. You can see more details about the process in the readme for this repo.

If you'd like to help with this work, let me know, I'd be very happy to help you help us.

nrc added a commit that referenced this issue Apr 4, 2017
@nrc nrc mentioned this issue Apr 4, 2017
@nrc nrc added has-PR and removed ready-for-PR labels Apr 4, 2017
@nrc nrc closed this as completed in a3025c2 May 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants