linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Kent Gibson <warthog618@gmail.com>
To: Erik Schilling <erik.schilling@linaro.org>
Cc: linux-gpio@vger.kernel.org, brgl@bgdev.pl
Subject: Re: [libgpiod][PATCH 4/4] bindings: rust: examples: add dedicated examples
Date: Wed, 14 Jun 2023 16:18:47 +0800	[thread overview]
Message-ID: <ZIl35wFvID2uA5fg@sol> (raw)
In-Reply-To: <CTC7KTWRXMS4.1G8UBSCYLSMIT@fedora>

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.

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

  reply	other threads:[~2023-06-14  8:19 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-14  3:54 [libgpiod][PATCH 0/4] dedicated examples Kent Gibson
2023-06-14  3:54 ` [libgpiod][PATCH 1/4] core: examples: add " Kent Gibson
2023-06-14  3:54 ` [libgpiod][PATCH 2/4] bindings: cxx: " Kent Gibson
2023-06-14  3:54 ` [libgpiod][PATCH 3/4] bindings: python: " Kent Gibson
2023-06-14  3:54 ` [libgpiod][PATCH 4/4] bindings: rust: " Kent Gibson
2023-06-14  7:52   ` Erik Schilling
2023-06-14  8:18     ` Kent Gibson [this message]
2023-06-14  8:29       ` Erik Schilling
2023-06-14 13:03 ` [libgpiod][PATCH 0/4] " Bartosz Golaszewski
2023-06-14 13:21   ` Kent Gibson
2023-06-14 13:26     ` Bartosz Golaszewski
2023-06-14 13:57       ` Kent Gibson
2023-06-14 15:11         ` Bartosz Golaszewski
2023-06-14 16:00           ` Kent Gibson
2023-06-15 15:16             ` Bartosz Golaszewski
2023-06-15 15:39               ` Kent Gibson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZIl35wFvID2uA5fg@sol \
    --to=warthog618@gmail.com \
    --cc=brgl@bgdev.pl \
    --cc=erik.schilling@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).