Add ability to add child BONChainables to style text between tags. #144

Imperiopolis · 2016-05-12T03:35:00Z

This adds the ability to parse and style substrings between arbitrary tags with assigned BONChainable objects. This potentially resolves #69.

Excerpt from the readme:

Tag Styles

BonMot can style text between arbirtrary tags using a <tag></tag> format and \ as an escape character.

BONChain *boldChain = BONChain.new.fontNameAndSize(@"Baskerville-Bold", 15.0f);
BONChain *italicChain = BONChain.new.fontNameAndSize(@"Baskerville-Italic", 15.0f);

BONChain *chain = BONChain.new.fontNameAndSize(@"Baskerville", 17.0f)
    .tagStyles(@[BONTagMake(@"bold", boldChain), BONTagMake(@"italic", italicChain)])
    .string(@"<bold>This text is wrapped in a \\<bold> tag.</bold>\n<italic>This text is wrapped in an \\<italic> tag.</italic>");

NSAttributedString *string = chain.attributedString;

Outputs:

BonMot can also style text between any arbitrary start and end strings using any escape string.

BONChain *boldChain = BONChain.new.fontNameAndSize(@"Baskerville-Bold", 15.0f);
BONChain *italicChain = BONChain.new.fontNameAndSize(@"Baskerville-Italic", 15.0f);

BONChain *chain = BONChain.new.fontNameAndSize(@"Baskerville", 17.0f)
.tagStyles(@[BONTagComplexMake(@"~start~", @"!end", @"escape", boldChain)])
.string(@"~start~This text is wrapped in a escape~start~ tag.!end");

NSAttributedString *string = chain.attributedString;

Note: Tag styles do not support nested or interleaved tags. The first tag matched will be applied, any additional tags between the start end end will be ignored.

ZevEisenberg · 2016-05-12T15:28:57Z

@Imperiopolis cool, thanks for posting this! I'm looking forward to digging in to it.

A question to start things off: why do you have the option to supply custom start, end and escape characters? Is it something you're using in your app? I ask because the syntax for .tagStyles could be simplified if you don't have to wrap tag/chain pairs in a constructor:

.tagStyles(@{@"bold": boldChain, @"italic": italicChain})

Of course, this looks better in Swift:

.tagStyles(["bold": boldChain, "italic": italicChain])

ZevEisenberg · 2016-05-12T15:30:54Z

Also, taking a straight dictionary of [String: BONTextable] also makes it easier to compose things. Your app could have a base dictionary with bold, italics, etc., and then if you need a special style at a particular call site, you could do:

.tagStyles(Appearance.defaultStyles + ["specialCase": specialChain])

(Can you tell I'm ready to be done with Obj-C?)

Imperiopolis · 2016-05-12T16:22:34Z

I initially wrote this exactly as you described, but as I was tidying things up I realized that the system was flexible enough to allow any arbitrary tag and it seemed worth exposing.

Some use cases I see for this:

Parse tags that don't use <> – stuff like markdown style *text*, etc
Allow for more complex tag handling – thinking stuff like <span class="a-thing">text</span>

For my particular uses, the former is not super useful but I intend to use the latter. I am utilizing this as a lightweight sort of HTML parser, such that my copy can essentially be shared between mobile and web with web using CSS styles and mobile using BonMot.

There are other libraries with more fully flushed out ways to handle complex HTML like this (utilizing CSS), but they all seem to incur a pretty significant performance hit to build out the full DOMTree. Since the styling I need to support is quite simple, I built out this lightweight version in BonMot to accommodate.

All that said, I think it could still be reasonably easy to accommodate your dictionary method by adding a function that takes the dictionary and converts it to the appropriate simple BONTag objects. I'm not sure I feel a solid need for doing that though. With this method, composition is still relatively straight forward.

.tagStyles(Appearance.defaultStyles + [BONTagMake("specialCase", specialChain)])

Will need to think on it a bit.

Imperiopolis · 2016-05-12T17:29:10Z

I added in a BONTagsFromDictionary function which allows doing something like

.tagStyles(BONTagsFromDictionary(Appearance.defaultStyles + ["specialCase": specialChain]))

Also considering adding a .tagStylesDictionary property to allow for something like the following that automatically does the conversion and updates .tagStyles

.tagStylesDictionary(Appearance.defaultStyles + ["specialCase": specialChain])

ZevEisenberg · 2016-05-13T22:34:31Z

Also considering adding a .tagStylesDictionary property

I like this. Maybe make the default one, which takes a [String : BONChain] or [String : BONTextable], be .tagStyles, and the advanced/complex one be .advancedTagStyles or .customTagStyles or .complexTagStyles? I'd prefer the basic API to look simple, rather than defined by the absence of the complex API.

Imperiopolis · 2016-05-13T23:32:38Z

That makes sense. I'll make that tweak :)

On May 13, 2016, at 15:34, Zev Eisenberg notifications@github.com wrote:

Also considering adding a .tagStylesDictionary property

I like this. Maybe make the default one, which takes a [String : BONChain] or [String : BONTextable], be .tagStyles, and the advanced/complex one be .advancedTagStyles or .customTagStyles or .complexTagStyles? I'd prefer the basic API to look simple, rather than defined by the absence of the complex API.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub

…mplexStyles to handle advanced cases.

ZevEisenberg · 2016-05-17T15:36:48Z

Example/BonMot/Cells/TagStylesCell.m

+    BONChain *italicChain = BONChain.new.fontNameAndSize(@"Baskerville-Italic", 15);
+
+    BONChain *baseChain = BONChain.new.fontNameAndSize(@"Baskerville", 17)
+                              .tagStyles(@[ BONTagMake(@"bold", boldChain), BONTagMake(@"italic", italicChain) ])


#req build warning: Incompatible pointer types passing 'NSArray *' to parameter of type 'NSDictionary<NSString *,id<BONTextable>> * _Nullable'

^ this results in a crash if you scroll down to the bottom in the sample app.

Ooops, forgot to update this one when I switched the default to take a dictionary.

ZevEisenberg · 2016-05-17T16:06:16Z

@Imperiopolis it would be nice if <bold>This text</> worked, or possibly even any </*>. If you disagree, I'd be interested to hear why, since I'm new to parsers and grammars, and I may be missing things. I was thinking that <bold>some bold text</> and much later, some <italic>italic</> text would be a convenient way to write stuff.

ZevEisenberg · 2016-05-17T16:09:10Z

Example/BonMot.xcodeproj/project.pbxproj

@@ -66,7 +64,12 @@
 		CD6DF05A1BF6B98500676E2D /* NSDictionary+BONEquality.m in Sources */ = {isa = PBXBuildFile; fileRef = CD6DF04F1BF6B53100676E2D /* NSDictionary+BONEquality.m */; };
 		CD78701E1CA5F8EC00B2714F /* EBGaramond12-Regular.otf in Resources */ = {isa = PBXBuildFile; fileRef = CD6DF03E1BF6AFAA00676E2D /* EBGaramond12-Regular.otf */; };
 		CDE658581C24ADD8009C7D09 /* BONUnderlineAndStrikethroughTestCase.m in Sources */ = {isa = PBXBuildFile; fileRef = CDE658571C24ADD8009C7D09 /* BONUnderlineAndStrikethroughTestCase.m */; };
+		EC2F199E1CEA324900D9F1FE /* BONTagDictionaryStylesTestCase.m in Sources */ = {isa = PBXBuildFile; fileRef = EC2F199C1CEA324300D9F1FE /* BONTagDictionaryStylesTestCase.m */; };
+		EC433DB31C88B7D9001B3ABE /* BONTagStylesTestCase.m in Sources */ = {isa = PBXBuildFile; fileRef = EC433DB11C88B65B001B3ABE /* BONTagStylesTestCase.m */; };


#req rename the test case files to match what they're testing (they weren't updated when you changed the names of the tag style properties).

Imperiopolis · 2016-05-17T17:30:24Z

@Imperiopolis it would be nice if This text</> worked, or possibly even any </*>. If you disagree, I'd be interested to hear why, since I'm new to parsers and grammars, and I may be missing things. I was thinking that some bold text</> and much later, some italic</> text would be a convenient way to write stuff.

I see a few advantages to having an explicit close tag for all start tags:

Allows for pseudo HTML parsing (things like <strong></strong>) which allows text to be shared across platforms
Prepares for future support of nested tags
Compared to a </*> version, drastically simplifies matching logic to not need to do any wildcard searching

That said, there's nothing stopping this method from being used as <bold></> and if you have a strong argument for it, I'm okay with changing the defaults to work that way. My gut feeling is these default should be HTML-esque, though.

ZevEisenberg · 2016-05-17T17:41:16Z

Shared parsing across platforms is a good case. I don't think nested tags is, though: you can easily do nested tags with </>, which just closes the innermost tag. I rescind my idea to support </*>, because it sounds messy and potentially misleading.

How bad/slow would it make the code to support </>? I'm not super attached to the idea, however. Could be an added feature later, so feel free to ignore it.

Imperiopolis · 2016-05-17T17:53:14Z

Yes, I suppose you can do nested tags with </>, I just really prefer the explicit nature of specific closed tags.

For example, it's pretty unclear what's happening here:
this is a <bold>bold thing with some <italic>bold italic</> stuff</> and regular stuff</>

Which one of these was this meant to be?
this is a bold thing with some bold italic stuff and regular stuff</>
this is a bold thing with some bold italic stuff and regular stuff</>

What was the extraneous trailing </> for? Did I forget a start tag or..?

Where as with explicit tags, you can quickly realize that "oh, forgot the opening h2"
this is a <bold>bold thing with some <italic>bold italic</italic> stuff</bold> and regular stuff</h2>

And that this pretty clearly produces:
this is a bold thing with some bold italic stuff and regular stuff

How bad/slow would it make the code to support </>?

There is no performance impact switching to </> vs a more explicit </bold> – that impact was really only for the </*> route. This should just work if you use </> as the endTag on BONTag

ZevEisenberg · 2016-05-17T18:06:08Z

@Imperiopolis OK, you've convinced me with the "makes editing harder" argument. Let's leave </> out for now. We can always add it as an enhancement later without breaking anything.

ZevEisenberg · 2016-05-17T18:41:13Z

Pod/Classes/BONTag_Private.h

+ *
+ *  @return An array of ranges representing the escaped tags.
+ */
+ (BONNonnull BONGeneric(NSArray, NSValue *) *)escapedRangesInString:(NSString *BONCNonnull *BONCNonnull)string withTags:(BONNonnull BONGeneric(NSArray, BONTag *) *)tags stripEscapeCharacters:(BOOL)stripEscapeCharacters;


#req document the fact that *string is mutated to contain the version of the string with unescaped tags stripped.

ZevEisenberg · 2016-05-17T18:43:12Z

Pod/Classes/BONTag.m

+
+#pragma mark - Tag Matching
+
+ (NSArray *)escapedRangesInString:(NSString **)string withTags:(NSArray *)tags stripEscapeCharacters:(BOOL)stripEscapeCharacters


#req please search the project for NSArray * and replace with BONGeneric

ZevEisenberg · 2016-05-17T18:49:02Z

@Imperiopolis is it intentional that you can have different escape characters for each tag in a scenario where you're using complex tags? What's the use case?

ZevEisenberg · 2016-05-17T18:55:47Z

Pod/Classes/BONTag.m

+    }
+}
+
+ (NSArray *)rangesInString:(NSString **)string betweenTags:(NSArray *)tags stripTags:(BOOL)stripTags


#? why would you ever not strip tags? You never pass NO here, in the tests or the example app.

Good question, there really is no reason I can think of. Will remove this code path.

ZevEisenberg · 2016-05-17T19:01:41Z

@Imperiopolis I may have missed something, but I don't think you test escaping the escape character anywhere. \\ would be the simplest example of this, but I didn't see \\\\, which you would need to express that, in the source code.

ZevEisenberg · 2016-05-17T19:18:15Z

Pod/Classes/BONTag.m

+
+        [escapedRanges addObject:[NSValue valueWithRange:range]];
+
+        searchRange = NSMakeRange(NSMaxRange(range), [theString length] - NSMaxRange(range));


#pp theString.length for idempotent getters

ZevEisenberg · 2016-05-17T20:01:42Z

@Imperiopolis OK, I’m done with my initial pass at the code review. Tiny tweaks may crop up, but in general it’s looking fantastic. Thanks so much for all the work you've put into this!

Imperiopolis · 2016-05-17T20:27:59Z

@Imperiopolis I may have missed something, but I don't think you test escaping the escape character anywhere. \ would be the simplest example of this, but I didn't see \, which you would need to express that, in the source code.

Escaping the escape character isn't something this currently supports – what's the goal of this? To handle if for some reason you needed a \ right before a tag, but didn't want to escape?

ZevEisenberg · 2016-05-17T20:39:55Z

I think the major reason to support escaping the escape character is because people would expect that to work. I tried putting \\ in a string and it came out as \, so maybe it already works? Either way, I think the behavior should be defined and tested. You should be able to use the escape character in the string, definitely by escaping it, and if you don't have to escape it, that's cool too, I guess, although it's a bit weird.

Imperiopolis · 2016-05-17T20:44:05Z

@Imperiopolis is it intentional that you can have different escape characters for each tag in a scenario where you're using complex tags? What's the use case?

Yes. I'm not really sure there is a great use case for this, but since these are complex options I figured it'd be nice to expose it if someone needed it.

ZevEisenberg · 2016-05-17T20:45:44Z

Are there tests for cases with different escapes for different tags?

Imperiopolis · 2016-05-17T21:28:43Z

Are there tests for cases with different escapes for different tags?

There are now!

Imperiopolis force-pushed the nt-tag-styling branch from a5312f4 to 52fb458 Compare May 12, 2016 04:06

Add ability to add child BONChainables to style text between tags.

0102251

Imperiopolis force-pushed the nt-tag-styling branch from 52fb458 to 0102251 Compare May 12, 2016 17:27

Change tagStyles property on BONChain to take a dictionary, add tagCo…

993f2a1

…mplexStyles to handle advanced cases.

Imperiopolis force-pushed the nt-tag-styling branch from 1f56e1e to 993f2a1 Compare May 16, 2016 16:58

ZevEisenberg reviewed May 17, 2016
View reviewed changes

Imperiopolis force-pushed the nt-tag-styling branch 3 times, most recently from b8ea5ba to 624ca58 Compare May 17, 2016 21:28

Cleanup based on Zev's PR review.

f2350f2

Imperiopolis force-pushed the nt-tag-styling branch from 624ca58 to f2350f2 Compare May 18, 2016 01:25

Mark BONTagMake functions unavailable in swift.

d5d617f

ZevEisenberg merged commit 5a7f599 into Rightpoint:master May 20, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ability to add child BONChainables to style text between tags. #144

Add ability to add child BONChainables to style text between tags. #144

Imperiopolis commented May 12, 2016 •

edited

ZevEisenberg commented May 12, 2016 •

edited

ZevEisenberg commented May 12, 2016

Imperiopolis commented May 12, 2016 •

edited

Imperiopolis commented May 12, 2016

ZevEisenberg commented May 13, 2016

Imperiopolis commented May 13, 2016

ZevEisenberg May 17, 2016

ZevEisenberg May 17, 2016 •

edited

Imperiopolis May 17, 2016

ZevEisenberg commented May 17, 2016

ZevEisenberg May 17, 2016

Imperiopolis commented May 17, 2016

ZevEisenberg commented May 17, 2016

Imperiopolis commented May 17, 2016 •

edited

ZevEisenberg commented May 17, 2016

ZevEisenberg May 17, 2016

ZevEisenberg May 17, 2016

ZevEisenberg commented May 17, 2016

ZevEisenberg May 17, 2016

Imperiopolis May 17, 2016

ZevEisenberg commented May 17, 2016

ZevEisenberg May 17, 2016

ZevEisenberg commented May 17, 2016

Imperiopolis commented May 17, 2016

ZevEisenberg commented May 17, 2016

Imperiopolis commented May 17, 2016 •

edited

ZevEisenberg commented May 17, 2016

Imperiopolis commented May 17, 2016


		#pragma mark - Tag Matching

		+ (NSArray )escapedRangesInString:(NSString )string withTags:(NSArray )tags stripEscapeCharacters:(BOOL)stripEscapeCharacters


		[escapedRanges addObject:[NSValue valueWithRange:range]];

		searchRange = NSMakeRange(NSMaxRange(range), [theString length] - NSMaxRange(range));

Add ability to add child BONChainables to style text between tags. #144

Add ability to add child BONChainables to style text between tags. #144

Conversation

Imperiopolis commented May 12, 2016 • edited

Tag Styles

ZevEisenberg commented May 12, 2016 • edited

ZevEisenberg commented May 12, 2016

Imperiopolis commented May 12, 2016 • edited

Imperiopolis commented May 12, 2016

ZevEisenberg commented May 13, 2016

Imperiopolis commented May 13, 2016

Choose a reason for hiding this comment

ZevEisenberg May 17, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ZevEisenberg commented May 17, 2016

Choose a reason for hiding this comment

Imperiopolis commented May 17, 2016

ZevEisenberg commented May 17, 2016

Imperiopolis commented May 17, 2016 • edited

ZevEisenberg commented May 17, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ZevEisenberg commented May 17, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ZevEisenberg commented May 17, 2016

Choose a reason for hiding this comment

ZevEisenberg commented May 17, 2016

Imperiopolis commented May 17, 2016

ZevEisenberg commented May 17, 2016

Imperiopolis commented May 17, 2016 • edited

ZevEisenberg commented May 17, 2016

Imperiopolis commented May 17, 2016

Imperiopolis commented May 12, 2016 •

edited

ZevEisenberg commented May 12, 2016 •

edited

Imperiopolis commented May 12, 2016 •

edited

ZevEisenberg May 17, 2016 •

edited

Imperiopolis commented May 17, 2016 •

edited

Imperiopolis commented May 17, 2016 •

edited