Line wrapping for object arguments about dprint-plugin-typescript HOT 9 CLOSED

To be honest, the very fact that the code contains a string of 80 characters is a sign itself that you need to fix it. Is it an error message? Sure, place those in a single file where you disable the "max characters per lines" rule. Is it a variable name? Chop that down.

I just think the source of the problem lies elsewhere and should be addressed before trying to patch it up quick and dirty.

@lazarljubenovic it is nice for it to be able to handle these edge cases well so it's good to discuss them :)

Agree though that if it's too complicated then the code should just be changed and this is not a big deal. I'm not sure what the expected output is though.

from dprint-plugin-typescript.

@dsherret Thanks for looking into this. Since it wasn't clear to some people, the first one does not contain a string or even a line of more than 80 chars. :) Prettier would leave it the way it is. First one is source and expected, second is actual. Using options:

ConfigurationBuilder::new()
  .line_width(80)
  .indent_width(2)
  .next_control_flow_position(NextControlFlowPosition::SameLine)
  .force_multi_line_parameters(true)
  .force_multi_line_arguments(true)
  .binary_expression_operator_position(OperatorPosition::SameLine)
  .build()

@lazarljubenovic

Neither of the examples looks good

A highly subjective thing to predicate your argument on. Judging by your first paragraph, I'd say you're distracted by that specific example I gave. That was intended to minimally represent the structure while achieving +80 for the whole statement. Here's a more realistic one:

walk("path/to/subdir", {
  followSymlinks: true,
  includeFiles: false,
  skip: excludePatterns
});

Is it ugly? Would you suggest I pull the object out as a variable, betraying the meaning of the options object pattern? Is this an edge case? People are calling this a complex statement, but it objectively isn't. It just happens to be a long one. That's exactly what automatic line wrapping is for.

from dprint-plugin-typescript.

To be honest, the very fact that the code contains a string of 80 characters is a sign itself that you need to fix it. Is it an error message? Sure, place those in a single file where you disable the "max characters per lines" rule. Is it a variable name? Chop that down.

While I appreciate tools being strictly defined and predictable, the point of a formatter is no get rid of:

1. thinking too much about it;
2. everyone formatting code differently;
3. everyone automatically re-formatting the code differently, making noise in the diff.

Neither of the examples looks good, and instead of fixing the printer, I think it's a sign that the code itself needs fixing. This will help with reason (1). Or -- just let it format one way, and leave it like that, and you'll avoid issues related to the reasons (2) and (3).

I don't mean that this shouldn't be decided or thought through, or that you shouldn't have opened the issue. I just think the source of the problem lies elsewhere and should be addressed before trying to patch it up quick and dirty.

from dprint-plugin-typescript.

@nayeemrmn Is that the expected output you'd like? I think this does what you'd like when forceMultiLineArguments is on:

https://dprint.dev/playground/#code/GYCgRAhl07fwxTnzAGgAQG8CwAoDDCALgwEZ8BfASgG4g/config/N4IgNglgdgpg6hAJgFwBYgFwA4AMAaEaRGKZBFdDAJgIFcBnGAFQEMAjezAMxbEYMYBbCAGEA9mDFROGZACdaMAdADmYGAEVaY5DBk8+SkPJYRIUFeMGCWMkLABuMOSDqMAQnJYBjPZhAA7qgkAHI6AMqq6gAy0DCuIGxevgAKYvQQyBBS-rAAHsixsACSXAASLBaqCRkW6u5iiACeaRlZORggNtDIplAJ+cjipHISAGKSAa2Z2f2dg0XxBGIADs4syGJy0+1z9jAFiwlcW74AsrRgWYsAgnIqtIIkyDLyigQncueX13EpLF4nro5PpePwQCRHgARGDeMAAjazAB0T0EbGc4RWPmqnW6pD6CQBowCY1oUG8uwAonkVnI9BkpEiGDB-nTSMFGHY8b1oCAAL5AA

Ryan turned that on in deno in the recent PR, so I think that should be good. Let me know if I'm misunderstanding.

from dprint-plugin-typescript.

@nayeemrmn wait... actually I think I am misunderstanding. Could you provide the expected output and what the current output is? Right now the output is the same as prettier unless forceMultiLineArguments is off.

from dprint-plugin-typescript.

Oh, this is a bug that only occurs with forceMultilineArguments. I see that now. Thanks! Will fix this soon (probably tonight).

Playground link:

https://dprint.dev/playground/#code/GYCgRAhl07fwxTnTAGgAQG8CwAoDDCALgwEZ8BfASgG4g/config/N4IgNglgdgpg6hAJgFwBYgFwA4AMAaEaRGKZBFdDAFgIFcBnGAFQEMAjezAMxbEYMYBbCAGEA9mDFROGZACdaMAdADmYGAEVaY5DBk8+SkPJYRIUFeMGCWMkLABuMOSDqMAQnJYBjPZhAA7qgkAHI6AMqq6gAy0DCuIGxevgAKYvQQyBBS-rAAHsixsACSXAASLBaqCRkW6u5iiACeaRlZORggNtDIplAJ+cjipHISAGKSAa2Z2f2dg0XxBGIADs4syGJy0+1z9jAFiwlcW74AsrRgWYsAgnIqtIIkyDLyigQncueX13EpLF4nro5PpePwQCRHgARGDeMAAjazAB0T0EbGc4RWPmqnW6pD6CQBowCY1oUG8uwAonkVnI9BkpEiGDB-nTSMFGHY8b1oCAAL5AA

from dprint-plugin-typescript.

@nayeemrmn How do you think this should format?

call({
    prop
}, "testing");

call({
    prop
}, {
    prop
});

It seems prettier splits them up and makes them multiline. Wouldn't be hard to keep them as-is though...

I'm thinking we could have an argument list meet either of these requirements in order to be considered "multi-line":

1. A collection of arguments exceeds the line width.
2. OR, an argument start position is the first token on that line.

So the above would format as-is. Does that sound good?

from dprint-plugin-typescript.

I've discovered various issues while making forceMultiLineArguments the default. Currently fixing them and making this more robust.

from dprint-plugin-typescript.

Thanks so much for reporting @nayeemrmn! I have fixed this bug and it's a pretty bad bug IMO.

Additionally I updated the config to the following:

• forceMultiLineParameters -> Replaced by preferHangingParameters with a default of false.
• forceMultiLineArguments -> Replaced by preferHangingArguments with a default of false.

I should be able to catch bugs for this more often now that I've made it the default behaviour.

from dprint-plugin-typescript.