Update CLI error and helper messages#2650
Update CLI error and helper messages#2650paf31 merged 5 commits intopurescript:masterfrom hatashiro:message-fix
Conversation
This is concerning recent changes in executables, and also #2573.
| , " psc-package install psci-support" | ||
| , "" | ||
| , "For help getting started, visit https://github.com/purescript/documentation/blob/master/PSCi.md" | ||
| [ "purs repl: No input files, or no psci-support loaded; try running `pulp psci` instead." |
There was a problem hiding this comment.
It's straightforward enough to distinguish these two cases that I think we should be doing so. I'd rather leave the supportModuleMessage more or less as it is and define a new noInputMessage similar to what you have here, but which is only displayed when no input files are provided.
The place to do that seems to be here: https://github.com/noraesae/purescript/blob/005a84f031e975224a31564309296134bbe1a4ad/app/Command/REPL.hs#L313-L315
There was a problem hiding this comment.
Sounds appropriate. Will upload a follow-up commit to fix it.
|
I updated the PR with a patch to distinguish |
| prologueMessage :: String | ||
| prologueMessage = unlines | ||
| [ "PSCi, version " ++ showVersion Paths.version | ||
| [ "PureScript REPL, version " ++ showVersion Paths.version |
There was a problem hiding this comment.
I think it's fine to continue to call the app "PSCi", but when referring to the command, it should say purs repl.
There was a problem hiding this comment.
Will revert this part :)
|
|
||
| noInputMessage :: String | ||
| noInputMessage = unlines | ||
| [ "purs repl: No input files; try running `pulp psci` instead." |
| , " psc-package install psci-support" | ||
| , "" | ||
| , "For help getting started, visit https://github.com/purescript/documentation/blob/master/PSCi.md" | ||
| [ "purs repl: psci-support is not loaded; try running `pulp psci` instead." |
There was a problem hiding this comment.
Now that the "no input files" case is separately handled, I think the current message is actually more appropriate. I'm specifically thinking about a case where someone has invoked purs repl via pulp psci but psci-support is not installed.
There was a problem hiding this comment.
I think removing the try running ... part from supportModuleMessage could be enough, because for both pulp psci and purs repl cases, what's needed is to install psci-support and if it's not already installed pulp psci doesn't help.
There was a problem hiding this comment.
Right, although I don't think "psci-support is not loaded" is informative enough that people new to PureScript will be able to understand what they need to do; this is why I prefer the current message.
There was a problem hiding this comment.
To clarify, I realise we are trying to remove hardcoded assumptions of particular package managers in the compiler, but for me that doesn't need to extend as far as changing this message; it really is more directed at e.g. psc-publish which currently assumes that you are using Bower as your package manager and simply does not work otherwise. The aim of this is to ease a transition away from Bower, at least that's how I see it. So I don't think we really gain anything by removing this information from this error message.
There was a problem hiding this comment.
I understand and agree. How about:
purs repl: PSCi requires the psci-support package to be installed.
You can install it using psc-package or Bower as follows:
psc-package install psci-support
bower i purescript-psci-support --save-dev
For help getting started, visit https://github.com/purescript/documentation/blob/master/guides/PSCi.md
There was a problem hiding this comment.
Let's just remove the reference to psc-package while we're at it. It's not going to be a supported option any more.
There was a problem hiding this comment.
Then, like below?
purs repl: PSCi requires the psci-support package to be installed.
You can install it using Bower as follows:
bower i purescript-psci-support --save-dev
For help getting started, visit https://github.com/purescript/documentation/blob/master/guides/PSCi.md
app/Command/Compile.hs
Outdated
| input <- globWarningOnMisses (unless pscmJSONErrors . warnFileTypeNotFound) pscmInput | ||
| when (null input && not pscmJSONErrors) $ do | ||
| hPutStrLn stderr "psc: No input files." | ||
| hPutStrLn stderr $ unlines [ "purs compile: No input files." |
There was a problem hiding this comment.
I think this will insert an extra line break; if we're using unlines, then I think the hPutStrLn should change to hPutStr
There was a problem hiding this comment.
You are right. I will fix.
|
Removed extra newlines and updated the error messages to be more informative. |
3 participants
This is concerning recent changes in executables(#2632, #2633), and also #2573.