Generate documentation from doc comments #748

bend-n · 2024-06-09T14:40:49Z

closes #178

lilizoey

i've actually also worked a little bit on this too, but i dont think this as written is the best solution. i'll try to avoid nitcpicking since it's a draft pr, but i think the better way of doing this is:

Proc macros

gets the doc-comments
parses markdown (using the markdown crate or something)
converts the parsed markdown into xml, but not the top-level <class> xml since that will need to happen later
registers plugins using the plugin system

There will then be two doc-plugins, one for the #[derive(GodotClass)] and one (or two if we can add documentation to overridden virtual methods?) for the #[godot_api].

We could probably just add the doc-plugins to the existing Struct and InherentImpl plugins.

Godot-core

gathers up these plugins
puts the documentation into a <class> tag
calls the registration method

godot-macros/src/docs.rs

godot-core/src/init/mod.rs

godot/Cargo.toml

lilizoey

Thanks!

I think this implementation is good for now, there are things that arent supported however. Like linking to other classes with [classname] syntax for instance. Could you maybe add a comment somewhere about what is explicitly not supported?

Additionally you should write some documentation on the relevant derive macros (GodotClass and godot_api) about how this documentation works. This could be a good place to document what isn't supported.

Also what happens if you try to use this in godot 4.2? Do you get a nice error message? does it just silently do nothing?

Also you're lacking much in terms of testing, could you add some explicit tests of this functionality? it'd be ideal if we could actually test that the documentation as registered with godot actually corresponds to what we expect. I think some of this can actually be unit tested as well which would be great.

Most of my comments are about code-style/organization, im sorry if it's a bit much. feel free to take your time going through it.

examples/dodge-the-creeps/rust/src/main_scene.rs

examples/dodge-the-creeps/rust/src/mob.rs

godot-macros/src/docs/md2bb.rs

godot-core/src/init/mod.rs

bend-n · 2024-06-17T02:17:31Z

Actually [classname] works just fine.

lilizoey

I think we should try to more thoroughly test the possible markdown syntaxes, to ensure that it doesn't break anything and to know what the output is gonna be. This will also ensure that if we later update our converter to better handle these cases then we'll already have a test that will cover those cases.

headings:

/// # Some heading

lists:

/// - lists
/// - like this
/// * maybe with `*` as well

links with back-references:

/// Blah blah [foo]
/// [foo]: https://example.org

footnotes:

/// We cannot florbinate the glorb[^florb]
/// [^florb]: because the glorb doesn't flibble.

task lists:

/// We must ensure that we've completed
/// - [ ] task 1
/// - [x] task 2

tables:

/// | Header1 | Header2 |
/// |---------|---------|
/// | abc     | def     |

images:

/// ![Image](http://url/a.png)

blockquotes:

/// > Some cool quote

numbered lists:

/// 1. thing one
/// 2. thing two

horizontal rules:

/// Something here
/// ---
/// And here

smart punctuation

/// test --- a -- b ... foo "asd" or 'hi'

maybe also some custom html somewhere

godot-macros/src/docs.rs

godot/tests/docs.rs

lilizoey · 2024-06-30T15:44:20Z

godot/tests/docs.rs

+
+#[godot_api]
+impl INode for ExtremelyDocumented {
+    fn init(base: Base<Node>) -> Self {


this function should also be documented imo, to check that it's not exported to godot either

it is exported to godot actually although right now it doesnt register properly

godot/tests/docs.rs

bend-n · 2024-06-30T16:22:10Z

Quite a few of those things arent supported by godot / this implementation but ill test the others.

lilizoey · 2024-06-30T17:16:40Z

Quite a few of those things arent supported by godot / this implementation but ill test the others.

sure but we'd ideally not want people's code to break just because they add documentation that isn't supported by godot. so at least testing that they dont break things would be useful imo.

lilizoey · 2024-07-01T17:38:53Z

What does the current state of the docs look like in the editor btw? especially with all the <unsupported> stuff?

If you could include a side-by-side of the rustdoc output and the godot documentation that'd be really useful too.

bend-n · 2024-07-02T01:21:38Z

godot-macros/src/docs.rs

Bromeon · 2024-07-08T13:26:11Z

godot-macros/src/docs.rs

+fn params<'a, 'b>(params: impl Iterator<Item = (&'a Ident, &'b TypeExpr)>) -> String {
+    params
+        .enumerate()
+        .fold(String::default(), |mut acc, (index, (name, ty))| {
+            use std::fmt::Write;
+            _ = write!(
+                acc,
+                r#"<param index="{index}" name="{name}" type="{ty}" />"#,
+                ty = ty.to_token_stream()
+            );
+            acc
+        })
+}


Also here, I think a let mut result = String::new(); followed by a for loop is easier to understand.

I think the best way is a map to a format! and then a join is easiest to read, but clippy doesnt like it, should i just #[allow]?

What does clippy not like or suggest instead?

This review would be so much faster if you could provide the relevant information without us always having to ask for it 😔

If you need #[allow], then rather use the variant with for loop. It might also need fewer allocations if a single string is extended.

clippy suggests this.

it wont be more efficient as we still need a format!

Clippy suggests replacing a good old for loop with a fold() call? I find that hard to believe.

Can you not keep the simplest possible procedural code?

Also here, I think a let mut result = String::new(); followed by a for loop is easier to understand.

godot-macros/src/docs/markdown_converter.rs

Bromeon · 2024-07-08T13:30:04Z

godot-macros/src/docs/markdown_converter.rs

+        // bbcode supports lists but docs dont
+        List(_) | BlockQuote(_) | FootnoteReference(_) | FootnoteDefinition(_) | Table(_) => {
+            "unsupported".into()
+        }


Suggested change

// bbcode supports lists but docs dont

List(_) | BlockQuote(_) | FootnoteReference(_) | FootnoteDefinition(_) | Table(_) => {

"unsupported".into()

}

// BBCode supports lists, but Godot docs don't.

List(_) | BlockQuote(_) | FootnoteReference(_) | FootnoteDefinition(_) | Table(_) => {

"unsupported".into()

}

Also, should we not try to keep the original as much as possible, even if unformatted?

Or what's the idea of replacing them with "unsupported" literal?

godot-macros/src/lib.rs

godot/tests/docs.rs

godot-macros/src/docs.rs

Bromeon

Thanks a lot for this great addition and the patience during review!

Bromeon · 2024-07-22T19:00:24Z

godot-macros/src/docs/markdown_converter.rs

+        List(_) | BlockQuote(_) | FootnoteReference(_) | FootnoteDefinition(_) | Table(_) => {
+            "".into()
+        }


Note to self: we still need to implement this at some point (at least unformatted) 🙂

Generate documentation from doc comments

lilizoey reviewed Jun 9, 2024

View reviewed changes

godot-macros/src/docs.rs Outdated Show resolved Hide resolved

godot-macros/src/docs.rs Outdated Show resolved Hide resolved

godot-core/src/init/mod.rs Outdated Show resolved Hide resolved

godot/Cargo.toml Outdated Show resolved Hide resolved

bend-n force-pushed the docs branch 3 times, most recently from feaadc1 to 6ebf19d Compare June 15, 2024 06:42

bend-n requested a review from lilizoey June 15, 2024 07:22

bend-n force-pushed the docs branch 10 times, most recently from 0c5d8ea to 9b73845 Compare June 16, 2024 06:38

bend-n marked this pull request as ready for review June 16, 2024 06:48

bend-n force-pushed the docs branch 3 times, most recently from 449749d to 19bf2bb Compare June 16, 2024 09:39

lilizoey reviewed Jun 16, 2024

View reviewed changes

Bromeon added feature Adds functionality to the library c: register Register classes, functions and other symbols to GDScript labels Jun 16, 2024

bend-n force-pushed the docs branch 7 times, most recently from efc5749 to 1f7e32e Compare June 17, 2024 02:15

bend-n force-pushed the docs branch from dd5b610 to cdfded1 Compare June 27, 2024 15:13

lilizoey reviewed Jun 30, 2024

View reviewed changes

bend-n force-pushed the docs branch from cdfded1 to 016f156 Compare June 30, 2024 18:12

bend-n force-pushed the docs branch from 016f156 to f106f9a Compare July 2, 2024 01:26

Bromeon reviewed Jul 8, 2024

View reviewed changes

bend-n force-pushed the docs branch 2 times, most recently from 4c67a82 to 4839237 Compare July 9, 2024 05:07

Bromeon reviewed Jul 9, 2024

View reviewed changes

godot-macros/src/docs.rs Outdated Show resolved Hide resolved

godot-macros/src/docs.rs Outdated Show resolved Hide resolved

bend-n force-pushed the docs branch 4 times, most recently from adc3595 to fbd8f52 Compare July 9, 2024 13:48

docs

b586b80

bend-n force-pushed the docs branch from fbd8f52 to b586b80 Compare July 22, 2024 11:32

bend-n requested a review from Bromeon July 22, 2024 11:34

Bromeon added this to the 0.2 milestone Jul 22, 2024

Bromeon approved these changes Jul 22, 2024

View reviewed changes

Bromeon added this pull request to the merge queue Jul 22, 2024

github-merge-queue bot pushed a commit that referenced this pull request Jul 22, 2024

Merge pull request #748 from bend-n/docs

e53f9d6

Generate documentation from doc comments

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 22, 2024

Bromeon added this pull request to the merge queue Jul 22, 2024

Merged via the queue into godot-rust:master with commit f601eb6 Jul 22, 2024
15 checks passed

bend-n deleted the docs branch July 22, 2024 23:08

Bromeon mentioned this pull request Jul 23, 2024

Docs comments do not follow usual markdown newline rules #811

Closed

Bromeon mentioned this pull request Aug 4, 2024

Invalid syntax in docs xml #837

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Generate documentation from doc comments #748

Generate documentation from doc comments #748

bend-n commented Jun 9, 2024

lilizoey left a comment

lilizoey left a comment

bend-n commented Jun 17, 2024

lilizoey left a comment •

edited

Loading

lilizoey Jun 30, 2024

bend-n Jun 30, 2024

bend-n commented Jun 30, 2024

lilizoey commented Jun 30, 2024

lilizoey commented Jul 1, 2024 •

edited

Loading

bend-n commented Jul 2, 2024

Bromeon Jul 8, 2024

bend-n Jul 8, 2024

Bromeon Jul 8, 2024

Bromeon Jul 8, 2024

bend-n Jul 9, 2024

bend-n Jul 9, 2024 •

edited

Loading

Bromeon Jul 9, 2024

Bromeon Jul 8, 2024

Bromeon Jul 8, 2024

Bromeon left a comment

Bromeon Jul 22, 2024

Generate documentation from doc comments #748

Generate documentation from doc comments #748

Conversation

bend-n commented Jun 9, 2024

lilizoey left a comment

Choose a reason for hiding this comment

Proc macros

Godot-core

lilizoey left a comment

Choose a reason for hiding this comment

bend-n commented Jun 17, 2024

lilizoey left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bend-n commented Jun 30, 2024

lilizoey commented Jun 30, 2024

lilizoey commented Jul 1, 2024 • edited Loading

bend-n commented Jul 2, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bend-n Jul 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Bromeon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lilizoey left a comment •

edited

Loading

lilizoey commented Jul 1, 2024 •

edited

Loading

bend-n Jul 9, 2024 •

edited

Loading