Skip to content

[vscode] Add proposed dropMetadata API#12736

Merged
msujew merged 2 commits intomasterfrom
msujew/dropMetadata
Jul 25, 2023
Merged

[vscode] Add proposed dropMetadata API#12736
msujew merged 2 commits intomasterfrom
msujew/dropMetadata

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
@JonasHelming JonasHelming requested a review from tsmaeder July 21, 2023 07:43
@msujew msujew force-pushed the msujew/dropMetadata branch from 5133e3a to 813ee11 Compare July 21, 2023 14:19
@tsmaeder
Copy link
Member

@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
Member

I've opened #12748

Copy link
Member

@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
@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