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

feat: copy id or document to clipboard #419

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

SethFalco
Copy link
Contributor

@SethFalco SethFalco commented May 28, 2022

Description

Adds 2 new context menu actions.

  • mdb.copyDocumentIdFromTree - To copy the ID of a document to the clipboard.
  • mdb.copyDocumentFromTree - To copy the value of a document to the clipboard.

Sometimes I can recognize an ID, or especially for small collections (i.e. during development) I know the document at the bottom is the one I need to interact with. It'd be nice to be able to just copy the ID directly from the tree view and just paste it in the playground.

body = body.slice(1, body.length - 1);
Just to explain this particular line. Copies the documentId property, but if it's a string, which it is in most cases, I thought we should remove the quotes when copying.
For any other type of ID (namely objects) the output doesn't start or end with quotes anyway.

Checklist

  • New tests and/or benchmarks are included
    • Not sure if we need tests for this one?
  • Documentation is changed or added

Motivation and Context

  • Bugfix
  • New feature
  • Dependency update
  • Misc

Dependents

Types of changes

  • Backport Needed
  • Patch (non-breaking change which fixes an issue)
  • Minor (non-breaking change which adds functionality)
  • Major (fix or feature that would cause existing functionality to change)

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.

Thank you for the PR! I think we’d need at least two things more, both for this one and for #420:

return false;
}

let body = JSON.stringify(documentId);
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens here if documentId contains non-JS bson datatypes, e.g. Binary() or similar? Should this be EJSON instead of JSON?

Comment on lines +502 to +504
if (body.startsWith('"')) {
body = body.slice(1, body.length - 1);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

If the idea here is to leave string document ids as-is, it might be better to check e.g. if (typeof body !== 'string') { body = JSON.stringify(documentId); } instead, since this currently still leaves JSON escapes in body

const tabSize = vscode.workspace
.getConfiguration('editor')
.get('tabSize', 2);
const body = JSON.stringify(document, undefined, tabSize);
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar question here :)

@Anemy
Copy link
Member

Anemy commented Nov 10, 2022

Added view document and copy document in this pr: #445 - it'll be in the next release. No copy id, yet, though.

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.

None yet

3 participants