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

add plugin overriding feature #1200

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

prevostcorentin
Copy link

@prevostcorentin prevostcorentin commented Jan 22, 2023

PR related to issue #1201.

@prevostcorentin
Copy link
Author

prevostcorentin commented Jan 22, 2023

This PR can be improved:

If two plug-ins defines override=true, the second one in the read order will replace the first one. Because the plug-in specification table is filled asynchronously, two overrides will bring hazardous situations.

Two solutions comes in mind:

  • Explicitly forbid more than one override for a given plug-in
  • Explicitly specifiy an override priority

I don't see any situation where two overrides would be necessary. This feature should be used when you are developing a feature you need on a plugin you pull from github and the feature is not yet merged. While waiting for the feature to be merged or not, you want to keep a configuration as clean possible (as I like to do).

I would personally disabling the ability to define more than one override for a given plug-in.

Best regards.

@@ -1,39 +1,42 @@
local a = require('plenary.async_lib.tests')
Copy link
Author

@prevostcorentin prevostcorentin Jan 22, 2023

Choose a reason for hiding this comment

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

Many undesirable changes are brought by an auto format feature.
It can be removed if needed.

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.

1 participant