Define default directory for haskell-stack-ghc checker #1007
Conversation
| @@ -6976,6 +6976,36 @@ Otherwise return the previously used cache directory." | |||
| (or flycheck-haskell-ghc-cache-directory | |||
| (make-temp-file "flycheck-haskell-ghc-cache" 'directory)))) | |||
|
|
|||
| (defun flycheck-haskell--default-directory (checker) | |||
| "Come up with a sutable default directory for Haskell to run checker in." | |||
There was a problem hiding this comment.
Come up sounds strange. Why not simply "find"?
Please also extend the docstring to explain what a "suitable" directory would be, i.e. document the behaviour of the function as well.
| (replace-regexp-in-string | ||
| (rx (* (any " \t\n")) eos) | ||
| "" | ||
| (shell-command-to-string "stack path --project-root")))) |
There was a problem hiding this comment.
Please use process-lines instead of shell-command-to-string. We should not use shell commands without need.
|
Thanks. I've done a cursory review, will look at the details tomorrow. |
|
@flycheck/haskell @purcell Would you also take a look? Maybe also test this PR on different Haskell projects? |
|
@lunaryorn I've amended commit with your suggestions. |
|
@sergv, Looks good, thank you for writing this! I'm looking forward to using it. |
|
LGTM. I tried this patch on top of flycheck HEAD with a fairly vanilla stack project and all was well, so I'm pretty confident this will work. |
| @@ -6976,6 +6976,45 @@ Otherwise return the previously used cache directory." | |||
| (or flycheck-haskell-ghc-cache-directory | |||
| (make-temp-file "flycheck-haskell-ghc-cache" 'directory)))) | |||
|
|
|||
| (defun flycheck-haskell--find-default-directory (checker) | |||
| "Come up with a sutable default directory for Haskell to run | |||
|
I've added quick review notes as well. Thanks for this PR! |
|
@purcell Thanks for the review-of-the-review :)) |
|
@cpitclaudel I've rebased on master and added commit that fixes review notes |
| (process-lines "stack" "path" "--project-root")) | ||
| (stack-dir | ||
| (when stack-output | ||
| (car stack-output)))) |
There was a problem hiding this comment.
@sergv The when guard is redundant. (car nil) is perfectly safe and evaluates to nil so it's perfectly safe to just do (car (process-lines …)).
|
@sergv Would you add a note about this change to |
|
LGTM. Thanks for the change 😊 I think though that #1012 should go in first. |
|
@lunaryorn Indeed, I missed it. Thank you! |
Default directory is detected either by looking for
stack.yamlor askingstack path --project-rootforhaskel-stack-ghcchecker or by looking for cabal file in case ofhaskell-ghcchecker.@mrkkrp Please comment, if you think something should be done differently.