-
Notifications
You must be signed in to change notification settings - Fork 419
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
Check if file exist before trying to create MetadataReference. #1045
Check if file exist before trying to create MetadataReference. #1045
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -148,7 +148,7 @@ private void ScriptReferencesChanged(object sender, ReferencesChangedEventArgs e | |||
var document = solution.GetDocument(documentId); | |||
var project = document.Project; | |||
|
|||
var metadataReferences = e.References.Select(reference => MetadataReference.CreateFromFile(reference, documentation: GetDocumentationProvider(reference))); | |||
var metadataReferences = GetMetadataReferences(e.References); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you may want to use https://github.com/OmniSharp/omnisharp-roslyn/blob/master/src/OmniSharp.Roslyn/Utilities/MetadataReferenceEqualityComparer.cs to avoid duplicates
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, e.References
should be ISet<string>
and not IReadOnlyCollection<string>
, so duplicates should already be avoided.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@filipw is referring to the issue of adding the same assembly reference to a project twice. This can happen if there are multiple copies of the same assembly with different file paths.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, then I understand. I'll update!
Just asking out of curiosity -- should you really just be using the MetadataFileCacheService? |
@DustinCampbell yes, I should be using that! 😄 Will rework this PR to use that instead. |
- Use MetadataFileReferenceCache when loading metadatareferences - ReferencesChangedEventArgs.References should be of type ISet<string>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good, the test failure is caused by nuget failure atm
AppVeyor seems to have died getting nuget pacakges. kicking. |
Just to avoid throwing any uneccesary exceptions.