Extract code action #3337
Extract code action #3337
Conversation
|
@michaelpj I think you might be interested |
|
Could you rebase this? I think that will make the diff nicer now your previous PR has been merged. |
santiweight
force-pushed
the
extract-plugin
branch
2 times, most recently
from
a45c5ee to
a78fdac
Compare
santiweight
force-pushed
the
extract-plugin
branch
from
a78fdac to
0d91dd1
Compare
|
This is now ready to review: all tests pass. I have added extensive documentation to guide the reader through my quick-and-dirty solution for finding free variables. @ozkutuk If you're interested I have not CPPed yet because I can imagine the code will change a fair amount during review... |
plugins/hls-qualify-imported-names-plugin/src/Ide/Plugin/QualifyImportedNames.hs
Show resolved
Hide resolved
plugins/hls-refactor-plugin/src/Development/IDE/GHC/ExactPrint.hs
Outdated
Show resolved
Hide resolved
plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs
Outdated
Show resolved
Hide resolved
| fromLspList :: List a -> [a] | ||
| fromLspList (List a) = a | ||
|
|
||
| -- | Find the text delineated by a given Range from a source Text. |
There was a problem hiding this comment.
I think this entire function can be implemented quite idiomatically using the text-rope package that we use in lsp. See here for something similar: https://github.com/haskell/lsp/blob/master/lsp/src/Language/LSP/VFS.hs#L535
There was a problem hiding this comment.
Maybe we should also export a function like this in that module that handles also trimming based on the character offsets.
plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction/Extract.hs
Outdated
Show resolved
Hide resolved
| -- | From a function name and list of arguments, generate a new function with the given LHsExpr as the rhs. | ||
| generateNewDecl :: Monad m => LIdP GhcPs -> [RdrName] -> LHsExpr GhcPs -> TransformT m (HsDecl GhcPs) | ||
| generateNewDecl name args expr = do | ||
| sp1 <- uniqueSrcSpanT |
There was a problem hiding this comment.
when do you use this rather than generatedSrcSpan?
| firstContainedExprQ = \case | ||
| lexpr@(L (SrcSpanAnn _ span@(RealSrcSpan realSrcSpan _)) _) :: LHsExpr GhcPs | ||
| | rangeSrcSpan <- rangeToSrcSpan (fromString $ unpackFS $ srcSpanFile realSrcSpan) (unUnpaddedRange range) | ||
| , isSameSrcSpanModuloFile rangeSrcSpan span |
There was a problem hiding this comment.
why modulo file? Maybe we should be turning the srcspans into Ranges and comparing those instead?
| import Debug.Trace (traceM, traceShowM) | ||
|
|
||
|
|
||
| pattern R :: UInt -> UInt -> UInt -> UInt -> Range |
There was a problem hiding this comment.
I'm sure this exists somewhere else as well... suggests it should maybe go in lsp!
|
Oh, and this will need doc in |
| -- Before: | ||
| -- foo = x + y + z | ||
| -- After | ||
| -- newDefinition y + z = y + z |
There was a problem hiding this comment.
| -- newDefinition y + z = y + z | |
| -- newDefinition = y + z |
This change adds a code action for extracting an expression as specified by a user's selected range. There is logic to ensure that the behavior of the plugin is as expected by ignoring any padded whitespace in the user's range.
There are currently two outstanding problems:
newDefinition. I'm not sure how to create fresh variables. We can definitely merge with this limitationThis code action does not add arguments for variables that are no longer in scopeImplementedIf anyone is interested, I could use pointers on (2). I am going to try to take inspiration from wingman for now, however...