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

Remove dependency on AsRef trait for TextBuffer #1824

Merged
merged 1 commit into from Aug 2, 2022

Conversation

Kryptos-FR
Copy link
Contributor

Summary
Remove the dependency on the AsRef<str> trait for TextBuffer.

Rationale
That dependency is not needed as it is trivial to implement fn as_str(&self) -> &str for both str and String which are the default implementations provided by egui and very likely the one that most user will ever need. Then TextEdit can just use as_str() in places where it was using as_ref() before, which in my opinion makes it more consistent with the trait definition.

In one of the project I am working on we did implement a custom TextBuffer for our struct Var. The forced dependency on AsRef<str> caused some pains because:

  • our struct Var is a kind of container that can have all kind of underlying data type (not only string).
  • in other places, it forced us to use the turbofish where it was not needed before:
// before implementing TextBuffer for Var
let headers: &mut Var = self.some_function();
let mut headers: Table = headers.as_ref().try_into()?; // here it uses the usual as_ref()

// after
let headers: &mut Var = self.some_function();
let mut headers: Table = <&mut Var as AsRef<Var>>::.as_ref(&headers).try_into()?; // here as_ref() was ambiguous

We anticipate that other occurrences will appear over time and we would like to avoid them.

Checklist

  • It is a trivial change.
  • I have run ./sh/check.sh.
    • There are errors but unrelated to this change.

Copy link
Owner

@emilk emilk left a comment

Choose a reason for hiding this comment

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

Sounds good to me!

@Kryptos-FR Kryptos-FR marked this pull request as ready for review July 26, 2022 08:32
@emilk emilk merged commit e288ca8 into emilk:master Aug 2, 2022
@Kryptos-FR Kryptos-FR deleted the textbuffer-trait-improvement branch August 2, 2022 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants