public inbox for pdm-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Wolfgang Bumiller <w.bumiller@proxmox.com>
To: Maximiliano Sandoval <m.sandoval@proxmox.com>
Cc: pdm-devel@lists.proxmox.com
Subject: Re: [pdm-devel] [PATCH proxmox-api-types] generator: Remove needless borrows
Date: Wed, 19 Feb 2025 15:20:34 +0100	[thread overview]
Message-ID: <jzcop4innwagfh6zj55fsjmniqu75xe6ormts7oz42l4vr7ep5@33jooabb7bvx> (raw)
In-Reply-To: <20250219140630.368105-1-m.sandoval@proxmox.com>

NAK:

So this changes

    let url = "some &str";
    self.0.get(&url);

into

    let url = "some &str";
    self.0.get(url);

I mentioned an alternative off-list which is a simpler change to the
generator which we can do once edition 2024 lands (which will hit stable
tomorrow...), that is, unconditionally change the use of `url` to `&url`
and turn the `format!` calls into borrows by prepending `&`. In the new
edition, the temporary will be kept alive, which is IMO a very awkward
and unexpected thing to do, but apparently I'm wrong in that way of
thinking 😄

Iow. we'd change:

    let url = format!("some url{query}");
    self.0.get(&url);

and

    let url = "some &str";
    self.0.get(&url);

into

    let url = &format!("some url{query}");
    self.0.get(url);

and

    let url = "some &str";
    self.0.get(url);

(Note how the `.get(url)` is the same in both cases.)

This way we don't need to keep track of whether the url is owned, and
the generator is already quite messy to look at, so I'd prefer that over
this patch.

On Wed, Feb 19, 2025 at 03:06:30PM +0100, Maximiliano Sandoval wrote:
> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
> ---
>  pve-api-types/generator-lib/Schema2Rust.pm | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/pve-api-types/generator-lib/Schema2Rust.pm b/pve-api-types/generator-lib/Schema2Rust.pm
> index 84f36f6..173d66f 100644
> --- a/pve-api-types/generator-lib/Schema2Rust.pm
> +++ b/pve-api-types/generator-lib/Schema2Rust.pm
> @@ -453,6 +453,7 @@ my sub print_method_without_body : prototype($$$$$) {
>      }
>  
>      my $input;
> +    my $is_url_owned = 0;
>      if (defined($input = $def->{input_type})) {
>          print {$out} "    params: $input,\n";
>          print {$out} ") -> Result<$def->{output_type}, Error> {\n";
> @@ -486,6 +487,7 @@ my sub print_method_without_body : prototype($$$$$) {
>                  print {$out} "    add_query_arg(&mut query, &mut sep, \"$name\", &p_$rust_name);\n";
>              }
>          }
> +        $is_url_owned = 1;
>          print {$out} "    let url = format!(\"/api2/extjs$def->{url}\{query}\");\n";
>      } elsif (defined($input = $def->{input})) {
>          for my $arg ($input->@*) {
> @@ -510,6 +512,7 @@ my sub print_method_without_body : prototype($$$$$) {
>                      print {$out} "    add_query_arg(&mut query, &mut sep, \"$name\", &$rust_name);\n";
>                  }
>              }
> +            $is_url_owned = 1;
>              print {$out} "    let url = format!(\"/api2/extjs$def->{url}\{query}\");\n";
>          } else {
>              print {$out} "    let url = \"/api2/extjs$def->{url}\";\n";
> @@ -523,7 +526,8 @@ my sub print_method_without_body : prototype($$$$$) {
>          print {$out} "    let url = \"/api2/extjs$def->{url}\";\n";
>      }
>  
> -    my $call = return_expr($def, "self.0.$method(&url).await?");
> +    my $maybe_borrowed_url = $is_url_owned ? '&url' : 'url';
> +    my $call = return_expr($def, "self.0.$method($maybe_borrowed_url).await?");
>      print {$out} "    $call\n";
>  
>      print {$out} "}\n\n";
> @@ -554,7 +558,7 @@ my sub print_method_with_body : prototype($$$$$) {
>      }
>      # print {$out} "    // self.login().await?;\n";
>      print {$out} "    let url = \"/api2/extjs$def->{url}\";\n";
> -        my $call = return_expr($def, "self.0.${method}(&url, &params).await?");
> +        my $call = return_expr($def, "self.0.${method}(url, &params).await?");
>          print {$out} "    $call\n";
>      print {$out} "}\n\n";
>  }
> -- 
> 2.39.5


_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel

      reply	other threads:[~2025-02-19 14:20 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-19 14:06 Maximiliano Sandoval
2025-02-19 14:20 ` Wolfgang Bumiller [this message]

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=jzcop4innwagfh6zj55fsjmniqu75xe6ormts7oz42l4vr7ep5@33jooabb7bvx \
    --to=w.bumiller@proxmox.com \
    --cc=m.sandoval@proxmox.com \
    --cc=pdm-devel@lists.proxmox.com \
    /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
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal