Automatically detect packageManager #1646
Conversation
@microsoft-github-policy-service agree |
|
First time making a PR to a language server so please let me know if I missed something. |
wilkinson4
force-pushed
the
ww/automatically-detect-package-manager
branch
from
3b34940 to
35e2a76
Compare
wilkinson4
force-pushed
the
ww/automatically-detect-package-manager
branch
from
35e2a76 to
28910c7
Compare
| } | ||
| const realpath = fs.realpathSync.native(result); | ||
| // Only use the real path if only the casing has changed. | ||
| if (realpath.toLowerCase() === result.toLowerCase()) { |
There was a problem hiding this comment.
It seems like your PR made some unintended whitespace changes. I suggest removing them for a cleaner git history.
There was a problem hiding this comment.
These changes came from the npm run lint script that I ran with the --fix option. I can undo them if we don't want the changes.
There was a problem hiding this comment.
Oh, in that case yeah we should leave them to make sure the codebase is properly linted. Kind of inconvenient they're not in their original PRs though. Maybe we should have a lint PR check @dbaeumer?
the vscode command.
wilkinson4
force-pushed
the
ww/automatically-detect-package-manager
branch
from
28910c7 to
5db8956
Compare
|
@dbaeumer anything else that needs to be addressed in this PR? |
|
IMO the change can be breaking for users that had different values set for ESLint and the package manager extension. IMO we should deprecated the value and code wise do the following:
If both values are the same pint a message to the channel that the eslint value is deprecated and will be removed in the future. Use either of them If the values are different print a message to the channel that the value is deprecated and that we will not honor it in the future and that we moved to Breaking changes usually cause swirl especially since we will receive issue reports for it. This is why I try to avoid this. Sorry for detecting this so late. |
wilkinson4
force-pushed
the
ww/automatically-detect-package-manager
branch
from
600bc0a to
9c4004b
Compare
wilkinson4
force-pushed
the
ww/automatically-detect-package-manager
branch
from
9c4004b to
5c3a156
Compare
client/src/client.ts
Outdated
| client.warn('Detected package manager matches the one in the deprecated packageManager setting. This setting will be removed in the future, so you can remove it from your configuration.', {}, true); | ||
| return detectedPackageMananger; | ||
| } | ||
| client.warn('Detected package manager differs from the one in the deprecated packageManager setting. We will honor this setting until it is removed.', {}, true); |
There was a problem hiding this comment.
We should include the two values in the message so that the user already knows about them and can make a decision how to prceed.
|
@wilkinson4 Forgot to say thanks for working on this. |
wilkinson4
force-pushed
the
ww/automatically-detect-package-manager
branch
from
5c3a156 to
26a6862
Compare
No problem! |
@dbaeumer Following up on this one. Made the recommended changes. |
Deprecate the packageManager setting, and automatically detect it with the vscode command.