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

General look and feel #7

Closed
nrc opened this issue Sep 3, 2016 · 8 comments
Closed

General look and feel #7

nrc opened this issue Sep 3, 2016 · 8 comments

Comments

@nrc
Copy link
Member

nrc commented Sep 3, 2016

This is a thread to debate a general/overall look and feel for formatting, to try and avoid going off-topic in other threads.

References:

Rustfmt of course has its own current aesthetic, although that is mostly a refinement of the Rust style guide and the style used in the Rust repo.

@nrc nrc mentioned this issue Sep 3, 2016
@Diggsey
Copy link

Diggsey commented Sep 3, 2016

Moving my argument for ubsan's guide here:

The stated goal of being "easy to manually format" is very far from being met with the current style, and visual indentation is horrible, both when writing and refactoring code, even if you're lucky enough to have an editor with support for it (I've never actually encountered an editor that handled this well, probably because of the inherent ambiguity in visual indentation...)

The way I see it, it's a trade off between being subjectively very slightly prettier in some cases (the current style) and being objectively simpler in all cases (ubsan's style). The latter has the distinct advantage of having almost no additional edge cases to consider, having no ambiguity so editors can easily do a good job of refactoring without having to reformat the entire file with an external tool, and a big one: eliminating unnecessary rightward drift!

@nikomatsakis
Copy link

nikomatsakis commented Sep 4, 2016

So on the question of visual indent, I wrote this comment in the general principles section, which basically says "ease of manual formatting is probably worth preserving, since most people don't use emacs, but it makes me a bit sad".

That said, there are a many things in @ubsan's guide that I quite like, but a few things that I would change (and I'm curious what @ubsan thinks). I think the prime one is that, if I am going to break things across multiple lines, I prefer to go to one thing per line at that point. So for example this code snippet:

fn quite_a_few_args(
    first_parameter_is: f64, second_parameter_is: f64,
) -> f64 {
    first_parameter_is + second_parameter_is
}

I would prefer to see written as:

fn quite_a_few_args(
    first_parameter_is: f64,
    second_parameter_is: f64,
) -> f64 {
    first_parameter_is + second_parameter_is
}

I find this much easier to read at a glance. Basically, once I have a list of things, I want to see them listed out one per line. Similarly, this code snippet:

long_function_name(
    long_argument_name1, long_argument_name2,
    &long_argument_name3.struct_member as *const StructMember,
    FourthArgument::new(), FIFTH_ARGUMENT, &sixth_argument[..],
);

I think would be better written:

long_function_name(
    long_argument_name1,
    long_argument_name2,
    &long_argument_name3.struct_member as *const StructMember,
    FourthArgument::new(),
    FIFTH_ARGUMENT, &sixth_argument[..],
);

Not in particular how complex the third argument is -- with a newline in between arguments, that does not prove an impediment to parsing. As a bonus, if I now go and compare this call to long_function_name against its definition, it's relatively easy to match up the arguments being provided with the parameters. I just count down lines. If any of the arguments were so complex as to stretch across lines, then the subsequent lines would generally be indented, so that's not a problem.

@nikomatsakis
Copy link

One other thing that I was thinking: instead of including an entire style guide, it'd be interesting to try and extract out the key underlying principles. Those are perhaps things worth raising on #4.

@nrc
Copy link
Member Author

nrc commented Sep 4, 2016

to be clear, I'm not proposing incorporating any style guide into the RFCs, they're just references for the general look of some existing styles.

@draivin
Copy link

draivin commented Sep 15, 2016

I agree with @nikomatsakis on the one item per line, my biggest issue with multiple items per line is that when you add new parameters to the function (mainly as a first parameter or a middle parameter), you end up having to shift the other lines in a frustrating way, breaking some lines and joining others.

If you have a function call like this:

long_function_name(
    long_argument_name1, long_argument_name2,
    long_argument_name3, long_argument_name4,
);

and then add a new parameter in the first position, you have to reformat your code to this:

long_function_name(
    new_first_argument, long_argument_name1,
    long_argument_name2, long_argument_name3,
    long_argument_name4,
);

Which takes a lot more effort than simply adding a new line.

@bruno-medeiros
Copy link

@draivin Why is you need to reformat the code like that? Why can't it just be

long_function_name(
    new_first_argument, 
    long_argument_name1, long_argument_name2, 
    long_argument_name3, long_argument_name4,
);

? The above can work if the formatter doesn't force you to join existing lines, it only breaks them if they exceed the limit. (Again, ideally there would be an option to control this. If the formatter offers no customization, that's when it becomes an issue)

@draivin
Copy link

draivin commented Sep 16, 2016

@bruno-medeiros
As I see it, you follow a style guide to achieve consistency, if the style expects multiple items per line, you should respect it in a top down flow. If you broke the "flow" for new items, and introduced them in a new line each, you may as well use one item per line to start with, and avoid the mishmash of short and long lines.

Disclaimer: This is purely subjective and based on my personal opinion.

@nrc
Copy link
Member Author

nrc commented Feb 5, 2018

Closing since, we are past the point of this issue being useful

@nrc nrc closed this as completed Feb 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants