GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Editor support for link tag by sandersn · Pull Request #41877 · microsoft/TypeScript

Conversation

@sandersn

@sandersn Nathan Shively-Sanders (sandersn) commented Dec 8, 2020 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Fixes #35524 (80% solution)

This PR makes the parser parse @link tag and the language service offer symbols for entity names found at the beginning of those tags. Links are stored redundantly with comment text, so they're only used to provide (1) a span to an editor to format in some way (2) a symbol for the declaration of the link's target.

Future PRs

  1. Parse # as type-space element access: Class#method. As well as, possibly, the normal TS syntax Class["method"], which doesn't currently parse either.

Open questions

  1. I parse @see {@link ...} as a @see with no reference and a link in its comment text. I think that's OK.
  2. I intentionally do not parse { @link X}, with a space separating { and @. I saw this only twice in my corpus, but Andrew Casey (@amcasey) points out that it's easy to support with an additional skipWhitespace call. I originally skipped it because I had slightly less lookahead when the next token after { was @, but that made the code unreadable, so I dropped it.

Resolved questions

  1. I introduced JSDocText and JSDocLink nodes into the AST, which better reflects the semantic relation of comments to their tags. It is more expensive, though, it means that jsdoc comment text is treated as a first-class member of the parse tree now.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

jerone, takaya1992, Kingwl, viceice, xmedeko, briandipalma, jasonkuhrt, Robbie-Cook, eps1lon, and btoo reacted with thumbs up emoji Kingwl, Robbie-Cook, and btoo reacted with laugh emoji amaechler, Kingwl, eternalphane, Robbie-Cook, cherryblossom000, and btoo reacted with hooray emoji hasparus, Kingwl, Robbie-Cook, and btoo reacted with heart emoji arthropodSeven, yGuy, Kingwl, Robbie-Cook, and btoo reacted with rocket emoji Kingwl reacted with eyes emoji
All reactions
  • 👍 10 reactions
  • 😄 3 reactions
  • 🎉 6 reactions
  • ❤️ 4 reactions
  • 🚀 5 reactions
  • 👀 1 reaction
Nathan Shively-Sanders (sandersn) added 18 commits September 10, 2020 14:19
@sandersn
Initial scribbles
72f19ef
@sandersn
Compiles but provides spans instead of location pairs
c08258b
Probably need to fork the services/server types and provide a conversion
with Session.toFileSpan. Not sure where to put the conversion.
@sandersn
Switch to DocumentSpan
2f1d113
In theory this is already better supported, but not sure practise bears
that out.
@sandersn
Merge branch 'master' into editor-support-for-links
256a60a
@sandersn
Builds w/protocol types + conversions
8ba5dc8
@sandersn
cleanup:better names and scrub TODOs
ca1b273
@sandersn
fix test harness too
867ab34
@sandersn
Misc
5d518d6
1. Simplify protocol after talking to @mjbvz.
2. Add more tests.
3. Initial notes about where to add parsing.
@sandersn
Parse and store links in the compiler
c07fe40
The text of the link is still stored in the comment text, but that's now
kept in an object instead of just a string. Each link has the parse for
the entity reference, if there is one.

Needs lots more tests -- this just makes all the existing jsdoc tests
pass.
@sandersn
Merge branch 'master' into editor-support-for-links
82c11fe
@sandersn
more tests and some fixes
90feb3e
@sandersn
Merge branch 'master' into editor-support-for-links
035bb4a
@sandersn
Fix other failing tests
fc813c2
@sandersn
Merge branch 'master' into editor-support-for-links
917314c
@sandersn
fix bad merge
7dd7cab
@sandersn
polish parser
c0e8d47
@sandersn
improve names and array types
8bbc8b4
@sandersn
slight tweaks
070da0b
@typescript-bot

TypeScript Bot (typescript-bot) commented Dec 8, 2020

Copy link
Copy Markdown
Contributor

Thanks for the PR! It looks like you've changed the TSServer protocol in some way. Please ensure that any changes here don't break consumers of the current TSServer API. For some extra review, we'll ping Sheetal Nandi (@sheetalkamat), Andrew Casey (@amcasey), Matt Bierner (@mjbvz), Mine Starks (@minestarks) for you. Feel free to loop in other consumers/maintainers if necessary

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@typescript-bot TypeScript Bot (typescript-bot) assigned Nathan Shively-Sanders (sandersn) Dec 8, 2020
@typescript-bot TypeScript Bot (typescript-bot) added the Author: Team label Dec 8, 2020
@sandersn
Nathan Shively-Sanders (sandersn) requested a review from Sheetal Nandi (sheetalkamat) December 8, 2020 17:12
@typescript-bot TypeScript Bot (typescript-bot) added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Dec 8, 2020
@sandersn
Nathan Shively-Sanders (sandersn) requested a review from Andrew Branch (andrewbranch) December 8, 2020 17:13
@sandersn

Nathan Shively-Sanders (sandersn) commented Dec 8, 2020 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

Matt Bierner (@mjbvz) and Andrew Casey (@amcasey), you should maybe also look at this (at least the protocol part)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sandersn
remove some done TODOs
fce33f1
@andrewbranch

Andrew Branch (andrewbranch) commented Dec 8, 2020

Copy link
Copy Markdown
Member

TypeScript Bot (@typescript-bot) pack this

Only thing I’m curious about is if there are some places where JSDoc can go, but otherwise entity names could not go, and therefore might expose a hole in getSymbolAtLocation.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@typescript-bot

TypeScript Bot (typescript-bot) commented Dec 8, 2020 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Heya Andrew Branch (@andrewbranch), I've started to run the tarball bundle task on this PR at fce33f1. You can monitor the build here.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sandersn

Nathan Shively-Sanders (sandersn) commented Dec 8, 2020

Copy link
Copy Markdown
Member Author

Andrew Branch (@andrewbranch) I indeed tried writing more tests and discovered a hole. I'll put up a commit that makes getSymbolAtLocation work with identifiers in links.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sandersn
more tests + resulting fixes
7ac40d4
@sandersn

Nathan Shively-Sanders (sandersn) commented Mar 5, 2021

Copy link
Copy Markdown
Member Author

The most recent commit avoids breaking VS, and I'm working on tests for all 4 possible combinations of the rich/full matrix.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Nathan Shively-Sanders (sandersn) added 5 commits March 8, 2021 11:56
@sandersn
Real server tests
52d64db
@sandersn
Disambiguate {@link} and @PARAM x {type}
5a21851
They are ambiguous; previously the parser preferred the type
interpretation, but will now look ahead and parse links instead when the
prefix is `{@link`.
@sandersn
Add explanatory comment in test
ab340f4
@sandersn
Merge branch 'master' into editor-support-for-links
3e5aa10
@sandersn
Merge branch 'master' into editor-support-for-links
618bcec
@sandersn

Nathan Shively-Sanders (sandersn) commented Mar 10, 2021

Copy link
Copy Markdown
Member Author

This is ready for review now.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

jessetrinity
Jesse Trinity (jessetrinity) reviewed Mar 10, 2021
View reviewed changes
Comment thread src/server/protocol.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

Nathan Shively-Sanders (sandersn) added 2 commits March 10, 2021 12:10
@sandersn
fix location in richResponse in protocol
5a29c2e
@sandersn
update API baseline
67c4966
amcasey
Andrew Casey (amcasey) approved these changes Mar 15, 2021
View reviewed changes

@amcasey Andrew Casey (amcasey) left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

The behavior and interface changes generally LGTM. Some suggestions below.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread src/compiler/types.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread src/server/protocol.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread src/server/protocol.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread src/server/protocol.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread src/server/protocol.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread src/services/jsDoc.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread src/services/codefixes/inferFromUsage.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread tests/cases/fourslash/gotoDefinitionLinkTag1.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

Nathan Shively-Sanders (sandersn) added 3 commits March 16, 2021 10:17
@sandersn
Merge branch 'master' into editor-support-for-links
2da204a
@sandersn
Address PR comments
d0f3048
1. Add a cross-file goto-def test.
2. Switch from per-message args to UserPreference.
@sandersn
use arraysEqual from core
4cfdaee
@sandersn

Nathan Shively-Sanders (sandersn) commented Mar 16, 2021

Copy link
Copy Markdown
Member Author

I think this PR has had enough review to be ready to use. I'll merge it and address any follow-up in subsequent PRs.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sandersn
Nathan Shively-Sanders (sandersn) merged commit ec77bff into master Mar 16, 2021
@sandersn
Nathan Shively-Sanders (sandersn) deleted the editor-support-for-links branch March 16, 2021 23:26
@Kingwl

Wenlu Wang (Kingwl) commented Mar 17, 2021

Copy link
Copy Markdown
Contributor

Cool!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Nathan Shively-Sanders (sandersn) added a commit that referenced this pull request Mar 18, 2021
@sandersn
Revert "Editor support for link tag (#41877)"
271c50c
This reverts commit ec77bff.
Nathan Shively-Sanders (sandersn) added a commit that referenced this pull request Mar 18, 2021
@sandersn
Revert "Editor support for link tag (#41877)" (#43302)
451d435
This reverts commit ec77bff.
Nathan Shively-Sanders (sandersn) added a commit that referenced this pull request Mar 19, 2021
@sandersn
Revert "Revert "Editor support for link tag (#41877)" (#43302)"
fb892f0
This reverts commit 451d435.
@sandersn Nathan Shively-Sanders (sandersn) mentioned this pull request Mar 19, 2021
@link support, second try #43312
Merged
Nathan Shively-Sanders (sandersn) added a commit that referenced this pull request Mar 22, 2021
@sandersn
@link support, second try (#43312)
dcc27eb
* Revert "Revert "Editor support for link tag (#41877)" (#43302)"

This reverts commit 451d435.

* Fix parsing @link at end of comment

* Parse comments as string when no @link occurs

* fix lint
@tmair tmair mentioned this pull request May 10, 2021
docs: fix broken links in documentation ReactiveX/rxjs#6380
Merged
@orta Orta Therox (orta) mentioned this pull request Jun 10, 2021
JSDoc @link not resolved across modules #43869
Closed
@brendankenny Brendan Kenny (brendankenny) mentioned this pull request Aug 12, 2021
deps: update to [email protected] GoogleChrome/lighthouse#12909
Merged
@caspardue Caspar Due (caspardue) mentioned this pull request Aug 29, 2021
jsdoc/tsdoc not parsed correctly gund/eslint-plugin-deprecation#29
Closed
@millsp

pierre (millsp) commented Sep 30, 2021

Copy link
Copy Markdown
Contributor

Docs can be updated now https://www.typescriptlang.org/docs/handbook/jsdoc-supported-types.html

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@akudev Andreas Kunz (akudev) mentioned this pull request Dec 29, 2021
@sapui5/ts-types-esm - broken links in docs UI5/typescript#303
Open
@microsoft Microsoft (microsoft) locked as resolved and limited conversation to collaborators Oct 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@DanielRosenwasser Daniel Rosenwasser (DanielRosenwasser) DanielRosenwasser left review comments
@andrewbranch Andrew Branch (andrewbranch) andrewbranch approved these changes
@sheetalkamat Sheetal Nandi (sheetalkamat) Awaiting requested review from sheetalkamat
+3 more reviewers
@amcasey Andrew Casey (amcasey) amcasey approved these changes
@minestarks Mine Starks (minestarks) minestarks left review comments
@jessetrinity Jesse Trinity (jessetrinity) jessetrinity left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@sandersn Nathan Shively-Sanders (sandersn)

Labels

Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug

Projects

PR Backlog
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Editor support for @see and {@link} in JSDoc comments tags

Uh oh!

There was an error while loading. Please reload this page.

10 participants

@sandersn @typescript-bot @andrewbranch @mjbvz @minestarks @Kingwl @millsp @DanielRosenwasser @amcasey @jessetrinity
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.