[core] Enforce straight quote #34686

oliviertassinari · 2022-10-09T14:48:34Z

The objective of this change is to:

To enforce non-straight quotes. It's a convention that we have followed since https://groups.google.com/a/mui.com/g/shared-accounts/c/wMZm8Fkzfp0/m/SlBNUsLBAQAJ but without an actual check, it's too easy to regress.
To set up the first custom rule linting our markdown. It's similar to our custom eslint rules. We might get more ideas going forward. For example, GitHub has written this one https://unpkg.com/browse/@github/[email protected]/no-default-alt-text.js. One that I would like to implement is to guarantee that the git diff format is correct, I have seen it done wrong so many times 😁.
Make it possible to remove the duplication in https://github.com/mui/mui-x/blob/next/.markdownlint.jsonc and https://github.com/mui/mui-toolpad/blob/master/.markdownlint.jsonc. It's painful to keep in sync with this repository. The solution is to use a JavaScript configuration over a JSON one so we can then do direct imports.

I had to move from markdownlint-cli to markdownlint-cli2 which is from the same author, but with extra features.

mui-bot · 2022-10-09T14:52:07Z

No bundle size changes

Generated by 🚫 dangerJS against 1108af6

oliviertassinari · 2022-10-09T14:49:42Z

scripts/prettier.js

@@ -48,7 +48,7 @@ function runPrettier(options) {
    });

  const files = glob
-    .sync('**/*.{js,jsx,md,tsx,ts,json,html,css,prisma,yml}', {
+    .sync('**/*.{js,jsx,md,tsx,ts,cjs,mjs,json,html,css,prisma,yml}', {


mjs is used in https://github.com/mui/material-ui/blob/d3010f1599af56b3b1dd9e576b27d5b91d760e9f/scripts/generateCodeowners.mjs
cjs was introduced in this PR.

oliviertassinari · 2022-10-09T17:00:45Z

.markdownlint-cli2.cjs

@@ -0,0 +1,28 @@
+const straightQuotes = require('./packages/markdownlint-rule-mui/straight-quotes');
+
+module.exports = {


This new configuration file is still compatible with https://marketplace.visualstudio.com/items?itemName=DavidAnson.vscode-markdownlint.

michaldudak

I'm OK with this change in general. Got just one comment.
It's worth letting @samuelsycamore know about this.

michaldudak · 2022-10-10T07:46:50Z

CHANGELOG.md

@@ -376,7 +376,7 @@ _Aug 22, 2022_

 A big thanks to the 11 contributors who made this release possible. Here are some highlights ✨:

- ✨ @michaldudak synced the Material Icons set with the latest from Google (#33988).  
+- ✨ @michaldudak synced the Material Icons set with the latest from Google (#33988).


This change is unrelated. Is there any specific reason for it?

@michaldudak I have VS Code configured to automatically remove trailing space on save, e.g. of a value: https://github.com/mui/mui-x/pull/6242/files#r990690291.

This shouldn't apply to markdown as trailing whitespace is meaningful there.

as trailing whitespace is meaningful there.

@michaldudak Would this be better b88ed0d? The existence of line break is not obvious to identify on HEAD:

material-ui/CHANGELOG.md

Lines 379 to 380 in e4c5bff

- ✨ @michaldudak synced the Material Icons set with the latest from Google (#33988).

A couple of icons changed their appearance. See the difference [on this Argos build](https://app.argos-ci.com/mui/material-ui/builds/4428]).

I think that it would be better with <br> to be able to predict how the markdown will render at first look. https://github.github.com/gfm/#hard-line-breaks also mentions that \ is available.

On this topic, I think that "GitHub release" is buggy, it adds a line break for a \n in the markdown but should it be for \n\n? For example, https://github.com/mui/material-ui/blob/master/CHANGELOG.md#muibase500-alpha98 is not the same as https://github.com/mui/material-ui/releases/tag/v5.10.6. WUT? I have opened community/community#35750.

I think that we can enable https://github.com/updownpress/markdown-lint/blob/master/rules/009-no-trailing-spaces.md next 😁

diff --git a/.markdownlint-cli2.cjs b/.markdownlint-cli2.cjs index 78ba7deec3..0b423d3689 100644 --- a/.markdownlint-cli2.cjs +++ b/.markdownlint-cli2.cjs @@ -4,6 +4,7 @@ module.exports = { config: { default: true, MD004: false, // MD004/ul-style. Buggy + MD009: true, // MD009/no-trailing-spaces MD013: false, // MD013/line-length. Already handled by Prettier. MD014: false, // MD014/commands-show-output. It's OK. MD024: { siblings_only: true }, // MD024/no-duplicate-heading/no-duplicate-header

I personally prefer the cleanliness of trailing spaces, but an HTML tag is acceptable as well.

cleanliness of trailing spaces

I have updated my previous comment, I think that the issue is with predictability.

The markdown document ideally should be well readable both as plain text and rendered. The inclusion of an HTML tag (<br />) harms the reading flow. The \ character could be a bit better in this context.

I have done the change from <br> to \ and pushed, it works https://github.com/mui/material-ui/blob/1108af61b61eeffdb0ee4b9149668ccde624a4a3/CHANGELOG.md#5102.

I think that for the markdown, it's more important to be able to predict how the source will render, as ultimately the rendered markdown will be read x10?, x100? more than the markdown source. So I think that <br> is clearer than \, but \ is already much clearer than a double space, so deal 😁.

oliviertassinari · 2022-10-10T10:54:10Z

It's worth letting @samuelsycamore know about this.

Great idea, we started a discussion about it at https://www.notion.so/Style-guide-2a957a4168a54d47b14bae026d06a24b?d=aff8a8dc579c48f8a2500de4253c6949#b17ab6d6eb6f4589ba084207caf33443.

In the worse case, if we don't want to enforce this quote style rule, points 2. and 3. are still wins :)

mapache-salvaje · 2022-10-14T16:15:33Z

To enforce non-straight quotes.

Do you mean "enforce straight quotes"? It looks like that's what this is doing—replacing all curly quotes with straight quotes. I'm cool with that. 👍

[core] Fix straight quote use

8937fe4

oliviertassinari added the core Infrastructure work going on behind the scenes label Oct 9, 2022

oliviertassinari added 2 commits October 9, 2022 18:55

fix ci

25dc603

a few more format issues

8d6fc1b

oliviertassinari commented Oct 9, 2022

View reviewed changes

oliviertassinari marked this pull request as ready for review October 9, 2022 17:00

oliviertassinari requested review from michaldudak and m4theushw as code owners October 9, 2022 17:00

oliviertassinari added the docs Improvements or additions to the documentation label Oct 9, 2022

more fixes

7f9031a

michaldudak reviewed Oct 10, 2022

View reviewed changes

oliviertassinari requested a review from mapache-salvaje October 10, 2022 10:53

oliviertassinari mentioned this pull request Oct 10, 2022

[core] Remove outdated docsearch.js dependency mui/mui-x#6242

Merged

oliviertassinari added 2 commits October 10, 2022 13:17

add line break

b88ed0d

line break

1108af6

michaldudak approved these changes Oct 12, 2022

View reviewed changes

oliviertassinari changed the title ~~[core] Fix straight quote use~~ [core] Enforce straight quote Oct 14, 2022

oliviertassinari merged commit c3d58bb into mui:master Oct 14, 2022

oliviertassinari deleted the markdownlint branch October 14, 2022 20:39

This was referenced Oct 14, 2022

[docs] Enforce no trailing spaces #34762

Merged

[docs] Improve markdownlint mui/mui-x#6518

Merged

[docs] Improve markdownlint mui/toolpad#1159

Merged

[core] Fix trailing spaces and git diff format mui/mui-x#6523

Merged

daniel-rabe pushed a commit to daniel-rabe/material-ui that referenced this pull request Nov 29, 2022

[core] Enforce straight quote (mui#34686)

b0b9317

feliperli pushed a commit to jesrodri/material-ui that referenced this pull request Dec 6, 2022

[core] Enforce straight quote (mui#34686)

ebbfe93

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[core] Enforce straight quote #34686

[core] Enforce straight quote #34686

oliviertassinari commented Oct 9, 2022 •

edited

Loading

mui-bot commented Oct 9, 2022 •

edited

Loading

oliviertassinari Oct 9, 2022

oliviertassinari Oct 9, 2022 •

edited

Loading

michaldudak left a comment

michaldudak Oct 10, 2022

oliviertassinari Oct 10, 2022 •

edited

Loading

michaldudak Oct 10, 2022

oliviertassinari Oct 10, 2022 •

edited

Loading

michaldudak Oct 11, 2022

oliviertassinari Oct 11, 2022

michaldudak Oct 11, 2022

oliviertassinari Oct 11, 2022 •

edited

Loading

oliviertassinari commented Oct 10, 2022 •

edited

Loading

mapache-salvaje commented Oct 14, 2022

		@@ -0,0 +1,28 @@
		const straightQuotes = require('./packages/markdownlint-rule-mui/straight-quotes');

		module.exports = {

	- ✨ @michaldudak synced the Material Icons set with the latest from Google (#33988).
	A couple of icons changed their appearance. See the difference [on this Argos build](https://app.argos-ci.com/mui/material-ui/builds/4428]).

[core] Enforce straight quote #34686

[core] Enforce straight quote #34686

Conversation

oliviertassinari commented Oct 9, 2022 • edited Loading

mui-bot commented Oct 9, 2022 • edited Loading

oliviertassinari Oct 9, 2022

Choose a reason for hiding this comment

oliviertassinari Oct 9, 2022 • edited Loading

Choose a reason for hiding this comment

michaldudak left a comment

Choose a reason for hiding this comment

michaldudak Oct 10, 2022

Choose a reason for hiding this comment

oliviertassinari Oct 10, 2022 • edited Loading

Choose a reason for hiding this comment

michaldudak Oct 10, 2022

Choose a reason for hiding this comment

oliviertassinari Oct 10, 2022 • edited Loading

Choose a reason for hiding this comment

michaldudak Oct 11, 2022

Choose a reason for hiding this comment

oliviertassinari Oct 11, 2022

Choose a reason for hiding this comment

michaldudak Oct 11, 2022

Choose a reason for hiding this comment

oliviertassinari Oct 11, 2022 • edited Loading

Choose a reason for hiding this comment

oliviertassinari commented Oct 10, 2022 • edited Loading

mapache-salvaje commented Oct 14, 2022

oliviertassinari commented Oct 9, 2022 •

edited

Loading

mui-bot commented Oct 9, 2022 •

edited

Loading

oliviertassinari Oct 9, 2022 •

edited

Loading

oliviertassinari Oct 10, 2022 •

edited

Loading

oliviertassinari Oct 10, 2022 •

edited

Loading

oliviertassinari Oct 11, 2022 •

edited

Loading

oliviertassinari commented Oct 10, 2022 •

edited

Loading