Fix #152510 #152955
Fix #152510 #152955
Conversation
…ttps://github.com/Harry-Hopkinson/vscode into add-path-suggestions-for-angled-brackets-markdown
| @@ -171,4 +171,14 @@ suite('Markdown path completion provider', () => { | |||
|
|
|||
| assert.ok(completions.some(x => x.insertText === 'file.md'), 'Has file from space'); | |||
| }); | |||
|
|
|||
| test('Should complete path for angled brackets', async () => { | |||
There was a problem hiding this comment.
Please also add tests for:
- Incomplete links:
[](<${CURSOR} - Links that appear inside text on the line:
text [](<${CURSOR}>) more text - Links with titles:
[](<${CURSOR}> "title") - Any other edge cases
I'm pretty sure the code will break for all of those today
| @@ -181,6 +181,24 @@ export class MdVsCodePathCompletionProvider implements vscode.CompletionItemProv | |||
| }; | |||
| } | |||
|
|
|||
| const angleBrackets = /^\s*\[(.*?)\]\(\s*<(.*?)>\s*$/; | |||
There was a problem hiding this comment.
Any reason this can't just be part of the linePrefixText regex? It's the same prefix, just with a < at the start of the link
| return undefined; | ||
| } | ||
|
|
||
| const suffix = lineSuffixText.match(/^[^\)\s]*/); |
There was a problem hiding this comment.
When completions for <> paths are accepted, they should not encode the path components. Make sure to add a test for completions with a space in the name to check that encoding doesn't happen
| @@ -181,6 +181,24 @@ export class MdVsCodePathCompletionProvider implements vscode.CompletionItemProv | |||
| }; | |||
| } | |||
|
|
|||
| const angleBrackets = /^\s*\[(.*?)\]\(\s*<(.*?)>\s*$/; | |||
There was a problem hiding this comment.
This regular expression should match up the cursor inside the link. Right now it tries to match the entire link (Notice how the normal link regex above doesn't require ^\s*) at the start
| @@ -181,6 +181,23 @@ export class MdVsCodePathCompletionProvider implements vscode.CompletionItemProv | |||
| }; | |||
| } | |||
|
|
|||
| const angleBracketsMatch = linePrefixText.match(this.referenceLinkStartPattern); | |||
There was a problem hiding this comment.
This match was already run so we should just re-use the result from above
| }); | ||
|
|
||
| test('Path component should not be encoded with <>', async () => { | ||
| const completions = await getCompletionsAtCursor(workspacePath('new.md'), joinLines( |
There was a problem hiding this comment.
This doesn't actually test that encoding isn't happening. For that you need to check the completions for a path with spaces in it. See Should escape spaces in path names as an example
There was a problem hiding this comment.
This also isn't testing the new behavior. This should test that completions inside <> links don't encode paths with spaces in them
There was a problem hiding this comment.
Sorry but this is still incorrect (and I believe will also fail as written at the moment). Here's what the test needs to be:
For any of the cases:
[](<./CURSOR
or
[](</CURSOR
or
[](<CURSOR
Completions for paths with spaces in their name use an insert text that does not %20 encode the spaces:
assert.ok(completions.some(x => x.insertText === 'sub with space/'), 'Has sub folder completion');
|
Looks like |
|
Sorry but I've merged #153158 which fixes this. I felt getting this PR into good shape was a mergable state would require a lot more back and forth |
This PR fixes #152510, adds path suggestions for angled brackets in markdown.