allow linking via workspace://<project-name>/path/to/file and project://path/to/file #98238
Conversation
|
|
…ide a text project://./example.ts -> ./example.ts
…en provided, otherwise display text
Fix/relative linking
…ode into feature/workspace-links
|
Thank you for your interest in this area! I discussed this PR with the TypeScript team and we have a concerns about introducing new url protocols such as With microsoft/TypeScript#35524, we are trying to find a solution that would work well across many different editors (or at least all editors that can use the TypeScript language service). I think we should wait to see how that work progresses first. The issue is surprisingly complex, so it may take some time. You can help out by contributing over on the TypeScript side (microsoft/TypeScript#35524) both by bringing the problems you want to see fixed and potentially helping out with code changes. Sorry that I have to say no here. The code changes are good and your detailed writeup on the problem was really great. Thank you |
|
@mjbvz while i understand, i would note that this works with link detection and all vscode features as I implemented all that in this (https://github.com/microsoft/vscode/pull/98238/files#diff-a3f68846509f98f95b8ec00503c1cf9c). It works independent of typescript LSP it ends up being a vscode feature rather than a typescript one by simply parsing the links directly via vscode internals. This would work for non-typescript languages as well for the link detection pieces on the top level if you try it out. The only part that is typescript only is the handling of {@link url|name} -- and in addition this fixes multiple bugs with the current implementation such as relative linking not working and cmd+clicking broken when doing so on a {@link url|name} |
This PR fixes a significant number of issues with the current linking functionality that exist. It fixes issues with relative linking in hovers/previews (or doesn't render them as links if it can't properly do so based on context), adds a project:// option to resolve links to the project base path, and a workspace:// for workspace linking.
This also fixes a decent amount of other bugs here or there, largely of which should be listed below.
I tried to follow code styling as much as possible and be as careful as I could not to go against any conventions. Would love to see this merged though as I think it's a beneficial feature for everyone to have!
What It Does
This PR aims to resolve the following:
Global Linking
workspace://<project-name>/path/to/fileproject://path/to/fileproject://./relative/path/to/fileproject://../relative/path/to/file[link name](scheme://path)(intercept and rewrite if necessary)Typescript
{@link ...}Linking{@link <any of the above>}{@link <any of the above>|Link Name}{@link <any of the above> Link Name}Typescript Preview Linking
Global Linkingas well as:file:///path/to/filefile://./relative/path/to/filefile://../relative/path/to/fileExample
Use Cases
While I feel the use cases are fairly apparent, I figured I would list some actual use cases where this would be useful within a project, especially with shared teams to provide powerful code navigation that can occur from anywhere that uses a given type value.
Tests
Happy to add all the test cases if this is something that would be merged. Figured I would submit and see what y'all think before finishing up. This PR is complete other than those tests which shouldn't take long.
Extra Notes
cmd + clicka link that is defined with a name ({@link <url>|Name}) it is not working due to the fact the link name is not separated from the url. For the above references this is resolved but it will still not work for file:// and https link types.workspace://seemed a bit odd since it isn't really shareable across systems easily at this time (although that would be nice to have -- sharing a workspace with the team and having everyone setup with all the projects and linking, etc). I really only needproject://for my own needs but figured I'd add it.Remaining Tasks
definitioncommand to resolve relative linking from the defined file rather than the open file. Will look into reason for this shortly but believe I know why.Potential Improvements & Considerations
file://../../blah.ts{@link link text after space}as jsdoc specifiesfile.ts#3,1)pathsproperty would be resolved as well. I wasn't sure how to accomplish this but think it would be worthwhile for some people.workspace://links have a strike-through and have a tooltip indicating the workspace was not found.module://orpackage://to allow resolving relative to the current package (closest package.json)Potentially provide support in [linkedText]Refactorwould potentially make relative links become incorrect. Wouldn't be terrible to have these update automatically but probably out of scope and not important(?)workspace://andproject://was to use magic strings of some kind when usingfile://. Flow allows for using<PROJECT_ROOT>in various paths as example so could be changed to replace<PROJECT_ROOT>and<WORKSPACE_ROOT>/project-namewith their absolute paths.