[WIP] - csproj intellisense #1214

cmurczek-it · 2017-02-09T22:43:19Z

preliminary task list to fix #1156

familiarize with NuGet V4 API
NuGet Gallery still runs on V3. I will get the feeds from the user scoped nuget.config, that way it'll "auto-update" as soon as the new api is referenced there. I'll put a test in for the feed name to make sure smthg breaks when that happens.
familiarize with how NuGet intellisense works in project.json
write tests and ~~enhance PackageManager~~ introduce NuGetClient
write tests and provider
wire up the provider with the extension

dnfclas · 2017-02-09T22:43:23Z

Hi @cmurczek, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla2.dotnetfoundation.org.

TTYL, DNFBOT;

DustinCampbell · 2017-02-12T13:55:47Z

test/packageReferenceProvider.test.ts

+
+    test('Retrieve packages from NuGet Gallery', () => {
+        let partialPackageId = 'newtonsoft';
+        let target = new PackageManager(platformInfo, logger);


Note that PackageManager in the VS Code extension has nothing to do with NuGet.

cmurczek-it · 2017-02-13T02:05:32Z

@DustinCampbell: Travis CI fails when I call PlatformInformation.GetCurrent() while setting up my tests. Any idea how to fix this? It's working on my (Windows) machine...

DustinCampbell · 2017-02-13T02:15:11Z

src/nuget.ts

+
+            fs.readFile(nuGetConfig, 'utf-8', (err, data) => {
+                if (err) {
+                    reject('Reading ' + nuGetConfig + ' failed with error ' + err);


Note that this will just carry below trying to access data. I suspect you want to return here.

DustinCampbell · 2017-02-13T02:17:20Z

PlatformInformation.GetCurrent() calls our execChildProcess utility, which relies on setExtensionPath being initialized here. This function called as part of extension activation.

Since you're just checking whether you're on windows, you might consider just checked process.platform === 'win32' instead of using PlatformInformation.

DustinCampbell · 2017-02-13T02:22:59Z

You should also be aware of this PR: #224. We'll be taking this soon to pull project.json completion into C# for VS Code (it lives in VS Code today). We'll want to unify these two to have the same behavior.

cmurczek-it · 2017-02-13T09:44:18Z

yeah, I'm using that one as a template. The NuGetClient class would be my shot at the future improvement that is mention in the PR regarding making the FEED_INDEX_URL configurable.

I'll refactor some more to mock away as many dependencies as possible and the push another update.

dnfclas · 2017-02-13T11:31:32Z

@cmurczek, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, DNFBOT;

DustinCampbell · 2017-02-13T14:41:53Z

I'll refactor some more to mock away as many dependencies as possible and the push another update.

Please don't over mock if you feel like you're going too far. After all, the tests run inside of VS Code to provide access to various VS Code services. So, if you must use PlatformInformation (which we don't use everywhere either -- only when we need bitness and Linux distro information), it's just a matter of ensuring that the extension is activated. I'm not sure what that looks like precisely, but it's certainly feasible.

cmurczek-it · 2017-02-13T15:54:58Z

And we're back on track. Thx for the heads up regarding test environment. I was coming from the fact that, the tests as they were, were dependent on an actual nuget.config file being present on the target system. Which in case of Travis CI is probably not the case. After refactoring processing of the config is separated from loading it from the file, so we're good, even on servers without any nuget installation/config.

There's however the question of testing the CompletionItemProvider for PackageReference elements. This will derive from AbstractProvider which requires me to pass an instance of OmniSharpServer. Is there a reasonable way to set up the infrastructure for such tests, or is that the reason why there aren't any tests for all the other providers? If so, I'd keep the PackageReferenceCompletionProvider as thin as possible on top of the NuGetClient.

DustinCampbell · 2017-02-13T16:17:31Z

I wouldn't bother deriving from AbstractProvider. It really doesn't buy you much and this shouldn't have any OmniSharp dependencies anyway.

DustinCampbell · 2017-02-13T17:26:34Z

I just wanted to point out that I've updated the PR for moving project.json completion and hover support into C# for VS Code: #1236. I'll likely be merging this soon. When I do, you might want to rebase onto the latest master.

cmurczek-it · 2017-02-13T18:02:22Z

Thx for letting me know. Will update when you signal. I guess that means I'll be tasked with unifying both of them too?

On a different note: What's your opinion on running live http-requests during tests? Right now Mocha flags them as slow, <1s but still. If it bothers you I could look into stubbing https calls using sinon, playing back canned responses. Might be worth the work anyway for the consistency of test data alone. Let me know.

DustinCampbell · 2017-02-13T18:08:00Z

I'm not expecting you to unify them. They can be separate for now and we can a work item to unify them later. You can stay focused. 😄

DustinCampbell · 2017-02-17T16:26:07Z

FYI that I merged the project.json IntelliSense yesterday.

the NuGetClient takes the activePackageSources from user scope NuGet.config and extracts the needed urls from the package source's index

probably needs more tests and mocking/stubbing of http calls

I had to update node from 4.4.5 to 6.9.5 in order to successfully complete the npm run compile script. Before the update it failed due to an unexpected = in executeChildProcess's parameters in common.js

this removes dependencies on external services (stubbing of http calls) as well as dependencies between tests

NinoFloris · 2017-03-09T00:12:37Z

Would this be easy to extend now @DustinCampbell?

NinoFloris · 2017-03-11T12:43:40Z

This being the project.json infra you already merged?

borgdylan · 2017-03-14T11:26:04Z

Will this only read PackageReferences via .csproj or will the extension allow loading of any arbitrary extension as long as it is an MSBuild project?

DustinCampbell · 2017-03-14T13:12:18Z

@borgdylan: I'm not sure I understand your question.

borgdylan · 2017-03-14T13:43:41Z

Will it be hardcoded to project that use ".csproj"? Or will it allow projects that are .".fsproj" etc. Intellisense for NuGet package versions is not C# specific and should be available for any project or else put in a separate extension that is generic.

DustinCampbell · 2017-03-14T14:12:33Z

@borgdylan: For C# for VS Code, it'll be specific to .csproj.

NinoFloris · 2017-03-30T18:39:28Z

So... Is this an abandoned project?

DustinCampbell · 2017-03-31T02:17:32Z

It seems like it might be. I'm awaiting @mlorbetske to deliver me a NuGet package of https://github.com/dotnet/ProjFileTools so we can use that within OmniSharp and provide completion to C# for VS Code.

mlorbetske · 2017-03-31T02:37:18Z

Sorry for the delay, I've been swamped of late. I got the go ahead to make a package for the project file tools earlier this week, just need to update the build and find someone to upload it to NuGet under one of the official accounts. I could also make it available on MyGet earlier (tonight maybe?) if that'd help.

mlorbetske · 2017-03-31T03:14:41Z

Filed dotnet/ProjFileTools#10 to get the packaging going as part of the official build.

For the time being, the package is available as ProjectFileTools.NuGetSearch v1.0.0 on https://dotnet.myget.org/F/temp-projfiletools/api/v3/index.json.

@DustinCampbell

cmurczek · 2017-03-31T05:46:28Z

Sorry I went AWOL for a couple of weeks. I'll have a couple of hours to kill while traveling tomorrow. I could try to wrap it up then, but I guess using the same told as VS2017 would be preferable. @DustinCampbell let me know

cmrigney · 2017-05-22T18:24:26Z

Any progress? This would be very useful.

xsoheilalizadeh · 2017-06-30T14:59:56Z

any plan? I Wait for this thanks :)

tintoy · 2018-03-13T23:23:41Z

Hi.

Just stumbled across this PR and, in case anybody's still wondering about intellisense for .csproj files (or any MSBuild project file, really), there's a separate extension that does this:

https://marketplace.visualstudio.com/items?itemName=tintoy.msbuild-project-tools

DustinCampbell · 2018-03-15T18:36:02Z

Given that it's been a year, I'm going to go ahead and close this PR. @tintoy's msbuild-project-tools extension is an excellent alternatively.

dnfclas added the cla-required label Feb 9, 2017

DustinCampbell reviewed Feb 12, 2017

View reviewed changes

DustinCampbell reviewed Feb 13, 2017

View reviewed changes

dnfclas added cla-signed and removed cla-required labels Feb 13, 2017

cmurczek-it force-pushed the packageRef-intellisense branch from 4eccbe8 to 3f424c9 Compare February 20, 2017 14:49

cmurczek-it added 11 commits February 20, 2017 16:59

create a base for the PR

f520c84

test added

5febd36

vscode complains about deprecated launch task attributes

a528dd1

introducing NuGetClient

2dbb59a

the NuGetClient takes the activePackageSources from user scope NuGet.config and extracts the needed urls from the package source's index

make tests without implementation pending

7f3f166

refactored NuGetClient for easier testing

df19a89

retrieving suggestions for partial packageId

07a811e

renamed the file, added more tests and embrace async/await

4dc8e72

retrieving versions for a given package

73ff173

making the NugetClient feature complete for this purpose

449b948

probably needs more tests and mocking/stubbing of http calls

updated min node versions to successfully compile

b3572a5

I had to update node from 4.4.5 to 6.9.5 in order to successfully complete the npm run compile script. Before the update it failed due to an unexpected = in executeChildProcess's parameters in common.js

lots of stubbing to remove dependencies

6da0224

this removes dependencies on external services (stubbing of http calls) as well as dependencies between tests

cmurczek-it force-pushed the packageRef-intellisense branch from 3f424c9 to 6da0224 Compare February 20, 2017 16:00

ghost removed the cla-signed label Dec 7, 2017

DustinCampbell closed this Mar 15, 2018

snyk-bot mentioned this pull request Mar 12, 2020

[Snyk] Fix for 1 vulnerabilities TheRealPiotrP/omnisharp-vscode#26

Open

snyk-bot mentioned this pull request Mar 30, 2021

[Snyk] Security upgrade nyc from 11.4.1 to 15.0.0 TheRealPiotrP/omnisharp-vscode#40

Open

snyk-bot mentioned this pull request Sep 15, 2021

[Snyk] Security upgrade nyc from 11.4.1 to 15.0.0 TheRealPiotrP/omnisharp-vscode#42

Open

snyk-bot mentioned this pull request Dec 30, 2021

[Snyk] Security upgrade nyc from 11.4.1 to 15.0.0 TheRealPiotrP/omnisharp-vscode#43

Open

TheRealPiotrP mentioned this pull request Jun 21, 2023

[Snyk] Fix for 1 vulnerabilities TheRealPiotrP/omnisharp-vscode#53

Open

TheRealPiotrP mentioned this pull request Nov 30, 2023

[Snyk] Fix for 1 vulnerabilities TheRealPiotrP/omnisharp-vscode#56

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] - csproj intellisense #1214

[WIP] - csproj intellisense #1214

cmurczek-it commented Feb 9, 2017 •

edited

Loading

dnfclas commented Feb 9, 2017

DustinCampbell Feb 12, 2017

cmurczek-it commented Feb 13, 2017

DustinCampbell Feb 13, 2017

DustinCampbell commented Feb 13, 2017

DustinCampbell commented Feb 13, 2017

cmurczek-it commented Feb 13, 2017

dnfclas commented Feb 13, 2017

DustinCampbell commented Feb 13, 2017

cmurczek-it commented Feb 13, 2017

DustinCampbell commented Feb 13, 2017

DustinCampbell commented Feb 13, 2017

cmurczek-it commented Feb 13, 2017

DustinCampbell commented Feb 13, 2017

DustinCampbell commented Feb 17, 2017

NinoFloris commented Mar 9, 2017

NinoFloris commented Mar 11, 2017

borgdylan commented Mar 14, 2017

DustinCampbell commented Mar 14, 2017

borgdylan commented Mar 14, 2017

DustinCampbell commented Mar 14, 2017

NinoFloris commented Mar 30, 2017

DustinCampbell commented Mar 31, 2017

mlorbetske commented Mar 31, 2017

mlorbetske commented Mar 31, 2017 •

edited

Loading

cmurczek commented Mar 31, 2017

cmrigney commented May 22, 2017

xsoheilalizadeh commented Jun 30, 2017

tintoy commented Mar 13, 2018

DustinCampbell commented Mar 15, 2018

[WIP] - csproj intellisense #1214

[WIP] - csproj intellisense #1214

Conversation

cmurczek-it commented Feb 9, 2017 • edited Loading

dnfclas commented Feb 9, 2017

DustinCampbell Feb 12, 2017

Choose a reason for hiding this comment

cmurczek-it commented Feb 13, 2017

DustinCampbell Feb 13, 2017

Choose a reason for hiding this comment

DustinCampbell commented Feb 13, 2017

DustinCampbell commented Feb 13, 2017

cmurczek-it commented Feb 13, 2017

dnfclas commented Feb 13, 2017

DustinCampbell commented Feb 13, 2017

cmurczek-it commented Feb 13, 2017

DustinCampbell commented Feb 13, 2017

DustinCampbell commented Feb 13, 2017

cmurczek-it commented Feb 13, 2017

DustinCampbell commented Feb 13, 2017

DustinCampbell commented Feb 17, 2017

NinoFloris commented Mar 9, 2017

NinoFloris commented Mar 11, 2017

borgdylan commented Mar 14, 2017

DustinCampbell commented Mar 14, 2017

borgdylan commented Mar 14, 2017

DustinCampbell commented Mar 14, 2017

NinoFloris commented Mar 30, 2017

DustinCampbell commented Mar 31, 2017

mlorbetske commented Mar 31, 2017

mlorbetske commented Mar 31, 2017 • edited Loading

cmurczek commented Mar 31, 2017

cmrigney commented May 22, 2017

xsoheilalizadeh commented Jun 30, 2017

tintoy commented Mar 13, 2018

DustinCampbell commented Mar 15, 2018

cmurczek-it commented Feb 9, 2017 •

edited

Loading

mlorbetske commented Mar 31, 2017 •

edited

Loading