Skip to content

Conversation

MeghanKulkarni
Copy link
Contributor

@MeghanKulkarni MeghanKulkarni commented Jun 22, 2023

Providing ability for pasting over highlighted Markdown text to turn the text into a link to the pasted URL

Fixes #179628

@MeghanKulkarni MeghanKulkarni requested a review from mjbvz June 22, 2023 22:33
@vscodenpa vscodenpa added this to the June 2023 milestone Jun 22, 2023
Copy link
Collaborator

@mjbvz mjbvz left a comment

Choose a reason for hiding this comment

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

Added a few more suggestions

Copy link
Collaborator

@mjbvz mjbvz left a comment

Choose a reason for hiding this comment

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

Added a few more points of feedback. The most important one is that to make sure we aren't going to change the existing file/uri-list paste flow

const snippetEdits = coalesce(activeEditor.selections.map((selection, i): vscode.SnippetTextEdit | undefined => {
const selectionText = activeEditor.document.getText(selection);
const snippet = createUriListSnippet(activeEditor.document, selectedFiles, {
const snippet = createUriListSnippet(activeEditor.document, selectedFiles, title, placeholderValue, {
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's the difference between these new arguments and the existing placeholderText / placeholderStartIndex values passed in below?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did not want to change to existing functionality of insertResource.ts, so I wanted to keep placeholderText and placeholderString as they are. I made an edit in shared.ts that will hopefully make more sense.

Copy link
Collaborator

@mjbvz mjbvz left a comment

Choose a reason for hiding this comment

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

Thanks for making the changes!

Approving so we can test this tomorrow. Please make sure to follow up on any remaining comments

@mjbvz mjbvz merged commit 6626f5f into microsoft:main Jun 27, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Aug 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide ability for pasting over highlighted Markdown text to turn the text into a link to the pasted URL
3 participants