Avoid having small right operand on its own line #180

jablko · 2019-08-03T23:19:35Z

Would you consider grouping the right operand, like language-js, "to avoid having a small right part like -1 be on its own line"?

codecov · 2019-08-03T23:21:29Z

Codecov Report

Merging #180 into master will decrease coverage by 0.31%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #180      +/-   ##
==========================================
- Coverage      99%   98.68%   -0.32%     
==========================================
  Files          74       74              
  Lines         601      609       +8     
  Branches       96      100       +4     
==========================================
+ Hits          595      601       +6     
- Misses          6        8       +2

Impacted Files	Coverage Δ
src/binary-operator-printers/exponentiation.js	`100% <100%> (ø)`	⬆️
src/binary-operator-printers/logical.js	`100% <100%> (ø)`	⬆️
src/binary-operator-printers/comparison.js	`100% <100%> (ø)`	⬆️
src/binary-operator-printers/arithmetic.js	`100% <100%> (ø)`	⬆️
src/clean.js	`33.33% <0%> (-66.67%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update df9dc77...083efb7. Read the comment docs.

Janther · 2019-08-03T23:38:42Z

prettier js formats the following:

veryVeryVeryVeryVeryLongVariableCalledA - veryVeryVeryVeryVeryLongVariableCalledB - 1 -1 -1 - 1 - 1 - 1 - 1;

like,

veryVeryVeryVeryVeryLongVariableCalledA -
  veryVeryVeryVeryVeryLongVariableCalledB -
  1 -
  1 -
  1 -
  1 -
  1 -
  1 -
  1;

have a look at #175 to see other ways prettier js groups ecuations that we are reviewing right now for solidity.

jablko · 2019-08-04T00:48:29Z

My mistake, however it formats

a(veryVeryVeryVeryVeryLongVariableCalledA + veryVeryVeryVeryVeryLongVariableCalledB) - 1;

like

a(
  veryVeryVeryVeryVeryLongVariableCalledA +
    veryVeryVeryVeryVeryLongVariableCalledB
) - 1;

Would you consider something like the revised version of this pull request, which handles both

veryVeryVeryVeryVeryLongVariableCalledA -
  veryVeryVeryVeryVeryLongVariableCalledB -
  1 -
  1 -
  1 -
  1 -
  1 -
  1 -
  1;

and

a(
  veryVeryVeryVeryVeryLongVariableCalledA +
    veryVeryVeryVeryVeryLongVariableCalledB
) - 1;

?

Janther · 2019-08-04T01:12:36Z

hahaha this one is a weird one.
If I try this in JS:

s = a(veryVeryVeryVeryVeryLongVariableCalledA + veryVeryVeryVeryVeryLongVariableCalledB) - 1;

2 - a(veryVeryVeryVeryVeryLongVariableCalledA + veryVeryVeryVeryVeryLongVariableCalledB) - 1;

a(veryVeryVeryVeryVeryLongVariableCalledA + veryVeryVeryVeryVeryLongVariableCalledB) - 1 + 2;

I get

s =
  a(
    veryVeryVeryVeryVeryLongVariableCalledA +
      veryVeryVeryVeryVeryLongVariableCalledB
  ) - 1;

2 -
  a(
    veryVeryVeryVeryVeryLongVariableCalledA +
      veryVeryVeryVeryVeryLongVariableCalledB
  ) -
  1;

a(
  veryVeryVeryVeryVeryLongVariableCalledA +
    veryVeryVeryVeryVeryLongVariableCalledB
) -
  1 +
  2;

I like having this particular edge case as long as the main rule is not broken, could you add test cases??
testing not only your solution but also combining it in different scenarios to check exactly how it behaves not just in a specific case but also in something broader.

Janther · 2019-08-04T01:13:39Z

src/binary-operator-printers/comparison.js

+    const right = concat([node.operator, line, path.call(print, 'right')]);
+    const parent = path.getParentNode();
+    const shouldGroup =
+      parent.type !== node.type && node.left.type !== node.type;


Could you also document why this particular type mismatch deserves a grouping?

👍 Will do!

jablko · 2019-08-05T15:59:32Z

I've rebased on #182: I added tests for

single operator/two operands
multiple operators
arithmetic, comparison, and logical operators
no context, assignment, if statement, function call, and return statement

mattiaerre · 2019-08-08T06:08:53Z

how can we fix this cov report issue? I'll try to reach out to the Codecov team and see what they say

mattiaerre · 2019-09-23T05:29:07Z

can you resolve the conflicts here too @jablko ? thanks

jablko · 2019-09-23T15:27:19Z

@mattiaerre Absolutely: Rebased.

mattiaerre · 2019-09-23T16:24:29Z

thanks so much @jablko

mattiaerre

over to you @jablko

mattiaerre · 2019-09-29T02:17:57Z

tests/BinaryOperators/__snapshots__/jsfmt.spec.js.snap

@@ -330,8 +326,7 @@ contract ArithmeticOperators {
                veryVeryVeryVeryVeryLongVariableCalledB;
        veryVeryVeryVeryVeryLongFunctionCalledA(
            veryVeryVeryVeryVeryLongVariableCalledB
-        ) **
-            c;
+        )**c;


just noticing that there is no space before ** and after it; is that what we want?

this was applied in #174

can we approve this PR?

jablko force-pushed the patch-3 branch from 8a05f8c to 7eab5b7 Compare August 4, 2019 00:38

Janther reviewed Aug 4, 2019

View reviewed changes

jablko force-pushed the patch-3 branch 2 times, most recently from 355fdc4 to 818f52e Compare August 5, 2019 15:51

jablko force-pushed the patch-3 branch 5 times, most recently from a303e72 to 7130b5a Compare August 7, 2019 15:46

jablko force-pushed the patch-3 branch 2 times, most recently from 13abec2 to 1a62513 Compare August 13, 2019 16:12

jablko added a commit to jablko/prettier-plugin-solidity that referenced this pull request Aug 13, 2019

Avoid having small right operand on its own line (prettier-solidity#180)

1a62513

jablko force-pushed the patch-3 branch from 1a62513 to 4431194 Compare August 13, 2019 16:17

jablko added a commit to jablko/prettier-plugin-solidity that referenced this pull request Aug 13, 2019

Avoid having small right operand on its own line (prettier-solidity#180)

4431194

jablko added a commit to jablko/prettier-plugin-solidity that referenced this pull request Sep 23, 2019

Avoid having small right operand on its own line (prettier-solidity#180)

4d9629f

jablko force-pushed the patch-3 branch from 4431194 to 4d9629f Compare September 23, 2019 01:11

Avoid having small right operand on its own line (prettier-solidity#180)

083efb7

jablko force-pushed the patch-3 branch from 4d9629f to 083efb7 Compare September 23, 2019 15:24

mattiaerre reviewed Sep 29, 2019

View reviewed changes

mattiaerre requested a review from Janther February 28, 2020 06:04

Janther approved these changes Feb 29, 2020

View reviewed changes

mattiaerre merged commit 4203efe into prettier-solidity:master Mar 2, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Avoid having small right operand on its own line #180

Avoid having small right operand on its own line #180

jablko commented Aug 3, 2019

codecov bot commented Aug 3, 2019 •

edited

Janther commented Aug 3, 2019

jablko commented Aug 4, 2019

Janther commented Aug 4, 2019 •

edited

Janther Aug 4, 2019

jablko Aug 4, 2019

jablko commented Aug 5, 2019

mattiaerre commented Aug 8, 2019

mattiaerre commented Sep 23, 2019

jablko commented Sep 23, 2019

mattiaerre commented Sep 23, 2019 •

edited

mattiaerre left a comment

mattiaerre Sep 29, 2019

Janther Sep 29, 2019

mattiaerre Sep 30, 2019

Avoid having small right operand on its own line #180

Avoid having small right operand on its own line #180

Conversation

jablko commented Aug 3, 2019

codecov bot commented Aug 3, 2019 • edited

Codecov Report

Janther commented Aug 3, 2019

jablko commented Aug 4, 2019

Janther commented Aug 4, 2019 • edited

Janther Aug 4, 2019

Choose a reason for hiding this comment

jablko Aug 4, 2019

Choose a reason for hiding this comment

jablko commented Aug 5, 2019

mattiaerre commented Aug 8, 2019

mattiaerre commented Sep 23, 2019

jablko commented Sep 23, 2019

mattiaerre commented Sep 23, 2019 • edited

mattiaerre left a comment

Choose a reason for hiding this comment

mattiaerre Sep 29, 2019

Choose a reason for hiding this comment

Janther Sep 29, 2019

Choose a reason for hiding this comment

mattiaerre Sep 30, 2019

Choose a reason for hiding this comment

codecov bot commented Aug 3, 2019 •

edited

Janther commented Aug 4, 2019 •

edited

mattiaerre commented Sep 23, 2019 •

edited