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

Configure and apply markdownlint #46

Merged
merged 10 commits into from Aug 15, 2020
Merged

Configure and apply markdownlint #46

merged 10 commits into from Aug 15, 2020

Conversation

kachkaev
Copy link
Member

@kachkaev kachkaev commented Aug 9, 2020

@jwoLondon WDYT of using https://github.com/DavidAnson/markdownlint as an extra linter? It supplements Prettier and helps us keep markdowns a bit cleaner semantically (Prettier only works with syntax).

@kachkaev kachkaev marked this pull request as ready for review August 9, 2020 21:17
@jwoLondon
Copy link
Member

Are you suggesting we just use it internally for consistent formatting of content we share in the litvis repo, or that we encourage/mandate for litvis users?

Copy link
Member

@jwoLondon jwoLondon left a comment

Choose a reason for hiding this comment

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

This looks good and should help us to create consistent and accessible markdown.

Perhaps we should also disable MD013 (long line length), as we sometimes use bulleted lists with multiple sentences that get picked up by the linter.

Otherwise, I agree on disabling first line headings (css and hidden elm blocks commonly used before first heading). And agree also on disabling no-inline-html (sometimes use subscripts/superscripts and sometimes html img where we can control image dimensions).

I have no strong feelings about no-duplicate heading and no-emphasis as heading, but presumably you have examples where these are necessary.

@jwoLondon
Copy link
Member

Have just noticed that the linter picks up MD004 'unordered list style' for lists that alternate between - and *. This is what Prettier inserts when we wish to have blank lines between list items. Is it better to use consistent list symbols and accept having no blank lines between items (subject to css styling), or do we drop this condition from the linter?

@jwoLondon
Copy link
Member

I think we should also modify MD026, trailing punctuation in heading, so that question marks are allowed.

"MD026": { "punctuation": ".,;:!"}

@kachkaev
Copy link
Member Author

Hi Jo! I'm at work now, so can't respond to emails – only seeing github notifications on my work laptop. WRT the rules we want to pick, I'm happy with any set – whatever you find convenient.

You'll notice that in this PR I'm extending @kachkaev/markdownlint-config – this is what I use in own projects (see index.json).

Perhaps, we don't need this extra abstraction in litvis and are better off without extending any third party config. In the future, when the set of rules you prefer settles, we can publish @gicentre/markdownlint-config and this will be the recommend starting point for giCentre projects.

In this PR, I mainly focus on adding the tool to the process, finding an ideal set of rules is out of scope. If you thing Markdownlint useful in principle, feel free to push any changes to this branch and merge. You'll probably want to install https://marketplace.visualstudio.com/items?itemName=DavidAnson.vscode-markdownlint too for a better local UX.

@@ -39,6 +41,7 @@
},
"lint-staged": {
"**/*": [
"suppress-exit-code markdownlint --fix",
Copy link
Member Author

Choose a reason for hiding this comment

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

Using suppress-exit-code here so that it was possible to commit changes even if there are markdownlint issues. We can remove this later when we are sure that markdowlint is a must-have tool in this repo.

@kachkaev
Copy link
Member Author

Hi Jo, the PR is ready for another review – thanks for your comments.

I also replaced ```md with ```markdown, because github syntax highlighter treats md as Matlab code or something.

Difference: before / after

@jwoLondon jwoLondon merged commit b848714 into master Aug 15, 2020
@kachkaev kachkaev deleted the markdownlint branch August 15, 2020 16:21
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.

None yet

2 participants