티스토리 수익 글 보기

티스토리 수익 글 보기

Add support for references when no [] exists · Pull Request #144781 · microsoft/vscode · GitHub
Skip to content

Add support for references when no [] exists – #144781

Merged
Matt Bierner (mjbvz) merged 9 commits into
mainfrom
unknown repository
Mar 17, 2022
Merged

Add support for references when no [] exists#144781
Matt Bierner (mjbvz) merged 9 commits into
mainfrom
unknown repository

Conversation

@ghost

@ghost Deleted user (ghost) commented Mar 9, 2022

Copy link
Copy Markdown

This PR fixes #141285

Code has been updated with the main branch and I have personally reviewed it.

Changes

Alters the regex to match […] and […][…]. Treats […] as if it’s the same as […][].

Testing

On the feature branch:

  1. Create a new markdown file.
  2. Add in some references. For example:
    [Works]
    [Works][]
    [Works][Works]
    [Still Works][Works]
    
    [Does Not Work]
    [Does Not Work][]
    [Does Not Work][DoesNotWork]
    [Works][DoesNotWork] Jokes
    
    [Works]
    
    [Works]: https://microsoft.com
  3. Make sure that the links that you’d expect to work continue to work, both in the editor display and the preview.
    • Expected behavior in editor: Ctrl-Click (or Cmd-Click on Mac) brings cursor to reference.
    • Expected behavior in preview: References turn blue and link to their targets.
    • Preview can be opened via running Markdown: Open Preview from the Command Palette.

Expected Output

image

@ghost ghost changed the title add single reference linking regex Add support for references when no [] exists Mar 9, 2022
@mjbvz

Copy link
Copy Markdown
Contributor

Thanks for taking a look! Can you please also add tests for this:

const testFile = vscode.Uri.joinPath(vscode.workspace.workspaceFolders![0].uri, ‘x.md’);

In the tests, also try adding test for potential edge cases or false positives where link detection should not occur

I’m also inclined to say that the link detection should not kick in for [Works]: https://microsoft.com. We already highlight the destination link in that case and highlighting the link id itself doesn’t strike me as useful here

@ghost

Deleted user (ghost) commented Mar 11, 2022

Copy link
Copy Markdown
Author

Will do!

@ghost

Deleted user (ghost) commented Mar 16, 2022

Copy link
Copy Markdown

CLA assistant check
All CLA requirements met.

@ghost

Copy link
Copy Markdown
Author

Sorry for the delay. Should be ready for review!

@mjbvz Matt Bierner (mjbvz) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks reasonable overall. Thanks for following up!

Can you please remove the binary file added under ./scripts though? Looks like it was committed by mistake

@ghost

Copy link
Copy Markdown
Author

Whoops! My bad for littering. Should be fixed, thanks for the heads up!

@ghost
ghost requested a review from Matt Bierner (mjbvz) March 16, 2022 21:33
@mjbvz Matt Bierner (mjbvz) added this to the March 2022 milestone Mar 17, 2022
@mjbvz
Matt Bierner (mjbvz) merged commit b6f6a37 into microsoft:main Mar 17, 2022
@mjbvz

Copy link
Copy Markdown
Contributor

Thanks! Will be in the next insiders build

@github-actions github-actions Bot locked and limited conversation to collaborators May 1, 2022
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.

Support references links when no [] exists

2 participants