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

Uri.parse doesnt render link correctly if the uri has a space in it #113328

Closed
ctf0 opened this issue Dec 23, 2020 · 13 comments
Closed

Uri.parse doesnt render link correctly if the uri has a space in it #113328

ctf0 opened this issue Dec 23, 2020 · 13 comments
Assignees
Labels
info-needed Issue requires more information from poster *not-reproducible Issue cannot be reproduced by VS Code Team member as described

Comments

@ctf0
Copy link

ctf0 commented Dec 23, 2020

Version: 1.53.0-insider
Commit: 4a875e23d20b64504a818834f3fa4c40adb8d480
Date: 2020-12-21T12:32:43.927Z
Electron: 11.1.0
Chrome: 87.0.4280.88
Node.js: 12.18.3
V8: 8.7.220.29-electron.0
OS: Darwin x64 19.6.0

Steps to Reproduce:

based on the docs
i have an ext that have the same setup but with a DoucmentLink instead of hover, which was working perfectly but now instead of getting a link i get the text

[remove "spatie/laravel-backup"](command:gotoPackage.removePackage?[{"cmnd":"composer remove spatie/laravel-backup","pkg":"spatie/laravel-backup"}] "Execute command gotoPackage.removePackage") (cmd + click)

and here is the console log

Screen Shot 2020-12-23 at 10 23 22 AM

am not sure if the api has changed or is it a genuine bug 😢

Does this issue occur when all extensions are disabled?: No

@jrieken jrieken added the info-needed Issue requires more information from poster label Dec 23, 2020
@jrieken
Copy link
Member

jrieken commented Dec 23, 2020

Please provide a simple repo case. I have tried with the provider below and everything is working fine

  vscode.languages.registerDocumentLinkProvider('*', new class implements vscode.DocumentLinkProvider {
        provideDocumentLinks(document: vscode.TextDocument): vscode.DocumentLink[] {
            return [
                new vscode.DocumentLink(new vscode.Range(0, 0, 1, 0), vscode.Uri.parse('command:extension.helloWorld'))
            ]
        }
    })

@ctf0
Copy link
Author

ctf0 commented Dec 23, 2020

u can try with the extension provided above, its basically a

i've even tried with an old version of the extension and still give the same result, whats really strange is the other 2 normal links "open file, open url" works without any problems.

also plz note that am using the second example of the docs where we pass arguments to the cmnd, maybe thats the problem am not sure.

@jrieken
Copy link
Member

jrieken commented Dec 23, 2020

u can try with the extension provided above, its basically a

Sorry, but asking you to distill a repo case from that

@ctf0
Copy link
Author

ctf0 commented Dec 23, 2020

sure np, i will ping you once done.

@ctf0
Copy link
Author

ctf0 commented Dec 23, 2020

https://github.com/ctf0/cnbd-uri-test, same ext downsized to bare min

steps

  • yarn
  • run debug
  • open package.json
  • hover a dependency

@ctf0 ctf0 changed the title Commands URIs no longer works with DoucmentLinks Commands URIs no longer works with DocumentLinks Dec 29, 2020
@ctf0
Copy link
Author

ctf0 commented Dec 29, 2020

finally found the issue.

if the uri have an empty space in it, it wont render correctly and will show the text instead.
this is related to the DocumentLink api, not Commands URIs api ex.

[resources/lang/en.json](vscode-insiders://ctf0.laravel-goto-lang/Users/xxx/test/resources/lang/en.json?hello world)

note hello world


[remove "spatie/laravel-backup"](command:gotoPackage.removePackage?[{"cmnd":"composer remove spatie/laravel-backup","pkg":"spatie/laravel-backup"}] "Execute command gotoPackage.removePackage")

note composer remove


removing the empty spaces will render the link as expected.

@ctf0 ctf0 changed the title Commands URIs no longer works with DocumentLinks DocumentLink doesnt render link correctly if the uri has a space in it Dec 29, 2020
@jeanp413
Copy link
Contributor

jeanp413 commented Jan 2, 2021

@jrieken I took a look at this and found that this is a regression from #112670
Not sure if it would be better to revert that commit or #85930 would be a better fix for this

@jrieken
Copy link
Member

jrieken commented Jan 4, 2021

@ctf0 This is confusing. How does #113328 (comment) relate to the document link provider API? That comment talks about markdown link syntax which seems unrelated to that API

@ctf0
Copy link
Author

ctf0 commented Jan 4, 2021

@jrieken actually u can use the same logic with any api that use vscode.Uri.parse & u r correct, the docs only mention MarkdownString but in reality its applicable with lots of other apis like document link.

so to thumbs up, i believe the issue is more related to vscode.Uri.parse in general no matter where u use it & not document link or MarkdownString in specific.

@ctf0
Copy link
Author

ctf0 commented Feb 12, 2021

a temp solution for anyone having this issue is to use encodeURI which will escape the empty space & decodeURI inside the UriHandler.

@ctf0 ctf0 changed the title DocumentLink doesnt render link correctly if the uri has a space in it Uri.parse doesnt render link correctly if the uri has a space in it Feb 12, 2021
@jrieken
Copy link
Member

jrieken commented Feb 12, 2021

This get more confusing. The make zero sense to me, Uri.parse doesn't render anything. Can't you just provide simple steps like:

  1. Doing xyz
  2. expecting foo
  3. seeing bar

@ctf0
Copy link
Author

ctf0 commented Feb 12, 2021

Can't you just provide simple steps like:

  • try this
let args  = encodeURIComponent(JSON.stringify([{cmnd: 'hello world'}]))

new vscode.DocumentLink(new vscode.Range(0, 0, 1, 0), vscode.Uri.parse(`command:extension.helloWorld?${args}`))
  • to fix the issue, wrap 'hello world' with encodeURI

@jrieken jrieken added the *not-reproducible Issue cannot be reproduced by VS Code Team member as described label Feb 12, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Mar 29, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
info-needed Issue requires more information from poster *not-reproducible Issue cannot be reproduced by VS Code Team member as described
Projects
None yet
Development

No branches or pull requests

4 participants
@jrieken @ctf0 @jeanp413 and others