Skip to content

feat: AsciiDoc and HTML outputs support the final specifier for classes and class templates #965

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.

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

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

gennaroprota
Copy link
Collaborator

No description provided.

@cppalliance-bot
Copy link

An automated preview of the documentation is available at https://965.mrdocs.prtest2.cppalliance.org/index.html

1 similar comment
@cppalliance-bot
Copy link

An automated preview of the documentation is available at https://965.mrdocs.prtest2.cppalliance.org/index.html

@gennaroprota
Copy link
Collaborator Author

gennaroprota commented Aug 13, 2025

@alandefreitas: Of course, feel free to squash the commits. (If you do that, the first commit message should be changed, as it states that we add "final" in a comment.)

@cppalliance-bot
Copy link

An automated preview of the documentation is available at https://965.mrdocs.prtest2.cppalliance.org/index.html

like `class A final;` is ill-formed (`final` can't be used on a
non-defining declaration). --}}
{{/unless}}{{#isSeeBelow}} { /* see-below */ }{{/isSeeBelow}}{{#if isFinal}} final{{else}};{{/if}}
Copy link
Collaborator

Choose a reason for hiding this comment

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

That still leaves the case where there are base classes.

@cppalliance-bot
Copy link

An automated preview of the documentation is available at https://965.mrdocs.prtest2.cppalliance.org/index.html

@cppalliance-bot
Copy link

An automated preview of the documentation is available at https://965.mrdocs.prtest2.cppalliance.org/index.html

1 similar comment
@cppalliance-bot
Copy link

An automated preview of the documentation is available at https://965.mrdocs.prtest2.cppalliance.org/index.html

@cppalliance-bot
Copy link

An automated preview of the documentation is available at https://965.mrdocs.prtest2.cppalliance.org/index.html

@cppalliance-bot
Copy link

An automated preview of the documentation is available at https://965.mrdocs.prtest2.cppalliance.org/index.html

…es and class templates

This was already supported in XML. In this commit, we also remove the
semicolon when that would produce invalid C++ code, e.g. in

class A final;

or

class A : B1, B2;

Instead, "{ /* see-below */ }" is now always followed by a semicolon,
even if the record is final and/or base classes are present.

This closes issue cppalliance#963.
@cppalliance-bot
Copy link

An automated preview of the documentation is available at https://965.mrdocs.prtest2.cppalliance.org/index.html

Comment on lines +27 to +28
{{~#isSeeBelow}} { /* see-below */ };{{/isSeeBelow~}}
Copy link
Collaborator

@alandefreitas alandefreitas Aug 21, 2025

Choose a reason for hiding this comment

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

{{/unless}}
{{!-- Unless we are going to show a class body ("{ ... }"), don't add a
      semicolon if the record is final, because something like `class A
      final;` is ill-formed (`final` can't be used on a non-defining
      declaration). Similarly, don't show a semicolon if there are base
      classes. --}}
{{~#unless (or isSeeBelow isFinal bases)}};{{/unless~}}
{{~#isSeeBelow}} { /* see-below */ };{{/isSeeBelow~}}

This is redundant because you're removing the ; when it's see-below then reincluding it when it is see-below (the reinclusion being part of this PR).

The non-redundant equivalent code would be something like the same logic, but not removing the first line and without the unnecessary condition (because see-below does include the ;):

{{/unless}}{{#isSeeBelow}} { /* see-below */ }{{/isSeeBelow}}; 
{{!-- Unless we are going to show a class body ("{ ... }"), don't add a
      semicolon if the record is final, because something like `class A
      final;` is ill-formed (`final` can't be used on a non-defining
      declaration). Similarly, don't show a semicolon if there are base
      classes. --}}
{{~#unless (or isFinal bases)}};{{/unless~}}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Your code would add two semicolons for a non-final, see-below record without bases; e.g.:

class A { /* see-below */ };;

and if you remove the semicolon after "{ /* see-below */ }" in the Handlebars code, that produces no semicolon (e.g.) for this:

class A final { /* see-below */ }

.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes. I incorrectly pasted it. My code was supposed to be:

{{/unless}}{{#isSeeBelow}} { /* see-below */ }{{/isSeeBelow}}
{{!-- Unless we are going to show a class body ("{ ... }"), don't add a
      semicolon if the record is final, because something like `class A
      final;` is ill-formed (`final` can't be used on a non-defining
      declaration). Similarly, don't show a semicolon if there are base
      classes. --}}
{{~#unless (or isFinal bases)}};{{/unless~}}

(it's to be something like that but it might obviously need some small indentation or change because I haven't tested it with all cases)

But this change is important because then things are in order, and the code reads as:

First condition) Render the code block if the code block is due
Second condition) Render the semicolon if the semicolon is due

instead of:

First condition) Render a semicolon that is due, but don't render a semicolon even if it's due if it's also a see-below symbol, because we're going to render this subcase of semicolon with the whole block later on
Second condition) Render the block now, even though we were already at the semicolon rendering stage one line above, but that's OK, but we put an exception there to ignore see-below, which is the only case where we need to render blocks for now.

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.

3 participants