Implement "split" tactic

[WorldSEnder]

Originated as isovector#30 this implements a split tactic for introducing specific constructors in holes on the user's request. For more details also see the linked PR.

[isovector]

Looks good to me. The test failures have me concerned though! I think I might know what's going wrong, so I'll try to fix that today.

[isovector]

Good catch! Not sure how I missed this.

[isovector]

I suspect the following diff will fix the tests:

diff --git a/plugins/tactics/src/Ide/Plugin/Tactic/Debug.hs b/plugins/tactics/src/Ide/Plugin/Tactic/Debug.hs
index ba91a7c..6c528da 100644
--- a/plugins/tactics/src/Ide/Plugin/Tactic/Debug.hs
+++ b/plugins/tactics/src/Ide/Plugin/Tactic/Debug.hs
@@ -1,3 +1,7 @@
+{-# LANGUAGE BangPatterns     #-}
+{-# LANGUAGE CPP              #-}
+{-# LANGUAGE TypeApplications #-}
+
 module Ide.Plugin.Tactic.Debug
   ( unsafeRender
   , unsafeRender'
@@ -9,17 +13,36 @@ module Ide.Plugin.Tactic.Debug
   , traceMX
   ) where
 
+import Control.DeepSeq
+import Control.Exception
 import Debug.Trace
 import DynFlags (unsafeGlobalDynFlags)
 import Outputable hiding ((<>))
+import System.IO.Unsafe (unsafePerformIO)
+
+#if __GLASGOW_HASKELL__ >= 808
+import PlainPanic (PlainGhcException)
+type GHC_EXCEPTION = PlainGhcException
+#else
+import Panic (GhcException)
+type GHC_EXCEPTION = GhcException
+#endif
+
 
 ------------------------------------------------------------------------------
 -- | Print something
 unsafeRender :: Outputable a => a -> String
 unsafeRender = unsafeRender' . ppr
 
+
 unsafeRender' :: SDoc -> String
-unsafeRender' = showSDoc unsafeGlobalDynFlags
+unsafeRender' sdoc = unsafePerformIO $ do
+  let z = showSDoc unsafeGlobalDynFlags sdoc
+  -- We might not have unsafeGlobalDynFlags (like during testing), in which
+  -- case GHC panics. Instead of crashing, let's just fail to print.
+  !res <- try @GHC_EXCEPTION $ evaluate $ deepseq z z
+  pure $ either (const "<unsafeRender'>") id res
+{-# NOINLINE unsafeRender' #-}
 
 traceMX :: (Monad m, Show a) => String -> a -> m ()
 traceMX str a = traceM $ mappend ("!!!" <> str <> ": ") $ show a

diff --git a/plugins/tactics/hls-tactics-plugin.cabal b/plugins/tactics/hls-tactics-plugin.cabal
index 9abb2b5..7fecc86 100644
--- a/plugins/tactics/hls-tactics-plugin.cabal
+++ b/plugins/tactics/hls-tactics-plugin.cabal
@@ -75,6 +75,28 @@ library
     , syb
     , text
     , transformers
+    , deepseq

[WorldSEnder]

I wonder what exactly is the difference now between the circle ci and the github tests, seeing that the latter worked fine, and only the former failed before?

[isovector]

Yeah, I have no idea. We're in #haskell-ide-engine on freenode trying to suss out wtf.

[jneira]

We were not able to come to a conclusion in irc, summarizin some facts:

• tests are failing in circleci but no in github wf
  • the former uses stack and the latter cabal
• both ci jobs uses -j1 for the test suite itself (but no for the build tool cabal/stack)
• tests are not failing locally (with cabal or stack)
• tests fail more frequently for ghc-8.8 but the fail occasionally for ghc-8.10. No fail ever for ghc-8.6
• the main diff between this pr and hlint one from the original pr that introduced tests is the bump of lsp-test from 0.11.0.4 to 0.11.0.5/0.11.0.6
  • the diff between both versions was the merge of Handle CustomClientMethod properly lukel97/lsp-test#74. It changed the handling of server shutdown
  • Fix a bug in tactics preventing split of split #520 is using lsp-test-0.11.0.5 and no fails 🤔

[jneira]

Ok, i've tried to add waitForDiagnostics for tactic func tests and it seems they are working at first:

• commit: jneira@8d1ef4c
• circleci build: https://app.circleci.com/pipelines/github/jneira/haskell-language-server/127/workflows/b8a860ce-8eb2-408f-8354-33d17b75ad21

[WorldSEnder]

Progress! Thank you @jneira that seems to have at least fixed stack-ghc-8.10. Locally stack-8.8.x is also no problem. The github action fail for windows-ghc-8.10.1 looks suspicious, although probably not connected to the PR.

[isovector]

@jneira uh oh. Now eval > Evaluation of expressions is failing with Exception: fd:3: hPutBuf: resource vanished (Broken pipe). Something is seriously wonky in the testing infrastructure.

[jneira]

i've hit it another one in hlint pr too, after rebasing waitForDiagnostics, but at least they are much less frequent now

[isovector]

@WorldSEnder everything should be cleared up if you merge master.

[jneira]

Rerunning circleci failed jobs

[isovector]

I'd like to get this merged. Mind if I take over, @WorldSEnder ?

[WorldSEnder]

Go ahead, the branch is set to accept commits from maintainers.

[Ailrun]

I guess this PR is not relevant anymore, as succeeded by #1379. If not, feel free to reopen this.

[isovector]

Different meaning of "split", I'm afraid!

[isovector]

Implemented in #1461