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

StyledObject does not implement Display #33

Closed
Rukenshia opened this issue Sep 20, 2018 · 6 comments
Closed

StyledObject does not implement Display #33

Rukenshia opened this issue Sep 20, 2018 · 6 comments

Comments

@Rukenshia
Copy link
Contributor

I am trying to upgrade from 0.3 to 0.4 but got over 60 errors in my project due to the breaking changes. I do a lot of calls where I just want to have some part of the output colored and use the println! macro a lot.

println!(
    "\nExample:\n\n\taws --profile {} s3 ls\n",
    crossterm.paint(&group.accounts[0].name).with(Color::Yellow)
);

I was trying to update the code with as little change as possible and ended up with attempting this:

println!(
    "\nExample:\n\n\taws --profile {} s3 ls\n",
    style(&group.accounts[0].name).with(Color::Yellow)
);

But that yields the following error

error[E0277]: `crossterm::StyledObject<&str>` doesn't implement `std::fmt::Display`

Any chance to implement Display? I don't really know how the library works so please forgive my ignorance. If this can't be done, I'll have to pull apart all of my print statements and squeeze style()..paint(&screen) between my print calls which definitely would be a huge pain. Maybe there's an easier way if you could guide me to one.

@TimonPost
Copy link
Member

TimonPost commented Sep 20, 2018

Yea It's a know problem unfortunately (if folks used the old API) allot has been changed and almost the whole API has been thrown over. It needed to be done and this crate had some crate benefits from it. If you have a few minutes I'll look into it. But am not sure there was a reason why I had removed it. I'll let you know...

@TimonPost
Copy link
Member

TimonPost commented Sep 20, 2018

Oke I have been experimenting and have come up with something. I can't work further on it now need to go. But I see that this is something that needs to be implemented because it is quite useful to have this kind of functionality. Look at the code below of how I have implemented it for now:


fn main()
{
    let screen = Screen::default();
    println!("\nExample:\n\n\taws --profile {} s3 ls\n", DisplayableObject::new(&screen, &style("test").with(Color::Yellow)));
}

use std::fmt::{Formatter, Error};

pub struct DisplayableObject<'a, D:Display + 'a>
{
    styled_object: &'a StyledObject<D>,
    screen: &'a Screen,
}

impl <'a, D: Display + 'a> DisplayableObject<'a, D>
{
    pub fn new(screen: &'a Screen, styled_object: &'a StyledObject<D>) -> DisplayableObject<'a, D>
    {
        DisplayableObject { screen, styled_object }
    }
}

impl<'a, D: Display + 'a> Display for DisplayableObject<'a, D>
{
    fn fmt(&self, f: &mut Formatter) -> Result<(), Error> {
        self.styled_object.paint(&self.screen);
        return Ok(())
    }
}

This will wrap a StyledObject inside a DisplayableObject which can be used in the write functions. It works but still need to experiment with it a little further. And yea maybe if this is the way I go I'll release the version tomorrow.

And also implementing Display for StyledObject needs to be investigated so work in progresss...

You could copy the code above for now but unfortunately, you still need to update 60 rows :(

@TimonPost
Copy link
Member

TimonPost commented Sep 21, 2018

I think this will be the way to go: I will add another function so you do not need to create the DisplayableObject itself manually instead StyledObject will have this function:

pub fn into_displayable(self, screen: &'a Screen) -> DisplayableObject<'a, D>
{
     return DisplayableObject::new(screen, self)
}

So now you can call it like this:

println!("\nExample:\n\n\taws --profile {} s3 ls\n", style("test").with(Color::Yellow).into_displayable(&screen));

If you have any feedback or you don't like it please let me know. Otherwise tonight I'll release a new version.

@Rukenshia
Copy link
Contributor Author

That looks pretty good to me! Thanks so much for the quick implementation of this @TimonPost

@TimonPost
Copy link
Member

I published the new version with the changes. So I'll close the issue.

@Rukenshia
Copy link
Contributor Author

Upgrade went pretty smooth, thanks for the quick turnaround on this.

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

No branches or pull requests

2 participants