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

fix(stylize)!: add Stylize impl for String #466

Merged
merged 1 commit into from
Sep 10, 2023
Merged

Conversation

joshka
Copy link
Member

@joshka joshka commented Sep 4, 2023

Although the Stylize trait is already implemented for &str which
extends to String, it is not implemented for String itself. This
commit adds an impl of Stylize that returns a Span<'static> for String
so that code can call Stylize methods on temporary Strings.

E.g. the following now compiles instead of failing with a compile error
about referencing a temporary value:

let s = format!("hello {name}!", "world").red();

BREAKING CHANGE: This may break some code that expects to call Stylize
methods on String values and then use the String value later. This
will now fail to compile because the String is consumed by set_style
instead of a slice being created and consumed.

This can be fixed by cloning the String. E.g.:

let s = String::from("hello world");
let line = Line::from(vec![s.red(), s.green()]); // fails to compile
let line = Line::from(vec![s.clone().red(), s.green()]); // works

Fixes https://discord.com/channels/1070692720437383208/1072907135664529508/1148229700821450833

Although the `Stylize` trait is already implemented for `&str` which
extends to `String`, it is not implemented for `String` itself. This
commit adds an impl of Stylize that returns a Span<'static> for `String`
so that code can call Stylize methods on temporary `String`s.

E.g. the following now compiles instead of failing with a compile error
about referencing a temporary value:

    let s = format!("hello {name}!", "world").red();

BREAKING CHANGE: This may break some code that expects to call Stylize
methods on `String` values and then use the String value later. This
will now fail to compile because the String is consumed by set_style
instead of a slice being created and consumed.

This can be fixed by cloning the `String`. E.g.:

    let s = String::from("hello world");
    let line = Line::from(vec![s.red(), s.green()]); // fails to compile
    let line = Line::from(vec![s.clone().red(), s.green()]); // works
@codecov
Copy link

codecov bot commented Sep 4, 2023

Codecov Report

Merging #466 (f94cec4) into main (343c6cd) will increase coverage by 0.09%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #466      +/-   ##
==========================================
+ Coverage   90.00%   90.10%   +0.09%     
==========================================
  Files          40       40              
  Lines       11156    11267     +111     
==========================================
+ Hits        10041    10152     +111     
  Misses       1115     1115              
Files Changed Coverage Δ
src/style/stylize.rs 100.00% <100.00%> (ø)

@joshka joshka merged commit ebd3680 into main Sep 10, 2023
31 checks passed
@joshka joshka deleted the fix-string-stylize branch September 10, 2023 00:05
@kdheepak kdheepak added the Type: Breaking Change This change will cause application code to break and must be noted in the breaking changes docs etc. label Sep 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Breaking Change This change will cause application code to break and must be noted in the breaking changes docs etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants