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

Import Godot docs for builtin types #584

Open
Bromeon opened this issue Jan 29, 2024 · 7 comments
Open

Import Godot docs for builtin types #584

Bromeon opened this issue Jan 29, 2024 · 7 comments
Labels
c: core Core components documentation Improvements or additions to documentation feature Adds functionality to the library

Comments

@Bromeon
Copy link
Member

Bromeon commented Jan 29, 2024

We should not manually write documentation for builtin types like Vector3, Quaternion, Callable, ... -- except in cases where the Rust API differs from Godot's. Otherwise, it would require us to copy-paste large swaths of Godot's docs, only for it to become outdated when upstream changes.

Instead, can automate it by pre-generating documentation in godot-codegen and importing it on demand:

// Generated code
macro_rules! godot_builtin_doc {
    (Quaternion.spherical_cubic_interpolate) => {
        "\n\nSome imported docs from Godot."
    };
    // all the stuff
}
impl Quaternion {
    /// A manually written first part of the doc.
    #[doc = godot_builtin_doc!(Quaternion.spherical_cubic_interpolate)]
    pub fn spherical_cubic_interpolate() {
        ...
    }
}

rustdoc output:

grafik

@Bromeon Bromeon added documentation Improvements or additions to documentation feature Adds functionality to the library c: core Core components labels Jan 29, 2024
@vortexofdoom
Copy link
Contributor

I assume we'd be reading the XML files from the engine repo? I don't think any of the current dependencies are well-suited to the task of parsing XML. roxmltree seems like a reasonable place to start looking? Not updated recently, but XML hasn't changed much lol.

@Bromeon
Copy link
Member Author

Bromeon commented Feb 3, 2024

extension_api.json now contains documentation, too, if generated with the right flag. We'd need to check if that's enough, otherwise I agree that roxmltree could be a valid candidate. We used it in gdnative, as well.

@vortexofdoom
Copy link
Contributor

vortexofdoom commented Feb 3, 2024

I'm looking at the json serialization stuff, are we currently just skipping over docs in json for the most part? It looks like we're basically just using DeJson to get strings and numbers with default deserializers and then re-parsing it all, but nothing that looks like it deals with docs in any way. I know serde ignores unspecified fields by default, I assume nanoserde is similar?

What/where are the flags for getting documentation from the extension_api?

@Bromeon
Copy link
Member Author

Bromeon commented Feb 3, 2024

I'm looking at the json serialization stuff, are we currently just skipping over docs in json for the most part?

At the moment, docs are not part of the JSON, but yes, the deserialization logic would skip it anyway.


What/where are the flags for getting documentation from the extension_api?

On the Godot executable, --dump-extension-api-with-docs.
We currently only use --dump-extension-api.

The caveat is that the output is much bigger (which can e.g. matter, since we store prebuilt artifacts with the JSON file, and might later publish those to crates.io).

Also, the doc dump wasn't available since 4.0, we'd need to conditionally support it.

@vortexofdoom
Copy link
Contributor

What would be the alternative to storing the larger JSON, I guess keep it locally but not publish it? Would this be a separate feature flag or just a version cfg? I'd imagine the latter?

@Bromeon
Copy link
Member Author

Bromeon commented Feb 4, 2024

I would only consider making this more complex if we run into actual issues with the size. Or if we see that the JSON doesn't contain all the information we require, and we need the separate XML docs anyway.

@vortexofdoom
Copy link
Contributor

Looks like many items have a description field, and some ALSO have a brief_description field that I think would be best to ignore, at least at first.

I don't think a single custom deserializer for a JsonDescription type would have to do much more than:

  • change [code] and [/code] attributes into '```'
  • Turning one '\n' into two
  • Translating link attributes like [method get_buffer] into [get_buffer()] or [`get_buffer`][fn] and similar. This would be the toughest one, since we'd have to tie it in to the renaming pipeline. It'd be good to support eventually, but I think just formatting it like code to start should be acceptable. It should definitely be handled separately, to make a dummy implementation easy to refine later.

It's hard to get a sense just from looking at the JSON whether it's sufficient but it seems pretty comprehensive.

I actually couldn't easily figure out how to make cargo update to the version with docs even replacing the argument passed to the binary, so I ended up just generating it myself, but that shouldn't change the serialization plumbing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: core Core components documentation Improvements or additions to documentation feature Adds functionality to the library
Projects
None yet
Development

No branches or pull requests

2 participants