-
-
Notifications
You must be signed in to change notification settings - Fork 21.4k
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
Generate docs from GDExtensions using --gdextension-docs
with --doctool
#91518
Conversation
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.
LGTM. The const could be added in the already specified section
76d5588
to
823522a
Compare
if (!gdextension_docs) { | ||
for (int i = 0; i < _doc_data_class_path_count; i++) { | ||
// Custom modules are always located by absolute path. | ||
String path = _doc_data_class_paths[i].path; |
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.
Just had a look at this again. Could this also be a const?
const String path = _doc_data_class_paths[i].path;
?
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.
It's pre-existing code (only indentation changed here), but yes it could be const.
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.
This one actually can't be const
because it's written to in the following two lines:
if (path.is_relative_path()) {
path = doc_tool_path.path_join(path);
}
However, the name
variable the next line down, could be const
, I think. That said, since this PR is just indenting this, I'd prefer to leave the code in this block as is.
Tested and it works great! See godotengine/godot-cpp#1374 (comment) for details. Might be worth addressing the issue with the XML schema URI in this PR too while we're at it. |
823522a
to
697db2b
Compare
697db2b
to
30db6ec
Compare
@paddy-exe @akien-mga @m4gr3d Thanks for the review!
In my latest push, I added a possible solution for the XML schema URI issue. It adds a new argument ( Actually, it looks like the only place we use the |
30db6ec
to
2e183d2
Compare
I've gone ahead and switched to doing it this way in my latest push. It looks like I'm the one who added the |
2e183d2
to
2c5c3ae
Compare
Thanks! |
Currently,
--doctool
won't generate docs for GDExtensions, only engine types.This adds a new option (
--gdextension-docs
) which will generate docs only for GDExtensions used in the current project. It's intended to be paired with--doctool
, similar to how the existing--gdscript-docs
option works.I'm not crazy about adding more stuff to
main.cpp
, but this is more-or-less in line with what we've already got there.This is intended to be used with godot-cpp PR godotengine/godot-cpp#1374