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

Proposal: make it easier to work with dotnet tools for C#/F# #4299

Merged

Conversation

Gastove
Copy link
Contributor

@Gastove Gastove commented Jan 13, 2024

This commit implements an approach proposed in #3906. Specifically, it provides new control variables to make it possible to use C# and F# language servers either as global or local dotnet tools. So far, I've done this by providing boolean variables to control command construction, but I'm not attached to this approach.

Closes #3906.

@jcs090218
Copy link
Member

cc @razzmatazz WDYT? 🤔

This commit allows users of this code to add extra directories to be ignored
during project loading, which can be extremely useful while also using toolsets
like Nix (which may symlink other F# projects in to the repo, thus confounding
project loading).
This commit implements an approach proposed in emacs-lsp#3906. Specifically, it provides
new control variables to make it possible to use C# and F# language servers
_either_ as global _or_ local dotnet tools. So far, I've done this by providing
boolean variables to control command construction, but I'm not attached to this
approach.

Closes emacs-lsp#3906.
@Gastove Gastove force-pushed the proposal/clean-up-dotnet-tool-management branch from a7e8932 to c6c272d Compare July 6, 2024 22:56
@github-actions github-actions bot added the client One or more of lsp-mode language clients label Jul 6, 2024
@Gastove
Copy link
Contributor Author

Gastove commented Jul 6, 2024

Hi y'all -- I think @razzmatazz / @yyoncho are the right "y'all" here. I've just rebased this branch so it's merge-able again. What do I need to do to get this merged? At this point, I think I've rebased my proposed changes from #4049 in here, and this PR now closes #4049 as well.

(if lsp-csharp-csharpls-use-local-tool
"dotnet tool list"
"dotnet tool list -g")))
)
Copy link
Member

Choose a reason for hiding this comment

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

Let's place the ) on the last line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh my lord, how did that get there 😆 🤦 Yes! One second.

@jcs090218
Copy link
Member

Can you resolve these compile errors?

In toplevel form:
lsp-csharp.el:131:2: Warning: custom-declare-variable `lsp-csharp-csharpls-use-dotnet-tool' docstring wider than 80 characters

In toplevel form:
lsp-fsharp.el:184:2: Warning: custom-declare-variable `lsp-fsharp-workspace-extra-exclude-dirs' docstring wider than 80 characters

@Gastove
Copy link
Contributor Author

Gastove commented Jul 7, 2024

Can you resolve these compile errors?

In toplevel form:
lsp-csharp.el:131:2: Warning: custom-declare-variable `lsp-csharp-csharpls-use-dotnet-tool' docstring wider than 80 characters

In toplevel form:
lsp-fsharp.el:184:2: Warning: custom-declare-variable `lsp-fsharp-workspace-extra-exclude-dirs' docstring wider than 80 characters

Absolutely.

@jcs090218
Copy link
Member

LGTM. Thanks!

@jcs090218 jcs090218 merged commit c69118c into emacs-lsp:master Jul 7, 2024
10 of 13 checks passed
@Gastove Gastove deleted the proposal/clean-up-dotnet-tool-management branch July 10, 2024 03:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client One or more of lsp-mode language clients
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Proposal/Enhancement: Allow C#/F# to use "local" tools for LSP servers
2 participants