-
-
Notifications
You must be signed in to change notification settings - Fork 389
Various PluginError PR suggestions I missed earlier #3737
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
Conversation
Merge fendor/enhance/plugin-logger-structure
Additionally further PluginError constructors and helpers refactoring
@@ -80,10 +81,17 @@ prettyResponseError err = errorCode <> ":" <+> errorBody | |||
errorCode = pretty $ show $ err ^. L.code | |||
errorBody = pretty $ err ^. L.message | |||
|
|||
pluginNotEnabled :: SMethod m -> [(PluginId, b, a)] -> Text | |||
pluginNotEnabled :: SMethod m -> [PluginId] -> Text |
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.
should this just be inlined into the function below now?
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.
Yes, good catch!
(first (toResponseError . (p,)) <$> runExceptT (f ide a)) `catchAny` -- See Note [Exception handling in plugins] | ||
(\e -> logAndReturnError' recorder (InR ErrorCodes_InternalError) (ExceptionInPlugin p (Some SMethod_WorkspaceApplyEdit) e)) | ||
res <- runExceptT (f ide a) `catchAny` -- See Note [Exception handling in plugins] | ||
(\e -> pure $ Left $ PluginInternalError (exceptionInPlugin p SMethod_WorkspaceExecuteCommand e)) |
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.
Why this method? And it was different before??
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.
Because that's it's actually method, see line 171. WorkspaceApplyEdit before was an error, either autocomplete typo, brain fog, or sloppy mistake on my part.
No description provided.