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

dartfmt should add braces if it splits a long if line #664

Closed
jcollins-g opened this issue Jan 11, 2018 · 6 comments
Closed

dartfmt should add braces if it splits a long if line #664

jcollins-g opened this issue Jan 11, 2018 · 6 comments
Labels
--fix Adding or changing a fix enhancement

Comments

@jcollins-g
Copy link
Contributor

@devoncarew
Tested on 2.0.0-dev.16.0.

If dartfmt sees a line like this:

    if (!identical(canonicalModelElement, this)) return canonicalModelElement?.href;

It will change it to:

   if (!identical(canonicalModelElement, this))
     return canonicalModelElement?.href;

I think it would be nice if this added braces.

@devoncarew
Copy link
Member

+1, but I think @munificent generally prefers that dart_style only manipulate whitespace?

@munificent
Copy link
Member

Yeah. I'm definitely sympathetic but, at least for now, it's considered out of scope. This has more detail:

https://github.com/dart-lang/dart_style/wiki/FAQ#why-doesnt-the-formatter-add-curlies-or-otherwise-clean-up-code

@natebosch
Copy link
Member

I don't see a canonical issue discussing adding braces as a --fix so I'll reopen this one.

The scope of the formatter has increased since we added --fix. We should consider including this as one since it's tedious and we are opinionated about it.

@munificent
Copy link
Member

Yeah, now that we have --fix, this is reasonable. Though it is somewhat tricky, for a couple of reasons:

  • Should it be symmetric and remove curlies if an if does fit on one line? Some (most?) users might not want that. We could probably have two fixes, but that's a lot of effort. Note that --fix means "apply all the fixes", so if a significant number of users don't want it to remove curlies, we should probably just not have that as a fix at all.

  • The fix requires knowing how a line gets split, but the formatter doesn't calculate line splits until after fixes have been applied, so this might be tricky to implement in a non-hacky way.

@natebosch
Copy link
Member

Should it be symmetric and remove curlies if an if does fit on one line?

IMO no it shouldn't.

@munificent munificent added --fix Adding or changing a fix enhancement labels Sep 24, 2019
@munificent
Copy link
Member

Closing this since we're removing dart format --fix in favor of dart fix (#1153). I believe the latter already supports this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
--fix Adding or changing a fix enhancement
Projects
None yet
Development

No branches or pull requests

4 participants