Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify reindent_multiline() signature #14101

Merged
merged 1 commit into from
Feb 7, 2025

Conversation

samueltardieu
Copy link
Contributor

  • reindent_multiline() always returns the result of reindent_multiline_inner() which returns a String. Make reindent_multiline() return a String as well, instead of a systematically owned Cow<'_, str>.
  • There is no reason for reindent_multiline() to force a caller to build a Cow<'_, str> instead of passing a &str directly, especially considering that a String will always be returned.

Also, both the input parameter and return value (of type Cow<'_, str>) shared the same (elided) lifetime for no reason: this worked only because the result was always the Cow::Owned variant which is compatible with any lifetime.

As a consequence, the signature changes from:

fn reindent_multiline(s: Cow<'_, str>,) -> Cow<'_, str> {}

to

fn reindent_multiline(s: &str,) -> String {}

changelog: none

@rustbot
Copy link
Collaborator

rustbot commented Jan 29, 2025

r? @y21

rustbot has assigned @y21.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jan 29, 2025
- `reindent_multiline()` always returns the result of
  `reindent_multiline_inner()` which returns a `String`. Make
  `reindent_multiline()` return a `String` as well, instead of a
  systematically owned `Cow<'_, str>`.
- There is no reason for `reindent_multiline()` to force a caller to
  build a `Cow<'_, str>` instead of passing a `&str` directly,
  especially considering that a `String` will always be returned.

Also, both the input parameter and return value (of type `Cow<'_, str>`)
shared the same (elided) lifetime for no reason: this worked only because
the result was always the `Cow::Owned` variant which is compatible with
any lifetime.

As a consequence, the signature changes from:

```rust
fn reindent_multiline(s: Cow<'_, str>, …) -> Cow<'_, str> { … }
```

to

```rust
fn reindent_multiline(s: &str, …) -> String { … }
```
@samueltardieu
Copy link
Contributor Author

Rebased

Copy link
Member

@y21 y21 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice cleanup 👍

@y21 y21 added this pull request to the merge queue Feb 7, 2025
Merged via the queue into rust-lang:master with commit 4a94ad6 Feb 7, 2025
11 checks passed
@samueltardieu samueltardieu deleted the push-wutpzooolmux branch February 7, 2025 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants