Crossterm Style API-discussion

[TimonPost]

In this issue, we will discuss all the API changes we want to make for crossterm_style.

[TimonPost]

@zrzka in reaction on the question to make fields from ObjectStyle private: "For Object Style I had the following toughts"

If yes, then I've got something like this on my mind ...

#[derive(Debug, Clone, Default)]
pub struct ObjectStyle {
    fg_color: Option<Color>,
    bg_color: Option<Color>,
    attrs: Vec<Attribute>,
}

impl ObjectStyle {
    pub fn apply<D: Display + Clone>(&self, val: D) -> StyledObject<D> {
        StyledObject {
            object_style: self.clone(),
            content: val,
        }
    }

    pub fn new() -> ObjectStyle {
        ObjectStyle::default()
    }

    pub fn background(mut self, color: Color) -> ObjectStyle {
        self.bg_color = Some(color);
        self
    }

    pub fn foreground(mut self, color: Color) -> ObjectStyle {
        self.fg_color = Some(color);
        self
    }

    pub fn attribute(mut self, attr: Attribute) -> ObjectStyle {
        self.attrs.push(attr);
        self
    }
}

In other words ...

• I'd make fields private
• I'd rename apply_to to apply
• I'd remove set_, ... and use just foreground, background, ...
• I'd change add_attr to attribute which should return self as other methods

... it will lead to something like ...

let style = ObjectStyle::new()
    .foreground(Color::Blue)
    .background(Color::Yellow)
    .attribute(Attribute::Italic);

let foo = style.apply("bar");
let bar = style.apply("baz");

[TimonPost]

ObjectStyle is most often not used directly by users. So the changes we make on this type are not affecting many people.

It is very common for a builder pattern to have no set so I have no problem removing that at all. As well after working some time with rust I see more value in having full names as well. So great idea!

One problem, although not big, is when we want to add multiple attributes this is going to look a bit weird:

let style = ObjectStyle::new()
    .foreground(Color::Blue)
    .background(Color::Yellow)
    .attribute(Attribute::Italic);
    .attribute(Attribute::Bold);
    .attribute(Attribute::Crossed);

1. Would you think it would be worth to keep attr, sounds pretty rusty in terms of naming things IMO? This matches the attr() of StyledObject as well
2. Although now that we have the private fields, how do you think we should make the getters for those fields since foreground(), background() etc. are already taken. We could do get_foreground() but that is a bit weird to the name foreground() since get if often left away as well.

[TimonPost]

I have 3 proposals as well:

1. Rename StyledObject to StyledContent
2. Rename ObjectStyle to ContentStyle

My motivation behind this is that StyledObject wraps Displayable content. And this is not really an 'Object'. So by renaming this to 'StyledContent' it is more clear what it is, styled content. The same principle goes for ObjectStyle -> ContentStyle

3. StyledObject contains public private fields as well. We should probably make those private as well.

pub struct StyledObject<D: Display + Clone> {
    object_style: ObjectStyle,
    content: D,
}

Then add the following functions (if change 1,2 as well are approved. Otherwise replace content_style with object_style).

fn content() -> D;
fn content_style() -> ObjectStyle

[zrzka]

Trait names - Styler vs Colorize -> Styler & Colorizer || Style & Colorize.

[TimonPost]

Styler & Colorizer although I have one problem with this, Colorizer is not a word in the dictionary and thus I decided to use Colorize a couple of months ago.

[zrzka]

Attribute::NoInverse should be renamed to Attribute::NoReverse.

[TimonPost]

Actual tasks are moved over here #286

[TimonPost]

Closing, because ideas were merged.