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.

Sign up for GitHub

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

Jump to bottom

FileSystemProvider whose uris use the query element is not properly handled by conflict resolution feature #104698

Closed
gjsjohnmurray opened this issue Aug 14, 2020 · 3 comments · Fixed by #104699
Closed

FileSystemProvider whose uris use the query element is not properly handled by conflict resolution feature #104698

gjsjohnmurray opened this issue Aug 14, 2020 · 3 comments · Fixed by #104699
Assignees
@bpasero
Labels
bug Issue identified by VS Code Team member as probable bug file-io File I/O verified Verification succeeded
Milestone
August 2020

Comments

@gjsjohnmurray
Copy link
Contributor

I am developing an extension that implements a FileSystemProvider for which the query element of the uri is significant.

When an edited document is saved and VS Code detects that the target file is newer, VS Code correctly offers this notification:

image

But when the 'Compare' option is taken, the uri sent to the FSP has lost its query string.

The fault is here:

vscode/src/vs/workbench/contrib/files/common/files.ts

Lines 166 to 172 in cc29a81

private static resourceToTextFile(scheme: string, resource: URI): URI {
return resource.with({ scheme, query: JSON.stringify({ scheme: resource.scheme }) });
}
private static textFileToResource(resource: URI): URI {
return resource.with({ scheme: JSON.parse(resource.query)['scheme'], query: null });
}

resourceToTextFile sequesters the query element to stash the original scheme when setting a new scheme. Later textFileToResource reverses this process, but in doing so it has lost the query element of the original uri.

I will submit a PR.
gjsjohnmurray added a commit to gjsjohnmurray/vscode that referenced this issue Aug 14, 2020
@gjsjohnmurray
fix microsoft#104698 Make query element of uri survive conflict resol…
b963948 
…ution massaging
@gjsjohnmurray gjsjohnmurray mentioned this issue Aug 14, 2020
Merged
@gjsjohnmurray gjsjohnmurray mentioned this issue Aug 16, 2020
Closed
@jrieken jrieken assigned bpasero and unassigned jrieken Aug 17, 2020
@bpasero bpasero added bug Issue identified by VS Code Team member as probable bug file-io File I/O labels Aug 17, 2020
@bpasero bpasero modified the milestones: July 2020, August 2020 Aug 17, 2020
bpasero pushed a commit that referenced this issue Aug 17, 2020
@gjsjohnmurray
fix #104698 Make query element of uri survive conflict resolution mas…
04f2a74 
…saging (#104699)

* fix #104698 Make query element of uri survive conflict resolution massaging

* Incorporate PR feedback

* Simplify

* Event simpler, and with comments

* Variant preferred by @bpasero
bpasero added a commit that referenced this issue Aug 17, 2020
@bpasero
Revert "fix #104698 Make query element of uri survive conflict resolu…
185bf25 
…tion massaging (#104699)"

This reverts commit 04f2a74.
@bpasero bpasero reopened this Aug 17, 2020
@bpasero
Copy link
Member

bpasero commented Aug 17, 2020

@gjsjohnmurray I oversaw test unit failures in

vscode/src/vs/workbench/contrib/files/test/browser/fileOnDiskProvider.test.ts

Line 23 in 185bf25

test('provideTextContent', async () => {
. I reverted the change and it would require a new PR + fix to address.

@bpasero bpasero removed this from the August 2020 milestone Aug 17, 2020
@bpasero bpasero added this to the August 2020 milestone Aug 18, 2020
@bpasero
Copy link
Member

bpasero commented Aug 18, 2020

@gjsjohnmurray thought I would have a closer look into it and noticed that the test was not ideal. All in all pushed it back via e55f6c9

@bpasero bpasero reopened this Aug 18, 2020
@bpasero bpasero closed this as completed Aug 18, 2020
@gjsjohnmurray
Copy link
Contributor Author

@bpasero thanks very much.

@connor4312 connor4312 added verified Verification succeeded z-author-verified labels Sep 3, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Oct 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@bpasero bpasero

Labels
bug Issue identified by VS Code Team member as probable bug file-io File I/O verified Verification succeeded
Projects
None yet
Milestone
August 2020
Development

Successfully merging a pull request may close this issue.

fix #104698 Make query element of uri survive conflict resolution massaging gjsjohnmurray/vscode
5 participants
@bpasero @jrieken @connor4312 @gjsjohnmurray and others