-
Notifications
You must be signed in to change notification settings - Fork 37.6k
add markdownlint to Mason auto-install #1624
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
base: master
Are you sure you want to change the base?
Conversation
As a side comment, I'd rather comment out markdownlint. Having a linter for .md is rather quite annoying. |
@@ -716,6 +716,7 @@ require('lazy').setup({ | |||
local ensure_installed = vim.tbl_keys(servers or {}) | |||
vim.list_extend(ensure_installed, { | |||
'stylua', -- Used to format Lua code | |||
'markdownlint', -- Markdown linter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest commenting out 'markdownlint',
as require 'kickstart.plugins.lint',
in init.lua is also commented out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good point.
User must be given clear instructions in that case, otherwise it will leave the point of friction lingering for some:
Enabling linting by uncommenting require 'kickstart.plugins.lint'
in init.lua
will also enable markdownlint
in lint.lua
.
Unless user explicitly disables markdownlint
in lint.lua
, they have to also ensure that thay have installed markdownlint
to prevent any issues.
Which can be done by uncommenting markdownlint
in 'mason-tool-installer'
setup.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO, kickstart is not a distribution but starting point. Therefore, kickstart doesn't have to be seamless. The users must experience this kind of small, tiny issue. And your PR provides a good pointer to solve it. If they see your PR, they will understand how kickstart works. If they don't have any issue when enabling require 'kickstar.plugin.lint'
, they won't understand how to add the other lint.
I think we need to add --markdownlint,
along with comments saying where to check to enable a lint. This is just my opinion. Let's hear from the others.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest just documenting that if you enable kickstart's lint plugin, you also have to uncomment markdownlint to be auto-installed by mason if the user does not want to see the error. Regardless if kickstart is supposed to be seamless or not, having to stumble upon a PR to find the solution to an issue is just a poor experience. kickstart's big thing is about docs, so let's rely on that.
Or maybe this also puts into question markdownlint being a dependency of the lint plugin's default implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm actually 100% on board with removing .md
as a default dependency. markdownlint
also requires npm
as a dependency when installing through Mason. I'm pretty sure majority of users will not see the bug, I was working on a clean MacOS install so I didn't have npm in my system.
TL;DR : It just seems like having .md
as a default is more trouble than what it's worth. Any reason why it was implemented in first place?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hah, thanks. Yea, commenting out markdownlint
seems like the most optimal choice imo. We can definitely provide further instructions to clarify things if needed.
Fixes #1510
lua/kickstart/plugins/lint.lua adds
markdownlint
as a linter. But it was not added to list of tools that Mason installed.This adds markdownlint under
ensure_installed
, which should solve the following error: