chore: run Prettier on games #19943

nschonni · 2022-08-24T22:19:18Z

Ran prettier on the games folder to see how it interacts with the Markdownlint setup. Found one definition list issue that needed manual intervention, but the rest looked OK

nschonni · 2022-08-24T22:22:09Z

files/en-us/games/examples/index.md

-**[Bullet Force](https://www.crazygames.com/game/bullet-force-multiplayer)**
-    3D multiplayer first-person shooter.
-
+- [Bullet Force](https://www.crazygames.com/game/bullet-force-multiplayer)


This was the manual intervention

files/en-us/games/anatomy/index.md

Josh-Cena · 2022-08-25T03:19:17Z

files/en-us/games/tutorials/html5_gamedev_phaser_device_orientation/index.md

+  this.borderGroup,
+  this.wallCollision,
+  null,
+  this


We need to turn on trailingComma: all.

Yes, the default, isn't it (I mean trailingComma is the default)

The default is es5, which removes trailing commas for function calls. They are changing it to all in v3, along with a few other changes (including using tabs and single quotes I think, which I don't like)

Do you have a link to v3? I have not found it. I'm curious to see what is coming.

rubiesonthesky · 2022-08-25T06:27:20Z

files/en-us/games/techniques/3d_on_the_web/building_up_a_basic_demo_with_a-frame/index.md

@@ -114,7 +110,8 @@ A camera entity can be created by adding an [`<a-camera>`](https://aframe.io/doc
  cursor-visible="true"
  cursor-scale="2"
  cursor-color="#0095DD"
-  cursor-opacity="0.5">
+  cursor-opacity="0.5"
+>


I think using bracketSameLine true is better but I don't know if you have agreed not to use that setting somewhere else :)

@wbamberg @Rumyra What do you think? Until now, I think this is the only default value we should change. It is working well with React or Vue.js, but I think, in other cases, it looks weird.

Yeah, I don't like the newline thing either, but I wanted to keep it to the defaults to start so I left it as-is

Yes, I think we should do this. This came up when we were doing the conversion - Prettier's behavior here was breaking the Markdown converter. I [can't remember | never understood] the full details, but see for instance:

HTML ➡️ Markdown: Web/JavaScript #5193 (comment) (the para "there is a more gnarly one")

Unexpected HTML for <pre> tag after prettier prettier/prettier#10950

...as a result of which I think Gregor restricted Prettier to formatting only HTML tables inside Markdown, when we did the conversion. Because bracketSameLine didn't exist then.

I'll try adding the setting and re-running to see if it "fixes" it back or not

Yup, looks like it fixed it by adding the .prettierrc.json with that config

rubiesonthesky · 2022-08-25T06:35:05Z

...s/en-us/games/techniques/tilemaps/square_tilemaps_implementation_colon__static_maps/index.md

-    1, 1, 1, 1, 2, 1, 1, 1,
-    1, 1, 1, 1, 2, 1, 1, 1,
-    1, 1, 1, 0, 0, 1, 1, 1
+    1, 3, 3, 3, 1, 1, 3, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 2, 1, 1, 1,


I think these should not be changed because it's showing correct row count etc. But it will be hard to prevent change in future.

We could use an alias to js(jsnolint) to exclude Prettier from updating these snippets. (I still need to check if it works).

I want to double-check if it would work.

I can wrap this one in the  /end if you want to keep it as-is

I think this is not so critical. The critical ones are in matrix examples.

IMO it woulds be good to keep previous styling, it's not that necessary. I wonder would adding trailing comments with row counting also disable the autoformat? That would also clarify why it was formatted like that. But could be that it just makes it harder to read.

Strangely I understood that a tile is a square matrix…

files/en-us/games/tutorials/2d_breakout_game_pure_javascript/build_the_brick_field/index.md

bsmth · 2022-08-25T07:36:30Z

files/en-us/games/anatomy/index.md

@@ -293,6 +303,7 @@ A separate update and draw method could look like the following example. For the
  main(performance.now()); // Start the cycle
 })();
 ```
+<!-- prettier-ignore-end -->

 Another alternative is to do certain things less often. If a portion of your update loop is difficult to compute but insensitive to time, you might consider scaling back its frequency and, ideally, spreading it out into chunks throughout that lengthened period. An implicit example of this was found over at The Artillery Blog for Artillery Games, where they [adjust their rate of garbage generation](https://web.archive.org/web/20161021030645/http://blog.artillery.com/2012/10/browser-garbage-collection-and-framerate.html) to optimize garbage collection. Obviously, cleaning up resources is not time sensitive (especially if tidying is more disruptive than the garbage itself).


I have a general comment:

I find lines like this hard to read where you have a paragraph on one line, essentially. If there are changes to a word in a sentence in the middle, the entire line is affected in a diff.

Prettier can format these using proseWrap in prettier config:

{ "proseWrap": "always", }

An alternative is to write one sentence per line, but this is a manual effort.

We don't have a consensus on this:

Some authors write one (long) sentence per line

Some authors write one paragraph per line

Some tooling break anywhere between a word if the line is greater than 80 or 120 chars

Some authors put every clause on a different line. (There was a blog post about this, but I can't find this.)

I think we are far from a consensus. More, I'm not sure we can find a solution that we agree on, and that is enforceable easily to the long tail of editors.

To be fair, I don't think it is a coincidence that Prettier does nothing about this by default.

My two cents: we're taking the initiative to remove the ambiguity and discussions about code style in example code using Prettier, we could do the same for the .md prose so that there's consistency there, also. Thanks for the context, though, I appreciate this is not simple 😄

I think this is a discussion worth having once we have set up Prettier and/or Markdownlint. "Stück für Stück" 🤣 .

Stück für Stück

Sounds good to me 😂

I think we certainly had a strong consensus on this: #6936 .

we're taking the initiative to remove the ambiguity and discussions about code style in example code using Prettier, we could do the same for the .md prose so that there's consistency there, also. Thanks for the context, though, I appreciate this is not simple 😄

I agree with this though, we should run Prettier on Markdown. When we converted from HTML, we ran it through Prettier as well. ~~There are things markdownlint does that Prettier does not (AFAIK things like italic syntax or heading syntax) so we want both~~.

(Apparently I'm wrong about this. I don't know whether there are things markdownlint does that Prettier does not.)

I have a strong preference for not wrapping at all—or at least not wrap by column width. I configured my editor to soft wrap and I encourage everyone to do so (you can also do that on GitHub).

If you wrap by column width, and you add one word on the first line, you would possibly trigger tens of lines of diff due to the reflow.

I'm okay with soft breaking by sentences (in that I won't call it out or change it if others do that), but IMO it makes reading the content from the source a little "bumpy", like reading poetry or one of those poorly structured Twitter threads.

…uild_the_brick_field/index.md Co-authored-by: rubiesonthesky <[email protected]>

teoli2003

Looks good to me. Let's merge this.

caugner · 2022-08-26T07:26:36Z

.prettierrc.json

@@ -0,0 +1,3 @@
+{


Just as with .eslintrc.js, I think it would be better to go with .prettierrc.js (using module.exports = { ... } syntax, as this allows adding comments (while JSON) doesn't allow this.

caugner · 2022-08-26T07:26:50Z

files/en-us/games/anatomy/index.md

@@ -55,13 +56,14 @@ But do not immediately assume animations require frame-by-frame control. Simple

 There are two obvious issues with our previous main loop: `main()` pollutes the `{{ domxref("window") }}` object (where all global variables are stored) and the example code did not leave us with a way to _stop_ the loop unless the whole tab is closed or refreshed. For the first issue, if you want the main loop to just run and you do not need easy (direct) access to it, you could create it as an Immediately-Invoked Function Expression (IIFE).

+<!-- prettier-ignore-start -->


Curious: When do you add these?

MMmh. When I reviewed this PR, this morning, it happened that GitHub hid some previously-changed files . I would really like to avoid these as much as possible (read maybe 3-4 places on the whole of MDN), but this can be fixed in a follow-up PR. (The way games/ is doing examples is unusual for MDN).

caugner · 2022-08-26T07:29:10Z

@nschonni FWIW Conventional Commits uses style: prefix for

Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)

See: https://github.com/angular/angular/blob/22b96b9/CONTRIBUTING.md#type

chore: run Prettier on games

ca1db7a

nschonni requested a review from a team as a code owner August 24, 2022 22:19

nschonni requested review from bsmth and removed request for a team August 24, 2022 22:19

github-actions bot added the Content:Other Any docs not covered by another "Content:" label label Aug 24, 2022

nschonni commented Aug 24, 2022

View reviewed changes

This comment was marked as resolved.

Sign in to view

Josh-Cena reviewed Aug 24, 2022

View reviewed changes

files/en-us/games/anatomy/index.md Outdated Show resolved Hide resolved

fix: use prettier-ignore for explicit semicolon

45bf133

Josh-Cena reviewed Aug 25, 2022

View reviewed changes

rubiesonthesky reviewed Aug 25, 2022

View reviewed changes

files/en-us/games/tutorials/2d_breakout_game_pure_javascript/build_the_brick_field/index.md Outdated Show resolved Hide resolved

bsmth reviewed Aug 25, 2022

View reviewed changes

teoli2003 and others added 2 commits August 25, 2022 09:54

Update files/en-us/games/tutorials/2d_breakout_game_pure_javascript/b…

3e59fef

…uild_the_brick_field/index.md Co-authored-by: rubiesonthesky <[email protected]>

chore: use Prettier bracketSameLine

c744033

nschonni requested a review from a team as a code owner August 25, 2022 15:13

teoli2003 approved these changes Aug 26, 2022

View reviewed changes

teoli2003 merged commit e4783c0 into mdn:main Aug 26, 2022

caugner requested changes Aug 26, 2022

View reviewed changes

nschonni deleted the prettier-games branch August 26, 2022 15:59

nschonni mentioned this pull request Sep 7, 2022

chore: Add Husky/lint-staged for local precommit #20397

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: run Prettier on games #19943

chore: run Prettier on games #19943

nschonni commented Aug 24, 2022

nschonni Aug 24, 2022

This comment was marked as resolved.

Josh-Cena Aug 25, 2022

teoli2003 Aug 25, 2022

Josh-Cena Aug 25, 2022

teoli2003 Aug 25, 2022

rubiesonthesky Aug 25, 2022

teoli2003 Aug 25, 2022

nschonni Aug 25, 2022

wbamberg Aug 25, 2022

nschonni Aug 25, 2022

nschonni Aug 25, 2022

rubiesonthesky Aug 25, 2022

teoli2003 Aug 25, 2022 •

edited

Loading

nschonni Aug 25, 2022

rubiesonthesky Aug 25, 2022

teoli2003 Aug 26, 2022

bsmth Aug 25, 2022 •

edited

Loading

teoli2003 Aug 25, 2022

bsmth Aug 25, 2022

teoli2003 Aug 25, 2022

bsmth Aug 25, 2022

wbamberg Aug 25, 2022

wbamberg Aug 25, 2022 •

edited

Loading

Josh-Cena Aug 25, 2022 •

edited

Loading

teoli2003 left a comment

caugner Aug 26, 2022

caugner Aug 26, 2022

teoli2003 Aug 26, 2022

caugner commented Aug 26, 2022

		@@ -55,13 +56,14 @@ But do not immediately assume animations require frame-by-frame control. Simple

		There are two obvious issues with our previous main loop: `main()` pollutes the `{{ domxref("window") }}` object (where all global variables are stored) and the example code did not leave us with a way to _stop_ the loop unless the whole tab is closed or refreshed. For the first issue, if you want the main loop to just run and you do not need easy (direct) access to it, you could create it as an Immediately-Invoked Function Expression (IIFE).

		<!-- prettier-ignore-start -->

chore: run Prettier on games #19943

chore: run Prettier on games #19943

Conversation

nschonni commented Aug 24, 2022

Choose a reason for hiding this comment

This comment was marked as resolved.

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

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

teoli2003 Aug 25, 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

bsmth Aug 25, 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

wbamberg Aug 25, 2022 • edited Loading

Choose a reason for hiding this comment

Josh-Cena Aug 25, 2022 • edited Loading

Choose a reason for hiding this comment

teoli2003 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

caugner commented Aug 26, 2022

teoli2003 Aug 25, 2022 •

edited

Loading

bsmth Aug 25, 2022 •

edited

Loading

wbamberg Aug 25, 2022 •

edited

Loading

Josh-Cena Aug 25, 2022 •

edited

Loading