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 — Add see tag support by Kingwl · Pull Request #39760 · microsoft/TypeScript

Conversation

@Kingwl

@Kingwl Wenlu Wang (Kingwl) commented Jul 27, 2020 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

Fixes a part of #35524

seetag

Sorry, something went wrong.

Uh oh!

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

MuTsunTsai, wilf312, jasonwilliams, Zzzen, vivilor, amyXia1994, and binury reacted with thumbs up emoji jerone, ValfarDeveloper, daniel-shuy, danilofuchs, btoo, jasonwilliams, lsfgrd, tfoxy, vivilor, matthewjosephtaylor, and dmrazzy reacted with hooray emoji zachhardesty7, Quramy, xmedeko, doggy8088, wsmd, sidvishnoi, and vivilor reacted with heart emoji
All reactions
  • 👍 7 reactions
  • 🎉 11 reactions
  • ❤️ 7 reactions
@Kingwl
Add see tag parser
4394731
@Kingwl
Wenlu Wang (Kingwl) marked this pull request as draft July 27, 2020 07:50
@typescript-bot TypeScript Bot (typescript-bot) added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Jul 27, 2020
Wenlu Wang (Kingwl) added 2 commits July 27, 2020 16:04
@Kingwl
add baseline
c89574c
@Kingwl
fix symbol resolve
5b49a14
@Kingwl Wenlu Wang (Kingwl) changed the title Add see tag parser Add see tag support Jul 27, 2020
@Kingwl

Wenlu Wang (Kingwl) commented Jul 27, 2020

Copy link
Copy Markdown
Contributor Author

Friendly ping Nathan Shively-Sanders (@sandersn) .

I'm not sure what is the expected of unittests:: JSDocParsing

All reactions

Sorry, something went wrong.

Uh oh!

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

@Kingwl
add more case
d916393
@Kingwl
Wenlu Wang (Kingwl) marked this pull request as ready for review July 27, 2020 14:31
@sandersn

Nathan Shively-Sanders (sandersn) commented Jul 27, 2020

Copy link
Copy Markdown
Member

Couple of quick comments -- I've only glanced at the code, haven't really read it yet.

  1. The jsDocParsing unittests are pretty basic. I think the existing test for @see there is sufficient.
  2. The tests I'd like to see are goto-def tests, obviously.
  3. My understanding of jsdoc is that braces are not required here since these aren't type references, but we might as well parse them because people may still use them ... and they aren't ambiguous, so why not?
All reactions

Sorry, something went wrong.

Uh oh!

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

Wenlu Wang (Kingwl) added 3 commits July 28, 2020 11:38
@Kingwl
fix unittests
2611909
@Kingwl
improve tests and parser
3d47343
@Kingwl
accept baseline
bcef14f
@Kingwl

Wenlu Wang (Kingwl) commented Jul 28, 2020

Copy link
Copy Markdown
Contributor Author

Hoops. Seems not my fault.

All reactions

Sorry, something went wrong.

Uh oh!

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

@Kingwl

Wenlu Wang (Kingwl) commented Aug 3, 2020

Copy link
Copy Markdown
Contributor Author

UP ↑

All reactions

Sorry, something went wrong.

Uh oh!

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

Andrew Casey (amcasey) and others added 15 commits August 24, 2020 18:39
@amcasey @Kingwl
Adopt package-lock.json and npm ci
5442afd
@amcasey @Kingwl
Add a workflow to update package-lock.json daily
5521ba5
@amcasey @Kingwl
Git ignore package-lock.json and forcibly update in workflow
95462f5
@amcasey @Kingwl
Update bot email address
7c667cb
@amcasey @Kingwl
Delete extra npm update
fd29771
@typescript-bot @Kingwl
Update package-lock.json
37700f5
@wyze @Kingwl
Add compactDisplay and signDisplay to NumberFormatOptions (microsoft#…
ef08789
…40039)
@cherryblossom000 @Kingwl
Fix typo in (Readonly)Set.keys comment (fixes microsoft#40164) (micro…
6a75377
…soft#40176)
@a-tarasyuk @Kingwl
fix(26325): use a unique name for reserved words in 'constructor like…
85501e6
…' function name (microsoft#39684)
@a-tarasyuk @Kingwl
fix(25770): add diagnostic message for the possible mapped type used …
b4393fe
…as an index (microsoft#39973)
@a-tarasyuk @Kingwl
fix(31046): add new diagnostic message for incompatible constructor s…
e6dabbe
…ignature (microsoft#40073)
@typescript-bot @Kingwl
Update package-lock.json
d0ae77e
@typescript-bot @Kingwl
Update package-lock.json
d187c63
@Kingwl
Merge branch 'master' of github.com:Kingwl/TypeScript
acbcd61
@Kingwl
Merge branch 'master' of github.com:microsoft/TypeScript
e46ce36
Erich Gamma (egamma) and others added 5 commits September 1, 2020 20:51
@egamma
Revert "fix anders"
8cac241
This reverts commit b3178d4.
@andrewbranch
Fix call hierarchy item serialization and server tests (microsoft#40348)
3b502f4
@Kingwl
Merge branch 'master' into see-tag
eb9ad2a
@Kingwl
Avoid error
6d90fb3
@Kingwl
accept baseline
51b67d7
@Kingwl

Wenlu Wang (Kingwl) commented Sep 2, 2020

Copy link
Copy Markdown
Contributor Author

Nathan Shively-Sanders (@sandersn) Is that a valid syntax?... I'm not sure what are you expected...

TypeScript Bot (@typescript-bot) pack this.

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 Sep 2, 2020 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

Heya Wenlu Wang (@Kingwl), I've started to run the tarball bundle task on this PR at 51b67d7. You can monitor the build here.

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 Sep 2, 2020 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

Hey Wenlu Wang (@Kingwl), I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/83979/artifacts?artifactName=tgz&fileId=9A8F21D752ED3524E8C7785D7AE26622ABF7CCFFE3F5BCDE9FA311FFB577030F02&fileName=/typescript-4.1.0-insiders.20200902.tgz"
    }
}

and then running npm install.


There is also a playground for this build.

All reactions

Sorry, something went wrong.

Uh oh!

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

@weswigham
Wesley Wigham (weswigham) force-pushed the master branch from 3b502f4 to 838a5e6 Compare September 2, 2020 18:47
@sandersn

Nathan Shively-Sanders (sandersn) commented Sep 3, 2020

Copy link
Copy Markdown
Member

Based on my comment on the original bug, here are some more test cases that need to be added. They don't have to pass, but I want to make sure they don't crash:

  1. @see Class#method -- this should, ideally, resolve to Class at least.
  2. @see {@link M.f} -- doesn't need to work yet.
  3. @see M.f() -- should, ideally, resolve to M.f and ignore the trailing ().

I'm going to do a bit more looking to see how well we'll be able to resolve module references in things like lodash. I think a lot of uses just make up a name for the module they are inside, even though it's never declared inside the module itself.

All reactions

Sorry, something went wrong.

Uh oh!

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

@Kingwl
Add more tests
2c6f1bd
@sandersn

Nathan Shively-Sanders (sandersn) commented Sep 9, 2020

Copy link
Copy Markdown
Member

The only question I have left before merging this is how to resolve x here:

/** @param x @see x */
function f(x) { }

Right now, @see x doesn't resolve in the scope of f.

But I don't actually think people write this. I'm going to figure out if that's the case.

Either way, I think the best thing is to resolve starting from the parameter list of a function declaration.

All reactions

Sorry, something went wrong.

Uh oh!

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

@Kingwl

Wenlu Wang (Kingwl) commented Sep 9, 2020

Copy link
Copy Markdown
Contributor Author

Maybe we could peek the parameters first if jsdoc attached on a function like declaration.
But that will point to two different symbol. Is that ok?

All reactions

Sorry, something went wrong.

Uh oh!

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

@sandersn

Nathan Shively-Sanders (sandersn) commented Sep 9, 2020

Copy link
Copy Markdown
Member

If we resolveName starting from the parameter list, I think you can get it to resolve to just the parameter. So @see on a function should start from the parameter list probably. (or it may be the function itself, although I'd rather not resolve to locals of a function.)

All reactions

Sorry, something went wrong.

Uh oh!

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

sandersn
Nathan Shively-Sanders (sandersn) requested changes Sep 9, 2020
View reviewed changes

@sandersn Nathan Shively-Sanders (sandersn) 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.

My suggestion fixes the parameter name resolution, still need a test like

const x = 1
/** @see x */
function f(x) { }

To make sure that x resolves to the parameter.

Sorry, something went wrong.

Uh oh!

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

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

Uh oh!

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

Wenlu Wang (Kingwl) added 2 commits September 10, 2020 01:09
@Kingwl
Merge branch 'master' into see-tag
1effe1b
@Kingwl
Add signature name resolve
6451424
sandersn
Nathan Shively-Sanders (sandersn) approved these changes Sep 9, 2020
View reviewed changes
@sandersn
Nathan Shively-Sanders (sandersn) merged commit ee5f51b into microsoft:master Sep 9, 2020
This was referenced Sep 9, 2020
Editor support for @see and {@link} in JSDoc comments tags #35524
Closed
@see tag doesn't render the entity it refers to #40453
Closed
@Kingwl
Wenlu Wang (Kingwl) deleted the see-tag branch September 15, 2020 07:21
@postspectacular

Karsten Schmidt (postspectacular) commented Dec 2, 2020

Copy link
Copy Markdown

Could someone from the TS team please clarify if this current @see implementation is at all considering the new tsdoc syntax, and more generally, if there's any progress from the TS/VSCode team(s) in terms of better support for tsdoc, especially the {@link ...} syntax and its various link declaration variations? I can't find any information about if/when better tsdoc support is planned by the TS/VSC teams. The tsdoc website seems to indicate some interactions between the 3 MS teams, though... is that still ongoing?

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 4, 2020 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

Work on @link is ongoing at the editor-support-for-links branch. I'm just writing tests right now, so it shouldn't be long before I have a PR ready.
Both @link and @see initially parse and resolve entity names only (dotted names). Right now the branch can successfully parse an example like {@link A.B.C ...} where ... is any text you want to put there (it should work with a trailing |link text too -- but I need to add a test for that).

Can you add a deeper link to the tsdoc syntax you're talking about? I talked with Pete Gonzalez (@octogonz) and concluded that TS should attempt to support a basic subset of entity names. Ron Buckton (@rbuckton) pointed me to a grammar he made, but I'd have to look through my notes to find the link.

Edit: Here is what we're targetting in the near term: #35524 (comment)

postspectacular and jerone reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

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

@octogonz

Pete Gonzalez (octogonz) commented Dec 4, 2020

Copy link
Copy Markdown

The TSDoc spec for @see gives these examples which might be useful test cases:

/**
 * Parses a string containing a Uniform Resource Locator (URL).
 * @see {@link ParsedUrl} for the returned data structure
 * @see {@link https://tools.ietf.org/html/rfc1738|RFC 1738}
 * for syntax
 * @see your developer SDK for code samples
 * @param url - the string to be parsed
 * @returns the parsed result
 */
function parseURL(url: string): ParsedUrl;
All reactions

Sorry, something went wrong.

Uh oh!

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

@octogonz

Pete Gonzalez (octogonz) commented Dec 4, 2020 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown

and more generally, if there's any progress from the TS/VSCode team(s) in terms of better support for tsdoc,

TSDoc and the compiler seem to take somewhat different approaches today:

  • My impression is the compiler's priority is to correctly parse a subset of very common coding patterns, as determined by a forensic sample of real world code. This means its grammar is heuristic and relatively simplistic. It will necessarily misinterpret many TSDoc syntaxes. For example, you can't reliably parse `@see` or \@see or <tag title="@see" /> without full a Markdown-like engine.

  • By contrast, TSDoc's approach is prescriptive: Although the spec is incomplete today, the aim is to arrive at a rigorous spec. In strict mode, the parser actually issues error messages for malformed inputs (whereas CommonMark has no concept of an "incorrect" input, for example). It's also pretty good at handling comments that weren't authored for TSDoc, but that's a secondary aim.

As such TSDoc would be a somewhat heavyweight piece to incorporate into the compiler. (Not in terms of kilobytes, but in terms of complexity and upkeep.) And doing so might drag the compiler into the rabbit hole of enterprise documentation systems, which is a delightful but surprisingly complex space. This would benefit developers who write rich documentation and adopt TSDoc, but they are a minority. So the current situation is understandable, if not ideal.

An easy way out would be for the compiler to expose a plugin API enabling its comment parser to be replaced. This way, developers could opt-in to TSDoc, and there wouldn't need to be any close coordination between the two projects. This idea was brought up in my last chat with the TypeScript folks, and it seemed to align pretty well with their roadmap for extensibility. It seems like a feasible and pretty good solution.

postspectacular and jerone reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

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

@RomainMuller

Romain Marcadier (RomainMuller) commented Feb 1, 2021 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown

There's a weird thing going on right now where @see http://lmgtfy.com/ will actually result in symbol.getJsDocTags() to return an entry where the text is http ://lmgtfy.com (note the extra whitespace after http). This looks like a bug?

All reactions

Sorry, something went wrong.

Uh oh!

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

@z0gSh1u ZHUO Xu (z0gSh1u) mentioned this pull request Feb 2, 2021
Symbol#getJsDocTags adds whitespace in URLs #42581
Closed
@github-actions github-actions Bot mentioned this pull request Jul 14, 2021
Translate 1 file to ko - TypeScript 4.1.md microsoft/TypeScript-Website-Localizations#94
Closed
@microsoft Microsoft (microsoft) locked as resolved and limited conversation to collaborators Oct 16, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@sandersn Nathan Shively-Sanders (sandersn) sandersn approved these changes

Uh oh!

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

Assignees

No one assigned

Labels

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.

Uh oh!

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

12 participants

@Kingwl @sandersn @typescript-bot @postspectacular @octogonz @RomainMuller @amcasey @wyze @cherryblossom000 @a-tarasyuk @egamma @andrewbranch
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.