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.

Sign up for GitHub

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

Jump to bottom

Fix Bugs in How 'slice2swift' Handled 'misc' Comment Components #3008

Closed
wants to merge 1 commit into from
Closed

Fix Bugs in How 'slice2swift' Handled 'misc' Comment Components #3008

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions cpp/src/slice2swift/SwiftUtil.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -657,7 +657,7 @@ SwiftGenerator::writeDocSummary(IceInternal::Output& out, const ContainedPtr& p)

if (!doc.misc.empty())
{
out << "///" << nl;
Copy link
Member Author

@InsertCreativityHere InsertCreativityHere Oct 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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);
}

Expand Down Expand Up @@ -789,8 +789,8 @@ SwiftGenerator::writeOpDocSummary(IceInternal::Output& out, const OperationPtr&

if (!doc.misc.empty())
{
out << nl;
writeDocLines(out, doc.misc, false);
Comment on lines -792 to -793
Copy link
Member Author

Choose a reason for hiding this comment

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

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!

out << nl << "///";
writeDocLines(out, doc.misc);
}
}

Expand Down Expand Up @@ -838,7 +838,8 @@ SwiftGenerator::writeProxyDocSummary(IceInternal::Output& out, const InterfaceDe

if (!doc.misc.empty())
{
writeDocLines(out, doc.misc, false);
Copy link
Member Author

@InsertCreativityHere InsertCreativityHere Oct 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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.

out << nl << "///";
writeDocLines(out, doc.misc);
}
}

Expand Down Expand Up @@ -878,7 +879,8 @@ SwiftGenerator::writeServantDocSummary(IceInternal::Output& out, const Interface

if (!doc.misc.empty())
{
writeDocLines(out, doc.misc, false);
out << nl << "///";
writeDocLines(out, doc.misc);
}
}

Expand Down Expand Up @@ -907,6 +909,7 @@ SwiftGenerator::writeMemberDoc(IceInternal::Output& out, const DataMemberPtr& p)

if (!doc.misc.empty())
{
out << nl << "///";
Copy link
Member Author

Choose a reason for hiding this comment

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

Added an extra newline to the comment, to be consistent with how we generate other sections. Also, I just think it looks better.

writeDocLines(out, doc.misc);
}

Expand Down
11 changes: 11 additions & 0 deletions swift/test/Slice/escape/Key.ice
View file
Open in desktop
Copy link
Member Author

@InsertCreativityHere InsertCreativityHere Oct 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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.

Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,12 @@ enum continue
var
}

/// The guard struct.
/// @see defer
struct guard
{
/// A default
/// @foo Test
int default;
}

Expand All @@ -20,8 +24,15 @@ struct defer
string else;
}

/// The break interface.
/// @foo Test
interface break
{
/// The case operation.
/// @foo Test1
/// @param catch A catch
/// @param try A try
/// @bar Test2
["amd"] void case(int catch, out int try);
}

Expand Down
Loading