Skip to content
This repository has been archived by the owner on Nov 5, 2021. It is now read-only.

Build tag text correctly for all tags #81

Merged
merged 2 commits into from Jun 11, 2021

Conversation

spahnke
Copy link
Contributor

@spahnke spahnke commented Jun 11, 2021

Fixes microsoft/monaco-editor#2523

Is this the correct way for all tag types? The typings suggest that tag.text is now always an array.

/cc @orta

@@ -576,7 +576,7 @@ function tagToString(tag: ts.JSDocTagInfo): string {
tagLabel += `\`${paramName.text}\``;
if (rest.length > 0) tagLabel += ` — ${rest.map(r => r.text).join(' ')}`;
} else if (tag.text) {
tagLabel += ` — ${tag.text}`;
tagLabel += ` — ${tag.text.map(r => r.text).join(' ')}`;
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this is determined by a user option: UserPreferences.displayPartsForJSDoc
So I think this is close, but it could be either a string or the array and so we probably want to handle that as a separate if?


    export interface JSDocTagInfo {
        /** Name of the JSDoc tag */
        name: string;
        /**
         * Comment text after the JSDoc tag -- the text after the tag name until the next tag or end of comment
         * Display parts when UserPreferences.displayPartsForJSDoc is true, flattened to string otherwise.
         */
        text?: string | SymbolDisplayPart[];
    }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I pushed a change to accomodate for that. Where does the discrepancy in typescriptServices.d.ts come from though? I don't see the string option there

interface JSDocTagInfo {
        name: string;
        text?: SymbolDisplayPart[];
    }

Copy link
Contributor

Choose a reason for hiding this comment

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

IMO, this is likely a bug in the compiler codebase - there are a few types which end up repeated in services vs the compiler. For this example, typescriptServices.d.ts has both:

// 6123
    interface JSDocTagInfo {
        name: string;
        text?: SymbolDisplayPart[];
    }

// 7433
    interface JSDocTagInfo {
        /** Name of the JSDoc tag */
        name: string;
        /**
         * Comment text after the JSDoc tag -- the text after the tag name until the next tag or end of comment
         * Display parts when UserPreferences.displayPartsForJSDoc is true, flattened to string otherwise.
         */
        text?: string | SymbolDisplayPart[];
    }

It compiles fine because of interface merging - I'll make a PR which makes them consistent to the compiler and see what someone closer to the codebase thinks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, that makes sense. Thank you for the fast review on this PR!

@alexdima alexdima added this to the June 2021 milestone Jun 11, 2021
@alexdima
Copy link
Member

Thank you!

@alexdima alexdima merged commit dda62a5 into microsoft:main Jun 11, 2021
@spahnke spahnke deleted the deprecated-hover-fix branch June 14, 2021 07:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants