Cleanup Javascript Doc Comment Generation #3086
Conversation
| @@ -219,50 +219,6 @@ namespace | |||
| return ostr.str(); | |||
| } | |||
|
|
|||
| void writeDocSummary(Output& out, const ContainedPtr& p, bool includeDeprecated = true) | |||
There was a problem hiding this comment.
merged with writeDocCommentFor. Both methods were doing pretty much the same.
cpp/src/slice2js/Gen.cpp
Outdated
| { | ||
| _out << nl << " * @deprecated"; | ||
| if (!dc->deprecated().empty()) | ||
| _out << nl << " * @deprecated "; |
There was a problem hiding this comment.
This emits a trailing space when there's no message to write.
We should wait to add the space until we're in one of the blocks.
cpp/src/slice2js/Gen.cpp
Outdated
| } | ||
| else if (auto deprecated = p->getDeprecationReason()) | ||
| { | ||
| _out << *deprecated << "."; |
There was a problem hiding this comment.
I don't think we have any 'official guidance' for how to write your deprecation message.
But if you look at our Slice files, we usually end ours in a period, so this would generate a second period.
I think it's best to just write the message as-is, and if the user needs a period, they can add it in their comment.
| } | ||
|
|
||
| if (includeDeprecated && dc && dc->isDeprecated()) | ||
| if (includeDeprecated && (comment->isDeprecated() || p->isDeprecated())) |
There was a problem hiding this comment.
I'd add a comment here explaining why we have this javascript specific deprecation logic
(because JS doesn't have a deprecated attribute to map to).
| lines.push_back(line); | ||
| } | ||
| // There's nothing to write for this doc-comment. | ||
| return; |
There was a problem hiding this comment.
There is a small issue here, definition marked as deprecated with metadata without a doc comment would be not marked as deprecated in JavaScript because of this early return.
Fixing in a follow up PR.
Fix #3069