refactor: Make InLvl a handler instead of a command #31
Conversation
brat/Brat/Checker/Monad.hs
Outdated
| @@ -60,8 +60,8 @@ data Context = Ctx { globalVEnv :: VEnv | |||
|
|
|||
| data CheckingSig ty where | |||
| Fresh :: String -> CheckingSig Name | |||
| -- Run a sub-process on a new namespace-level | |||
| InLvl :: String -> Checking a -> CheckingSig a | |||
| GetNS :: CheckingSig Namespace | |||
There was a problem hiding this comment.
How about a single SplitNS :: String -> CheckingSig Namespace which both updates the namespace inside handler (to the second half of the split) and returns the first half of the split?
There was a problem hiding this comment.
This makes things much nicer! 😄
brat/Brat/Checker/Monad.hs
Outdated
| -> Either Error (v,Context,([TypedHole],Graph),Namespace) | ||
| handler (Ret v) ctx g ns = return (v, ctx, ([], g), ns) | ||
| -> Either Error (v,Context,([TypedHole],Graph)) | ||
| handler (Ret v) ctx g _ = return (v, ctx, ([], g)) |
There was a problem hiding this comment.
I am even wondering if we could remove the Namespace parameter to handler. Have the top-level command(s) (i.e. GetNS+SetNS or SplitNS) die with an error, and rely on running a localNS inside it. You could make run still take a Namespace and invoke handler (localNS c).... (so, a bit like localFC)
There was a problem hiding this comment.
I was thinking the same thing!
brat/Brat/Checker/Monad.hs
Outdated
| @@ -300,7 +290,10 @@ typeErr = err . TypeErr | |||
|
|
|||
| instance FreshMonad Checking where | |||
| freshName x = req $ Fresh x | |||
| str -! c = req $ InLvl str c | |||
| str -! c = do | |||
There was a problem hiding this comment.
Oh, nice - I wasn't expecting it to be so easy to keep FreshMonad
| @@ -153,7 +154,7 @@ loadStmtsWithEnv ns (venv, oldDecls, oldEndData) (fname, pre, stmts, cts) = addS | |||
| let vdecls = map fst entries | |||
| -- Now generate environment mapping usernames to nodes in the graph | |||
| venv <- pure $ venv <> M.fromList [(name, overs) | ((name, _), (_, overs)) <- entries] | |||
| ((), (holes, newEndData, graph, _)) <- run venv kcStore ns $ withAliases aliases $ do | |||
| ((), (holes, newEndData, graph)) <- run venv kcStore newRoot $ withAliases aliases $ do | |||
There was a problem hiding this comment.
It'd be more consistent with "globals" to do the split outside run here, or to use -! above (as the old code did), but I'm not too bothered if you want to be different!
There was a problem hiding this comment.
I don't think we need to change the split here, since we don't do anything with the new namespace
No description provided.