-
Notifications
You must be signed in to change notification settings - Fork 21
feat: support the final specifier for classes and class templates, in AsciiDoc and HTML #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
Conversation
An automated preview of the documentation is available at https://965.mrdocs.prtest2.cppalliance.org/index.html |
1 similar comment
An automated preview of the documentation is available at https://965.mrdocs.prtest2.cppalliance.org/index.html |
@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.) |
An automated preview of the documentation is available at https://965.mrdocs.prtest2.cppalliance.org/index.html |
share/mrdocs/addons/generator/common/partials/symbol/signature/record.hbs
Outdated
Show resolved
Hide resolved
An automated preview of the documentation is available at https://965.mrdocs.prtest2.cppalliance.org/index.html |
An automated preview of the documentation is available at https://965.mrdocs.prtest2.cppalliance.org/index.html |
1 similar comment
An automated preview of the documentation is available at https://965.mrdocs.prtest2.cppalliance.org/index.html |
An automated preview of the documentation is available at https://965.mrdocs.prtest2.cppalliance.org/index.html |
An automated preview of the documentation is available at https://965.mrdocs.prtest2.cppalliance.org/index.html |
An automated preview of the documentation is available at https://965.mrdocs.prtest2.cppalliance.org/index.html |
share/mrdocs/addons/generator/common/partials/symbol/signature/record.hbs
Outdated
Show resolved
Hide resolved
An automated preview of the documentation is available at https://965.mrdocs.prtest2.cppalliance.org/index.html |
1 similar comment
An automated preview of the documentation is available at https://965.mrdocs.prtest2.cppalliance.org/index.html |
classes. --}} | ||
{{~#isSeeBelow}} { /* see-below */ }{{/isSeeBelow~}} | ||
{{#if (or isSeeBelow (and (not isFinal) (isEmpty bases)))}};{{/if}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additionally, it's essential to have a formal explanation for why not bases
did not work. Because in general, the inverse conditional with #unless
shouldn't work either. That's an important question.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not bases
always gives false
because of this. It doesn't matter whether the array is empty or not. Now, are you saying {{#unless <condition>}}
gets internally translated to {{#if (not (<condition>))}}
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now, are you saying {{#unless }} gets internally translated to {{#if (not ())}}?
That's not how it works internally, but yes, in the sense that they're logically equivalent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But we have a problem with this point. If you look at if_fn
in the same file, you'll see #if
uses isEmpty
and not isTruthy
. Other built-in helpers should follow suit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My hunch is the problem has nothing to do with if
and unless
. The problem is the and
, or
, and all other logical helpers.
They're not built-in helpers. They came from the Antora default UI bundle. The comments should have references to their implementation: https://gitlab.com/antora/antora-ui-default/-/tree/master/src/helpers
Their implementation relies on isTruthy
, and we replicated that when porting it. But they should really depend on isEmpty
because a condition that always returns true
or false
is useless. Not only that, but it's confusing because it doesn't match the behavior of #if
and #unless
. This should be fixed in all Antora helpers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You want to change other built-in helpers to use isEmpty()
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No. I said that:
- All built-in helpers should already use isEmpty (not isTruthy)
- All logical and antora helpers should use isEmpty instead of isTruthy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not happy that it returns true for 0
(that is, for all integers) though. Let's not change that for now.
You could just use the helper to get the array size (0
would decay to false
with isTruthy
), and everything else about handlebars in the PR becomes unnecessary.
This way, the problem is solved with minimal effort, and we can consider the isEmpty issue later.
An automated preview of the documentation is available at https://965.mrdocs.prtest2.cppalliance.org/index.html |
… AsciiDoc and HTML 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.
An automated preview of the documentation is available at https://965.mrdocs.prtest2.cppalliance.org/index.html |
Fixes issue #963.