-
-
Notifications
You must be signed in to change notification settings - Fork 388
Deduplicate HLS plugins #3112
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Deduplicate HLS plugins #3112
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.
LGTM!
I think it would be better to use a Map, and then use an explicit priority (a simple Perhaps we also want a richer graph structure in the future so we can express dependencies between plugins, but lets just go for explicit priorities for now. |
41446c8
to
2a7561b
Compare
Agreed, explicit priorities are much better. Added now |
76a6cff
to
7baaaab
Compare
Use a smart constructor to prevent duplicated plugins. We cannot use a set since order matters
7baaaab
to
ccefa4e
Compare
@@ -113,6 +152,8 @@ instance Show (IdeCommand st) where show _ = "<ide command>" | |||
data PluginDescriptor (ideState :: *) = | |||
PluginDescriptor { pluginId :: !PluginId | |||
-- ^ Unique identifier of the plugin. | |||
, pluginPriority :: Natural |
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.
What happens if two plugins have the same priority?
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.
Unspecified order as decided by unordered-containers
|
||
-- --------------------------------------------------------------------- | ||
|
||
descriptor :: PluginId -> PluginDescriptor IdeState | ||
descriptor plId = (defaultPluginDescriptor plId) | ||
{ pluginHandlers = mkPluginHandler J.STextDocumentCodeAction codeActionProvider | ||
<> mkPluginHandler J.STextDocumentCompletion completion | ||
, pluginPriority = ghcideCompletionsPluginPriority + 1 |
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.
worth explaining
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.
Argh, I forgot adding a line there.
Use a smart constructor to prevent duplicated plugins. We cannot use a
set since order matters for plugin execution.