neovimhaskell / nvim-hs

Neovim API for Haskell plugins as well as the plugin provider
Other
267 stars 18 forks source link

Function types #12

Closed osa1 closed 9 years ago

osa1 commented 9 years ago

I'm currently writing some plugins to see how convenient the API is. I realized two things:

  1. Non-stateful function type is pretty useless, because even though it lets us do IO, we can't evaluate vim expressions and commands using it, because we need Neovim monad for those. So we can't read any vim state, for example I can't do getpos("'<") and get beginning of visual selection.
  2. Stateful functions can get method name using reqMethod, so a single stateful functions can be used for multiple functions in a plugin(and they don't need multiple channels, although a channel for each different function call may be used).

So it seems like we can remove stateless functions and use only stateful ones instead. @saep what do you think?

Here's an example stateful function that wraps the selection with parens:

pwrap :: TQueue SomeMessage -> Neovim cfg s ()
pwrap q = do
    req <- awaitRequest q
    -- reqMethod req

    Right (ObjectArray [ObjectInt _, ObjectInt bline, ObjectInt bcol, ObjectInt _]) <-
      atomically' =<< vim_eval "getpos(\"'<\")"

    Right (ObjectArray [ObjectInt _, ObjectInt eline, ObjectInt ecol, ObjectInt _]) <-
      atomically' =<< vim_eval "getpos(\"'>\")"

    _ <- atomically' =<< (vim_command $ "normal! " ++ show bline ++ "gg" ++
                            show bcol ++ "|i(")
    _ <- atomically' =<< (vim_command $ "normal! " ++ show eline ++ "gg" ++
                            show (ecol + if bline == eline then 1 else 0) ++
                            "|a)")

    respond (reqId req) (Right ())
    pwrap q

It's not possible to implement this using stateless functions, since it's not possible to evaluate vim expressions in stateless functions. Note that there's no need to keep this function looping in a thread, but currently we don't have way to implement this in any other way. In the worst case multiple functions can be implemented as a single stateful function and function dispatch can be handled using multiple TChans or reqMethod function.

saep commented 9 years ago

The type of stateless functions is definitively bad. You linked me a blog post that made a reasonable case against ExceptT bla IO blah which is our current type. Anyhow, I think that stateless functions do have some advantages. Conceptually, something like [Object] -> Neovim () () Object should be simple to use and fix the issue of not being able to call Vim functions. We could hide this low level Object handling with Template Haskell and let the user write a function signature like this:

f :: String -> Text-> Int -> Neovim' (Map ByteString Double)

Neovim' is just a type synonym for Neovim () ().

The generated code will ideally respond with error messages such as:

Invalid number of arguments for function f.

Wrong type for the 2nd argument in f.

Edit: typos

saep commented 9 years ago

Writing on a phone is really annoying. ;)

osa1 commented 9 years ago

Oh, it's good that you mentioned error handling. We should find a way to lift functions of some types to Neovim functions, by automatically wrapping those functions with functions that do type checking of arguments and wrapping return values with Neovim/Msgpack types.

As an example, in hslua I'm doing this using this instance: https://github.com/osa1/hslua/blob/master/src/Scripting/Lua.hsc#L632

We can do same thing unless we have a better idea.

(I'll be writing some more plugins in the meantime to come up with other improvements)

saep commented 9 years ago

The Neovim transformer stack also uses ExceptT on top of IO. I think we should get rid of that as well.

saep commented 9 years ago

I am currently writing a function generator in Template Haskell. I'll create a pull request for that once it is generic enough.

saep commented 9 years ago

I think the test-function branch provides a reasonable base to have a convenient way to create functions.

I think we should write a convenient interface for creating stateless functions first as I have the feeling we just have to generalize them to have more convenient interface for stateful functions.

I would also suggest to replace type of the functions field inside the Plugin type to a new data type which looks something like this:

data Export
    = Function Text ([Object] -> Neovim' Object)
   -- ^ The first argument is the name of the function
    | Command Text  ([Object] -> Neovim' Object) 
    -- ^ The first argument is the name of the command which must start with an uppercase letter
   {- ... and possibly more things -}

The point of this data type is, that they will also do additional work when they are registered upon startup. A value with the Function constructor will execute a function registration snippet, so that you can call it from neovim with e.g. call myFunction(). The same goes for a value created with the Command constructor which will make the command "Foo" available as :Foo inside neovim.

A code example for defining a plugin should then look like this:

myFunction :: Neovim' Int
myFunction = return 42

myFooCommand :: Neovim' ()
myFooCommand = return () -- NOOP

myPlugin = return $ SomePlugin Plugin {
    name = "Sample plugin",
    functions = [ $(function "myFunction" 'myFunction)
                , $(command "Foo" 'myFooCommand)
                ]

We could also provide the command' and function' splices that generate the name for the function/command automatically from the function definition.

saep commented 9 years ago

I think the design for stateful functions (or functionality) needs a redesign. We currently have to do a lot of things manually in the tests. This approach seems a bit fragile and repetitive to me. We should probably wrap the mutable state in an STVar, so that we can dispatch to other functions completely asynchronously. Also, we should group the stateful functions by the state they share and thus the TQueue loop that dispatches them. I envision a type something like this:

data Plugin r st = Plugin {
    ...
    , stateFulFUnctions :: [(r, st, [ExportedFunctionality])]
    ...
    }

instead of this. Note that we can drop the services field because the grouping inside statefulFunctions assumes to access the same state and hence the same TQueue. Also note, that we can almost use the same function generation code for stateful functions as for stateless functions as currently being implemented in #19. It may suffice to extend ExportedFunctionality with the types of the reader and state monad states.

saep commented 9 years ago

I'm quite happy with the state of #19 as of right now. The boiler-plate is mostly gone and the main module has become less cluttered with ad-hoc function registration code. I'd like to close this ticket and write some extensive Haddock documentation on hire to write plugins.