Re: [PATCH v5 12/13] scripts: generate_rust_analyzer.py: define scripts

[Tamir Duberstein]

On Thu, Apr 17, 2025 at 3:25 AM Trevor Gross <tmgross@xxxxxxxxx> wrote:
>
> On Tue, Mar 25, 2025 at 3:07 PM Tamir Duberstein <tamird@xxxxxxxxx> wrote:
> >
> > Generate rust-project.json entries for scripts written in Rust. This is
> > possible now that we have a definition for `std` built for the host.
> >
> > Use `str::rstrip` for consistency.
> >
> > Reviewed-by: Daniel Almeida <daniel.almeida@xxxxxxxxxxxxx>
> > Tested-by: Daniel Almeida <daniel.almeida@xxxxxxxxxxxxx>
> > Reviewed-by: Fiona Behrens <me@xxxxxxxxxx>
> > Signed-off-by: Tamir Duberstein <tamird@xxxxxxxxx>
> > ---
> >  scripts/generate_rust_analyzer.py | 16 +++++++++++++++-
> >  1 file changed, 15 insertions(+), 1 deletion(-)
>
> > +    with open(scripts / "Makefile") as f:
> > +        makefile = f.read()
>
> Since we are using pathlib, this can be `makefile = (scripts /
> "Makefile").open().read()`

I assume you mean `makefile = (scripts / "Makefile").read_text()`;
`.open().read()` would leak a file descriptor.

>
> > +    for path in scripts.glob("*.rs"):
> > +        name = path.name.rstrip(".rs")
>
> `stem` should provide the file name without extension, so
>
>     name = path.stem
>
> > @@ -267,7 +281,7 @@ def generate_crates(
> >      for folder in extra_dirs:
> >          for path in folder.rglob("*.rs"):
> >              logging.info("Checking %s", path)
> > -            name = path.name.replace(".rs", "")
> > +            name = path.name.rstrip(".rs")
>
> The above applies here too.
>
> A couple of optional nits if you resend but nothing to block on:
>
> Reviewed-by: Trevor Gross <tmgross@xxxxxxxxx>