Closed kosyak closed 3 years ago
@kosyak wow, this is a great addition! Thanks so much for contributing back your work so that others can benefit ❤️
I'd love to see some unit tests to make sure it keeps working as advertised. Is that something you'd be willing to add? I can help if you run into trouble.
@kosyak This is great, thank you! I would also love to see the docs updated, with maybe an advanced/dynamic XML section explaining usage of this new functionality (similar to your PR description): https://github.com/Rightpoint/BonMot/blob/master/README.md#styling-parts-of-strings-with-xml
Thanks for your support! I totally understand the need of tests and docs for this, will come up with those, won’t hesitate to ask for help :)
@ZevEisenberg I've added unit tests (they're based on testAdditiveFontFeatures
where I've found some code with .xmlRules
). They may be in the wrong place or overcomplicated, please point me to the right direction.
@ZevEisenberg I've pushed fixes that hopefully will resolve some of your concerns.
I'm wondering if we can come up with better names for the new cases?
I'm looking at some Kotlin code right now, and there's lazyMessage
here. We can have
case .lazyStyle(String, ([String: String]) -> StringStyle)
(this is good, I think), .lazyEnter
, .lazyExit
(not so good...)
Yeah, the Kotlin naming doesn't seem to fit too well here. What about styler(String, ([String: String]) -> StringStyle)
to pair with the style(String, StringStyle)
we already have? You could potentially even typealias Styler = ([String: String]) -> StringStyle
.
I'm fine with .styler
. Should we rename .enterBlock
, .exitBlock
?
Damn. I forgot about those. I'd be fine with enterStyler
and exitStyler
. But now styler
by itself seems to be missing something. I'm probably fine with that, unless we can think of a good word for stylerThatIsNeitherEnterNorExit
.
typealias Styler = ([String: String]) -> StringStyle
I'm not sure where to put it - should it be near XMLStyleRule
?. It could be within XMLStyleRule but there's already XMLStyleRule.Styler
https://github.com/Rightpoint/BonMot/pull/391/files#diff-c9b2f909282ce5a1468d61cb0337c704R136.
Also, won't there be a confusion between our freshly renamed XMLStyleRule.styler
and XMLStyleRule.Styler
?
Here's one more naming option: .dynamicStyle
, .dynamicEnter
, .dynamicExit
(or even .deferred...
) - just yo be sure we thought of them all :)
Wow, I'm glad one of the two of us is paying attention! 😅 I haven't worked on this code in a while, so I'm a little rusty. You make excellent points. I'll have a think about it when I've got some time, and I definitely hope to get this in soon.
Thanks for the merge, will work on readme/docs this week.
Hello and thank you for this fine library!
I propose addition of several
XMLStyleRule
s to dynamically generate and apply styles, enter- and exit-insertions of elements.With these changes, you can use them like this:
We have a BonMot patched with these changes for quite some time, they are very convenient for us :)