[Language Server] Support for VS Code variables in MSBuild global property overrides

[tintoy]

This would allow populating MSBuild property values by expanding expressions such as ${workspaceRoot}.

For example:

{
    "msbuildProjectTools.msbuild.globalProperties": {
        "MyProperty": "${workspaceRoot}/Foo.sln"
    }
}

Relates to #164.

[tintoy]

Related: microsoft/vscode#70769.

[tintoy]

Also related: microsoft/vscode#46471.

Looks like the issue is well-and-truly undead (been unaddressed for 6+ years now), so we may need to look into alternative mechanisms, such as vscode-variables.

[matherm-aboehm]

Hi,
I'm answering here as follow-up of #164, like proposed.

Sorry I didn't want to destroy your weekends with that. I know these situations quite good for myself.
Unfortunately, I can't follow all of your last explanations. It looks like that I've confused you more than I was with my last comments about all these changes and the PR #167.
What do you mean with "closed it"? What did you close? The PR isn't redundant and wasn't just closed, it was merged into master, as I wrote in my comment and which is perfectly fine, because it contains more then only the submodule commit.
But I was not about to start a new discussion on that topic.
I just tried to let you know that for me there was still a use case for implementing the new features described earlier in #164.
But now that you closed this issue again, I don't think that will happen, so I will try to support on this new issue here.

Also sorry to hear that currently your collaborators are mostly leaving you alone with this great extension.
For #166 I think that it was resolved by PR #167, so the issue could be closed. In the commit messages and PR title you speak of "initial support", so you could see this new issue as a follow up (or sub-issue), if you want to link them together. So you could delay the close of the former issue until this issue is resolved.

Regarding your last paragraph, could you please explain more what your concerns are about the implementation of this feature?
Why should a project work only correctly when in VS Code?
Isn't it already the case with the initial implementation of #166, even when you use just static property overrides, that projects that rely on this, would not work correctly outside of VS Code?
I don't see any relation with this problem to this feature here.
And if these concerns are only about compilation, in all cases that I know of, other tools (also CLI) have the ability that you can provide such properties explicitly for the invocation of the tool, so it would Just Work™. 😄

So, finally on topic of this issue:
It's bad to see that there is no native support for that. I had the impression that this was implemented somehow, because I've already seen variables in the settings file, but they maybe are for internal features of VS Code not extensions, so the implementation of this variable support could also be internal. For example the terminal settings terminal.integrated.profiles.windows/profilename/path or terminal.integrated.profiles.windows/profilename/args can use ${env:...} VS Code variables (looks like PowerShell, but is actually resolved by VS Code).

Currently in your LSP server you just use the raw JSON data you get from the LSP client notification here or initialization here.
When you want to use vscode-variables, you would need to call back from there to the LSP client by using 'workspace/configuration' request and implement everything for the variables in the LSP client (pay attention to the last paragraph in this doc section).
Unfortunately, I can't see any hint in the docs if the LSP client would automatically send (for initialization and notification) the configuration data of the workspace.configuration client capability when it is implemented.

[tintoy]

I’ll start at the end and work backwards 🙂

It’s been a while but I’m pretty sure we can supply initial configuration data to send to the language server when we first initialise it from the extension; we can expand properties there if necessary.

As for working only in VS Code, I guess that I meant automatically populating well-known MSBuild properties from well-known VSCode variables would result in project files that expect those properties to be populated in a way that other IDEs would not do (by default, anyway). A bit like the original SolutionDir issue.

However, allowing the user to override MSBuild properties (and use VSCode variables to do so, effectively establishing their own mappings/transformations) feels to me like a good middle ground.

And yes, VSCode’s variables support is largely internal-only (and even then only supported in a couple of places). While I understand their rationale for not exposing it elsewhere, I do think it’s worth being able to consume that functionality and so I’m happy to use that 3rd-party package to do so.

It’s 7:30am here (Australia) at the moment so I’m gong to have a nice big cup of coffee before going over the rest of this message (that way I can give it the attention it deserves 🙂).

But the TLDR version is that, yes, I think your idea has merit and I’d like to implement it as described in this issue’s description. I’m hoping to have time to get an initial implementation going, this week, so we have something concrete to discuss.

[tintoy]

PS. Thanks for taking the time to discuss this - I appreciate your feedback 🙂

[tintoy]

Ah, I see - we rely on the native config synchronisation mechanism. I think maybe that’s where some of my other-IDE concerns might have come in; the language server is also used from Eclipse (which has different variables/behaviour).

I’ll have a think about it this morning and work out how we can reliably support this in a way that still gracefully downgrades to the regular config style if the client doesn’t support variables.

[tintoy]

Looks like pulling configuration is the preferred model anyway:

https://github.com/microsoft/vscode-languageserver-node/blob/3412a17149850f445bf35b4ad71148cfe5f8411e/client/src/common/configuration.ts#L132