Better error messages for unsupported uninstall arguments#1786
Merged
charlespierce merged 2 commits intomainfrom Jul 10, 2024
Merged
Better error messages for unsupported uninstall arguments#1786charlespierce merged 2 commits intomainfrom
uninstall arguments#1786charlespierce merged 2 commits intomainfrom
Conversation
chriskrycho
force-pushed
the
chriskrycho/better-reporting
branch
from
24d3c78 to
f2d7514
Compare
chriskrycho
force-pushed
the
chriskrycho/better-reporting
branch
from
f2d7514 to
fda493e
Compare
Instead of parsing whatever string the user supplied as the tool name and supplying a default `VersionSpec`, attempt to parse the value as a full specifier which may include a version. This means we can provide better errors when the user passes a version. Specifically we can report that Volta does not yet support uninstalling specific versions of tools. Previously, we would report something like this: ``` warning: No package 'typescript@5.4' found to uninstall ``` Notice that the old message treated `'typescript@5.4'` as the name of the tool, when it should have been treating it as a tool and a version specifier. Now, we instead report that uninstalling specific versions of tools is unsupported. The original motivation here was noticing that we printed errors like that if the user tried to uninstall a runtime or a package manager with a version specifier. This fixes that as well, since it no longer parses a string like `node@20.14.5` as a package, but rather as a runtime and version specifier, and can fall into the normal handling for runtimes.
chriskrycho
force-pushed
the
chriskrycho/better-reporting
branch
from
fda493e to
66da382
Compare
charlespierce
approved these changes
Jul 10, 2024
Contributor
charlespierce
left a comment
charlespierce
left a comment
There was a problem hiding this comment.
LGTM! Love to have better / clearer error messages 💥
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Instead of parsing whatever string the user supplied as the tool name and supplying a default
VersionSpec, attempt to parse the value as a full specifier which may include a version. This means we can provide better errors when the user passes a version. Specifically we can report that Volta does not yet support uninstalling specific versions of tools. Previously, we would report something like this:Notice that the old message treated
'typescript@5.4'as the name of the tool, when it should have been treating it as a tool and a version specifier. Now, we instead report that uninstalling specific versions of tools is unsupported.The original motivation here was noticing that we printed errors like that if the user tried to uninstall a runtime or a package manager with a version specifier. This fixes that as well, since it no longer parses a string like
node@20.14.5as a package, but rather as a runtime and version specifier, and can fall into the normal handling for runtimes.Background and Miscellanies.
volta_uninstallvs.direct_uninstallmeant, so I commented those.