Re: [libgpiod][PATCH 4/4] bindings: rust: examples: add dedicated examples

["Erik Schilling" <erik.schilling@linaro.org>]

On Wed Jun 14, 2023 at 10:18 AM CEST, Kent Gibson wrote:
> On Wed, Jun 14, 2023 at 09:52:20AM +0200, Erik Schilling wrote:
> > On Wed Jun 14, 2023 at 5:54 AM CEST, Kent Gibson wrote:
> > > Add rust equivalents of the core examples.
> > >
> > > Signed-off-by: Kent Gibson <warthog618@gmail.com>
> > 
> > Reviewed-by: Erik Schilling <erik.schilling@linaro.org>
> > 
> > Some nit-picks below, but those are a matter of taste and the change
> > looks ok either way.
> > 
> > > +
> > > +use libgpiod::line;
> > 
> > I think one could also just import the other used modules. Or, since
> > this is an example anyway, just `use libgpiod::*`.
> > 
>
> I'm never keen on using `::*`, as subsequent changes could pull in symbols
> that conflict with locals.
>
> And as this is an example I wanted to be explicit as to where the symbols
> originate, especially as there is some overlap, e.g. line::Config and
> request::Config.
> The general rule is, if it is only used once then use the full name.
> But there are so many line attributes that using the slightly shortened
> form made it more readable.

Yeah, I assumed that you followed this rule. Fine with me.

> > > +                "line: {}, type: {}, event #{}",
> > > +                event.line_offset(),
> > > +                match event.event_type()? {
> > > +                    line::EdgeKind::Rising => "Rising ",
> > > +                    line::EdgeKind::Falling => "Falling",
> > > +                },
> > > +                event.line_seqno()
> > > +            );
> > 
> > println!("{: <8}") could also be used to pad things (would allow
> > removing the trailing space).
> > 
>
> So add 4 chars to remove 1?
>
> Ideally the padding would go after the comma, and then you start getting
> into compound fields, so this was a case of KISS.

As I said, may be a matter of taste. I am fine with either way (thats
why I already provided the Review tag). The extra " " just jumped
to my eye since with rustfmt it is not aligned in a way that made it
immediately obvious.

- Erik