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

[vscode] Add proposed dropMetadata API #12736

Merged
merged 2 commits into from
Jul 25, 2023
Merged

[vscode] Add proposed dropMetadata API #12736

merged 2 commits into from
Jul 25, 2023

Conversation

msujew
Copy link
Member

@msujew msujew commented Jul 20, 2023

What it does

Closes #12632

This PR adds the groundworks for supporting the proposed API. Note that while the API is supported on a type level, our current monaco version isn't able to actually make use of the changes. I've annotated all places in the code with @monaco-uplift so that any contributor can improve on this behavior once we have performed the uplift to ^1.79.0.

How to test

There isn't really anything to test, no behavior should have changed. Confirm that the changes in #12125 still work as expected.

Review checklist

Reminder for reviewers

@msujew msujew added the vscode issues related to VSCode compatibility label Jul 20, 2023
@tsmaeder
Copy link
Contributor

@msujew this seems to work fine. The only strange thing when I test according to the instruction in #12125 is this: I get both a new editor with the file I'm dropping and I get the file contents inserted into the editor I dropped into. Is this the desired behavior?

@msujew
Copy link
Member Author

msujew commented Jul 24, 2023

@tsmaeder I assume that this is an untested combination of #12065 and #12125. Since both PRs were submitted at similar times, I assume that the latter one didn't include the former yet, so I didn't notice the strange behavior during testing. And no, this isn't desired behavior. I believe that vscode has a special mode (activated via shift) to change between file dropping and resolving via the DocumentDropEditProvider. We don't seem to do that.

However, fixing that is imo out of scope for this PR.

@tsmaeder
Copy link
Contributor

I've opened #12748

Copy link
Contributor

@tsmaeder tsmaeder left a comment

Choose a reason for hiding this comment

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

Since the observed weird behavior cannot possibly be due to the changes in this PR, this looks good to me.

@msujew msujew merged commit 34e0d13 into master Jul 25, 2023
6 checks passed
@msujew msujew deleted the msujew/dropMetadata branch July 25, 2023 13:12
@github-actions github-actions bot added this to the 1.40.0 milestone Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
vscode issues related to VSCode compatibility
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[vscode][proposed] dropMetadata proposed API updates for 1.79
2 participants