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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxx>
> 
> Reviewed-by: Erik Schilling <erik.schilling@xxxxxxxxxx>
> 
> 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.

> > +
> > +    let value = request.value(line_offset)?;
> > +    println!("{:?}", value);
> 
> Could also be:
> +    println!("{value:?}");
> (same below)
> 

Fair enough.  I'm old school so I tend to prefer printf style.

> > +                "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.

Cheers,
Kent.



[Index of Archives]     [Linux SPI]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux