-
-
Notifications
You must be signed in to change notification settings - Fork 370
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
Draft: Support add to where command #3289
base: master
Are you sure you want to change the base?
Conversation
pure $ MG xMg (L locMatches matches') originMg | ||
|
||
insertAtStart' :: (Monad m, HasDecls ast) => ast -> HsDecl GhcPs -> TransformT m ast | ||
insertAtStart' old newDecl = do |
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.
@alanz This is a function that I wrote in order to do "the right thing" when inserting a new declaration at the start of a where clause. insertAtStart
wasn't doing the right thing in a few situations, such as the following:
-- insert a "bar" decl with noLocA
foo = 1
-- becomes
foo = 1
wherebar = 1
foo = 1
where
baz = 1
-- becomes
foo
wherebar = _ baz = 1
Inserting an L (... DifferentLine 1 0 ...) decl
causes a whole different type of issues:
foo = 1
where
baz = 1
-- becomes
foo = 1
where
bar = 1
baz = 1
Also, it appears that the insertAtStart
from Exactprint does the wrong thing for comments (I've tagged you in another comment on the test that shows the problem)
liftIO $ contentAfterAction @?= T.unlines foo' | ||
, testSession "untyped error" $ do | ||
let foo = | ||
[ "module Foo where" |
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.
@alanz This test case (foo
is the before, foo'
is the after), shows how comments appear to cause problems when using insertAt
from Exactprint. I tried some stuff to make it work but to no avail...
The issue persists even when using insertAt
from exact print without doing anything except returning the old decls:
foo = 1
where
-- hi
baz = 1
-- becomes
foo = 1
where
____
-- hibaz = 1
-- NOTE used 4 underscores to indicate 4 space characters
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.
Comments are tough.
Which version of ghc-exactprint?
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.
@santiweight you should be using the high-level API as demonstrated in the various addLocalDeclX
tests from https://github.com/alanz/ghc-exactprint/blob/master/tests/Test/Transform.hs#L318
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.
And for current ghc-exactprint master, I reproduced your example with a changer test using
addLocaLDecl7 :: Changer
addLocaLDecl7 libdir lp = do
let
doAddLocal = do
decls0 <- hsDecls lp
replaceDecls lp decls0
(lp',_,_w) <- runTransformT doAddLocal
debugM $ "log:[\n" ++ intercalate "\n" _w ++ "]log end\n"
return lp'
And test file
module AddLocalDecl7 where
foo = 1
where
-- hi
baz = 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.
Thanks for these examples @alanz
I realize now that the newer versions of ghc-exactprint are not available to me until 9.4.x is supported in HLS...
I am using 9.2.4 for the refactoring actions I have been working on up until now, which I believe corresponds to ghc-exactprint 1.6.
I am thinking it might be best to hold out for newer versions of ghc-exactprint to be available with 9.4 and then use newer versions of the API then... Alternatively, I might try again to use the exact-ring available in HLS and see if the results can be better than what I've been producing before I saw these examples in the ghc-exactprint repo!
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.
9.4 is supported in HLS
b612a39
to
c89225e
Compare
c89225e
to
96cc578
Compare
This MR is still in progress because comments are handled in a breaking way and I'm not totally sure why yet...
This MR is based on the add-argument MR that is merging soon.
Here are a few examples: