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

turning highlighted Markdown text to link to pasted URL #185924

Merged

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

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
5 checks passed
@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