Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

keep braces and brackets in tags comment. #1001

Closed
wants to merge 5 commits into from

Conversation

capraynor
Copy link
Contributor

Please see the comments in code.
I just saw some bad code in 2015.
These two lines of code removes braces and square-brackets in tags comment.

This metod only used in parsing tag's comment, so we think tha these two lines of code should be removed.

Another question is, could you please create a hot-fix for us? we eagerly need this thing to be fixed.

Looking forward for your reply
Thousand Thanks!

@Gerrit0
Copy link
Collaborator

Gerrit0 commented May 7, 2019

The replacement is done to remove types inside the braces for projects migrating from pure JS and JSDoc. They should be removed in a comment like this:

/**
 * @param {string} name - the name
 */
function func(name: string) {}

I recognize your issue is still a problem, but this shouldn't be how it is addressed. I think changing the first regular expression to ignore braces with tags inside should be the solution.

As an aside, I have no clue why the second line there exists. It removes pairs of single brackets, but not double brackets (I'm guessing because they are used for links?), but doesn't remove them if there is nothing between them ([]). There might be some behavior that relies on this, but I can't find it if there is, and it seems like unexpected behavior to me.


Also, while the description of why you removed the lines is helpful, that should go in the PR, not in the source code. If there were a dozen lines of comments describing how each line was changed the code would quickly become unreadable. Someone who wonders what changed can use git blame to see when the code was last changed and find the discussion in the PR.

@capraynor
Copy link
Contributor Author

Hi, @Gerrit0
Thanks for your explanation, and sorry for my late response.

The replacement is done to remove types inside the braces for projects migrating from pure JS and JSDoc.

Are there any requirements on "remove the JSDoc annotatios in tags"? IMHO, the tool should obey user's content. Maybe our tool did too much to our user.

The "Square Brackets" line.

The square brackets in JSDoc is "Optional Params"

The only thing I found is this page, please search "optional" on this page.

Write the Comments in PR, but not in code

Very sorry for that. This is my work style, and I don't know GitHub users discuss in PR but don't provide information in code.

What's your Opinion on the first two issues?

Thanks for your TypeDoc!

@aciccarello
Copy link
Collaborator

@capraynor Thanks for being willing to discuss this.

Are there any requirements on "remove the JSDoc annotatios in tags"? IMHO, the tool should obey user's content. Maybe our tool did too much to our user.

I would say yes because we want to separate the JSDoc annotations from the string descriptions. We already show types (from TypeScript) so we don't want the type to appear again from the JSDoc comment. This would result in output similar to the below.

Screen Shot 2019-05-15 at 9 22 58 AM

You do raise a valid question of whether TypeDoc is doing too much. Looking at the page you liked to, there is a lot of variation in JSDoc syntax which we don't account for (including square brackets for optional params). A few things come to mind:

  1. We should investigate whether we can reasonably support TS in JS users
  2. Can we limit the bracket removal to the first word after @param
  3. Does TS do any comment parsing for us that we aren't yet taking advantage of?

@Gerrit0
Copy link
Collaborator

Gerrit0 commented May 15, 2019

TypeScript does parse JSDoc, but the result is stored on the internal jsDocComments property. https://sandersn.github.io/manual/Typescript-compiler-implementation.html#jsdoc

Once we eventually start using @microsoft/tsdoc, this will be automatically solved, but that will probably be a while, so it would be nice to have a fix now.

@sergeyt
Copy link

sergeyt commented Aug 21, 2019

@capraynor consider just improving these regexps to keep TypeDoc annotations and remove JSDoc annotations, so everyone wins

@Gerrit0 Gerrit0 closed this in eefdb2c Feb 16, 2020
@Gerrit0
Copy link
Collaborator

Gerrit0 commented Feb 16, 2020

I pulled in this fix with a couple changes in eefdb2c, released in 0.16.10. The {@link} example will no longer be stripped :)

Sorry this took so long!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants