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

Improve no plugin messages #3864

Merged
merged 12 commits into from
Jan 4, 2024
35 changes: 21 additions & 14 deletions ghcide/src/Development/IDE/Plugin/HLS.hs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import qualified Data.List as List
import Data.List.NonEmpty (NonEmpty, nonEmpty, toList)
import qualified Data.List.NonEmpty as NE
import qualified Data.Map as Map
import Data.Maybe (mapMaybe)
import Data.Some
import Data.String
import Data.Text (Text)
Expand Down Expand Up @@ -72,16 +73,17 @@ instance Pretty Log where
<> pretty method <> ": " <> viaShow exception
instance Show Log where show = renderString . layoutCompact . pretty

noPluginEnabled :: Recorder (WithPriority Log) -> SMethod m -> [PluginId] -> IO (Either ResponseError c)
noPluginEnabled :: Recorder (WithPriority Log) -> SMethod m -> [(PluginId, PluginStatus)] -> IO (Either ResponseError c)
noPluginEnabled recorder m fs' = do
logWith recorder Warning (LogNoPluginForMethod $ Some m)
let err = ResponseError (InR ErrorCodes_MethodNotFound) msg Nothing
msg = pluginNotEnabled m fs'
return $ Left err
where pluginNotEnabled :: SMethod m -> [PluginId] -> Text
where pluginNotEnabled :: SMethod m -> [(PluginId, PluginStatus)] -> Text
pluginNotEnabled method [] = "No plugin installed for this " <> T.pack (show method) <> " request."
pluginNotEnabled method availPlugins =
"No plugin enabled for " <> T.pack (show method) <> ", potentially available: "
<> (T.intercalate ", " $ map (\(PluginId plid) -> plid) availPlugins)
"No plugin enabled for this " <> T.pack (show method) <> " request.\n Plugins installed for this method, but not enabled for this request are:\n"
joyfulmantis marked this conversation as resolved.
Show resolved Hide resolved
<> (T.intercalate "\n" $ map (\(PluginId plid, pluginStatus) -> plid <> " " <> (renderStrict . layoutCompact . pretty) pluginStatus) availPlugins)

pluginDoesntExist :: PluginId -> Text
pluginDoesntExist (PluginId pid) = "Plugin " <> pid <> " doesn't exist"
Expand Down Expand Up @@ -213,8 +215,8 @@ executeCommandHandlers recorder ecs = requestHandler SMethod_WorkspaceExecuteCom
res <- runExceptT (f ide a) `catchAny` -- See Note [Exception handling in plugins]
(\e -> pure $ Left $ PluginInternalError (exceptionInPlugin p SMethod_WorkspaceExecuteCommand e))
case res of
(Left (PluginRequestRefused _)) ->
liftIO $ noPluginEnabled recorder SMethod_WorkspaceExecuteCommand (fst <$> ecs)
(Left (PluginRequestRefused r)) ->
liftIO $ noPluginEnabled recorder SMethod_WorkspaceExecuteCommand [(p,PluginDisabled (PluginRejected r))]
(Left pluginErr) -> do
liftIO $ logErrors recorder [(p, pluginErr)]
pure $ Left $ toResponseError (p, pluginErr)
Expand All @@ -236,11 +238,13 @@ extensiblePlugins recorder plugins = mempty { P.pluginHandlers = handlers }
(IdeMethod m :=> IdeHandler fs') <- DMap.assocs handlers'
pure $ requestHandler m $ \ide params -> do
config <- Ide.PluginUtils.getClientConfig
-- Only run plugins that are allowed to run on this request
let fs = filter (\(_, desc, _) -> pluginEnabled m params desc config) fs'
-- Only run plugins that are allowed to run on this request, save the
-- list of disabled plugins incase that's all we have
let (fs, dfs) = List.partition (\(_, desc, _) -> pluginEnabled m params desc config == PluginEnabled) fs'
let disabledPluginsReason = (\(x, desc, _) -> (x, pluginEnabled m params desc config)) <$> dfs
-- Clients generally don't display ResponseErrors so instead we log any that we come across
case nonEmpty fs of
Nothing -> liftIO $ noPluginEnabled recorder m ((\(x, _, _) -> x) <$> fs')
Nothing -> liftIO $ noPluginEnabled recorder m disabledPluginsReason
Just neFs -> do
let plidsAndHandlers = fmap (\(plid,_,handler) -> (plid,handler)) neFs
es <- runConcurrently exceptionInPlugin m plidsAndHandlers ide params
Expand All @@ -251,9 +255,12 @@ extensiblePlugins recorder plugins = mempty { P.pluginHandlers = handlers }
Nothing -> do
let noRefused (_, PluginRequestRefused _) = False
noRefused (_, _) = True
filteredErrs = filter noRefused errs
case nonEmpty filteredErrs of
Nothing -> liftIO $ noPluginEnabled recorder m ((\(x, _, _) -> x) <$> fs')
(asErrors, asRefused) = List.partition noRefused errs
convertPRR (pId, PluginRequestRefused r) = Just (pId, PluginDisabled (PluginRejected r))
convertPRR _ = Nothing
asRefusedReason = mapMaybe convertPRR asRefused
case nonEmpty asErrors of
Nothing -> liftIO $ noPluginEnabled recorder m (disabledPluginsReason <> asRefusedReason)
Just xs -> pure $ Left $ combineErrors xs
Just xs -> do
pure $ Right $ combineResponses m config caps params xs
Expand All @@ -274,8 +281,8 @@ extensibleNotificationPlugins recorder xs = mempty { P.pluginHandlers = handlers
(IdeNotification m :=> IdeNotificationHandler fs') <- DMap.assocs handlers'
pure $ notificationHandler m $ \ide vfs params -> do
config <- Ide.PluginUtils.getClientConfig
-- Only run plugins that are allowed to run on this request
let fs = filter (\(_, desc, _) -> pluginEnabled m params desc config) fs'
-- Only run plugins that are enabled for this request
let fs = filter (\(_, desc, _) -> pluginEnabled m params desc config == PluginEnabled) fs'
case nonEmpty fs of
Nothing -> do
logWith recorder Warning (LogNoPluginForMethod $ Some m)
Expand Down
Loading
Loading