allow linking via workspace://<project-name>/path/to/file and project://path/to/file

[bradennapier]

Also noticed Typescript 4.0 is looking at some improvements around this as well which can build on this PR when ready microsoft/TypeScript#35524

Note that the integration test failure seems unrelated to this PR as it was passing previously and no changes to the reason were made / other tests failing for other PR as well

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/file
• project://path/to/file
• project://./relative/path/to/file
• project://../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

• supports any from Global Linking as well as:
• file:///path/to/file
• file://./relative/path/to/file
• file://../relative/path/to/file

Example

/**
 * - file:///Users/user/file.ts <- Absolute FileSystem
 * - file://./index.ts <- Relative File
 * - project://./index.ts <- Relative File
 * - [Relative to index via file://./index.ts]( file://./index.ts ) <- Relative File
 * - [Relative to index via project://./index.ts](project://./index.ts)
 * - [Absolute to project file via project://src/index.ts](project://src/index.ts)
 * - {@link file://./index.ts Relative to index via @link and file scheme}
 * - {@link workspace://my-project-invalid/index.ts|Link to missing workspace folder}
 * - {@link workspace://my-project/index.ts Link to valid workspace folder}
 * - {@link file://./index.ts#3,2 Link to index.ts line 3, column 2}
 * - {@linkcode https://www.google.com Google Link as linkcode}
 * - https://www.google.com
 * - [Link To Google](https://www.google.com)
 */
export const links = {} 

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.

import * as types from './types';

export type ButtonProps = {
    /**
     * A type preset from the exported css fragments from {@link project://src/lib/css/types.ts|types.ts}
     * which will be merged into the button style.
     * 
     * @example
     *  <Button type="primary">My Button</Button>
     *  <Button type="success">My Button</Button>
     */
    type: keyof typeof types
}

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

• I noticed that with the current implementation of @link, when you cmd + click a 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.
• Supporting 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 need project:// for my own needs but figured I'd add it.

Remaining Tasks

• Just realized there is one window that is not properly using the definition command 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.
  • @see Fix/relative linking bradennapier/vscode#1
• Writes tests should proposal be accepted

Potential Improvements & Considerations

• Provide support for relative back links, upstream currently can not handle file://../../blah.ts
• Provide support for {@link link text after space} as jsdoc specifies
• Allow the use of fragments for top-level and preview links (currently broken in vscode upstream) (allowing linking to specific line/column via file.ts#3,1)
• Intercept and rewrite markdown links if necessary (currently markdown links will not work in preview hover if they use file:// relative paths or the new protocols). 6eb7d58
• Signature helper was not given the definition call as I could find no case that typescript returns the necessary definition details to allow us to render relative links. Absolute/project/https and such will still render like normal.
• Parse project and workspace paths using the appropriate tsconfig so that the paths property would be resolved as well. I wasn't sure how to accomplish this but think it would be worthwhile for some people.
• Fix bug in current implementation of @link to bring the fixes that allow cmd + click of named links using project:// and workspace:// to work for file:// and https:// cases (see Extra Notes)
• Check if the path exists and only render as link if it does. I wasn't sure if this would be too large of a perf hit and didn't want to overcomplicate the PR, but happy to add if we think it is worth it.
  • This is being done for invalid workspace links since we can detect if a workspace is incorrect without any extra filesystem calls. Invalid workspace:// links have a strike-through and have a tooltip indicating the workspace was not found.
• file:// relative linking is not working properly (see extra notes), happy to resolve this as well
• Allow linking with fragments to symbol names. I wasn't sure how to accomplish this but happy to dig deeper if deemed worthwhile.
  • Looks like Typescript considering for 4.0 roadmap adding helpers for this that would simplify this Editor support for @see and {@link} in JSDoc comments tags TypeScript#35524
• Allow for module:// or package:// to allow resolving relative to the current package (closest package.json)
• Potentially provide support in [linkedText]
• Refactor would potentially make relative links become incorrect. Wouldn't be terrible to have these update automatically but probably out of scope and not important(?)
• Another option I had considered instead of workspace:// and project:// was to use magic strings of some kind when using file://. Flow allows for using <PROJECT_ROOT> in various paths as example so could be changed to replace <PROJECT_ROOT> and <WORKSPACE_ROOT>/project-name with their absolute paths.
  • The reason I went against this was largely around the display of the url in the preview pain, the magic path ends up making the docs look less than ideal.

[msftclas]

All CLA requirements met.

[mjbvz]

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 project and workspace. Such urls would only be supported by VS Code and would not work in many of VS Code's other features, such as link detection.

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

[bradennapier]

@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}