allow prettier to format more files #1966

petertseng · 2022-02-17T05:14:56Z

It's my opinion that for these 15 files, we should not have a strong
opinion on the formatting, and therefore should just accept what
prettier gives us.

This leaves only 18 files ignored. That leaves much less manual work
needed.

petertseng

you will notice that some of these appear to cause internal inconsistency, in that some JSON files an array will go on a single line, while in the same JSON file some other arrays will have each element on their own line. While I regret the inconsistency, I believe it's simpler to let prettier do as much as it can, because whatever tool we write to reformat the remaining files is maintained by us, and the less work it has to do, the less work it is to maintain it.

None of the files formatted in this PR are files where the array elements are representing rows of a 2D grid, so the formatting doesn't affect our ability to understand the input.

If the idea is that test cases are immutable, we don't run into problems where adding an element to an array causes a larger diff than necessary, since we simply cannot add elements to those arrays.

It's my opinion that for these 20 files, we should not have a strong opinion on the formatting, and therefore should just accept what prettier gives us. This leaves only 13 files ignored. That leaves much less manual work needed.

petertseng · 2022-02-17T05:44:14Z

exercises/spiral-matrix/canonical-data.json

@@ -52,10 +52,10 @@
        "size": 4
      },
      "expected": [
-        [ 1,  2,  3, 4],
+        [1, 2, 3, 4],


while I regret that we are losing the padding, I do not think it is important enough to preserve. I do not think losing the padding is going to cause trouble understanding this expected output.

SaschaMann · 2022-02-17T06:32:37Z

exercises/book-store/canonical-data.json

+        "basket": [
+          1, 1, 1, 1, 1, 1, 2, 2, 2, 2, 2, 2, 3, 3, 3, 3, 3, 3, 4, 4, 5, 5
+        ]


See #1917 why this is not desirable.

Are you referring to:

This makes copy and pasting harder for no reason.

If so, I don't really see why the new format is much harder, nor why copy-pasting is important here.

To avoid bikeshedding, let me assure you from the standpoint of someone who intends to write the formatting tool that the difference is going to be small - it would be only a small extra cost to add this one as an exception. There is an exercise where I already intend to change a multi-line array into a single-line one (scale-generator), so adding another one isn't an additional code path; it's just adding another exercise that takes that code path.

Since the difference is small, I don't intend to spend a lot of effort on this decision. I actually am supportive of optimising in favour of track maintainers (restoring the original), in exchange for a small extra maintenance cost. Since ultimately that benefits the students, which is why we're here. No need to make it any harder than necessary to implement an exercise from this JSON file.

However. I do need to gain clarification on which exercises this idea needs to be applied to. I apologise for engaging in what-about-ism, but understanding this concern is key to making sure we are helping track maintainers and our future selves. It seems to me like book-store, bowling, change, and variable-length-quantity would fall under the same category here - that their inputs would all benefit from being on the same line as their []. That would be four exercises. Please confirm whether my understanding is correct.

If so, I don't really see why the new format is much harder, nor why copy-pasting is important here.

Line breaks make it annoying to copy into a REPL or into the code file (where it most likely looks like foo([...])). It's important because it saves a lot of time when manually implementing an exercise. For tracks that use generators, the formatting is irrelevant, so I don't see why we shouldn't optimise it for human-use whenever possible. I don't see any advantage of the new format for reading or implementing the exercise, only disadvantages.

However. I do need to gain clarification on which exercises this idea needs to be applied to. I apologise for engaging in what-about-ism, but understanding this concern is key to making sure we are helping track maintainers and our future selves. It seems to me like book-store, bowling, change, and variable-length-quantity would fall under the same category here - that their inputs would all benefit from being on the same line as their []. That would be four exercises. Please confirm whether my understanding is correct.

Yes, that sounds right. I'm not sure if there are other exercise but those are the main ones where it makes a big difference.

Sometimes the added line breaks add readability (see Pascal's Triangle) but for a list of numbers like in these exercises, I don't see that being the case.

Line breaks make it annoying to copy into a REPL or into the code file
It's important because it saves a lot of time when manually implementing an exercise.

Well, many tracks don't have a REPL nor do I think that "it saves a lot of time" is necessarily true (I feel that is an assumption).
That said, we should indeed optimize for human-readability, so if it is do-able for Peter to make these exceptional cases, then I am fine with it.

Well, many tracks don't have a REPL nor do I think that "it saves a lot of time" is necessarily true (I feel that is an assumption).

Do those tracks gain anything from spreading it out over several lines? Maybe there are advantages, I just can't think of any.

There are very few programming languages that don't have a REPL at all. Whether or not people use it as part of their workflow is a different story, though.

(I feel that is an assumption).

Try it and measure it yourself if you want, I don't see how it wouldn't be slower to remove line breaks for every paste compared to just pasting them. 🤷

SaschaMann · 2022-02-17T06:32:48Z

exercises/dominoes/canonical-data.json

+        "dominoes": [
+          [1, 2],
+          [2, 3],
+          [3, 1],
+          [2, 4],
+          [2, 4]
+        ]


See #1917 why this is not desirable.

If I understand correctly, the idea here is that seeing [1, 2], [2, 3], [3, 1] all next to each other makes it more visually apparent that they could connect to each other, and that is lost if putting them on separate lines.

I have thought about it and I am sufficiently convinced.

May I remind from the standpoint of a person intending to write the formatting tool that I expect the extra maintenance cost incurred by this to be small. Given it is small, I think it's worth it.

If I understand correctly, the idea here is that seeing [1, 2], [2, 3], [3, 1] all next to each other makes it more visually apparent that they could connect to each other, and that is lost if putting them on separate lines.

Indeed.

SaschaMann · 2022-02-17T06:33:46Z

exercises/pascals-triangle/canonical-data.json

+      "expected": [
+        [1],
+        [1, 1],
+        [1, 2, 1],
+        [1, 3, 3, 1],
+        [1, 4, 6, 4, 1],
+        [1, 5, 10, 10, 5, 1],
+        [1, 6, 15, 20, 15, 6, 1],
+        [1, 7, 21, 35, 35, 21, 7, 1],
+        [1, 8, 28, 56, 70, 56, 28, 8, 1],
+        [1, 9, 36, 84, 126, 126, 84, 36, 9, 1]
+      ]


Perhaps these should all be triangles in this file.

I will note this for consideration when writing the array formatter. After it is incorporated into CI I will try applying that to pascals-triangle and see how it looks. For the time being, I let prettier do as it will.

Sounds good.

ErikSchierboom

I approve of all these changes.

ErikSchierboom · 2022-02-17T06:55:47Z

exercises/book-store/canonical-data.json

+        "basket": [
+          1, 1, 1, 1, 1, 1, 2, 2, 2, 2, 2, 2, 3, 3, 3, 3, 3, 3, 4, 4, 5, 5
+        ]


Are you referring to:

This makes copy and pasting harder for no reason.

If so, I don't really see why the new format is much harder, nor why copy-pasting is important here.

(vlq = variable-length-quantity)

I am making a ruling in favour of track maintainers, given that the additional cost to problem-specifications maintenance is going to be small. in applicable exercises, I have applied my decision

ErikSchierboom · 2022-03-04T10:21:30Z

Requesting a review from @exercism/reviewers

kotp

Nothing looks awry, the formatting seems to be reasonable. The things that are meant to be formatted different appear to be not affected in a harmful way.

race condition between "Exercise allergies test case" exercism#1969 and "allow prettier to format more files" exercism#1966 Since this file is now being formatted by prettier, the test case added in the latter PR must be formatted according to prettier. There are alternate approaches, such as directing format-array to always format `expected` with one item on each line, but it doesn't seem like this is particular important for the `allergies` exercise.

race condition between "Exercise allergies test case" exercism#1969 and "allow prettier to format more files" exercism#1966 Since this file is now being formatted by prettier, the test case added in the latter PR must be formatted according to prettier. Otherwise, as will be observable from any PR after exercism#1966, CI fails on main. There are alternate approaches, such as directing format-array to always format `expected` with one item on each line, but it doesn't seem like this is particular important for the `allergies` exercise.

race condition between "Exercise allergies test case" #1969 and "allow prettier to format more files" #1966 Since this file is now being formatted by prettier, the test case added in the latter PR must be formatted according to prettier. Otherwise, as will be observable from any PR after #1966, CI fails on main. There are alternate approaches, such as directing format-array to always format `expected` with one item on each line, but it doesn't seem like this is particular important for the `allergies` exercise.

suggested as a follow-up action to format-array being merged, in at least two separate occasions: exercism#1968 (comment) exercism#1966 (comment)

suggested as a follow-up action to format-array being merged, in at least two separate occasions: exercism#1968 (comment) exercism#1966 (comment) The reason for the suggestion is that helps visualise the human-readable form of the triangle that we are used to seeing.

suggested as a follow-up action to format-array being merged, on at least two separate occasions: exercism#1968 (comment) exercism#1966 (comment) The reason for the suggestion is that helps visualise the human-readable form of the triangle that we are used to seeing.

suggested as a follow-up action to format-array being merged, on at least two separate occasions: #1968 (comment) #1966 (comment) The reason for the suggestion is that helps visualise the human-readable form of the triangle that we are used to seeing.

petertseng commented Feb 17, 2022

View reviewed changes

allow prettier to format more files

32cfed5

It's my opinion that for these 20 files, we should not have a strong opinion on the formatting, and therefore should just accept what prettier gives us. This leaves only 13 files ignored. That leaves much less manual work needed.

petertseng force-pushed the more-formats branch from af37010 to 32cfed5 Compare February 17, 2022 05:40

petertseng commented Feb 17, 2022

View reviewed changes

SaschaMann previously requested changes Feb 17, 2022

View reviewed changes

ErikSchierboom approved these changes Feb 17, 2022

View reviewed changes

petertseng added 2 commits February 17, 2022 08:09

dominoes: we affirm that inputs should be on one line

ac3cbd2

book-store,bowling,change,vlq: inputs on one line

a62dc2d

(vlq = variable-length-quantity)

petertseng force-pushed the more-formats branch from 2de3797 to a62dc2d Compare February 17, 2022 08:18

SaschaMann approved these changes Feb 17, 2022

View reviewed changes

This was referenced Feb 19, 2022

auto-format portions of some JSON files #1967

Merged

add format-array (formats JSON arrays) #1968

Merged

kotp approved these changes Mar 4, 2022

View reviewed changes

SaschaMann merged commit b820099 into exercism:main Mar 4, 2022

petertseng deleted the more-formats branch March 4, 2022 12:36

petertseng mentioned this pull request Mar 4, 2022

allergies: format #1978

Merged

petertseng mentioned this pull request Mar 31, 2022

pascals-triangle: format each row as one line #1996

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

allow prettier to format more files #1966

allow prettier to format more files #1966

petertseng commented Feb 17, 2022 •

edited

Loading

petertseng left a comment

petertseng Feb 17, 2022

SaschaMann Feb 17, 2022

ErikSchierboom Feb 17, 2022

petertseng Feb 17, 2022

SaschaMann Feb 17, 2022 •

edited

Loading

SaschaMann Feb 17, 2022

ErikSchierboom Feb 17, 2022

SaschaMann Feb 17, 2022 •

edited

Loading

SaschaMann Feb 17, 2022

petertseng Feb 17, 2022

SaschaMann Feb 17, 2022

SaschaMann Feb 17, 2022

petertseng Feb 17, 2022

SaschaMann Feb 17, 2022

ErikSchierboom left a comment

ErikSchierboom Feb 17, 2022

ErikSchierboom commented Mar 4, 2022

kotp left a comment

allow prettier to format more files #1966

allow prettier to format more files #1966

Conversation

petertseng commented Feb 17, 2022 • edited Loading

petertseng left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SaschaMann Feb 17, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SaschaMann Feb 17, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ErikSchierboom left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ErikSchierboom commented Mar 4, 2022

kotp left a comment

Choose a reason for hiding this comment

petertseng commented Feb 17, 2022 •

edited

Loading

SaschaMann Feb 17, 2022 •

edited

Loading

SaschaMann Feb 17, 2022 •

edited

Loading