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

Prettier-style line breaking #246

Open
Tracked by #58093
jtheisen opened this issue Jul 10, 2019 · 41 comments
Open
Tracked by #58093

Prettier-style line breaking #246

jtheisen opened this issue Jul 10, 2019 · 41 comments
Labels
Feature Request This issue is requesting an enhancement or new feature

Comments

@jtheisen
Copy link

It's my understanding the the project doesn't do line breaking the way Prettier does: Inserting and removing line breaks depending on the length of the resulting lines.

I can't find a rationale for this and I can't find a ticket mentioning this.

It's technically easy to implement (using Roslyn), but it's not entirely independent from other formatting jobs (eg. whether opening braces should always be on a new line).

So what's going on? Is there no love for Prettier's approach?

@sharwell
Copy link
Member

I've never seen an automated line breaking strategy that wasn't also a readability breaking strategy. In other words, the tool produced worse results on average than if it didn't change wrapping at all. I have some theories about things that might change this, but overall this is a very, very hard problem.

@jtheisen
Copy link
Author

You've seen Prettier? You think it gives sub-par results?

@sharwell
Copy link
Member

I haven't seen Prettier specifically, especially on C# code. Is the line break strategy documented in detail?

@jtheisen
Copy link
Author

It can't do C# and my good impression comes from what it does to JavaScript.

I just looked at the docs and it says it used this paper. That seems a bit more complicated that I would have thought at first glance.

I can tell that it is at least doing this: When you have a node with more than one child (array literal elements, function arguments, etc.) and it can't fit it all in a line, each child goes on its own line. I've seen this to be the crucial issue, but maybe there's more to it.

@jmarolf
Copy link
Contributor

jmarolf commented Jul 10, 2019

basically my question is how well does Prettier handle re-flowing comments in javascript? I've read the paper you linked but it doesn't address this problem. The approach they use mentions an AST, but ASTs don't contain comments. Handling comment layout is one of the problems we will need to solve if we want to tackle this.

@OneCyrus
Copy link

prettier handles line breaks very well. i know a lot of teams who enforces prettier formatting as validation gates in PRs.
it completely removed the discussions around code style for my team as well. it works so good that i‘m not aware of one single example where prettier did a bad job with typescript projects.

if we could get C# code to the same high quality standard it would save us a lot of time.

@jmarolf
Copy link
Contributor

jmarolf commented Jul 10, 2019

looking at this test it doesn't appear to handle comments at all. You can even get it to place things outside of the minimum width in pathological cases.

I believe that re-flowing syntax can be done. The issue every time we research this on our side is how do you correctly re-flow constructs that do not have syntactic meaning such as comments. If someone has a proposal for this I am all ears.

@jtheisen
Copy link
Author

jtheisen commented Jul 11, 2019

@jmarolf They talk about why they don't do comments here: prettier/prettier#265

I'm not sure why a lack of support for comments renders the other parts of the formatting useless.

Prettier also still leaves the programmer some other kinds of freedom, such as empty lines for separation of statements.

@jmarolf
Copy link
Contributor

jmarolf commented Jul 11, 2019

I'm not sure why a lack of support for comments renders the other parts of the formatting useless.

A re-flow strategy that only handles some language constructs is not a complete solution. If I run dotnet format and my code is less readable I am not going to be inclined to use dotnet format.

That doesn't mean we can't start on this though. @jtheisen if you submit a PR for Prettier style re-flow I would be happy to work with you to get it in.

I assume if this was implemented we would want to specify the column width in the editor config file. presumably one labeled printWidth?

@jtheisen
Copy link
Author

Thanks for the offer. We'll see if I get struck with dedication.

Btw, why is the formatter operating on a project rather than a single file?

@jmarolf
Copy link
Contributor

jmarolf commented Jul 11, 2019

@jtheisen there are some formatting questions that require semantic information and there fore need to know settings from the project file. We could re-write things to allow a subset of the rules to work without a project file being present.

@jtheisen
Copy link
Author

Can you be more specific? What rules require more than the file in question?

@jmarolf
Copy link
Contributor

jmarolf commented Jul 12, 2019

For example, there is a style rule that allows you to prefer keywords such as int over class names such as Int32. Without the information in your project file we cannot determine if a class is the Int32 class from the framework or just a similarly named class that you or a nuget package has defined. I can go through the style rules and give you a list

@jtheisen
Copy link
Author

I see, that makes sense. No need for a full list at this point.

@JoeRobich JoeRobich added the Feature Request This issue is requesting an enhancement or new feature label Mar 17, 2020
@snebjorn
Copy link

I concur. A zero config, catch all, highly opinionated formatter for C# like what prettier is for JavaScript/TypeScript would be an absolute game changer! 🚀

I found this repo looking for a prettier-like solution to my C# code.
Prettier is the de facto formatting standard for everything JS/TS and it's awesome! I really really hope this will become something similar ❤️

About the single file formatting.
I believe this to be an essential feature, because the next step is to have git pre-commit hooks run formatting on every file committed. That'll be odd when having to run on folders.
Isn't looking for the project file up the directory tree an option? Much like editorconfig looks for its config?

@jmarolf
Copy link
Contributor

jmarolf commented Mar 30, 2020

@snebjorn Prs have been added in the meantime @JoeRobich I believe it is possible to run this tool on a single file now correct?

@JoeRobich
Copy link
Member

@snebjorn You can invoke dotnet format against a single file with the following command dotnet format -f . -files {relative-file-path}

  • the -files option will become the -include option in 4.x and switch from being a comma-separated list of relative paths to a space separated list of globs

@MaStr11
Copy link
Contributor

MaStr11 commented Apr 3, 2020

The related feature request in roslyn is dotnet/roslyn#15406 and it is in the backlog.

@mgasparel
Copy link

I concur. A zero config, catch all, highly opinionated formatter for C# like what prettier is for JavaScript/TypeScript would be an absolute game changer! 🚀

If you've never used a language with a community-standard auto-formatter, it's hard to overstate how big of a deal it is. This is by far my top feature request across the entire .net ecosystem. I think it would be a big win for .net

@nathan-c
Copy link

We currently use the resharper cli in a hook to format our solution, which works well for line wrapping (and is free to use) however it is very very slow (even when filtering to just staged files).

I would love to switch away from it as dotnet-format performance is considerably better, however line wrapping is a deal breaker right now.

My 2 cents on comments would be that if the rest of the code is wrapped you are more likely to write properly wrapped comments. If they aren't wrapped automatically it is still a huge improvement.

FWIW We also use prettier+eslint to format out react+typescript web frontend so have experience with that.

@davzucky
Copy link

davzucky commented Sep 1, 2020

After two out of .Net and spending my time on python. I can confirm that language like that exists. The python community has a pep that defines the code formatting pep8. Today the community is using a tool name black.
This simple convention that defines as well the max line size simplifies a lot of code readability and developer won't have a argue about the code format during code review.
I'm looking forward to seeing that on C# as well

@laktak
Copy link

laktak commented Feb 5, 2021

There is a prettier csharp plugin, it's unfinished though: https://github.com/warrenseine/prettier-plugin-csharp

upd - this one is in development: https://github.com/belav/csharpier

@mauve
Copy link

mauve commented Jun 10, 2022

clang-format does this very well for C++ code and has been a game-changer for our development at my current place and at Spotify, at both places we enforce it as a PR-gate.

@Fed03
Copy link

Fed03 commented Jun 10, 2022

This feature would be a game changer for our team. no we are forced to use JB CLI tool and its proprietary rules

@loraderon
Copy link

This feature would be a game changer for our team. no we are forced to use JB CLI tool and its proprietary rules

https://github.com/belav/csharpier is the game changer you are looking for, it really is like prettier but for C#.

@Meligy
Copy link

Meligy commented Aug 17, 2022

CSharpier is awesome, but the fact that it doesn't seem to use dotnet analyzers (I'm assuming here because it doesn't work with dotnet format) makes it always a bit silly to have 2 tools and also have to use one or the other (for example in VS Code, either use omnisharp formatter or CSharpier extension formatter).

This really belongs to dotnet format, which does already have style / whitespace aspects - or Rosluyn analyzers in general.

If you configure your analyzers nicely and maybe bring in some 3d parties like Roslynator, this issue becomes the most, even the only, obvious difference that makes tools like Resharper, etc. shine compared to Roslyn-based solutions.

@OneCyrus
Copy link

What‘s your use case with dotnet analyzers? I think you don’t need the overhead of those analyzers to format code files. Dotnet format is pretty slow and still has inconsistent formatting. Prettier does a pretty good job by being fast and consistent. IMHO that‘s the reason why it is so successful.
Completely rewriting the code with changed syntax shouldn‘t be a part of formatting.

@stinos
Copy link

stinos commented Nov 16, 2022

FWIW I tried a combination of csharpier + format, in that order, because we'd really want consistent line breaking but also want most format features plus we do get the reationaly behind opinionated formatting but the shift from if( stuff ) to csharpier-stylle if (stuff) is a bit much for a huge codebase. Unfortunately doesn't work 100% (as in: repeatedly applying still changes some whitespace) though it's close.

@Meligy
Copy link

Meligy commented Nov 16, 2022

csharpier has some edge cases (it failed on what seemed to be a totally innocent file in my experience), but the most annoying thing about it is that it seems to cause some issues with roslyn format options, that csharpier even recommends turning off the format analyzer. This is a bit disappointing even if it makes sense.

It also resists the idea of configurability (like prettier when it first started, later prettier added configurability I think), which wouldn't work for everyone.

@loraderon
Copy link

@stinos & @Meligy

Unfortunately doesn't work 100% (as in: repeatedly applying still changes some whitespace) though it's close.

csharpier has some edge cases (it failed on what seemed to be a totally innocent file in my experience)

I haven't seen any of those problems with CSharpier, maybe you could create issues for your problems?

It should play nice with other linters according to https://csharpier.com/docs/IntegratingWithLinters.

@Meligy
Copy link

Meligy commented Nov 17, 2022

It should play nice with other linters according to csharpier.com/docs/IntegratingWithLinters.

Yeah, I'm not concerned about the specific file as it can be an issue to solve.

But lack of configuration is bad when other formatting analyzers exist. The link you put itself shows the issue I talked about regarding conflicting whitespace formatting rules in the built in analyzers vs CSharpier. They say do not run the dotnet format whitespace command, but if enforce code style on build and you have the default formatting rule on (IDE0055), you get the same issue.

@OneCyrus
Copy link

the whole point of csharpier and prettier is that there are no configuration options. it‘s an opinionated formatter which fully formats the code. all the formatting rules of dotnet format need to be skipped as they have no use in such a scenario.

@stinos
Copy link

stinos commented Nov 18, 2022

I haven't seen any of those problems with CSharpier, maybe you could create issues for your problems?

@loraderon note that I meant a sequence like

dotnet-format
dotnet-csharpier
dotnet-format
git commit -am "formatted"
dotnet-csharpier
dotnet-format
git diff
<diff shows whitespace changes in indentation>

but I didn't look into where the problem is exactly so perhaps it is not CSharpier.

@Meligy
Copy link

Meligy commented Nov 18, 2022

the whole point of csharpier and prettier is that there are no configuration options. it‘s an opinionated formatter which fully formats the code. all the formatting rules of dotnet format need to be skipped as they have no use in such a scenario.

This sounds like an argument why the default tooling should have a formatter too, or a better formatter, because it already offers one (it already has whitespace and style support), it just needs to cover more cases.

Even prettier has some config and several plugins, and can play well with ESLint, etc. Csharpier is a bit too small to have such an ecosystem, and it's implementation is kind of a hack compared to dotnet format which integrates with analyzers, etc. This is the already existing plugin ecosystem, analyzers. And dotnet format already uses it. It just needs to cover more cases by default in its whitespace/code style support, like fluent/chained method calls, etc.

@belav
Copy link

belav commented Feb 2, 2023

It just needs to cover more cases by default in its whitespace/code style support, like fluent/chained method calls, etc.

I think you are underestimating how much of a leap it is to go from adding whitespace around a given node, to deciding when to add line breaks based on the nodes above and below that node and how wide the output of those nodes will be when they are actually printed. I don't see how it could be done without the intermediate representation that understands everything that needs to be printed and how to break it apart when it gets too wide.

Even prettier has some config and several plugins, and can play well with ESLint

Prettier regrets adding a number of the configuration that they currently support and refuses to discuss adding more.

The reason that it can play nice with ESLint is because they offer a configuration for ESLint that turns off any ESLint rules that conflict with the formatting Prettier produces. As far as I am aware there is no way to offer a sharable configuration of the analyzers that conflict with the output of CSharpier.

I'd happily turn CSharpier into a plugin system if there was demand. The only languages that probably make sense are F#, VB.Net, Powershell or Razor. Beyond that I don't know what else someone would want to format that isn't already covered by another opinionated formatter or a prettier plugin. CSharpier itself started as a prettier plugin until I ran into major performance issues and ported portions of prettier to c#. And to be clear the performance issue was with how I was using prettier to start a dotnet cli app to parse c# with roslyn and convert it to json that could be used by prettier.

@jtheisen
Copy link
Author

jtheisen commented Feb 2, 2023

@belav I guess that depends on how much the formatter changes. For example, your CSharpier doesn't change between var and an explicit type in definitions, between block and expression bodies, or between int and Int32. The last example is something this project modifies as per @jmarolf's comment to me above.

The more the formatter touches, the less you're going to get away with providing no options.

Personally I think the main clue of prettier is the way it handles line breaks, not that it provides only few options. It's also possible that C# has just more demand for some more options, as all examples I listed above simply have no equivalent in JavaScript.

@Meligy
Copy link

Meligy commented Feb 7, 2023

Options or no options, the point of "format" is to format the whole file. Something like spaces and line breaks are common format features, and as mentioned in previous comment are part of what makes prettier useful. We see something similar in golang format too.

And C# can really use it, as it has a lot of method call chains either from fluent APIs or LINQ etc. Currently there's no way other than CSharpier and maybe Resharper to format these consistently.

@piotrwiewiura
Copy link

I've used dotnet-format for the first time today and was really surprised it didn't break long lines, which to me is one of the main points of "formatting".
So yes, this feature would make dotnet-format much more useful and true to its name.

@will-molloy
Copy link

will-molloy commented Nov 12, 2023

+1

I want to use dotnet format over csharpier! It is the standard tool provided by dotnet and it is more powerful in other areas, e.g. converting to file-scoped namespaces, fixing missing braces, and removing redundant statements - csharpier does none of these (atm).

However, refusing to respect max_line_length is a deal breaker. This is the main thing (related to formatting) we comment on in PRs and want automated.

Read any other code style doc, e.g. Google Java Style Guide, one of the first things mentioned is line length/width!

@akhanalcs
Copy link

Can we set this up as Prettier style "format on save"?

@UnstoppableMango
Copy link

Can we set this up as Prettier style "format on save"?

That sounds orthogonal to issue at hand? Whether line breaks are applied automatically on save or when a format command is run doesn't really matter, what we want is for dotnet-format to apply line breaks at all.

Also going to throw out that fantomas handles line breaks for F# very nicely

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Request This issue is requesting an enhancement or new feature
Projects
None yet
Development

No branches or pull requests