Fix Bugs in How 'slice2swift' Handled 'misc' Comment Components #3008
Conversation
| @@ -657,7 +657,7 @@ SwiftGenerator::writeDocSummary(IceInternal::Output& out, const ContainedPtr& p) | |||
|
|
|||
| if (!doc.misc.empty()) | |||
| { | |||
| out << "///" << nl; | |||
There was a problem hiding this comment.
This was just backwards, causing us to generate:
/// therefore proper security precautions should be taken.
/// and be registered in an object blah blah.///
/// @remarks something.
Now we generate:
/// therefore proper security precautions should be taken.
/// and be registered in an object blah blah.
///
/// @remarks something.
| out << nl; | ||
| writeDocLines(out, doc.misc, false); |
There was a problem hiding this comment.
This is the part that fixes #2088.
The false parameter here meant: "don't put a triple slash in front of this line`.
So this code would generate:
/// - parameter current: `Ice.Current` - The Current object for the dispatch.
@remarks This is a test!
With no leading triple slash, which rightfully angered the compiler.
Now, we generate:
/// - parameter current: `Ice.Current` - The Current object for the dispatch.
///
/// @remarks This is a test!
| @@ -838,7 +838,8 @@ SwiftGenerator::writeProxyDocSummary(IceInternal::Output& out, const InterfaceDe | |||
|
|
|||
| if (!doc.misc.empty()) | |||
| { | |||
| writeDocLines(out, doc.misc, false); | |||
There was a problem hiding this comment.
This was arguably worse than #2088. It caused us to just slap the @remarks on whatever line we were already writing to. No compiler error, but a silent failure:
/// Process Methods:
///
/// - writeMessage: Write a message @remarks something.
versus what we now generate:
/// Process Methods:
///
/// - writeMessage: Write a message
///
/// @remarks something.
| @@ -907,6 +909,7 @@ SwiftGenerator::writeMemberDoc(IceInternal::Output& out, const DataMemberPtr& p) | |||
|
|
|||
| if (!doc.misc.empty()) | |||
| { | |||
| out << nl << "///"; | |||
There was a problem hiding this comment.
Added an extra newline to the comment, to be consistent with how we generate other sections. Also, I just think it looks better.
There was a problem hiding this comment.
We have 0 testing of comments in slice2swift, so I wanted to at least add something.
I manually checked the output of this to make sure it looked good, and it will catch future errors which might introduce un-compilable code.
There was a problem hiding this comment.
This is not a good fix.
All unrecognized tags (including @remarks) are put in a special misc field,
This is an unexpected and unreasonable behavior, and we should not keep it and try it to make somewhat more correct.
The reasonable behavior for unknown tags is to:
a) fail
or
b) issue a warning and ignore the tag and associated text entirely
When I opened this issue, I was under the impression we supported the @remarks tag. Just like we support the @deprecated tag even though it's not documented.
Also, it's good to add tests, but please don't add the test to an unrelated test!
|
No surprise to anyone, I think option B is the right way to go here. |
This PR fixes #2088 (Adding
@remarksto a doc comment causedslice2swiftto generate uncompilable code).That being said, I think #2088 is also a strange issue to open, since Slice has no support for
@remarkscomment tags.All unrecognized tags (including
@remarks) are put in a specialmiscfield, the contents of which are just pasted on to the end of generated comments, because we don't know what else to do with them.