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

function calls #65

Closed
nrc opened this issue Feb 9, 2017 · 15 comments
Closed

function calls #65

nrc opened this issue Feb 9, 2017 · 15 comments
Labels

Comments

@nrc
Copy link
Member

nrc commented Feb 9, 2017

And static method calls, method calls should be specified separately.

@nrc nrc mentioned this issue Feb 9, 2017
14 tasks
@nrc
Copy link
Member Author

nrc commented Mar 22, 2017

Suggestion from @vitiral (#73):

very_long_module_name::this_is_a_really_long_function(
    arg1, arg2, arg3, 
    "this is a raw string and is terible")

rustfmt currently uses the Method 1 way too often. I'm not sure the right balance here. Like @joshtriplett said in #47 , you really want to define "simple". IMO "simple" would be:

  • If the function and all args fit within the line length, just do that.
  • Otherwise use Method 1 if Method 1 only takes up two lines
  • Otherwise use Method 2:
    - arguments always start on their own indented line
    - arguments are listed normally as long as they don't go over the line length limit
    - any argument that would go over the line length limit goes on it's own line anything over 25 characters goes on it's own line

@nrc
Copy link
Member Author

nrc commented Mar 22, 2017

I think we probably want to use block rather than visual indentation. As discussed on other issues, we should avoid putting multiple args on one line if we are in multi-line mode.

@vitiral
Copy link

vitiral commented Mar 22, 2017

@nrc so you're saying it will use (approximately) the format suggested there? That would be awesome!

Edit: I think that an exception should be if the end of the ( takes up 4 spaces or less. For instance, this would be bad:

a(
    this, is, a, lot, of, arguments, being,
    "passed into function call a")

should instead be:

a(this, is, a, lot, of, arguments, being,
  "passed into function call a")

@nrc
Copy link
Member Author

nrc commented Mar 22, 2017

I'm proposing:

a(
    this,
    is,
    a,
    lot,
    of,
    arguments,
    being,
    "passed into function call a"
)

I don't think we should make such an exception for short names - they tend to be rare, and when they do occur they tend not to spill onto multiple lines (anyone who makes a one-char function name which takes enough complex arguments to trigger multi-line formatting deserves what they get).

@vitiral
Copy link

vitiral commented Mar 22, 2017

that's a good point.

I would like to suggest that we do:

some_function_call(
    this, is, a, &lot, of, arguments, being,
    "passed into function call a"
)

I think that anywhere people are using just variable (or &variable) this is what they want, and anywhere they are doing something more complicated they want it on it's own line. What you suggest would make a function call with lots of arguments take up a LOT of line real estate, when that isn't really helpful for reading it.

For something like a struct, it makes sense for them to be their own line because they are passed in key: value form (which is more complicated), but when it is by position this makes more sense.

Agh... as I talk about it more I am not sure, but I really don't want something like this:

kinda_long_function(x, y, z)

to become this:

kinda_long_function(
    x,
    y,
    zebra
)

just because I changed z -> zebra.

@joshtriplett
Copy link
Member

joshtriplett commented Mar 23, 2017

In the context of function definitions, we talked about allowing an intermediate style, where if breaking the arguments onto their own line made them fit on one line, they didn't require breaking onto a line each. We ended up dropping that for simplicity. I wonder if we should reconsider it for function calls? (And for consistency, function definitions, because having the two differ on that point seems odd.)

Function calls make a compelling case for it; I do prefer this:

long_function_name(
    arg1, arg2, arg3
);

instead of this:

long_function_name(
    arg1,
    arg2,
    arg3,
);

On the other hand, I'm really tempted to just go with "if you have to break onto multiple lines, use one line per argument", perhaps along with what we bounced around as a println! / format! exception for placement of the format string.

@nrc
Copy link
Member Author

nrc commented Mar 28, 2017

I wonder if we should reconsider it for function calls?

I prefer not to for the same reason as before - you save some vertical space at the expense of clarity. It is rare for arguments to calls to be single variables, much more likely to be sub-expressions, and there I think you get into the complexity argument for using multiple lines quickly.

@nrc
Copy link
Member Author

nrc commented Mar 28, 2017

OK, an attempt at a somewhat full description:

A function call consists of a callee and expression arguments:

callee(expr_args)

callee is an expression (most often a name; it may include generic arguments)

A method call has a receiver a method name, generic args, and expr args:

receiver.name::<generics>(exp args)

To simplify things, I'm going to discuss only the function form, for methods we assume callee == receiver.name::<generics>. Some special cases might be discussed in the paths or chains issues.

  • Prefer to keep everything on one line if possible:
foo(a, b, c)
  • If the callee terminates with punctuation, put a space before the (. Otherwise use no spaces.

  • If not possible, put each argument on its own line using block indentation. In this case, use a trailing comma:

function_name(
    a,
    b,
    c,
)
  • If there is still not enough room for the callee, split it across multiple lines according to the rules for that sub-expression. Keep the ( on the same line. In this case always use the multi-line form:
|blah| {
    // ...
} (
    an_argument
)
  • If any argument spans multiple lines, use the multi-line form (see Combining openings and closings #61 for exceptions).
  • If the arguments are not short (see Define 'short' #47), use the multi-line form.
  • These rules also apply to function-like (i.e., using ()) macro uses where the tokens in the () can be parsed as a comma-separated list of expressions.

nrc referenced this issue in killercup/waltz Mar 28, 2017
@gsingh93
Copy link

I like the rules above, but I don't see the reasoning behind: "If the callee terminates with punctuation, put a space before the (."

@solson
Copy link
Member

solson commented Apr 13, 2017

@gsingh93 Same here. In fact, there's a relatively common case where I specifically prefer the opposite: calling a function pointer field.

(foo.x)(42)

@gsingh93
Copy link

Can we clarify the question I raised above, and then move to FCP?

@joshtriplett
Copy link
Member

@gsingh93 I don't know what that refers to either. And it seems directly untrue; for instance, ::<turbofish> ends in punctuation but we don't put a space between the > and (.

@est31
Copy link
Member

est31 commented May 1, 2017

I'm not sure, would enum constructors be covered by this issue?

I don't like the way rustfmt treats code like:

return Ok(FancyStruct {
    member_a: stuff,
    member_b: stuff,
});

it gets reformated to:

return Ok(
    FancyStruct {
        member_a: stuff,
        member_b: stuff,
    }
);

And no way to turn it off :(

@nrc
Copy link
Member Author

nrc commented May 1, 2017

@est31 this is covered by #61 and just hasn't been implemented in rustfmt yet

@nrc
Copy link
Member Author

nrc commented May 9, 2017

Entering FCP based on #65 (comment) (note: edited to remove punctuation/spacing thing).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants