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

fn_call_style has no "Compressed" variant #2010

Closed
gnzlbg opened this issue Sep 28, 2017 · 6 comments
Closed

fn_call_style has no "Compressed" variant #2010

gnzlbg opened this issue Sep 28, 2017 · 6 comments

Comments

@gnzlbg
Copy link
Contributor

gnzlbg commented Sep 28, 2017

Check this out. Before:

let a = i8x32::new(
             0, 1, -1, 2,	
             -2, 3, -3, 4,
             -4, 5, -5, std::i8::MAX,	
             std::i8::MIN + 1, 100, -100, -32,	
             0, 1, -1, 2,	
             -2, 3, -3, 4,	
             -4, 5, -5, std::i8::MAX,	
             std::i8::MIN + 1, 100, -100, -32);

After

let a = i8x32::new(
            0,
            1,
            -1,
            2,
            -2,
           3,
            -3,
            4,
             -4,
             5,
             -5,
             std::i8::MAX,
             std::i8::MIN + 1,
             100,
            -100,
            -32,
             0,
            1,
             -1,
             2,
             -2,
             3,
             -3,
             4,
             -4,
             5,
             -5,
             std::i8::MAX,
             std::i8::MIN + 1,
             100,
             -100,
             -32,
         );
@nrc
Copy link
Member

nrc commented Nov 2, 2017

Should probably just skip calls like this.

@nrc nrc closed this as completed Nov 2, 2017
@gnzlbg
Copy link
Contributor Author

gnzlbg commented Nov 2, 2017

@nrc is this fixed, or should I just not write this method in the first? we are using rustfmt in stdsimd, and... if you have an u8x32 then the new` constructor really does take 32 arguments... such is life :D

@nrc
Copy link
Member

nrc commented Nov 2, 2017

You should use a skip attribute on a call like this.I think it is unlikely that rustfmt ever will (or even could) format a call like this nicely.

@gnzlbg
Copy link
Contributor Author

gnzlbg commented Nov 2, 2017

This is what I ended up doing (skiping these calls). This issue was filled because I thought that adding a Compressed variant to fn_call_style that wraps as many arguments as possible in a line would fix this everywhere for us. Turning such a feature on would probably eliminate 90% of the skips in the stdsimd code base.

@nrc
Copy link
Member

nrc commented Nov 2, 2017

The trouble is that for most function calls, compressed is a pretty bad formatting style and we have no way of forcing an option on for some calls. We'd need to be 'smart' about when to apply different styles and people generally don't like these kind of 'heuristic' changes.

@gnzlbg
Copy link
Contributor Author

gnzlbg commented Nov 2, 2017

I personally like to maximize use of vertical space so compressing function arguments is not that bad.

I agree that it doesn't look very nice, but nice-looking code is not what I expect from an automatic formatting tool, the only thing I expect is that the resulting code doesn't need to be horrible. I am willing to live with overall worse looking code if that saves me having to write a lot of skip annotations (because for me the most important thing is to not have to worry about formatting at all).

The stdsimd crate is not a typical crate, so such an option shouldn't be the default, and probably it is not worth it to implement it, but I would personally find it handy for this particular crate.

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

2 participants