Don't insert trailing commas over the max width #473
Conversation
facebook-github-bot
added
the
CLA Signed
| | ) | ||
| | bar( | ||
| | someArg, | ||
| | exactlyAtMaaax |
There was a problem hiding this comment.
Is that was @Rawa was referring to in #472?
What I understood was that the comma should have forced the line break, respecting the line max width option, which for this particular case here, where we cannot respect it, the comma would still be included.
@Rawa, is my understanding correct or were you also thinking something along the lines of what @nreid260 did here?
There was a problem hiding this comment.
This isn't an ideal solution, more of a hack to mitigate complaints. I can't think of an efficient algorithm for:
- Remove unnecessary trailing commas
- Insert line breaks between items in long lists
- Insert trailing commas in broken lists
It's the last part that's being reported as a bug, because when the list item is exactly 100 columns, the trailing comma is inserted into column 101. Shorter items put the comma within bounds, longer items get wrapped during formatting.
Running prettyPrint twice would be slow, and also makes tests hard to debug.
The key thing is that tailing commas are still 100% automated.
|
TBH I'd prefer to have the code as is vs this modification here. I'm happy to hear arguments countering that though, but if you don't have any, just keep this PR closed. |
Closes #472