rename pallet struct in #[cfg(doc)]

[kianenigma]

... to something that represents the pallet itself, for example PalletBalances or BalancesPallet instead of Pallet. The reason for this is that reading pages like this is kinda hard:

Although, I am also mildly worried that this will bring about more confusion that being helpful.

• Action item: Share this issue with the Rust lang forum (or in the Rust issue tracker) and get feedback on feasibility @wentelteefje

[kianenigma]

cc @sam0x17 @gupnik

[bkchr]

Then rename the struct Pallet to whatever you want to have it named. Otherwise the docs are starting to get useless.

[daredevil3435]

where to start for this issue?

[sam0x17]

I would second what @bkchr said that we should actually name the pallet struct something meaningful.

This could potentially be done by tweaking the pallet expansion code to be OK with this, i.e:

1. in the pallet parsing code, collect the pallet struct name by hooking into #[pallet::pallet] and make this accessible, let's say through a variable pallet_struct_ident for the sake of argument
2. in the expansion code for #[pallet::pallet], have it also spit out a type alias pub type Pallet = #pallet_struct_ident;

This will force you to use a name other than Pallet since that name will be taken by the type alias, but all code expecting Pallet to still be there will still compile because of the alias. This would be a good starting point imo and then we can see what we can do from there

I would also prefer that this pallet struct ident be extremely accessible, probably on the root pallet Def so we can access it elsewhere in pallet macro expansion and use this as the pallet name. I know there is some logging code that could make use of this where we are hard-coding a const to have a name for logs instead of somehow asking the pallet what its name is, which should be a thing!

[kianenigma]

@sam0x17 as it, we cannot rename struct Pallet. Can you make a PR to to pallet macros to at least allow this? Banning Pallet is probably too disruptive, but we can at least start renaming it in our code base.

Ideally, I would name Pallet and Config in each pallet like StakingPallet and StakingConfig. The rest don't seem to matter much.

Your point about logging is also correct. We may be able to can auto-generate a fn log on Pallet if need be and if we have access to the name in the broader scope. Essentially, replace the macro_rules! log that is defined in many pallets.

[sam0x17]

@kianenigma yeah no problem I will do it

[bkchr]

Your point about logging is also correct. We may be able to can auto-generate a fn log on Pallet if need be and if we have access to the name in the broader scope. Essentially, replace the macro_rules! log that is defined in many pallets.

You are working everywhere to make it less magic and then you want to generate a log method... Just declare a LOG_TARGET in your crate and then use log::whatever, no need to autogenerate this.

[sam0x17]

My point was more that the pallet name should be accessible enough both at runtime and compile-time that something automatic can hook into it instead of _needing_ to do something like `LOG_TARGET` in every crate

[bkchr]

You don't know the pallet name at compile time. We moved away from this assumption quite some time ago. The pallet name is set in the runtime. You can name your stuff StakingPallet or whatever, but in the runtime it could be FuckItBestStakingEver.

LOG_TARGET is really not such a big problem and it also enables people to grep for this, which was already asked multiple times.

[kianenigma]

FYI, #[doc(alias = "FastUnstakePallet")] helps only with search, but nothing more.

[sam0x17]

We moved away from this assumption quite some time ago

Why did we move away from this. Seems like a step backward?

[bkchr]

I mean the name of the pallet. This name is used for example to prefix the storage items to make them unique. You could call the pallet struct as you like. But there can be multiple instances of the same pallet and each of it having a different name in the runtime.

[sam0x17]

OK so these two things are de-coupled because instancing can give the pallet different names at runtime. This doesn't mean we can't also have a name at compile-time that can be relied on for various purposes and that doesn't necessairly track with the instancing name used at runtime. Beter to have that than just "well we know it is a pallet and that is about it". This compile-time name could also act as a default for the runtime name, allowing it to go unspecified unless it needs to be overridden at runtime because of instancing/etc.

[bkchr]

This compile-time name could also act as a default for the runtime name, allowing it to go unspecified unless it needs to be overridden at runtime because of instancing/etc.

You specify a name any way in the runtime, always, by default. No need to have some "other" default option. As I said, support renaming Pallet to custom names, but that's it. No log method generation or whatever. I mean we just talking about get method being to confusing and then we want to introduce a log method/macro. Doesn't sound that logic to me.

[kianenigma]

I think we're going a bit on a sidetrack here. Seems like we agree on "let's rename struct Pallet" and disagree on some improvement to the logging macros.

The log matter is entirely different because it is not a function of the name of the pallet struct, and is instead a function of the name that the pallet is given at by the top level runtime.

Even if something can be done about a nicer logging, it is a different concern (again, because it has nothing to do with struct Pallet), so I suggest we focus on the first part for now and further discuss the log matter elsewhere, if needed.

[gui1117]

in the doc, hovering over the name will give the full path. This helps a little.

But I agree seeing Pallet and Config doesn't render nicely in the doc.

Maybe all item could be named for consistency: GenesisConfig, Event, Error, Pallet and Config.

EDIT: Also we don't need to make it mandatory.

[gui1117]

Another idea could be to force those specific types to be rendered with their full path in the doc.
I don't know if such option exist yet, but I could see it being useful for other usecase as well.

[gui1117]

Another idea could be to force those specific types to be rendered with their full path in the doc.
I don't know if such option exist yet, but I could see it being useful for other usecase as well.

Result doesn't render very good I think:

I tried to make those specific items to always render with full path in librustdoc in rustc

diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs
index f26d74629dd..14c49fe3a55 100644
--- a/src/librustdoc/html/format.rs
+++ b/src/librustdoc/html/format.rs
@@ -801,6 +801,14 @@ fn resolved_path<'cx>(
 ) -> fmt::Result {
     let last = path.segments.last().unwrap();
 
+    let last_name_as_str = last.name.as_str();
+    let force_absolute = last_name_as_str == "Pallet"
+        || last_name_as_str == "Config"
+        || last_name_as_str == "GenesisConfig"
+        || last_name_as_str == "Event"
+        || last_name_as_str == "Error"
+        || last_name_as_str == "Runtime";
+
     if print_all {
         for seg in &path.segments[..path.segments.len() - 1] {
             write!(w, "{}::", if seg.name == kw::PathRoot { "" } else { seg.name.as_str() })?;
@@ -809,7 +817,7 @@ fn resolved_path<'cx>(
     if w.alternate() {
         write!(w, "{}{:#}", &last.name, last.args.print(cx))?;
     } else {
-        let path = if use_absolute {
+        let path = if use_absolute || force_absolute {
             if let Ok((_, _, fqp)) = href(did, cx) {
                 format!(
                     "{}::{}",

but the result is too verbose, maybe only use absolute path for type in foreign crate
but it looks difficult anyway, allowing user to use unique name would be the way to go I feel.

here are some screenshots

[kianenigma]

This is great exploration @thiolliere, thanks for sharing your outcomes.

Possibly, we can couple this with the rest of ideas in paritytech/polkadot-sdk-docs#12, and maintain our our little fork of librustdoc.

[bkchr]

maintain our our little fork of librustdoc.

I hope this is a joke ;)

[kianenigma]

In general I agree that this, while being somewhat useful for docs, is too much magic and can cause more confusion than help. Closing. Someday, if we are to revise again, it is probably easiest to do it via injecting a custom JS into the doc header.