Skip to content

Copyright comments are not preserved when generating d.ts files#5472

Merged
yuit merged 1 commit into
microsoft:masterfrom
MartyIX:issue-5183
Nov 2, 2015
Merged

Copyright comments are not preserved when generating d.ts files#5472
yuit merged 1 commit into
microsoft:masterfrom
MartyIX:issue-5183

Conversation

@MartyIX
Copy link
Copy Markdown
Contributor

@MartyIX MartyIX commented Oct 30, 2015

This is a proposed fix for #5183 ... WIP

Comment thread src/compiler/utilities.ts Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There is already an isPinnedComments elsewhere. Also, can you rename it to isPinnedComment?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

isPinnedComments function was in emitter.ts and I put it inside emitDetachedComments because it is not used anywhere else.

Yes, I can rename it.

@mhegazy
Copy link
Copy Markdown
Contributor

mhegazy commented Oct 30, 2015

unit tests?

Comment thread src/compiler/utilities.ts Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think it will be better to not pass in the detachedCommentsInfo array into the function and return currentDetachedCommentInfo and then have a function in emitter call emitDetachedCommentsAndUpdateCommentsInfo that calls into this function and do the following logic

if (detachedCommentsInfo) {
   detachedCommentsInfo.push(currentDetachedCommentInfo);
}
else {
  detachedCommentsInfo = [currentDetachedCommentInfo];
}

So that in the case of using the function in declarationEmitter.ts, we will not be doing throw away work.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

also please add return type. And return type and parameter type into JSDoc

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

also please add return type. And return type and parameter type into JSDoc

I'm not sure how to do that becase I see no other @return or @param in utilities.ts. Can you elaborate?

@MartyIX
Copy link
Copy Markdown
Contributor Author

MartyIX commented Oct 31, 2015

@mhegazy Yes, I added some.

@yuit
Copy link
Copy Markdown
Contributor

yuit commented Nov 2, 2015

Thanks for fast iteration. Can you add one more case with similar content but with // @removeComments: true? To make sure that we will preserve copy-right comment but nothing else in declaration emit.

@MartyIX
Copy link
Copy Markdown
Contributor Author

MartyIX commented Nov 2, 2015

@yuit Sure

@yuit
Copy link
Copy Markdown
Contributor

yuit commented Nov 2, 2015

lgtm. @MartyIX thanks for the PR 🍪

yuit added a commit that referenced this pull request Nov 2, 2015
Fix copyright comments are not preserved when generating d.ts files
@yuit yuit merged commit f503169 into microsoft:master Nov 2, 2015
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants