6 ms·

I read this before working on prettier and it was a bit scary to be honest :) I wasn’t convinced that the exhaustive approach with weigths was a good idea. It

by vjeux 8y ago

I read this before working on prettier and it was a bit scary to be honest :) I wasn’t convinced that the exhaustive approach with weigths was a good idea. It makes it computationally super intensive and hard to predict what the printer would do. Instead prettier runs on a simple idea: if something doesn’t fit in one line, break the outermost parent. This simple rule (implemented via the Wadler IR) is efficient to implement and makes the output generally look good. Then the hard part was to go through the very looong (took me ~6 months full time) process of adding special cases for every constructs that human write in a special way. There is nothing in there that’s hard at a computer science level, it’s just a lot of special cases. I don’t believe that there’s a general solution for it, you just need to slog through it. And the last annoying thing are comments, they can be everywhere and are super annoying to print correctly. Overall, it wasn’t really hard, but a lot of work.

ukulele 8y ago
First, thanks for putting the work in on prettier! It's a great tool, and it's helping a lot of devs & teams. I find your approach much more reasonable than the one outlined in the article, which to me feels like complexity for complexity's sake. There might be some minor win to breaking lines by analyzing billions of possibilities, but it just seems like overkill, and it's certainly not predictable for the programmer.
- skybrian 8y ago
  As another alternative, gofmt solved it a different way, by not having a limit on line length. I haven't seen it add line breaks at all.
  - vjeux 8y ago
    gofmt took a different approach where it respects the author's line breaks. So the following are all going to remain as is with gofmt: f(a, b) f( a, b) f(a, b, ) f(a, b) f( a, b, ) My original motivation for prettier was this particular example where I was sick of having to add/remove all those newlines when the line would go below or above 80 columns. So prettier only allows the following two forms and will format code for you as you save: f(a, b) f( a, b, )
    - skybrian 8y ago
      Well okay, but this raises the question of why code needs to always fit within 80 columns. The Go developers decided that it doesn't, and it's fine to check in the code as-is. (In particular, automatic refactoring tools don't need to wrap lines when they get longer.) And everything seems to have worked out fine. It's not the only way to do it, but I think it's interesting how getting everyone to agree that a problem doesn't need solving sometimes simplifies things a lot.
      - bradleyjg 8y ago
        If you do code reviews and your code reviewers take thier role seriously, and so will push back with comments like “this is hard to read, consider breaking it into two lines” then you don’t need automated enforcement. But if code reviewers look for sufficient units tests, obvious errors, and then dash off a LGTM, software style enforcement is second best.
        skybrian 8y ago
        In practice, this doesn't come up in review and Go code is quite readable anyway. Apparently people just don't need much help to get line lengths more or less right.
      - vjeux 8y ago
        You want to break --at some point--. The question is, who decides where to break? For gofmt, they decided that the person writing the code is going to make this decision everywhere. For prettier, we decided that the tool was going to make that decision, so that the humans don't have to. If we want to do it automatically, we need to figure out an algorithm to do that. It turns out that if you use 80 columns as a heuristic, the vast majority of the code will look fine. A lot of people (myself included) tried to use different algorithms but couldn't beat that heuristic. If you know of a better way, please let me know :)
        mbell 8y ago
        > It turns out that if you use 80 columns as a heuristic, the vast majority of the code will look fine. I'm not sure what corpus was used to determine that heuristic (nor what tab size). But it would trash a lot the projects I work on.
  - jaggederest 8y ago
    I would just have it raise an error or warning, but maybe that's me. My first questions when a problem starts to spiral are "Do we really need to solve this problem? And if so, do we really need to solve it right now?"
piinbinary 8y ago
Is this the paper describing Walder IR you referred to? https://homepages.inf.ed.ac.uk/wadler/papers/prettier/prettier.pdf https://homepages.inf.ed.ac.uk/wadler/papers/prettier/pretti...
- vjeux 8y ago
  Yes! If you want something less mathematical, you can read the explanation of the various commands: https://github.com/prettier/prettier/blob/master/commands.md https://github.com/prettier/prettier/blob/master/commands.md
munificent 8y ago
> I wasn’t convinced that the exhaustive approach with weigths was a good idea. It makes it computationally super intensive and hard to predict what the printer would do. Yes, you are right on both accounts. I'm still not entirely sure it's the right approach for dartfmt. I started with a corpus of code that was hand-formatted by people who I felt had great style. And then I worked backwards to figure out what kinds of formatting mechanics I would need to be able to automate that. Unfortunately, there are some very common formatting idioms in Dart that really just don't play nice with any simpler formalism. For example, it's common to use embedded function literals as if they were blocks: group("test some stuff", () { test("test a thing with a very long" "multi-line description of the thing being tested", () { expect(thing, isSomeThing); }); }); Note here how the body of the innermost function literal does not respect the surrounding expression nesting, even though it is technically all an argument to test(). But in other cases, a function parameter should work like a normal indented expression: things .map((thing) { ... }) .where((thing) => thing.isFoo); .toList(); I probably could have said, "Well, these idioms are going away and this code will look different." and then designed a much simpler formatter. A huge part of software engineering is picking the right problem to solve and knowing when to push against requirement and when to take them as given. For dartfmt, I did my best to keep the original requirements even thought it meant a much more complex formatter. That made my implementation job harder. But it made the political job of convincing everyone inside Google (and most users outside) to use dartfmt much easier because the output was closer to what they wanted.
- vjeux 8y ago
  > For dartfmt, I did my best to keep the original requirements even thought it meant a much more complex formatter. That made my implementation job harder. But it made the political job of convincing everyone inside Google (and most users outside) to use dartfmt much easier because the output was closer to what they wanted. I couldn't have said it better :) I added a ton of special cases in the step that takes an AST and outputs the IR to handle those two things (and many many more). For 1), I special case functions that look like `test(` and completely ignore the 80 column rule so that the name stays in one line. https://github.com/prettier/prettier/blob/3e0dceda9954658860ffdbc7e04465fcf5737d12/src/language-js/printer-estree.js#L1047-L1058 https://github.com/prettier/prettier/blob/3e0dceda9954658860... For 2), member chains is actually the most complex piece of prettier, but it was important to get right. There's plenty of comments in the implementation: https://github.com/prettier/prettier/blob/3e0dceda9954658860ffdbc7e04465fcf5737d12/src/language-js/printer-estree.js#L4222-L4584 https://github.com/prettier/prettier/blob/3e0dceda9954658860... I'm sure you did something similar as well :)

by vjeux 8y ago

ukulele 8y ago

First, thanks for putting the work in on prettier! It's a great tool, and it's helping a lot of devs & teams. I find your approach much more reasonable than the one outlined in the article, which to me feels like complexity for complexity's sake. There might be some minor win to breaking lines by analyzing billions of possibilities, but it just seems like overkill, and it's certainly not predictable for the programmer.

skybrian 8y ago
As another alternative, gofmt solved it a different way, by not having a limit on line length. I haven't seen it add line breaks at all.
- vjeux 8y ago
  gofmt took a different approach where it respects the author's line breaks. So the following are all going to remain as is with gofmt: f(a, b) f( a, b) f(a, b, ) f(a, b) f( a, b, ) My original motivation for prettier was this particular example where I was sick of having to add/remove all those newlines when the line would go below or above 80 columns. So prettier only allows the following two forms and will format code for you as you save: f(a, b) f( a, b, )
  - skybrian 8y ago
    Well okay, but this raises the question of why code needs to always fit within 80 columns. The Go developers decided that it doesn't, and it's fine to check in the code as-is. (In particular, automatic refactoring tools don't need to wrap lines when they get longer.) And everything seems to have worked out fine. It's not the only way to do it, but I think it's interesting how getting everyone to agree that a problem doesn't need solving sometimes simplifies things a lot.
    - bradleyjg 8y ago
      If you do code reviews and your code reviewers take thier role seriously, and so will push back with comments like “this is hard to read, consider breaking it into two lines” then you don’t need automated enforcement. But if code reviewers look for sufficient units tests, obvious errors, and then dash off a LGTM, software style enforcement is second best.
      - skybrian 8y ago
        In practice, this doesn't come up in review and Go code is quite readable anyway. Apparently people just don't need much help to get line lengths more or less right.
    - vjeux 8y ago
      You want to break --at some point--. The question is, who decides where to break? For gofmt, they decided that the person writing the code is going to make this decision everywhere. For prettier, we decided that the tool was going to make that decision, so that the humans don't have to. If we want to do it automatically, we need to figure out an algorithm to do that. It turns out that if you use 80 columns as a heuristic, the vast majority of the code will look fine. A lot of people (myself included) tried to use different algorithms but couldn't beat that heuristic. If you know of a better way, please let me know :)
      - mbell 8y ago
        > It turns out that if you use 80 columns as a heuristic, the vast majority of the code will look fine. I'm not sure what corpus was used to determine that heuristic (nor what tab size). But it would trash a lot the projects I work on.
- jaggederest 8y ago
  I would just have it raise an error or warning, but maybe that's me. My first questions when a problem starts to spiral are "Do we really need to solve this problem? And if so, do we really need to solve it right now?"

piinbinary 8y ago

Is this the paper describing Walder IR you referred to? https://homepages.inf.ed.ac.uk/wadler/papers/prettier/prettier.pdf https://homepages.inf.ed.ac.uk/wadler/papers/prettier/pretti...

vjeux 8y ago
Yes! If you want something less mathematical, you can read the explanation of the various commands: https://github.com/prettier/prettier/blob/master/commands.md https://github.com/prettier/prettier/blob/master/commands.md

munificent 8y ago

> I wasn’t convinced that the exhaustive approach with weigths was a good idea. It makes it computationally super intensive and hard to predict what the printer would do. Yes, you are right on both accounts. I'm still not entirely sure it's the right approach for dartfmt. I started with a corpus of code that was hand-formatted by people who I felt had great style. And then I worked backwards to figure out what kinds of formatting mechanics I would need to be able to automate that. Unfortunately, there are some very common formatting idioms in Dart that really just don't play nice with any simpler formalism. For example, it's common to use embedded function literals as if they were blocks: group("test some stuff", () { test("test a thing with a very long" "multi-line description of the thing being tested", () { expect(thing, isSomeThing); }); }); Note here how the body of the innermost function literal does not respect the surrounding expression nesting, even though it is technically all an argument to test(). But in other cases, a function parameter should work like a normal indented expression: things .map((thing) { ... }) .where((thing) => thing.isFoo); .toList(); I probably could have said, "Well, these idioms are going away and this code will look different." and then designed a much simpler formatter. A huge part of software engineering is picking the right problem to solve and knowing when to push against requirement and when to take them as given. For dartfmt, I did my best to keep the original requirements even thought it meant a much more complex formatter. That made my implementation job harder. But it made the political job of convincing everyone inside Google (and most users outside) to use dartfmt much easier because the output was closer to what they wanted.

vjeux 8y ago
> For dartfmt, I did my best to keep the original requirements even thought it meant a much more complex formatter. That made my implementation job harder. But it made the political job of convincing everyone inside Google (and most users outside) to use dartfmt much easier because the output was closer to what they wanted. I couldn't have said it better :) I added a ton of special cases in the step that takes an AST and outputs the IR to handle those two things (and many many more). For 1), I special case functions that look like `test(` and completely ignore the 80 column rule so that the name stays in one line. https://github.com/prettier/prettier/blob/3e0dceda9954658860ffdbc7e04465fcf5737d12/src/language-js/printer-estree.js#L1047-L1058 https://github.com/prettier/prettier/blob/3e0dceda9954658860... For 2), member chains is actually the most complex piece of prettier, but it was important to get right. There's plenty of comments in the implementation: https://github.com/prettier/prettier/blob/3e0dceda9954658860ffdbc7e04465fcf5737d12/src/language-js/printer-estree.js#L4222-L4584 https://github.com/prettier/prettier/blob/3e0dceda9954658860... I'm sure you did something similar as well :)