Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automatically detect packageManager #1646

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion $shared/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,4 +172,4 @@ export type ConfigurationSettings = {
nodePath: string | null;
workspaceFolder: WorkspaceFolder | undefined;
workingDirectory: ModeItem | DirectoryItem | undefined;
};
};
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ This extension contributes the following variables to the [settings](https://cod
"eslint.lintTask.options": "-c C:/mydirectory/.eslintrc.json --ignore-path C:/mydirectory/.eslintignore ."
}
```
- `eslint.packageManager`: controls the package manager to be used to resolve the ESLint library. This has only an influence if the ESLint library is resolved globally. Valid values are `"npm"` or `"yarn"` or `"pnpm"`.
wilkinson4 marked this conversation as resolved.
Show resolved Hide resolved
- The old `eslint.packageManager` setting is now deprecated and can safely be removed. This controlled the package manager to be used to resolve the ESLint library. This has only an influence if the ESLint library is resolved globally. Valid values are `"npm"` or `"yarn"` or `"pnpm"`.
- `eslint.options`: options to configure how ESLint is started using either the [ESLint class API](http://eslint.org/docs/developer-guide/nodejs-api#eslint-class) or the [CLIEngine API](http://eslint.org/docs/developer-guide/nodejs-api#cliengine). The extension uses the ESLint class API if ESLint version 8 or higher is used or if ESLint version 7 is used and the setting `eslint.useESLintCLass` is set to true. In all other cases the CLIEngine API is used.
An example to point to a custom `.eslintrc.json` file using the new ESLint API is:
```json
Expand Down
8 changes: 5 additions & 3 deletions client/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -232,13 +232,15 @@ export namespace ESLintClient {
return {};
});

client.onRequest(NoESLintLibraryRequest.type, (params) => {
client.onRequest(NoESLintLibraryRequest.type, async (params) => {
const key = 'noESLintMessageShown';
const state = context.globalState.get<NoESLintState>(key, {});

const uri: Uri = Uri.parse(params.source.uri);
const workspaceFolder = Workspace.getWorkspaceFolder(uri);
const packageManager = Workspace.getConfiguration('eslint', uri).get('packageManager', 'npm');
const packageManager = await commands.executeCommand<PackageManagers>(
'npm.packageManager'
wilkinson4 marked this conversation as resolved.
Show resolved Hide resolved
);
const localInstall = {
npm: 'npm install eslint',
pnpm: 'pnpm install eslint',
Expand Down Expand Up @@ -934,4 +936,4 @@ export namespace ESLintClient {
}
}
}
}
}
2 changes: 1 addition & 1 deletion history/settings_1_9_x.md
Original file line number Diff line number Diff line change
Expand Up @@ -107,4 +107,4 @@ As with JavaScript validating TypeScript in a mono repository requires that you
{ "directory": "./client", "changeProcessCWD": true },
{ "directory": "./server", "changeProcessCWD": true }
]
```
```
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@
"pnpm"
],
"default": "npm",
"description": "The package manager you use to install node modules."
"description": "The package manager you use to install node modules.",
"deprecationMessage": "The setting is deprecated. The Package Manager is automatically detected now."
},
"eslint.problems.shortenToSingleLine": {
"type": "boolean",
Expand Down
2 changes: 1 addition & 1 deletion server/src/eslint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1368,4 +1368,4 @@ export namespace ESLint {
return Status.error;
}
}
}
}
10 changes: 5 additions & 5 deletions server/src/paths.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,11 @@ export function getFileSystemPath(uri: URI): string {
}
if (process.platform === 'win32' || process.platform === 'darwin') {
try {
const realpath = fs.realpathSync.native(result);
// Only use the real path if only the casing has changed.
if (realpath.toLowerCase() === result.toLowerCase()) {
result = realpath;
}
const realpath = fs.realpathSync.native(result);
// Only use the real path if only the casing has changed.
if (realpath.toLowerCase() === result.toLowerCase()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like your PR made some unintended whitespace changes. I suggest removing them for a cleaner git history.

Copy link
Contributor Author

@wilkinson4 wilkinson4 May 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

result = realpath;
}
} catch {
// Silently ignore errors from `fs.realpathSync` to handle scenarios where
// the file being linted is not yet written to disk. This occurs in editors
Expand Down