🧑‍💻 zv, zn: Improve Str, Owned*Name, BusName Debug impls #450

SeaDve · 2023-08-19T13:21:11Z

This PR reduces Debug impls verbosity but retains possibly useful information.

See also #394

Before:

Str::from_static("hello") = Str(
    Static(
        "hello",
    ),
)
Str::from("hello") = Str(
    Borrowed(
        "hello",
    ),
)
Str::from(String::from("hello")) = Str(
    Owned(
        "hello",
    ),
)
UniqueName::from_static_str_unchecked(":1.2") = UniqueName(
    Str(
        Static(
            ":1.2",
        ),
    ),
)
OwnedUniqueName::from(UniqueName::from_static_str_unchecked(":1.2")) = OwnedUniqueName(
    UniqueName(
        Str(
            Static(
                ":1.2",
            ),
        ),
    ),
)
WellKnownName::from_static_str_unchecked("org.gnome.Console") = WellKnownName(
    Str(
        Static(
            "org.gnome.Console",
        ),
    ),
)
OwnedWellKnownName::from(WellKnownName::from_static_str_unchecked("org.gnome.Console")) = OwnedWellKnownName(
    WellKnownName(
        Str(
            Static(
                "org.gnome.Console",
            ),
        ),
    ),
)
BusName::Unique(UniqueName::from_static_str_unchecked(":1.2")) = Unique(
    UniqueName(
        Str(
            Static(
                ":1.2",
            ),
        ),
    ),
)
BusName::WellKnown(WellKnownName::from_static_str_unchecked("org.gnome.Console")) = WellKnown(
    WellKnownName(
        Str(
            Static(
                "org.gnome.Console",
            ),
        ),
    ),
)
OwnedBusName::from(BusName::Unique(UniqueName::from_static_str_unchecked(":1.2"))) = OwnedBusName(
    Unique(
        UniqueName(
            Str(
                Static(
                    ":1.2",
                ),
            ),
        ),
    ),
)
OwnedBusName::from(BusName::WellKnown(WellKnownName::from_static_str_unchecked("org.gnome.Console"))) = OwnedBusName(
    WellKnown(
        WellKnownName(
            Str(
                Static(
                    "org.gnome.Console",
                ),
            ),
        ),
    ),
)
ErrorName::from_static_str_unchecked("org.gnome.Console.Error") = ErrorName(
    Str(
        Static(
            "org.gnome.Console.Error",
        ),
    ),
)
OwnedErrorName::from(ErrorName::from_static_str_unchecked("org.gnome.Console.Error")) = OwnedErrorName(
    ErrorName(
        Str(
            Static(
                "org.gnome.Console.Error",
            ),
        ),
    ),
)
InterfaceName::from_static_str_unchecked("org.gnome.Console.Interface") = InterfaceName(
    Str(
        Static(
            "org.gnome.Console.Interface",
        ),
    ),
)
OwnedInterfaceName::from(InterfaceName::from_static_str_unchecked("org.gnome.Console.Interface")) = OwnedInterfaceName(
    InterfaceName(
        Str(
            Static(
                "org.gnome.Console.Interface",
            ),
        ),
    ),
)
MemberName::from_static_str_unchecked("Member") = MemberName(
    Str(
        Static(
            "Member",
        ),
    ),
)
OwnedMemberName::from(MemberName::from_static_str_unchecked("Member")) = OwnedMemberName(
    MemberName(
        Str(
            Static(
                "Member",
            ),
        ),
    ),
)

After:

Str::from_static("hello") = "hello"
Str::from("hello") = "hello"
Str::from(String::from("hello")) = "hello"
UniqueName::from_static_str_unchecked(":1.2") = UniqueName(
    ":1.2",
)
OwnedUniqueName::from(UniqueName::from_static_str_unchecked(":1.2")) = OwnedUniqueName(
    ":1.2",
)
WellKnownName::from_static_str_unchecked("org.gnome.Console") = WellKnownName(
    "org.gnome.Console",
)
OwnedWellKnownName::from(WellKnownName::from_static_str_unchecked("org.gnome.Console")) = OwnedWellKnownName(
    "org.gnome.Console",
)
BusName::Unique(UniqueName::from_static_str_unchecked(":1.2")) = BusName::Unique(
    ":1.2",
)
BusName::WellKnown(WellKnownName::from_static_str_unchecked("org.gnome.Console")) = BusName::WellKnown(
    "org.gnome.Console",
)
OwnedBusName::from(BusName::Unique(UniqueName::from_static_str_unchecked(":1.2"))) = OwnedBusName::Unique(
    ":1.2",
)
OwnedBusName::from(BusName::WellKnown(WellKnownName::from_static_str_unchecked("org.gnome.Console"))) = OwnedBusName::WellKnown(
    "org.gnome.Console",
)
ErrorName::from_static_str_unchecked("org.gnome.Console.Error") = ErrorName(
    "org.gnome.Console.Error",
)
OwnedErrorName::from(ErrorName::from_static_str_unchecked("org.gnome.Console.Error")) = OwnedErrorName(
    "org.gnome.Console.Error",
)
InterfaceName::from_static_str_unchecked("org.gnome.Console.Interface") = InterfaceName(
    "org.gnome.Console.Interface",
)
OwnedInterfaceName::from(InterfaceName::from_static_str_unchecked("org.gnome.Console.Interface")) = OwnedInterfaceName(
    "org.gnome.Console.Interface",
)
MemberName::from_static_str_unchecked("Member") = MemberName(
    "Member",
)
OwnedMemberName::from(MemberName::from_static_str_unchecked("Member")) = OwnedMemberName(
    "Member",
)

Fixes #431.

This makes Debug impls of Str transparent Before: Str(Static("foo")) After: "foo"

zeenix · 2023-08-19T16:03:04Z

@SeaDve Many thanks! Before I look into the actual commits, could you please summarize what's the different with #394?

> Str::from_static("hello") = "hello"
> Str::from("hello") = "hello"
> Str::from(String::from("hello")) = "hello"
> UniqueName::from_static_str_unchecked(":1.2") = UniqueName(
>     ":1.2",
> )

Looking nice! should we maybe not split it over multiple lines anymore? I think it was only needed when the format was too verbose and hard to follow.

SeaDve · 2023-08-19T16:09:09Z

Before I look into the actual commits, could you please summarize what's the different with #394?

From user side, instead of making everything look like a string when printing its debug representation, it still retain the wrapper info, e.g., MemberName("foo"), WellKnownName("foo") etc.

On the similarities with #394, this also removed the information that it is Owned or Borrowed, etc., i.e., Str(Borrowed("foo")) is just "foo". Owned*Name("foo") is just *Name("foo"). I think the commit messages summarizes it pretty well.

should we maybe not split it over multiple lines anymore?

It is up to the formatter. I currently just copied the output from the dbg! macro

zeenix

LGTM otherwise. Thanks for writing good commit messages.

zbus_names/src/bus_name.rs

SeaDve · 2023-08-21T15:25:46Z

I am not sure how to handle this on OwnedBusName ; how would it print the inner? Before the latest commits, it prints UniqueName("foo"), but now it is just OwnedBusName("foo")

This makes Debug impls of Owned*Name transparent Before: Owned*Name(*Name("foo")) After: Owned*Name("foo")

zeenix · 2023-08-21T16:46:16Z

I am not sure how to handle this on OwnedBusName ; how would it print the inner? Before the latest commits, it prints UniqueName("foo"), but now it is just OwnedBusName("foo")

In case of OwnedBusName, why not have the inner field output its debug? So it becomes OwnedBusName(UniqueName("foo")).

SeaDve · 2023-08-21T16:47:42Z

I am not sure how to handle this on OwnedBusName ; how would it print the inner? Before the latest commits, it prints UniqueName("foo"), but now it is just OwnedBusName("foo")

In case of OwnedBusName, why not have the inner field output its debug? So it becomes OwnedBusName(UniqueName("foo")).

That would make it inconsistent with BusName

zeenix · 2023-08-21T16:52:25Z

In case of OwnedBusName, why not have the inner field output its debug? So it becomes OwnedBusName(UniqueName("foo")).

That would make it inconsistent with BusName

Right. Then I am confused what the problem is? 🤔 If BusName -> BusName("foo") then OwnedBusName -> OwnedBusName("foo") as well?

SeaDve · 2023-08-21T16:55:31Z

In case of OwnedBusName, why not have the inner field output its debug? So it becomes OwnedBusName(UniqueName("foo")).

That would make it inconsistent with BusName

Right. Then I am confused what the problem is? 🤔 If BusName -> BusName("foo") then OwnedBusName -> OwnedBusName("foo") as well?

BusName is currently UniqueName("foo") per the last commits in the PR.

I also think the well-known/unique distinction is pretty useful.

I wonder if we can do BusName::Unique("foo"), and thus OwnedBusName::Unique("foo"), if it is still idiomatic in rust debug impls

zeenix · 2023-08-21T17:00:51Z

BusName is currently UniqueName("foo") per the last commits in the PR.

I also think the well-known/unique distinction is pretty useful

ah ok, yeah I see the issue.

I wonder if we can do BusName::Unique("foo"), and thus OwnedBusName::Unique("foo"), if it is still idiomatic in rust.

Hmm.. why not? I'd say go for that.

SeaDve · 2023-08-21T17:02:46Z

Hmm.. why not? I'd say go for that.

Sure. However, I will update the PR tomorrow morning as I am currently in bed

zeenix · 2023-08-21T20:14:22Z

Sure. However, I will update the PR tomorrow morning as I am currently in bed

For sure, it's nothing urgent. :)

This makes Debug impl of BusName transparent Before: Unique(UniqueName("foo")) After: BusName::Unique("foo")

SeaDve · 2023-08-22T01:37:10Z

I have push the modifications, and also updated the PR comment on how debug prints look

zeenix · 2023-08-22T12:09:19Z

@SeaDve Many thanks for your efforts here!

🧑‍💻 zv: Improve Str Debug impl

7fef7a4

This makes Debug impls of Str transparent Before: Str(Static("foo")) After: "foo"

zeenix mentioned this pull request Aug 19, 2023

zv,zn: manually implement Debug for Str, *Name and ObjectPath #394

Closed

zeenix reviewed Aug 19, 2023

View reviewed changes

zbus_names/src/bus_name.rs Show resolved Hide resolved

SeaDve force-pushed the main branch from aa01d00 to 958eca9 Compare August 21, 2023 15:06

🧑‍💻 zn: Improve Owned*Name Debug impls

c9d0418

This makes Debug impls of Owned*Name transparent Before: Owned*Name(*Name("foo")) After: Owned*Name("foo")

SeaDve force-pushed the main branch from 958eca9 to b7d905a Compare August 21, 2023 15:28

SeaDve force-pushed the main branch from b7d905a to b7d93b3 Compare August 22, 2023 01:33

🧑‍💻 zn: Improve BusName Debug impl

813eb7a

This makes Debug impl of BusName transparent Before: Unique(UniqueName("foo")) After: BusName::Unique("foo")

SeaDve force-pushed the main branch from b7d93b3 to 813eb7a Compare August 22, 2023 01:35

zeenix approved these changes Aug 22, 2023

View reviewed changes

zeenix merged commit 412b349 into dbus2:main Aug 22, 2023
7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🧑‍💻 zv, zn: Improve Str, Owned*Name, BusName Debug impls #450

🧑‍💻 zv, zn: Improve Str, Owned*Name, BusName Debug impls #450

SeaDve commented Aug 19, 2023 •

edited

zeenix commented Aug 19, 2023

SeaDve commented Aug 19, 2023 •

edited

zeenix left a comment

SeaDve commented Aug 21, 2023

zeenix commented Aug 21, 2023 •

edited

SeaDve commented Aug 21, 2023

zeenix commented Aug 21, 2023

SeaDve commented Aug 21, 2023 •

edited

zeenix commented Aug 21, 2023

SeaDve commented Aug 21, 2023

zeenix commented Aug 21, 2023

SeaDve commented Aug 22, 2023

zeenix commented Aug 22, 2023

🧑‍💻 zv, zn: Improve Str, Owned*Name, BusName Debug impls #450

🧑‍💻 zv, zn: Improve Str, Owned*Name, BusName Debug impls #450

Conversation

SeaDve commented Aug 19, 2023 • edited

zeenix commented Aug 19, 2023

SeaDve commented Aug 19, 2023 • edited

zeenix left a comment

Choose a reason for hiding this comment

SeaDve commented Aug 21, 2023

zeenix commented Aug 21, 2023 • edited

SeaDve commented Aug 21, 2023

zeenix commented Aug 21, 2023

SeaDve commented Aug 21, 2023 • edited

zeenix commented Aug 21, 2023

SeaDve commented Aug 21, 2023

zeenix commented Aug 21, 2023

SeaDve commented Aug 22, 2023

zeenix commented Aug 22, 2023

SeaDve commented Aug 19, 2023 •

edited

SeaDve commented Aug 19, 2023 •

edited

zeenix commented Aug 21, 2023 •

edited

SeaDve commented Aug 21, 2023 •

edited