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

Add string, mutableString methods #2

Closed
wants to merge 1 commit into from
Closed

Conversation

muukii
Copy link

@muukii muukii commented Apr 18, 2016

I wrote an extension of TextAttributes to enable creating NSAttributedStrings directly, like this:

let attributedString = TextAttributes()
    .font(name: "HelveticaNeue", size: 16)
    .foregroundColor(white: 0.2, alpha: 1)
    .lineHeightMultiple(1.5)
    .string("The quick brown fox jumps over the lazy dog")

It will be even more easier to create attributed strings.
How do you think?

@delba
Copy link
Owner

delba commented Apr 22, 2016

Hi @muukii , thanks for the PR!

I'm sorry I won't merge it but I'll do my best to explain why and I hope it'll make sense.

The only "beef" I have with NSAttributedString / NSMutableAttributedString is the attribute dictionary:

  • there is no autocompletion: you have to remember the key names or to look them up in the documentation. It's getting better with the latest versions of Xcode thanks to the fuzzy autocomplete (you can now type NS... and then find your way) but it's still not ideal.
  • the key names are verbose and repetitive (.NS..AttributeName)
  • it's a loosely typed [String: AnyObject] dictionary. So if you want to get the font, you have to write if let font = dictionary[NSFontAttributeName] as? UIFont { print(font) }. It won't accept optionals either as values and you end up writing bangs: [NSFontAttributeName: UIFont(name: "Avenir", size: 14)!]

I like the current way of creating an attributed string though. NSAttributedString("Hello", attributes: attrs) creates a line between the content ("Hello") and the style (attrs).
Separating the content from the style also fosters reusability of the latter:

let base = TextAttributes()
    // setterMethods

label.attributedText = NSAttributedString("hello", attributes: base)

let highlight = base.clone()
    // setterMethods

Thanks again!

@delba delba closed this Apr 22, 2016
@muukii
Copy link
Author

muukii commented Apr 22, 2016

OK!
Thanks!

@muukii
Copy link
Author

muukii commented Apr 22, 2016

@delba
I've created utility library.
TextAttributes is awesome! Thank you!

https://github.com/muukii/TextAttributesUtil

@delba
Copy link
Owner

delba commented Apr 22, 2016

Awesome! Thank you 👍

Would you like me to add it in an Extensions/ folder (that will live outside of the target, like I did for JASON) or would you prefer a link to it in the README ?

@muukii
Copy link
Author

muukii commented Apr 22, 2016

Thank you!

That's great.
Please, add Extensions/!

@delba
Copy link
Owner

delba commented Apr 22, 2016

Could you create a PR please?

Why using a closure though ? Why not your original idea of string(_:) and mutableString(_:) ?

Thanks!

@muukii
Copy link
Author

muukii commented Apr 23, 2016

@delba

OK, I'll create PR.

Why not your original idea of string(​_:) and mutableString(_​:)

I thought better not add a method that ends method chaining, for API consistency.

Why using a closure though

When you need to pass in multiple lines of code, I think it looks better with {} than (). Particulary ending with many ) looks bad.
Also, with closure you can have multiple statements, if you need to.

@muukii
Copy link
Author

muukii commented Apr 23, 2016

@delba

Sorry,
I want to write in README.
Because, Extensions/ files do not include compile.

@delba
Copy link
Owner

delba commented Apr 25, 2016

Hi @muukii ,

No problem, I added TextAttributesUtil in the README 2bccfe4

Thank you!

@muukii
Copy link
Author

muukii commented Apr 25, 2016

@delba Thanks!

@delba delba mentioned this pull request May 1, 2016
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