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.

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

fix(VSCODE-118): Fix opening binary _id #220

Merged
merged 5 commits into from
Dec 4, 2020

Conversation

Anemy
Copy link
Member

@Anemy Anemy commented Dec 4, 2020

The solution introduced in #213 didn't fully cover all the opening of documents with different typed _id cases. When we open documents in vscode, in the original solution we sort of hackily added the _id as a ejsoned string on the document's uri in vscode. This doesn't work well because we have to ejson stringify, parse, uri encode, uri decode, and also we're bound by uri constraints like character limits in the query.
This PR updates how we pass a document's _id to the document provider in vscode. It adds a store which we then pass a reference to, so that we can avoid any encoding/decoding of the _id value.
On the upcoming editing documents work we may revisit how we show these documents so that we can be a bit more generalized and we can accommodate opening documents from playground results.

Tested with binary ids with + characters, numberlongs, and some other possible _ids.

opening all sorts of uuids

Should fix #131 as well.

@@ -286,7 +289,10 @@ export default class PlaygroundController {
.then(undefined, (error) => {
log.error('Evaluate playground with cancel modal error', error);

return resolve(undefined);
Copy link
Member Author

Choose a reason for hiding this comment

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

typescript & lint wasn't happy with this line which makes sense. This is an unrelated change.
I'll make a ticket to look into adding lint checks to this project.

Copy link
Contributor

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

Looks good! :)

@Anemy Anemy merged commit 213b793 into master Dec 4, 2020
@Anemy Anemy deleted the VSCODE-118/improve-binary-id-support branch December 4, 2020 10:12
@Anemy Anemy mentioned this pull request Jan 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to find document with NumberLong id
3 participants