Fix -Wall in retrie plugin #4071
Conversation
| @@ -212,20 +211,12 @@ runRetrieInlineThisCmd state token RunRetrieInlineThisParams{..} = do | |||
| -- Run retrie to get a list of changes | |||
| -- Select the change that inlines the identifier in the given position | |||
| -- Apply the edit | |||
| ast <- runActionE "retrie" state $ | |||
There was a problem hiding this comment.
All of these bindings were unused. Not sure if they can be removed (if something is relying on the side effects of these?) or not, but the tests are passing so.. 🤷♂️
There was a problem hiding this comment.
I don't think they are necessary.
There was a problem hiding this comment.
If the tests are passing, let's remove them for now and wait for bug reports :)
| case rn of | ||
| (HsGroup{hs_valds, hs_ruleds, hs_tyclds}, _, _, _) -> do | ||
| topLevelBinds <- case hs_valds of | ||
| ValBinds{} -> throwError $ PluginInternalError "getBinds: ValBinds not supported" |
There was a problem hiding this comment.
The above was giving warning:
Pattern match(es) are non-exhaustive
In a pattern binding:
Patterns of type ‘GHCGHC.RenamedSource’ not matched:
((HsGroup _ (ValBinds _ _ _) _ _ _ _ _ _ _ _ _ _), _, _, _)
Throwing error for the unhandled case seems far from ideal but I still consider it step forward (better visibility into where the error is coming from compared to "anonymous" pattern match failure).
| runRetrieCmd :: CommandFunction IdeState RunRetrieParams | ||
| runRetrieCmd state token RunRetrieParams{originatingFile = uri, ..} = ExceptT $ | ||
| withIndefiniteProgress description token Cancellable $ \_updater -> do | ||
| runExceptT $ do | ||
| _ <- runExceptT $ do |
There was a problem hiding this comment.
This is definitely suspicious: wouldn't we want to send the failure to client instead of ignoring it?
There was a problem hiding this comment.
agree, or we can atleast log the error?
There was a problem hiding this comment.
On second thought it's actually sending some errors notifications to the client within the ExceptT block if errors occur, so this is probably ok.
No description provided.