Skip to content

Conversation

@InsertCreativityHere
Copy link
Member

In Python, our doc comments (both generated and hand-written) are inconsistent and sub-optimal.

Comparison of the API reference pages before and after:

CompressBatch Before:
Screenshot 2026-01-02 at 14 54 56
CompressBatch After:
Screenshot 2026-01-02 at 14 55 11


EndpointSelectionType Before:
Screenshot 2026-01-02 at 14 55 31
EndpointSelectionType After:
Screenshot 2026-01-02 at 14 55 44

Current Behavior

Right now, we never generate doc-comments on the enumerators themselves, instead we generate a list of enumerators on the enclosing enum. There are 2 different styles we use at random:

Enumerators:

    - Yes:
        Compress the batch requests.

    - No:
        Don't compress the batch requests.

and

    Enumerators
    -----------
    Random : EndpointSelectionType
        Random causes the endpoints to be arranged in a random order.
    Ordered : EndpointSelectionType
        Ordered forces the Ice runtime to use the endpoints in the order they appeared in the proxy.

What this PR does

We no longer write this list of enumerators in the enum's doc-string.
Enumerator doc-comments are now placed directly on the enumerator itself:

class CompressBatch(Enum):
    """
    The batch compression option when flushing queued batch requests.
    """

    Yes = 0
    """
    Compress the batch requests.
    """

    No = 1
    """
    Don't compress the batch requests.
    """

This seems more logical, works better with the Python extension in VSCode, and looks better in the generated API reference.

out.inc();

writeEnumDocstring(p, out);
writeDocstring(p, out, "enum class");
Copy link
Member Author

Choose a reason for hiding this comment

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

Now that we don't generate this list of enumerators in the enum doc-comment, we don't need a special writeDocString method for enums.

It can just use the normal writeDocString that every other type uses.

out << nl;
for (const auto& enumerator : enumerators)
{
out << sp;
Copy link
Member Author

Choose a reason for hiding this comment

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

We weren't generating spaces between enumerators before.
Now we are.

remarks.push_back(
"The Slice compiler generated this " + generatedType + " from Slice " + p->kindOf() + " ``" + p->scoped() +
"``.");

Copy link
Member Author

Choose a reason for hiding this comment

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

I just moved these lines down to where we were doing the other writes to out. Instead of having these mixed in with where we're gathering doc-strings.

Comment on lines -2400 to -2401
remarks.push_back(
"The Slice compiler generated this enum class from Slice " + p->kindOf() + " ``" + p->scoped() + "``.");
Copy link
Member Author

Choose a reason for hiding this comment

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

I decided to not generate this remark for each enumerator, otherwise it looked really noisy seeing this 4 times back-to-back.

It's still on the enum though.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR improves the documentation formatting for Python enums by moving enumerator documentation from a list in the enum class docstring to individual docstrings on each enumerator. This change makes the documentation more intuitive, improves IDE integration, and produces better-looking API reference documentation.

Key Changes:

  • Refactored the C++ code generator to write enumerator docstrings directly on each enum member instead of aggregating them in the enum class docstring
  • Updated three example Python enum files (ToStringMode, EndpointSelectionType, CompressBatch) to demonstrate the new documentation format

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
python/python/Ice/ToStringMode.py Moved documentation for Unicode, ASCII, and Compat enumerators from class-level list to individual enumerator docstrings
python/python/Ice/EndpointSelectionType.py Moved documentation for Random and Ordered enumerators from class-level list to individual enumerator docstrings
python/python/Ice/CompressBatch.py Moved documentation for Yes, No, and BasedOnProxy enumerators from class-level list to individual enumerator docstrings
cpp/src/slice2py/PythonUtil.h Renamed writeEnumDocstring to writeEnumeratorDocstring and updated signature to accept EnumeratorPtr instead of EnumPtr
cpp/src/slice2py/PythonUtil.cpp Replaced writeEnumDocstring with writeEnumeratorDocstring, simplified the implementation to handle individual enumerators, and updated visitEnum to call the new function for each enumerator

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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.

2 participants