Add way to display Value nicely

[SeaDve]

I'm not sure if I missed any API, but the current nicest way to display that I know of is through its Debug implementation:

Structure {
    fields: [
        Str(
            Str(
                Borrowed(
                    ":1.202",
                ),
            ),
        ),
        Str(
            Str(
                Borrowed(
                    "",
                ),
            ),
        ),
        Str(
            Str(
                Borrowed(
                    ":1.202",
                ),
            ),
        ),
    ],
    signature: Signature(
        "(sss)",
    ),
}

It looks much more verbose and harder to look at compared to something from GVariant through g_variant_print:

(':1.202', '', ':1.202')

I would be willing to submit a pull request for the reimplementation of GVariant printing style on zvariant::Value if that will be fine.

[zeenix]

@SeaDve Thanks. Actually I also get annoyed with the output but I think it's the Display impl you want here, not Debug (where verbosity is good).

BTW for unique names like :1.202, you want to use out dedicated type, zbus::names::UniqueName.

[SeaDve]

BTW for unique names like :1.202, you want to use out dedicated type, zbus::names::UniqueName.

Thanks, but it is unfortunately out of control for me here since the variant came from unknown sources.

I think it's the Display impl you want here, not Debug (where verbosity is good).

I agree. So, to clarify, I think what we want here is to reimplement something like GVariant printing as the Display impl of Value?

[zeenix]

BTW for unique names like :1.202, you want to use out dedicated type, zbus::names::UniqueName.

Thanks, but it is unfortunately out of control for me here since the variant came from unknown sources.

What encoding comes from is not relevant here. You use that type on your side and from encoding pov it's all strings underneath still. You get type safety that way. There is also BusName enum in case the name could be either unique name or well-known name.

I think it's the Display impl you want here, not Debug (where verbosity is good).

I agree. So, to clarify, I think what we want here is to reimplement something like GVariant printing as the Display impl of Value?

Yes. If you do, it would be good to all to other types too (also the types in zbus_name crate that is re-exported as zbus::names).

[SeaDve]

What encoding comes from is not relevant here. You use that type on your side and from encoding pov it's all strings underneath still.

I got the structure from the body of a Message built from raw parts (I used a fork that made from_raw_parts public temporarily), so I don't think it would be possible, though would be indeed nice, to use specific names types instead of a str

[zeenix]

I got the structure from the body of a Message built from raw parts (I used a fork that made from_raw_parts public temporarily),

Ah it's very generic code. Nm then. :)

[MaxVerevkin]

I think Debug is more suitable here because

• To be consistent with the standard library (where containers implement nice Debug but not Display).
• A better Debug implementation would improve the derived Debug imps of already existing code.
• Display is for user-facing output. Arguably, Value should not be user-facing.

[MaxVerevkin]

Also, I'm not sure if it's possible with Display, but with Debug, "{:#?}" con be used to "pretty-print" the collection if it uses Formatter::debug_{map,list,tuple,...}.

[zeenix]

Display is for user-facing output. Arguably, Value should not be user-facing.

Hmm.. that's a good point.

Also, I'm not sure if it's possible with Display, but with Debug, "{:#?}" con be used to "pretty-print" the collection if it uses Formatter::debug_{map,list,tuple,...}.

This too. :)

@SeaDve Sorry to have sent you in the wrong direction but I think switching from Display to Debug shouldn't be a huge effort?

[zeenix]

@SeaDve Sorry to have sent you in the wrong direction but I think switching from Display to Debug shouldn't be a huge effort?

And yes, tbc @MaxVerevkin convinced me that your original suggestion of modifying the Debug impl was the way to go. Sorry again.

[SeaDve]

Don't worry about it

I think switching from Display to Debug shouldn't be a huge effort?

Yeah, it shouldn't be difficult (Can be done I guess simply by replacing Debug with Display). Though, implementing a "pretty-print" would be. Do we want it?

Also, I'm not sure if it's possible with Display, but with Debug, "{:#?}" con be used to "pretty-print" the collection if it uses Formatter::debug_{map,list,tuple,...}.

It's possible for both Debug and Display using the alternate method of Formatter, which is also used internally by Formatter::debug_{map,list,tuple,...} to check whether to "pretty-print".

[MaxVerevkin]

Is the GVariant consistency really important? Just delegating the Debug implementation of a couple of structs to str drastically improves the readability, very easy, and probably meets the expectation of most Rust devs:

Structure {
    fields: [
        ":1.202",
        "",
        ":1.202",
    ],
    signature: "(sss)",
}

[SeaDve]

Is the GVariant consistency really important?

Not much for me, but the compatibility may be useful on some scenario like parsing

Is the GVariant consistency really important? Just delegating the Debug implementation of a couple of structs to str drastically improves the readability, very easy, and probably meets the expectation of most Rust devs:

Structure {
    fields: [
        ":1.202",
        "",
        ":1.202",
    ],
    signature: "(sss)",
}

Hmm, we also have an option of doing that with the Debug implementation, and keeping this with the Display impl.

[MaxVerevkin]

but the compatibility may be useful on some scenario like parsing

Hm, could you elaborate please? Why would anyone want to parse the Debug output?

[SeaDve]

but the compatibility may be useful on some scenario like parsing

Hm, could you elaborate please? Why would anyone want to parse the Debug output?

I meant it with the Display/to_string() impl. If it is compatible with GVariant, it has a nice side effect of working with https://gtk-rs.org/gtk-rs-core/git/docs/glib/variant/struct.Variant.html#method.parse

[MaxVerevkin]

it has a nice side effect of working with https://gtk-rs.org/gtk-rs-core/git/docs/glib/variant/struct.Variant.html#method.parse

Interesting! In this particular case a From impl would be much better, thought (zvariant can add this conversion behind a feature flag).

[MaxVerevkin]

Maybe the GVariant compatible Display implementation can be exposed through a wrapper type, similar to how Path does not implement Display but has a .display() method which returns a struct which is displayable? Something like

pub struct<'a, T> GVariantDisplay(&'a T);

impl ... {
    pub fn gvarint_display(&self) -> GVariantDispay<'_, Self> {
        GVariantDisplay(self)
    }
}

impl Display for GVariantDisplay<...> {
   ...
}

impl Display for GVariantDisplay<...> {
   ...
}

println!("{}", val.gvariant_display());

But maybe I'm over-complicating things...

[zeenix]

But maybe I'm over-complicating things...

Yeah, maybe. :) I think we should just go for Display and Debug both. They can just be the same. As long as Debug has sufficient context for all debugging needs, we're good.

[zeenix]

Hmm, we also have an option of doing that with the Debug implementation, and keeping this with the Display impl.

That sounds like a good plan IMO.

[SeaDve]

So, I think we agree to keep #379 as it is and just improve the existing Debug impl?

I could do a separate PR :)

[zeenix]

So, I think we agree to keep #379 as it is and just improve the existing Debug impl?

Yup. @MaxVerevkin ?

I could do a separate PR :)

Probably better, yes (for the Debug improvement, that is).

[MaxVerevkin]

So, I think we agree to keep #379 as it is and just improve the existing Debug impl?

Yup. @MaxVerevkin ?

Seems reasonable