Skip to content

Always use canonical uri inside createModelReference#99658

Merged
jrieken merged 3 commits intomasterfrom
joh/uriIdent
Jun 9, 2020
Merged

Always use canonical uri inside createModelReference#99658
jrieken merged 3 commits intomasterfrom
joh/uriIdent

Conversation

@jrieken
Copy link
Copy Markdown
Member

@jrieken jrieken commented Jun 9, 2020

This is for #93368

@jrieken jrieken self-assigned this Jun 9, 2020

async createModelReference(resource: URI): Promise<IReference<IResolvedTextEditorModel>> {

resource = this.uriIdentityService.asCanonicalUri(resource);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@bpasero this is the brute force way which might be OK... However, there are still all callers that we might need to visit and should maybe move this call to where it matter most, e.g where we actually branch off into the file service. E.g here:

return this.textFileService.files.resolve(resource, { reason: TextFileLoadReason.REFERENCE });

All other places shouldn't be affected by URI "normalization" because only file systems can contribute rules.

Copy link
Copy Markdown
Member

@bpasero bpasero left a comment

Choose a reason for hiding this comment

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

@jrieken I only added a comment to explain the reasoning, this is a comment I also have in editor service to clear things up a bit.

We can move it closer to where we branch off into files, but on the other hand, that does not change things in anyway because since it only impacts resources with file providers anyway, we do not cause any harm for all other resources that are being passed in, right?

…solverService.ts

Co-authored-by: Benjamin Pasero <benjpas@microsoft.com>
@jrieken
Copy link
Copy Markdown
Member Author

jrieken commented Jun 9, 2020

we do not cause any harm for all other resources that are being passed in, right?

Yeah, no harm with that. Let's ship it and see how it works

@jrieken jrieken merged commit e4d8873 into master Jun 9, 2020
@jrieken jrieken deleted the joh/uriIdent branch June 9, 2020 12:53
@github-actions github-actions bot locked and limited conversation to collaborators Jul 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants