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

Wrap strings automatically with dartfmt #909

Open
kaspnilsson opened this issue Mar 23, 2020 · 5 comments
Open

Wrap strings automatically with dartfmt #909

kaspnilsson opened this issue Mar 23, 2020 · 5 comments

Comments

@kaspnilsson
Copy link

One of the best parts of Dart is the tooling, and the lack of comment and string wrapping (especially while those rules are enforced by presubmits) is a pretty major missing feature. AFAICT it should be reasonably fixable to wrap strings at 80ch (although I could easily be missing the complexity here)

@kaspnilsson
Copy link
Author

Any chance we can get this looked at and prioritized? This is pretty obnoxious in practice to not have, and lots of other format tools have already solved for this problem

@munificent
Copy link
Member

I agree this would be handy in many cases, but I think it would also do the wrong or at least confusing things in other cases. See #431 and https://github.com/dart-lang/dart_style/wiki/FAQ#why-does-the-formatter-only-touch-whitespace-by-default for some more context.

I'll leave this open mainly to so that others can find the issue instead of re-filing it, but I think it's unlikely that this will be feasible.

@kaspnilsson
Copy link
Author

It's disappointing to hear that this is still the Dart team's POV.

I would very, very strongly challenge your characterization of this problem as infeasible -- most other modern languages and toolings (Prettier.JS, gofmt, clang-format, etc) do this, at least in some form. Taking a look at the tooling that exists in other ecosystems would definitely expand your opinion as to what is feasible with a little bit of code.

I definitely can understand if this kind of work is not a priority right now, or if this particular tool is unstaffed or itself not a priority for the Dart team, etc, but I would really hope that this basic convenience is a feature of future formatters.

@munificent
Copy link
Member

most other modern languages and toolings (Prettier.JS, gofmt, clang-format, etc) do this, at least in some form.

  • Prettier does not and will not: "Closing this as it's definitely not going to happen in this project (Prettier core)."

  • gofmt does not do line breaking at all.

  • clang-format does have an option to support splitting strings. However, it does not appear to unsplit strings when they no longer need to be split. That violates one of the goals of dartfmt that its changes should generally be "reversible".

By reversible, I mean that if you:

  1. Start with some code.
  2. Change the code.
  3. Run dartfmt.
  4. Change the code back.
  5. Run dartfmt again.

You should end up with the same code at 4 that you had at 0. If you make a string longer on step 1 and dartfmt splits it on 2, and then you make it shorter again on 3 but dartfmt doesn't merge the string back on 4, then your code is left with a pointless split that dartfmt inserted but you don't realize can be removed.

@munificent
Copy link
Member

Leaving a note here that supporting this will likely be much more valuable when we ship the new style (#1253) and every Dart users starts migrating to it. That style change can sometimes change the indentation of lines containing strings and being able to split them if needed could be valuable.

That being said, I'm still not sure how to implement this. We don't know which strings need splitting until after line wrapping, but splitting strings will affect the behavior of line wrapping. It's not a trivial feature.

Also, it's not a reversible change, so if we support it at all, it will have to be with an opt-in flag like --fix.

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

2 participants