Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(): prettify all source code #8276

Merged
merged 4 commits into from
Sep 12, 2022
Merged

chore(): prettify all source code #8276

merged 4 commits into from
Sep 12, 2022

Conversation

asturur
Copy link
Member

@asturur asturur commented Sep 11, 2022

In theory no changes here.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 11, 2022

Coverage after merging prettify into master will be

82.39%

Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
HEADER.js54.90%48.15%0%65.22%105, 105, 105, 12, 14, 14, 14, 14, 14, 16–17, 21, 21–22, 22, 22, 24, 24, 26, 28, 30–31
src
   cache.ts97.06%90%100%100%57
   canvas.class.ts93.44%90.36%94.23%95.58%1077, 1077–1078, 1081, 1101, 1101, 1136, 1169–1170, 1198–1199, 1232, 1240, 1350–1351, 1353–1354, 1374–1375, 1533, 1538, 1548, 1552, 485–486, 491, 500, 649–651, 696–697, 761–762, 765, 767, 814–816, 858, 863–864, 892–893
   config.ts77.27%66.67%66.67%84.62%130, 138–140, 151–153
   constants.ts100%100%100%100%
   intersection.class.ts100%100%100%100%
   pattern.class.ts92.19%85.71%100%96.30%110, 116, 127, 136, 88
   point.class.ts100%100%100%100%
   shadow.class.ts95.95%90%100%100%172, 239, 8
   static_canvas.class.ts88.97%82.26%96.70%91.67%1152–1153, 1153, 1153–1154, 1288, 1354–1355, 1358, 1407–1408, 1501, 1516, 1520, 1546–1547, 1576–1577, 1610–1611, 1652–1653, 1656, 1658, 1658, 1658, 1658, 1662, 1662, 1662–1664, 1686–1687, 1728–1729, 1732, 1734, 1734, 1734, 1734, 1738, 1738, 1738–1740, 1809, 1809–1810, 1813, 1813–1814, 1873, 1875, 1875, 1875, 1875, 1875–1876, 1879–1880, 1880, 1880–1881, 1884, 1884, 1884, 1886, 1889, 1895, 1897–1898, 1898, 1898, 1901–1902, 1902, 1902, 1905, 278–279, 281–282, 284–285, 298–299, 301–302, 616, 642, 698–701, 881, 899–901, 901, 901–902, 904–905
src/brushes
   base_brush.class.ts100%100%100%100%
   circle_brush.class.ts2.99%0%0%3.92%100–101, 103, 105–107, 116, 116, 116, 118, 120, 122–124, 126–129, 137, 144, 146, 26, 31–32, 40–44, 48–52, 59–62, 70–74, 76, 84, 84, 84, 84, 84–85, 87, 87, 87–90, 92
   pattern_brush.class.ts5.26%0%0%8.33%16, 20–23, 25–26, 26, 26–29, 37–38, 40, 44, 55, 55, 55, 63–65, 65, 65, 72–73, 75–76, 76, 80
   pencil_brush.class.ts91.95%85.42%100%93.69%125–126, 155, 155–157, 279, 283, 288–289, 71–72, 87–88
   spray_brush.class.ts2.30%0%0%3.08%102–104, 106–107, 115, 115, 115, 115, 115–116, 118–119, 126–127, 129, 131–135, 144, 148–149, 149, 157, 157, 157–160, 162–165, 169–170, 172, 174–177, 180, 187–188, 190, 192–193, 195, 202–203, 205–206, 209, 209, 216, 216, 220, 25–26, 28–30, 30, 30–32, 36, 45, 52, 59, 66, 73, 80, 92–94
src/color
   color.class.ts91.67%84.51%100%94.44%325–326, 330–331, 334–335, 41, 45, 72–73, 73, 75, 75, 75–76, 78–79
   color_map.ts100%100%100%100%
   constants.ts100%100%100%100%
   index.ts100%100%100%100%
   util.ts100%100%100%100%
src/controls
   control.class.ts90.24%81.48%86.67%97.50%210, 304, 304, 348, 372, 6
   controls.actions.ts76.80%69.20%93.75%80.67%161, 163, 163, 163, 165, 167, 26, 28, 28, 28, 290–293, 321, 323, 330–331, 333, 333–334, 336–337, 341, 373, 375, 382–383, 385, 385–386, 388–389, 393, 421–422, 424, 426, 431, 434, 434, 434–435, 435, 435, 437, 437, 437–438, 438, 438, 441, 441, 441–442, 442, 442, 475–476, 478, 480, 485, 488, 488, 488–489, 489, 489, 491, 491, 491–492, 492, 492, 495, 495, 495–496, 496, 496, 520–522, 528, 528, 528–529, 532–535, 537, 537, 537–539, 539, 539–541, 543, 543, 543–545, 545, 545–546, 551, 551, 551–552, 554, 556–558, 57, 589–590, 592–594, 641–643, 8, 841
   controls.render.ts85.11%84.78%100%84.78%23, 27, 42–49, 58–59, 82, 86
   default_controls.ts94.29%50%100%100%115, 83
src/filters
   2d_backend.class.ts96.43%83.33%100%100%78
   WebGLProbe.ts60%80%57.14%52.17%37–38, 48–52, 66–69, 71, 77
   base_filter.class.ts28.90%31.82%36.36%26.17%100–102, 102, 102–103, 112–116, 116, 116–117, 124–125, 125, 125–128, 143, 159, 169–174, 178, 181, 181, 181–184, 184, 184–185, 185, 188–189, 195, 204–205, 210–214, 257–260, 273, 273, 273–274, 276, 292–294, 294, 294, 294, 294–295, 297, 299–300, 306–307, 309–311, 315–316, 318, 322–324, 328, 332, 352, 352, 352–356, 393, 78, 78, 78–79, 79, 79–80, 80, 80–81, 86–89, 89, 89–90, 99
   

@asturur
Copy link
Member Author

asturur commented Sep 11, 2022

If there is a stlye rule ( those are all defaults from prettier ) that you don't like this is the moment to talk about it.

Copy link
Contributor

@ShaMan123 ShaMan123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like

condition
? value
: other

and prefer

condition ?
   value :
   other;

BUT as long as VSCODE formats it for me f*** it

? [255, 255, 255, 0]
: Color.sourceFromHex(color) ||
Color.sourceFromRgb(color) ||
Color.sourceFromHsl(color) || [0, 0, 0, 1]; // color is not recognize let's default to black as canvas does
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like this

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see if is configurable and change it

@ShaMan123
Copy link
Contributor

ShaMan123 commented Sep 12, 2022

I wonder (and dread) how hard it will be to rebase PRs from before the migration

@asturur
Copy link
Member Author

asturur commented Sep 12, 2022

I wonder (and dread) how hard it will be to rebase PRs from before the migration

Copy and reapply the changes. Indeed we waited too much to do this.

@ShaMan123
Copy link
Contributor

which do you prefer? https://eslint.org/docs/latest/rules/operator-linebreak

@ShaMan123
Copy link
Contributor

What is the difference between this https://github.com/prettier/eslint-plugin-prettier and https://github.com/prettier/eslint-config-prettier?

Anyways I managed to get it more or less as I prefer but only when I run lint. prettier doesn't do it

@ShaMan123
Copy link
Contributor

It seems that https://github.com/prettier/eslint-config-prettier disables the rule I added when running with prettier

image

@asturur
Copy link
Member Author

asturur commented Sep 12, 2022

the issue is that that is a Lint rule. Prettier just does formatting and has a set of lint rules with the intent of being only disabled, to aovid overlapping.

eslint-config-prettier: Disable eslint rules that overlap with prettier.
eslint-plugin-prettier: Run prettier as a lint rule

I would prefer separation of concern between formatting and lintable bugs

@asturur
Copy link
Member Author

asturur commented Sep 12, 2022

ok maybe we have to swallow it. This is what a popular tool offer and debating where the question mark goes is wasted time.
I didn't like the else break in more lines,

}
else {

But i had to keep it for 7 long years

@ShaMan123
Copy link
Contributor

Yeah yeah sure.

This reverts commit 7efdc7a.
@ShaMan123 ShaMan123 self-requested a review September 12, 2022 08:42
@ShaMan123
Copy link
Contributor

Isn't .eslintrc.json a dead file?

@asturur
Copy link
Member Author

asturur commented Sep 12, 2022

Isn't .eslintrc.json a dead file?

Could be that without JS files anymore is dead.

@asturur asturur merged commit 5538c06 into master Sep 12, 2022
@asturur asturur deleted the prettify branch November 1, 2022 23:38
frankrousseau pushed a commit to cgwire/fabric.js that referenced this pull request Jan 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants