Clean up some error messages #2032
Conversation
Removes extra 'ERROR' in text where the `error` method is used. Converts some `write-host "error message"` to use the `error` method Converts some `write-host "error message"; exit 1` to use the `abort` method.
| @@ -65,7 +65,7 @@ $queue | % { | |||
|
|
|||
| if ($json.checkver -eq "github") { | |||
| if (!$json.homepage.StartsWith("https://github.com/")) { | |||
| write-host "ERROR: $name checkver expects the homepage to be a github repository" -f DarkYellow | |||
| error "$name checkver expects the homepage to be a github repository" | |||
There was a problem hiding this comment.
Even though the original was darkyellow text, changed to error instead of warn, since it seemed like an error condition.
|
Regarding the PS: there goes my plan to remove the |
If you want to go another way, it's fine by me :) |
|
The complete error handling of scoop has to be changed in order to remove the |
|
|
||
| $architecture = default_architecture | ||
| try { | ||
| $architecture = ensure_architecture ($opt.a + $opt.arch) | ||
| } catch { | ||
| error "ERROR: $_"; exit 1 | ||
| abort "ERROR: $_" |
There was a problem hiding this comment.
This is fine, but kinda confusing. Could abort either not prefix 'ERROR: ', or use a different prefix, like 'FATAL: '?
There was a problem hiding this comment.
It could, but I wouldn't worry too much about it. As @r15ch13 said, the whole error handling needs to be changed anyway. The worst cases ("ERROR ERROR...") have been taken care of.
|
Having thought about it some more, yeah this should all really be done a different way. IMO most inner functions shouldn't be writing error messages, or possibly anything to the screen directly, but rather returning a success value or throwing errors if something goes wrong. Only at the last moment should the result be translated into success or error messages on-screen. This makes it much easier to test and to know what's going on in the code. |
Cleans up code that outputs error messages:
error 'ERROR...errorfunction instead in some cases ofwrite-host '...' -f Red/DarkRed...exitcomes after it, in which case it's changed to useabort(in some cases this changes the text from darkred to red). Theabortmethod doesn't prepend any text.Haven't touched these cases:
[console]::error.writelineinstead, since I don't know exactly what this does/why it was usedwrite-host -f DarkRed "...", but I haven't changed them all to use the error function because I don't know if preceding every error message with "ERROR " was really desired.Additional note: For the scoop commands there's some inconsistency in what they do when arguments are missing. Some use
error(in turn usingwrite-host), eg:https://github.com/lukesampson/scoop/blob/35eb73b2dbc4a8cde6189c8b46eccf03113b9f3b/libexec/scoop-install.ps1#L62
Others just output the text, eg:
https://github.com/lukesampson/scoop/blob/35eb73b2dbc4a8cde6189c8b46eccf03113b9f3b/libexec/scoop-cache.ps1#L41