Fix doc comments not showing up if only some class members are documented #815

Yarwin · 2024-07-26T13:52:29Z

closes #810

Extend test case to feature some undocumented members of godot instances
Change map&unwrap in docs.rs to filter_map (allow to document only subset of all the properties of a given class)
~~Add "docs" feature to plugin in constant_tests~~
Generate documentation blocks for signals & constants on compile-time

Yarwin · 2024-07-26T14:07:05Z

Changing to draft; I just noticed in test/minimal reproduction that it doesn't cover the whole scope of the issue

Yarwin · 2024-07-26T14:38:28Z

fix'd – all the cases in minimal reproduction project has been covered.

It is worth nothing that example included in test docs causes documentation generation to fail silently (might be simply a length limit).

bend-n · 2024-07-26T15:46:14Z

itest/rust/src/register_tests/constant_test.rs

@@ -174,6 +174,8 @@ godot::sys::plugin_add!(
            register_methods_constants_fn: ::godot::private::ErasedRegisterFn {
                raw: ::godot::private::callbacks::register_user_methods_constants::<HasOtherConstants>,
            },
+            #[cfg(all(since_api = "4.3", feature = "docs"))]
+            docs: None


this isnt going to compile, itest doesnt have a docs feature

well it doesnt change anything, because it can never be enabled

Makes sense, missed leftover – I removed it since itest probably won't be called with godot-core/docs/godot/register-docs anyway

bend-n · 2024-07-26T15:53:34Z

godot-macros/src/docs.rs

-            .map(make_method_docs)
-            .collect::<Option<String>>()?;
+            .filter_map(make_method_docs)
+            .collect::<String>();


theres a new problem here-- i implemented the option stuff so as to save space when no documentation is provided for a function, but now you just have empty documentation. you need to check if the number of documented items is 0 and return none to save space here
else you're reverting a feature

Makes sense, fixed – method won't generate TokenStream in question if there is no need to do so

its not when the methods are empty its when the methods have no documentation at all, i.e methods.iter().filter(has_docs).count() == 0

you see when

impl X { #[func] fn y(…) { } }

we dont want to have a <methods> block

Sure thing – right now (on master) if no documentation for methods are present, the generated xml contains methods tag <methods></methods> with no children present, and same goes for signals and constants. I aimed to preserve this behaviour. I'll just check with test project if Godot has no issues with missing "methods" block in case if there is some documentation

I dont remember why i put the Options in at all then...

But if its working fine right now, you should be able to make the function just return Some, and thus remove the function from existence as it was a IIFE moved out (makeshift try)

I've taken a bit more direct approach – I've decided to keep all the declarations as options and just handle them accordingly while generating given docs block.

(it is kinda a requirement if we want to document only few out of four blocks (the blocks in question being virtual methods, methods, signals or constants))

Yarwin · 2024-07-27T10:34:41Z

From now on, empty tags are not generated in doc's xml if there isn't any documentation for blocks in question.

Another round of manual testing:

(frankly I could just generate given xml and call it a day, but I wanted to see if there isn't any weird, unexpected behavior or whatnot)

bend-n · 2024-07-27T10:58:07Z

godot-core/src/docs.rs

-    pub methods: &'static str,
-    pub signals: &'static str,
-    pub constants: &'static str,
+    pub methods: Option<&'static str>,


this was previously done as Option<InherentImplDocs>, please change the docs fields to be InherentImplDocs etc, as right now the type is Option<[Option<&'static str>; 3]>

I moved generation of signal & constant blocks to compile time, thus now InherentImplDocs holds 2x &'static str & one Option<&'static str>.

bend-n · 2024-07-27T11:06:31Z

godot-macros/src/docs.rs

+    virtual_methods
+        .is_empty()
+        .not()
+        .then(|| quote! { virtual_method_docs: #virtual_methods.into(), })
+        .unwrap_or_else(|| quote! { virtual_method_docs: None, })


why did you obfuscate the match?

Suggested change

virtual_methods

.is_empty()

.not()

.then(|| quote! { virtual_method_docs: #virtual_methods.into(), })

.unwrap_or_else(|| quote! { virtual_method_docs: None, })

match virtual_methods {

"" => quote! { virtual_method_docs: None, },

_ => quote! { virtual_method_docs: #virtual_methods.into(), }

}

why are you applying my suggestion weirdly? you seem to really want to use is_empty

bend-n · 2024-07-27T11:11:33Z

godot-macros/src/docs.rs

+        if methods.is_empty() && signals.is_empty() && constants.is_empty() {
+            return None;
+        }
+        let to_tokenstream = |s: String| -> TokenStream {
+            s.is_empty()
+                .not()
+                .then(|| quote!(Some(#s)))
+                .unwrap_or(quote! {None})
+        };
+        let (methods, signals, constants) = (
+            to_tokenstream(methods),
+            to_tokenstream(signals),
+            to_tokenstream(constants),
+        );


Why are you doing this s.is_empty().not().then().unwrap_or_else() thing?

Suggested change

if methods.is_empty() && signals.is_empty() && constants.is_empty() {

return None;

}

let to_tokenstream = |s: String| -> TokenStream {

s.is_empty()

.not()

.then(|| quote!(Some(#s)))

.unwrap_or(quote! {None})

};

let (methods, signals, constants) = (

to_tokenstream(methods),

to_tokenstream(signals),

to_tokenstream(constants),

);

let to_tokenstream = |s| match s {

"" => quote! { Some(#s) },

_ => quote! { None },

};

let (methods, signals, constants) = (

to_tokenstream(methods),

to_tokenstream(signals),

to_tokenstream(constants),

);

bend-n · 2024-07-27T11:14:02Z

godot-core/src/docs.rs

+fn block_docstring_or_empty(block: Option<&'static str>, tag: &'static str) -> String {
+    match block {
+        Some(s) => format!("<{tag}>{s}</{tag}>"),
+        _ => "".to_string(),
+    }
 }


Is there any chance you could move the tags into the block at macro time instead?

Suggested change

fn block_docstring_or_empty(block: Option<&'static str>, tag: &'static str) -> String {

match block {

Some(s) => format!("<{tag}>{s}</{tag}>"),

_ => "".to_string(),

}

}

fn block_docstring_or_empty(block: Option<&'static str>, tag: &'static str) -> String {

block.map(|s| format!("<{tag}>{s}</{tag}>")).unwrap_or_default()

}

It is an option for signals & constants – methods&virtual methods need to be formatted during runtime 🤔

GodotRust · 2024-07-27T12:40:50Z

API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdext/pr-815

Bromeon

Thank you!

There are some Clippy issues.

Bromeon · 2024-07-27T12:35:58Z

godot-core/src/docs.rs

+            let methods_block: String = match methods.is_some() || virtual_methods.is_some() {
+                true => format!("<methods>{m}{vm}</methods>", m=methods.unwrap_or_default(), vm=virtual_methods.unwrap_or_default()),
+                _ => "".to_string()
+            };


Use if instead of match

Also, String::new() instead of "".to_string()

Bromeon · 2024-07-27T12:37:28Z

godot-core/src/init/mod.rs

+            unsafe {
+                crate::docs::register();
+            }


Why this change?

Call to crate::docs::register(); is unsafe* and I can't compile test project without explicitly marking this call as unsafe 🤔 .

test project:

[dependencies] godot = { path = "../../gdext/godot", features = ["custom-godot", "register-docs"] }

irwin@toster:~/apps/godot/opensource-contr/missing_docs/bug-repro-godot-rust-docs-master/docstest$ cargo build Compiling godot-core v0.1.3 (/home/irwin/apps/godot/opensource-contr/missing_docs/gdext/godot-core) error: call to unsafe function `docs::register` is unsafe and requires unsafe block (error E0133) --> /home/irwin/apps/godot/opensource-contr/missing_docs/gdext/godot-core/src/init/mod.rs:147:17 | 147 | crate::docs::register(); | ^^^^^^^^^^^^^^^^^^^^^^^ call to unsafe function | = note: for more information, see issue #71668 <https://github.com/rust-lang/rust/issues/71668> = note: consult the function's documentation for information on how to avoid undefined behavior note: an unsafe function restricts its caller, but its body is safe by default --> /home/irwin/apps/godot/opensource-contr/missing_docs/gdext/godot-core/src/init/mod.rs:132:1 | 132 | unsafe fn gdext_on_level_init(level: InitLevel) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ note: the lint level is defined here --> /home/irwin/apps/godot/opensource-contr/missing_docs/gdext/godot-core/src/init/mod.rs:131:8 | 131 | #[deny(unsafe_op_in_unsafe_fn)] | ^^^^^^^^^^^^^^^^^^^^^^ error: could not compile `godot-core` (lib) due to 1 previous error

cargo test in godot with proper flags set for itself and godot-core:

/missing_docs/gdext/godot$ cargo test --features register-docs,api-custom,godot-core/docs,godot-core/api-custom -- --nocapture Compiling godot-macros v0.1.3 (/home/irwin/apps/godot/opensource-contr/missing_docs/gdext/godot-macros) Compiling godot-core v0.1.3 (/home/irwin/apps/godot/opensource-contr/missing_docs/gdext/godot-core) error: call to unsafe function `docs::register` is unsafe and requires unsafe block (error E0133) --> godot-core/src/init/mod.rs:147:13 | 147 | crate::docs::register(); | ^^^^^^^^^^^^^^^^^^^^^^^ call to unsafe function | = note: for more information, see issue #71668 <https://github.com/rust-lang/rust/issues/71668> = note: consult the function's documentation for information on how to avoid undefined behavior note: an unsafe function restricts its caller, but its body is safe by default --> godot-core/src/init/mod.rs:132:1 | 132 | unsafe fn gdext_on_level_init(level: InitLevel) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ note: the lint level is defined here --> godot-core/src/init/mod.rs:131:8 | 131 | #[deny(unsafe_op_in_unsafe_fn)] | ^^^^^^^^^^^^^^^^^^^^^^ error: could not compile `godot-core` (lib) due to 1 previous error

safety: must be called from the same thread that bindings are initialized from & bindings must be initialized beforehand

Somebody must have put that deny in there and forgotten to check the #[cfg]d out things i suppose...

@bend-n it seems so

Good remark, I'll make sure this feature is enabled in at least one CI run.

For now, you can add the unsafe block and prepend it with a safety assertion line:

// SAFETY: binding has been initialized at this point.

Done: #819
I can later rebase this PR onto master, to make sure the CI passes.

Bromeon · 2024-07-27T12:38:41Z

godot-macros/src/docs.rs

+        let to_tagged = |s: String, tag: &str| match s.is_empty() {
+            true => s,
+            _ => format!("<{tag}>{s}</{tag}>"),
+        };


bromeon what do you think of

match x { "" => …, _ => …, }

(was my original suggestion)

Bromeon · 2024-07-27T12:39:19Z

godot-macros/src/docs.rs

+        let constants_block = to_tagged(
+            constants
+                .iter()
+                .map(|ConstDefinition { raw_constant: x }| x)


Suggested change

.map(|ConstDefinition { raw_constant: x }| x)

.map(|ConstDefinition { raw_constant }| raw_constant)

Bromeon · 2024-07-27T12:40:15Z

godot-macros/src/docs.rs

+        let methods = match methods.is_empty() {
+            true => quote! { None },
+            _ => quote! {#methods.into()},
+        };


Bromeon · 2024-07-27T12:40:45Z

godot-macros/src/docs.rs

-        },
+        .filter_map(make_virtual_method_docs)
+        .collect::<String>();
+    match virtual_methods.is_empty() {


Please empty line before, to separate complex multi-line statements.
Again, if.

if'ed and multi-lined

Bromeon · 2024-07-27T12:53:56Z

(At least some) clippy issues are independent of this PR, I'll fix them.

Bromeon

Thanks! Could you

include the missing commits with the amendments [edit: seems resolved]
rebase onto master
squash the commits, in line with this?

Bromeon · 2024-07-27T13:43:52Z

godot-core/src/init/mod.rs

+            unsafe {
+                crate::docs::register();
+            }


Good remark, I'll make sure this feature is enabled in at least one CI run.

For now, you can add the unsafe block and prepend it with a safety assertion line:

// SAFETY: binding has been initialized at this point.

godot-macros/src/docs.rs

+        let to_tagged = |s: String, tag: &str| match s.is_empty() {
+            true => s,
+            _ => format!("<{tag}>{s}</{tag}>"),
+        };


Bromeon · 2024-07-27T15:16:55Z

Fixed the formatting and the commit message. Please don't refer to issues via link or # in commit message, because this creates a ton of spam on the tickets as you iterate:

There are still a few more things to do but we're getting closer, will add another comment 🙂

Bromeon

Thanks! Looks mostly good, but there's still one CI problem to be addressed.

To me it's still not quite clear why the handling of methods, signals and constants is different: some declared as Option<&'static str>, others as &'static str; some evaluated XML tags before others.

You mentioned this in multiple comments, but I didn't see why this is done. What's the rationale? Can we not do the processing for different symbols at the same stage?

Yarwin · 2024-07-27T16:37:44Z

@Bromeon initially I was generating all the required xml during initialization.

I was asked by @bend-n #815 (comment) to do so at compile time – but I don't know[1] how to deal with two separate method blocks (one coming from inherent impl (#[godot_api] impl MyType) and another coming from IGodot trait impl (#[godot_api] impl IGodot for MyType)) that are being processed independently 🤔 – thus I settled on weird solution, splitting it in half.

I can revert to generating all the docs blocks on runtime/during plugin registration in godot-core/src/docs.rs:gather_xml_docs.

[1] there are some ways to get around that – for example enum_dispatch uses "cache" https://gitlab.com/antonok/enum_dispatch/-/blob/master/src/cache.rs, but it is certainly over-engineered solution

bend-n · 2024-07-27T16:51:38Z

im talking about the tags part: i.e, instead of storing Some("<method>this is documentation</method>...") you can store Some("<methods><method></method></methods>"), but its not very important

Yarwin · 2024-07-27T16:56:14Z

I can revert to initial solution then

Bromeon · 2024-07-27T18:00:39Z

I don't know[1] how to deal with two separate method blocks (one coming from inherent impl (#[godot_api] impl MyType) and another coming from IGodot trait impl (#[godot_api] impl IGodot for MyType)) that are being processed independently 🤔 – thus I settled on weird solution, splitting it in half.

Ah, that makes sense. I would actually even argue not too much should be processed at compile time, because compile times in Rust are already slow. I'm not sure if one would feel much impact during load time (and it's not like released games need this feature, it's mostly during development and for plugins).

So I think you can leave this for now -- please just add a comment above the plugin declarations that explains this design rationale. 👍

Bromeon · 2024-07-28T16:26:53Z

godot-core/src/docs.rs

@@ -7,6 +7,7 @@
 use crate::registry::plugin::PluginItem;
 use std::collections::HashMap;

+#[doc(hidden)]


Is this necessary? If I check latest API docs, I don't find the symbol.

If it's indeed necessary, attributes should come after the /// comment block.

Bromeon · 2024-07-28T16:29:44Z

godot-core/src/docs.rs

+/// Documentation for signals and constants is being processed on compile time
+/// and can take a form of an already formatted xml `<block><doc></doc>…</block>` or an
+/// empty string if no such attribute has been documented.


Suggested change

/// Documentation for signals and constants is being processed on compile time

/// and can take a form of an already formatted xml `<block><doc></doc>…</block>` or an

/// empty string if no such attribute has been documented.

/// Documentation for signals and constants is being processed at compile time

/// and can take the form of an already formatted XML `<block><doc></doc>…</block>`, or an

/// empty string if no such attribute has been documented.

Bromeon · 2024-07-28T16:31:30Z

godot-core/src/docs.rs

+/// Since documentation for methods comes from two different sources
+/// – Inherent Implementation (`methods`) and IGodotType trait implementation (`virtual_method_docs`) –
+/// it is undesirable to merge these on compile time, and they are being kept as an optional strings of not-yet-parented xml tags
+/// (or nothing if no method has been documented).


Suggested change

/// Since documentation for methods comes from two different sources

/// – Inherent Implementation (`methods`) and IGodotType trait implementation (`virtual_method_docs`) –

/// it is undesirable to merge these on compile time, and they are being kept as an optional strings of not-yet-parented xml tags

/// (or nothing if no method has been documented).

/// Since documentation for methods comes from two different sources

/// -– inherent Implementation (`methods`) and I* trait implementation (`virtual_method_docs`) –-

/// it is undesirable to merge them at compile time. Instead, they are kept as optional strings of not-yet-parented XML tags

/// (or nothing if no method has been documented).

Why a -– (…) –- if I can ask?

Bromeon · 2024-07-28T16:32:19Z

godot-core/src/docs.rs

+            let methods_block: String = if methods.is_some() || virtual_methods.is_some() { format!("<methods>{m}{vm}</methods>", m=methods.unwrap_or_default(), vm=virtual_methods.unwrap_or_default()) } else { String::new()};
+
+        let brief = description.split_once("[br]").map(|(x, _)| x).unwrap_or_default();
+


I fixed the formatting of this in my commit, but you reverted it now. Please restore.

Bromeon · 2024-07-28T16:34:29Z

godot-macros/src/docs.rs

+        let methods = if methods.is_empty() {
+            quote! { None }
+        } else {
+            quote! {#methods.into()}


Suggested change

quote! {#methods.into()}

quote! { #methods.into() }

Also, what does the .into() do here? Is it possible to make the conversion more explicit? Since this is non-local generated code, things can be quite hard to understand otherwise.

The into is for T => Some(T)

Let's write Some(...) then. 10 times clearer.

That's one thing I truly hate about From, it sometimes encourages obfuscation with zero benefits.

I'll change it to { Some(…) }, I aimed to keep it consistent with earlier version

Bromeon · 2024-07-28T16:36:41Z

godot/tests/docs.rs

+    #[signal]
+    /// some user signal
+    ///
+    /// some multiline doc
+    fn documented_signal(p: Vector3, w: f64);


Also here, would be good to have Rustdoc before attributes.

We don't need to fix it everywhere (it may be good to have one other case just for testing), but for lines that are added or changed already.

Bromeon

Thanks for the patience, only few things left -- but you should be able to apply them directly 🙂

Bromeon · 2024-07-29T19:22:46Z

godot-macros/src/docs.rs

+    if virtual_methods.is_empty() {
+        quote! { virtual_method_docs: None, }
+    } else {
+        quote! { virtual_method_docs: #virtual_methods.into(), }


Suggested change

quote! { virtual_method_docs: #virtual_methods.into(), }

quote! { virtual_method_docs: Some(#virtual_methods), }

Bromeon · 2024-07-29T19:23:07Z

godot-core/src/docs.rs

+                    "<methods>{m}{vm}</methods>",
+                    m=methods.unwrap_or_default(),
+                    vm=virtual_methods.unwrap_or_default())


Suggested change

"<methods>{m}{vm}</methods>",

m=methods.unwrap_or_default(),

vm=virtual_methods.unwrap_or_default())

"<methods>{m}{vm}</methods>",

m = methods.unwrap_or_default(),

vm = virtual_methods.unwrap_or_default())

I missed this, sorry.

Bromeon · 2024-07-29T19:23:45Z

godot-core/src/docs.rs

@@ -23,40 +23,56 @@ pub struct StructDocs {
    pub members: &'static str,
 }

-/// Created for documentation on
+/// Keeps documentation for Inherent Implementation such as:


Suggested change

/// Keeps documentation for Inherent Implementation such as:

/// Keeps documentation for inherent `impl` blocks, such as:

Bromeon · 2024-07-29T19:24:58Z

godot-core/src/docs.rs

+///     #[signal]
+///     /// this signal signals
+///     fn documented_signal(p: Vector3, w: f64);
+///     #[constant]
+///     /// this constant consts
+///     const CON: i64 = 42;
+///


Suggested change

/// #[signal]

/// /// this signal signals

/// fn documented_signal(p: Vector3, w: f64);

/// #[constant]

/// /// this constant consts

/// const CON: i64 = 42;

///

/// #[signal]

/// /// this signal signals

/// fn documented_signal(p: Vector3, w: f64);

///

/// #[constant]

/// /// this constant consts

/// const CON: i64 = 42;

(I just moved the empty line from the end to between them, no clue why the diff is so weird)

Bromeon · 2024-07-29T19:28:11Z

godot-core/src/docs.rs

+/// Since documentation for methods comes from two different sources
+/// -– inherent Implementation (`methods`) and I* trait implementation (`virtual_method_docs`) –-
+/// it is undesirable to merge them at compile time. Instead, they are being kept as an optional
+/// strings of not-yet-parented XML tags
+/// (or nothing if no method has been documented).


Suggested change

/// Since documentation for methods comes from two different sources

/// -– inherent Implementation (`methods`) and I* trait implementation (`virtual_method_docs`) –-

/// it is undesirable to merge them at compile time. Instead, they are being kept as an optional

/// strings of not-yet-parented XML tags

/// (or nothing if no method has been documented).

/// Since documentation for methods comes from two different sources

/// -- inherent implementations (`methods`) and `I*` trait implementations (`virtual_method_docs`) --

/// it is undesirable to merge them at compile time. Instead, they are being kept as an optional

/// strings of not-yet-parented XML tags (or nothing if no method has been documented).

Bromeon · 2024-07-30T09:44:01Z

Thanks a lot!

bend-n

Some of these options seem slightly pointless, as what you seem to be doing is

let thing = x.is_empty().then(String::default).unwrap_or(x);

// later
if thing.is_some() && … {
	thing.unwrap_or_default()
}

Which could be done as

let thing = x;

if !thing.is_empty() && !… {
    thing
}

bend-n · 2024-07-30T09:49:27Z

godot-macros/src/docs.rs

+    if virtual_methods.is_empty() {
+        quote! { virtual_method_docs: None, }
+    } else {
+        quote! { virtual_method_docs: Some(#virtual_methods), }
    }


This is one of the options in question:

let methods_block = if methods.is_some() || virtual_methods.is_some() { format!( "<methods>{m}{vm}</methods>", m = methods.unwrap_or_default(), vm = virtual_methods.unwrap_or_default()) } else { String::new() };

can that be changed to just

(virtual_methods.is_empty() && methods.is_empty()) .then(String::default) .unwrap_or_else(|| format!("<methods>{methods}{virtual_methods}</methods>"))

and do this here: (and in the make method docs function)

Suggested change

if virtual_methods.is_empty() {

quote! { virtual_method_docs: None, }

} else {

quote! { virtual_method_docs: Some(#virtual_methods), }

}

quote! { virtual_method_docs: #virtual_methods }

?

thanks a lot! It actually works fine and looks like the cleanest and the least messy solution

Bromeon · 2024-07-30T10:04:39Z

Good that CI failed then 😁 there's an unrelated cargo-deny failure, no clue why exactly now...

…s are documented - Extend docs test case to feature some undocumented members of godot instances - Change map&unwrap in docs.rs to filter_map (allow to document only subset of all the properties of a given class) - Generate documentation blocks for signals & constants on compile-time

Yarwin changed the title ~~#810 Docs comments do not show up if only some of the class members/methods are documented~~ DRAFT: #810 Docs comments do not show up if only some of the class members/methods are documented Jul 26, 2024

Yarwin force-pushed the bugfix/810-docs-comments-do-not-show-up branch from 92739b9 to 5a81f7f Compare July 26, 2024 14:30

Yarwin changed the title ~~DRAFT: #810 Docs comments do not show up if only some of the class members/methods are documented~~ #810 Docs comments do not show up if only some of the class members/methods are documented Jul 26, 2024

Yarwin mentioned this pull request Jul 26, 2024

Docs comments do not follow usual markdown newline rules #811

Closed

bend-n reviewed Jul 26, 2024

View reviewed changes

Yarwin requested a review from bend-n July 27, 2024 10:44

bend-n suggested changes Jul 27, 2024

View reviewed changes

Yarwin requested a review from bend-n July 27, 2024 12:25

Bromeon reviewed Jul 27, 2024

View reviewed changes

Bromeon changed the title ~~#810 Docs comments do not show up if only some of the class members/methods are documented~~ Fix doc comments not showing up if only some class members are documented Jul 27, 2024

Bromeon added bug c: register Register classes, functions and other symbols to GDScript labels Jul 27, 2024

Bromeon added this to the 0.2 milestone Jul 27, 2024

Bromeon mentioned this pull request Jul 27, 2024

Include register-docs feature in CI #819

Merged

Yarwin force-pushed the bugfix/810-docs-comments-do-not-show-up branch 2 times, most recently from 4ff001e to 1ba8157 Compare July 27, 2024 14:33

Bromeon force-pushed the bugfix/810-docs-comments-do-not-show-up branch from 1ba8157 to 8c89daf Compare July 27, 2024 15:15

Bromeon reviewed Jul 27, 2024

View reviewed changes

Yarwin force-pushed the bugfix/810-docs-comments-do-not-show-up branch 2 times, most recently from 607912e to c62c448 Compare July 28, 2024 10:10

Bromeon reviewed Jul 28, 2024

View reviewed changes

Yarwin force-pushed the bugfix/810-docs-comments-do-not-show-up branch 4 times, most recently from d3338d0 to 28c4513 Compare July 29, 2024 16:53

Bromeon reviewed Jul 29, 2024

View reviewed changes

Yarwin force-pushed the bugfix/810-docs-comments-do-not-show-up branch from 28c4513 to 17b4880 Compare July 30, 2024 08:08

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

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

bend-n suggested changes Jul 30, 2024

View reviewed changes

Yarwin force-pushed the bugfix/810-docs-comments-do-not-show-up branch from 17b4880 to 6b46978 Compare July 30, 2024 11:51

Yarwin force-pushed the bugfix/810-docs-comments-do-not-show-up branch from 6b46978 to 87fa9ba Compare July 30, 2024 11:56

bend-n approved these changes Jul 30, 2024

View reviewed changes

Bromeon added this pull request to the merge queue Aug 2, 2024

Merged via the queue into godot-rust:master with commit 832415c Aug 2, 2024
14 checks passed

Bromeon mentioned this pull request Aug 4, 2024

Invalid syntax in docs xml #837

Closed

	.map(\|ConstDefinition { raw_constant: x }\| x)
	.map(\|ConstDefinition { raw_constant }\| raw_constant)

		let methods_block: String = if methods.is_some() \|\| virtual_methods.is_some() { format!("<methods>{m}{vm}</methods>", m=methods.unwrap_or_default(), vm=virtual_methods.unwrap_or_default()) } else { String::new()};

		let brief = description.split_once("[br]").map(\|(x, _)\| x).unwrap_or_default();

	quote! { virtual_method_docs: #virtual_methods.into(), }
	quote! { virtual_method_docs: Some(#virtual_methods), }

	/// Keeps documentation for Inherent Implementation such as:
	/// Keeps documentation for inherent `impl` blocks, such as:

Fix doc comments not showing up if only some class members are documented #815

Fix doc comments not showing up if only some class members are documented #815

Conversation

Yarwin commented Jul 26, 2024 • edited Loading

Yarwin commented Jul 26, 2024 • edited Loading

Yarwin commented Jul 26, 2024 • 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 Jul 26, 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

Yarwin Jul 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Yarwin Jul 27, 2024 • edited Loading

Choose a reason for hiding this comment

Yarwin commented Jul 27, 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

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

Yarwin Jul 27, 2024 • edited Loading

Choose a reason for hiding this comment

GodotRust commented Jul 27, 2024

Bromeon left a 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

Yarwin Jul 27, 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as resolved.

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Bromeon commented Jul 27, 2024 • edited Loading

Bromeon left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as resolved.

Bromeon commented Jul 27, 2024

Bromeon left a comment • edited Loading

Choose a reason for hiding this comment

Yarwin commented Jul 27, 2024 • edited Loading

bend-n commented Jul 27, 2024

Yarwin commented Jul 27, 2024

Bromeon commented Jul 27, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Yarwin Jul 29, 2024 • 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 Jul 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Yarwin Jul 29, 2024 • edited Loading

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

Yarwin commented Jul 26, 2024 •

edited

Loading

Yarwin commented Jul 26, 2024 •

edited

Loading

Yarwin commented Jul 26, 2024 •

edited

Loading

bend-n Jul 26, 2024 •

edited

Loading

Yarwin Jul 27, 2024 •

edited

Loading

Yarwin Jul 27, 2024 •

edited

Loading

Yarwin commented Jul 27, 2024 •

edited

Loading

Yarwin Jul 27, 2024 •

edited

Loading

Yarwin Jul 27, 2024 •

edited

Loading

Bromeon commented Jul 27, 2024 •

edited

Loading

Bromeon left a comment •

edited

Loading

Bromeon left a comment •

edited

Loading

Yarwin commented Jul 27, 2024 •

edited

Loading

Yarwin Jul 29, 2024 •

edited

Loading

bend-n Jul 28, 2024 •

edited

Loading

Yarwin Jul 29, 2024 •

edited

Loading

bend-n left a comment •

edited

Loading

bend-n Jul 30, 2024 •

edited

Loading