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

Allow configuring directory of ESLint package #1412

Merged

Conversation

PyvesB
Copy link
Contributor

@PyvesB PyvesB commented Nov 25, 2023

By default, the ESLint language server will try to find the ESLint package in node_modules at the root of the project. However, it may be in a different place, for example when using non-NPM package managers. This PR adds a preference to configure the location.

I've added the new preference to the existing JS/TS preferences page. Even though a different language server, ESLint is strongly tied to JS and TS, it didn't feel right to introduce a separate bespoke page just for this preference.

Signed-off-by: Pierre-Yves Bigourdan <10694593+PyvesB@users.noreply.github.com>
Copy link
Contributor

@mickaelistria mickaelistria left a comment

Choose a reason for hiding this comment

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

Unless I misunderstood, the preferences would configure the path to eslint package for all projects. So one couldn't easily work with project that have different layout, such as npm vs non-npm.
Could the preference(s) be used as a lookup list so that the first that finds an eslint directory wins? That would allow to set "Alternative paths to find eslint" with the preferences, but if those do not resolve to a package, they would then be skipped and other paths such as node_modules would be searched.
Note that the label should also mention that the path in preferences are better being project-relative paths (because absolute paths would then affect all projects, which is not desired for the case described above).

@PyvesB
Copy link
Contributor Author

PyvesB commented Nov 27, 2023

the preferences would configure XXXX for all projects. So one couldn't easily work with project that have different layout

I'd argue that this is the pattern that has historically been followed for all Wild Web Developer preferences. For example, formatting options such as line length or single vs. double quotes will typically be different from one project to another. There are a lot of WWD settings that fall in this case, yet they are all defined for all projects in the top level CSS, XML, etc. preference pages.

I'd argue that Wild Web Developer should allow configuring project-specific build path, compiler, style, and editor preferences, like JDT does, rather than implementing workarounds such as lookup lists.

@mickaelistria
Copy link
Contributor

I'd argue that this is the pattern that has historically been followed for all Wild Web Developer preferences. For example, formatting options such as line length or single vs. double quotes will typically be different from one project to another. There are a lot of WWD settings that fall in this case, yet they are all defined for all projects in the top level CSS, XML, etc. preference pages.

Yes, and this is basically an issue in WIld Web Developer, not a good pattern to reproduce.

I'd argue that Wild Web Developer should allow configuring project-specific build path, compiler, style, and editor preferences, like JDT does, rather than implementing workarounds such as lookup lists.

Indeed. But that's a lot of work that I don't expect anyone is likely to push now. In the meantime, we need to find pragmatic ways to improve things. Just making the preference check whether it's good for the project, and fall back to default value if the preference doesn't match anything relevant for a project is a step towards handling project-specifics more gracefully without having to start building a lot of project-specific preferences.

@PyvesB
Copy link
Contributor Author

PyvesB commented Nov 27, 2023

Just making the preference check whether it's good for the project, and fall back to default value if the preference doesn't match anything relevant for a project

Sounds good, I can add a check and fall back.

Note that the label should also mention that the path in preferences are better being project-relative paths

DirectoryFieldEditor only allows absolute paths if I'm not mistaken. Should I switch back to StringFieldEditor, or do you have another idea in mind?

@mickaelistria
Copy link
Contributor

Should I switch back to StringFieldEditor, or do you have another idea in mind?

I think StringFieldEditor is enough and allows further improvements more easily (such as usage of workspace variables of whatever we think makes sense here)

@PyvesB
Copy link
Contributor Author

PyvesB commented Nov 29, 2023

Have pushed some changes, let me know what you think.

@mickaelistria mickaelistria merged commit e34c51c into eclipse-wildwebdeveloper:master Nov 29, 2023
6 checks passed
@mickaelistria
Copy link
Contributor

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants