8000 Feature: edit server path and options in preference page by ghentschke · Pull Request #21 · eclipse-cdt/cdt-lsp · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Feature: edit server path and options in preference page #21

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Feb 27, 2023
Merged

Feature: edit server path and options in preference page #21

merged 1 commit into from
Feb 27, 2023

Conversation

ghentschke
Copy link
Contributor
@ghentschke ghentschke commented Feb 23, 2023 •
edited
Loading

The server path and options can be edited for the used server provider. The default values are the commands delivered from the server provider.

In Windows it looks like this:
image

TODO: add tests and create a new issue for the topic that the LS has to be restarted when the server binary or options changes

jonahgraham
jonahgraham reviewed Feb 25, 2023
View reviewed changes
Copy link
Member
@jonahgraham jonahgraham left a comment

Choose a reason for hiding this comment

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

This change points out a problem that we'll need to resolve at some future point. Different projects in the workspace may need different clangd installs. I don't think we should put this preference as a project setting as well as that would add hard coded paths to the project settings.

I think we can defer it for now, but it is something to keep in mind. This becomes of special interest to Renesas IIUC because they have special versions of clang(d) for different processors.

8000
@ghentschke
Copy link
Contributor Author
ghentschke commented Feb 27, 2023 •
edited
Loading

Maybe then it's a better approach to let C/C++ toolchain(s) provide the server path and settings. (That's what we do in our projects). The toolchain plugin could use the serverProvider extension point from org.eclipse.cdt.lsp to provide a server. Instead of using priorities to select a distinct server, we can use the isEnabledFor method in the org.eclipse.cdt.lsp.server.ICLanguageServerProvider interface to select a server provider for a project.

This leads to new LSP4E requirement: Run a language server instance per project.
This will solve another issue:
Currently the compile settings (examined from the found compile_commands.json) from the first opened project file which triggers the LS will be used workspace wide to examine (e.g. the C/C++ version to use) the included external (header) files.

Instead, the project specific compile settings should be used to examine external (#include) files included in a project file (e.g. a C++20 system header is included in a project where the C++20 standard has been defined). Another project in the workspace could use a different standard ans system library.

I opened a new issue to discuss this requirement further: #22

@ghentschke
Feature: edit server path and options in preference page
f6cdcfc 
The server path and options can be edited for the used server provider.
The default values are the commands delivered from the server provider.
jonahgraham
jonahgraham approved these changes Feb 27, 2023
View reviewed changes
Copy link
Member
@jonahgraham jonahgraham left a comment

Choose a reason for hiding this comment

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

LGTM now.

@ghentschke ghentschke merged commit 8e57a46 into eclipse-cdt:master Feb 27, 2023
@jonahgraham jonahgraham added this to the 1.0.0 milestone Sep 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonahgraham jonahgraham jonahgraham approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.0.0
Development

Successfully merging this pull request may close these issues.
2 participants
@ghentschke @jonahgraham
0