Proposal - Add haskell-process-wrapper-function

[ardumont]

Hello,

Following this PR and its possible improvment, I added the haskell-process-wrapper.

This permits to launch the haskell-process-type command (inside haskell-process-start function) inside a wrapper command (nix-shell for example, or others).

For example, using this:

(custom-set-variables '(haskell-process-wrapper '("nix-shell" "default.nix" "--command"))
                      '(haskell-process-type 'cabal-repl)))

For this example, this triggers the cabal repl command inside nix-shell (defined within the default.nix expression).

I tested manually - ardumont@3f6e21d with:

• haskell-process-wrapper (for nix-shell as above) with haskell-process-type in (cabal-repl cabal-ghci ghci).
• haskell-process-wrapper nil (the default) and haskell-process-type
  in (cabal-repl cabal-ghci ghci).

The code is generic, so this should work for the other commands that @chrisdone mentioned.

Note
I added unit tests (to enforce that I did not break anything) and I needed mocks (I added the deps inside the haskell-process-tests.el file but this must not be what you want.
If you have any suggestions on how to improve this, I'm all for it.

[ardumont]

Beware this HACK here to install the el-mock dependencies...
How can we do better?

[purcell]

Probably Cask would be the way to handle this robustly across the entire repo, because it has the concept of development dependencies.

[ardumont]

Yes, indeed.
I did not want to take it upon me to propose it (I already use it for org-trello).

So, shall I try and propose using Cask in this PR?

[ardumont]

Here we go #372 (in another PR to avoid cluttering this functionality).

[ardumont]

As a side note, here is the possible project overload of emacs config (that @chrisdone suggested) about .dir-labels:

((haskell-mode . ((haskell-process-wrapper . ("nix-shell" "haskell-lab.nix" "--command"))
                  (haskell-process-type . cabal-ghci))))

Cf. manual for interested readers.

And thanks @chrisdone, I did not know this possibility.

Cheers,

[ardumont]

Ok.

So running cask with emacs-24.3 works but with emacs-snapshot fails.
Is it normal that emacs-snapshot is an emacs 23 (there is a limit with Cask, only for emacs >= 24)?

I'll revert those changes and propose another PR about Cask, this way the main functionality which works stays intact :D

Cheers,

[ardumont]

Rebased on latest master + squash all (intermediary) commits into 1.

[ardumont]

@ocharles Hello, as a fellow nixos user, can you please give this a try?

[purcell]

This mostly looks good to me. One note would be that you're assuming the wrapped command will be passed to the wrapper as a single stringified argument. I'm not sure that would always be the case, because it would depend on the wrapper.

If that's something we care about, then how about defining the wrapper via a defcustom called something like haskell-process-wrapper-function, which would take a command + args list and return a new command + args list? The default value would be 'identity, and to customize it for nix-shell we might do something like:

(setq haskell-process-wrapper-function
      (lambda (argv) (append '("nix-shell" "haskell-lab.nix" "--command")
                             (shell-quote-argument argv))))

Then you don't have to special-case the code where haskell-process-wrapper is non-nil: you just call it unconditionally every time to get the final command line.

[ardumont]

This mostly looks good to me.

Yeah! Almost there :D (and this time, no over-complecting thing? :D)

One note would be that you're assuming the wrapped command will be passed to the wrapper as a single stringified argument.
I'm not sure that would always be the case, because it would depend on the wrapper.

Yes, I do! And yes, I'm not sure either.

I did not how to test with other wrappers.
@chrisdone mentionned docker but I do not know how to test with it (yet).

If that's something we care about, then how about defining the wrapper via a defcustom called something like haskell-process-wrapper-function, which would take a command + args list and return a new command + args list? The default value would be 'identity, and to customize it for nix-shell we might do something like:

(setq haskell-process-wrapper-function
        (lambda (argv) (append '("nix-shell" "haskell-lab.nix" "--command")
                          (shell-quote-argument argv))))

Then you don't have to special-case the code where haskell-process-wrapper is non-nil: you just call it unconditionally every time to get the final command line.

If I understood correctly, this is pretty neat!
But then, do we still need the defcustom haskell-process-wrapper?
Can't we just define the defcustom haskell-process-wrapper-function directly?

[purcell]

If I understood correctly, this is pretty neat!
But then, do we still need the defcustom haskell-process-wrapper?
Can't we just define the defcustom haskell-process-wrapper-function directly?

Yes, exactly, we wouldn't need haskell-process-wrapper, because haskell-process-wrapper-function would be everything we need.

[ardumont]

Yes, exactly, we wouldn't need haskell-process-wrapper, because haskell-process-wrapper-function would be everything we need.

Good.
I'll see what I can do.

Note
The 2 other PR I propose (if the team was ever interested) will need to be rebased to be consistent.

[ardumont]

Ok.

I'm working on it but I'm having a hard time unit testing this :D (flet, cl-letf, cl-flet, and whatnot for function binding in test context...).

I'm pretty tired right now so I'll get back as soon as I have something.

Cheers,

[ardumont]

Ok, so I got some more time and I could really test the following:

|-----------------------------------------------+------+------------+------------+-----------|
|                                               | ghci | cabal-repl | cabal-ghci | cabal-dev |
|-----------------------------------------------+------+------------+------------+-----------|
| Unit Tests                                    | OK   | OK         | OK         | OK        |
|-----------------------------------------------+------+------------+------------+-----------|
| Manual without wrapper function (the default) | OK   | OK         | OK         | N/A       |
| Manual with wrapper function                  | OK   | OK         | OK         | N/A       |
|-----------------------------------------------+------+------------+------------+-----------|

(as before I did not test cabal-dev because I do not know this one but I ensure the initial contract stays the same as before... in unit tests that is...)

Here is the possible .dir-local-variables I used:

((haskell-mode . ((haskell-process-wrapper-function . (lambda (argv) (append (list "nix-shell" "haskell-lab.nix" "--command" )
                                                                        (list (mapconcat 'identity argv " ")))))
                  (haskell-process-type . ghci)))) ;; ok

((haskell-mode . ((haskell-process-wrapper-function . (lambda (argv) (append (list "nix-shell" "haskell-lab.nix" "--command" )
                                                                        (list (mapconcat 'identity argv " ")))))
                  (haskell-process-type . cabal-repl)))) ;; ok

((haskell-mode . ((haskell-process-wrapper-function . (lambda (argv) (append (list "nix-shell" "haskell-lab.nix" "--command" )
                                                                        (list (mapconcat 'identity argv " ")))))
                  (haskell-process-type . cabal-ghci)))) ;; ok

Their equivalence as custom-set-variables:

(custom-set-variables '(haskell-process-wrapper-function (lambda (argv) (append (list "nix-shell" "haskell-lab.nix" "--command" )
                                                                           (list (mapconcat 'identity argv " ")))))
                      '(haskell-process-type ghci)) ;; ok

(custom-set-variables '(haskell-process-wrapper-function (lambda (argv) (append (list "nix-shell" "haskell-lab.nix" "--command" )
                                                                           (list (mapconcat 'identity argv " ")))))
                      '(haskell-process-type cabal-repl)) ;; ok

(custom-set-variables '(haskell-process-wrapper-function (lambda (argv) (append (list "nix-shell" "haskell-lab.nix" "--command" )
                                                                           (list (mapconcat 'identity argv " ")))))
                      '(haskell-process-type cabal-ghci)) ;; ok

Note
I tried to use shell-quote-argument but systematically resulted in command not found.

Here is the sample that did not work out

((haskell-mode . ((haskell-process-wrapper-function . (lambda (argv) (append (list "nix-shell" "haskell-lab.nix" "--command" )
                                                                        (list (shell-quote-argument (mapconcat 'identity argv " "))))))
                  (haskell-process-type . ghci)))) ;; shell-quote-argument does not work

[ardumont]

Rebased to the latest upstream/master.

[purcell]

The implementation here looks good to me. Anyone else want to express an opinion, including re. the el-mock dependency & installation trick?

[ocharles]

Ok - I can confirm it all works! I was able to load a file into cabal repl, and I can also run cabal build directly from Emacs - which previously failed. All good from me 👍

[ardumont]

Thanks. Will read it again as I clearly missed spots!
And will try again as soon as I can.

Following the right procedure, I did not encounter any issue.

Are you sure you have haskell-process-type cabal-repl and not haskell-process-type 'cabal-repl? > With the latter, I get past this error and am now able to test more :)

I tested like you (only the flag I remove).

Ok - I can confirm it all works! I was able to load a file into cabal repl, and I can also run cabal build directly from Emacs - which previously failed. All good from me 👍

Great news!

Thanks!

Cheers,

[chrisdone]

I'm good to merge this. I tested it with my docker script with a .dir-locals.el config, like this:

(haskell-mode . ((haskell-process-type . ghci)
                  (haskell-process-wrapper-function
                   . (lambda (argv) (append (list "docker-run" "exec" "--")
                                            argv)))))

[chrisdone]

Any objections to merging?

[purcell]

Not from me.

[chrisdone]

Merged as dcf4ad..44228b. Did not merge the Cask tests. They can be discussed in #372.

Thanks, @ardumont!

[ardumont]

Merged as dcf4ad..44228b. Did not merge the Cask tests. They can be discussed in #372.

Great news!

Thanks, @ardumont!

Thanks to the team for maintaining such a great mode!

Cheers,

[ardumont]

@chrisdone It would be great when you'll have some time to add in the haskell-process-wrapper-function docstring your docker function sample.

[chrisdone]

@ardumont Right now I use an internal company tool for launching the docker, but if that changes I'll document it. =)

[fatlazycat]

Hi, could some one point me at an emacs config to get haskell mode working with nix-shell. I have tried setting haskell-process-wrapper-function but on a haskell-process-load-or-reload it never finds cabal when setting up the project. The project compiles & runs etc if i enter the shell myself with nix-shell --pure etc.

Thanks

[ardumont]

@fatlazycat does none of those work?

#370 (comment)

[cocreature]

It was broken for me as well until 35633bf, maybe that fixed it for you.

[ardumont]

It was broken for me as well until 35633bf, maybe that fixed it for you.

Ok, indeed, without this code, that would not work :D
Thanks for the information.