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.

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

Implements using directives, using declarations and namespace aliases #545

Merged
merged 1 commit into from
Apr 24, 2024

Conversation

fpelliccioni
Copy link
Contributor

No description provided.

Copy link
Collaborator

@alandefreitas alandefreitas left a comment

Choose a reason for hiding this comment

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

CI aside, LGTM. It would be good if we were already using it in the handlebars templates.

@sdkrystian Couldn't a .inc file (like the ones LLVM generates) defining these Info types maybe be useful here? I lot of code going through these types defining things is looking very repetitive. (It would be a simple list of macro calls for each info name so we don'tt need a .td file of course.)

include/mrdocs/Metadata/Info.hpp Outdated Show resolved Hide resolved
src/lib/AST/ASTVisitor.cpp Outdated Show resolved Hide resolved
src/lib/Metadata/Finalize.cpp Outdated Show resolved Hide resolved
src/lib/AST/ASTVisitor.cpp Outdated Show resolved Hide resolved
src/lib/AST/ASTVisitor.cpp Outdated Show resolved Hide resolved
src/lib/AST/ASTVisitor.cpp Outdated Show resolved Hide resolved
src/lib/AST/ASTVisitor.cpp Show resolved Hide resolved
include/mrdocs/Metadata/Using.hpp Show resolved Hide resolved
src/lib/AST/ASTVisitor.cpp Outdated Show resolved Hide resolved
src/lib/AST/ASTVisitor.cpp Outdated Show resolved Hide resolved
src/lib/AST/ASTVisitor.cpp Outdated Show resolved Hide resolved
src/lib/Metadata/Finalize.cpp Show resolved Hide resolved
src/lib/AST/AnyBlock.hpp Outdated Show resolved Hide resolved
include/mrdocs/Metadata/Using.hpp Outdated Show resolved Hide resolved
src/lib/AST/BitcodeWriter.cpp Outdated Show resolved Hide resolved
src/lib/Gen/xml/XMLWriter.cpp Outdated Show resolved Hide resolved
src/lib/Metadata/DomMetadata.cpp Outdated Show resolved Hide resolved
src/lib/AST/ASTVisitor.cpp Outdated Show resolved Hide resolved
Copy link
Member

@sdkrystian sdkrystian left a comment

Choose a reason for hiding this comment

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

Add support for these new Info kinds to the adoc templates, and this should be good to go.

@fpelliccioni
Copy link
Contributor Author

Add support for these new Info kinds to the adoc templates, and this should be good to go.

@sdkrystian
This was already done a long time ago, here is the link to the file:

https://github.com/cppalliance/mrdocs/blob/374c13c7ece462806d8ee8eda65827c25d9557b5/share/mrdocs/addons/generator/asciidoc/partials/tranche.adoc.hbs

Or do you mean something else?

@sdkrystian
Copy link
Member

sdkrystian commented Apr 16, 2024

The XML schema needs to be updated.

With respect to updating the asciidoc templates, you need to add handlebars partials for rendering the documentation page of an alias and using here, and also add handlebars partials for rendering the signature of such symbols here (the signature of a using declaration would e.g. be using A::f)

@sdkrystian
Copy link
Member

@fpelliccioni Please try generating an asciidoc reference containing using declarations/using directives/namespace aliases. The asciidoc partials you wrote contain numerous errors, so it would be easier if you were able to see the output and adjust them accordingly :)

@vinniefalco
Copy link
Member

Why has this been sitting in the PR queue for 5+ weeks?

Copy link
Member

@sdkrystian sdkrystian left a comment

Choose a reason for hiding this comment

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

Make sure tests pass in CI, otherwise LGTM

@fpelliccioni
Copy link
Contributor Author

Why has this been sitting in the PR queue for 5+ weeks?

The PR is quite comprehensive, touching on 63 files across different parts of the project. It took some time to ensure everything was aligned and functioning as expected. The good news is that the PR has now passed all CI checks and has been approved. I'm merging it as we speak. Really appreciate your patience and support!

@fpelliccioni
Copy link
Contributor Author

Make sure tests pass in CI, otherwise LGTM

@sdkrystian I am not abled to merge it.

@sdkrystian sdkrystian merged commit 753823e into cppalliance:develop Apr 24, 2024
6 of 7 checks passed
@alandefreitas
Copy link
Collaborator

alandefreitas commented May 11, 2024

@fpelliccioni
Copy link
Contributor Author

fpelliccioni commented May 11, 2024 via email

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.

None yet

4 participants