-
-
Notifications
You must be signed in to change notification settings - Fork 314
docs: rework Installation & Setup sections, and other minor adjustments #1951
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
Conversation
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 noted two nits I noticed along the way, and thought they are worth a mention when already rewriting the docs. But they absolutely not necessary minor stuff.
Also, the ---@since 1.0.0
in the default config could maybe be removed? Not from the code (I assume they serve some automated docgen purpose), but in the README. Since we are now at v2, such references to v1 feel a bit outdated.
The following is the recommended setup when using `lazy.nvim`. It will set up the plugin for you, meaning **you don't have | ||
to call `require("mason").setup()` yourself**. |
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.
Is callingsetup
multiple times an issue for mason? If so, this could simply be fixed programmatically, by setting a masonWasAlreadySetup
variable that disallows running setup
twice. mason is one of the first plugins new users install, so keeping it as fool-proof as possible might be worthwhile.
Alternatively, mason could also support changing the config with subsequent setup
calls. Though not a hard standard, it is a common-ish practice among newer nvim plugins to do so.
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 already is has_setup
variable, but it's only used in mason-lspconfig.nvim
to ensure the correct setup order. Calling it multiple times is an issue right now because it'll result in duplicate registries, which causes issues during registry installation (working on a fix for that specifically), and a polluted PATH
environment variable.
The setup itself is very simple by design so allowing it to be called multiple times shouldn't be an issue, would require some minor adjustments. The only problem would be if certain settings (especially install_root_dir
) changes between setups and the user has somehow accessed Mason some time in-between, either programmatically via some 3rd party plugin or manually.
I'm not too interested in officially supporting multiple setups because it can cause some headaches and is difficult to support for all settings, but maybe keeping track of the amount of times setup has been called and provide a warning/error in the health check?
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.
Then how about using has_setup
in mason.nvim
as well to prevent multiple calls? Would be the simplest solution
They primarily serve documentation purposes, there's no docgen involved atm. I kinda like having them visible in the docs. The documentation for the default configuration is done very lazily atm, just copy pasting the actual Lua table as-is. |
In principle I like such extra info, going through the releaes, release 1.0.0 is more than 2 years old. So the "since 1.0.0" info is only relevant to people who have not upgraded their mason in more than 2 years. 😅 |
Yeah it's probably not relevant anymore for such old versions. I think the problem is with how it's displayed as a simple Lua table with Lua comments. Also not sure where one would draw the line in terms of when to display it and when not to. Anyways, I'll merge this in the current state |
Preview: https://github.com/mason-org/mason.nvim/tree/docs/setup.
@chrisgrieser What do you think?