[DetectorType][MessageLabel] Add custom, mention and hashtag detector type 👻 by JulienKode · Pull Request #913 · MessageKit/MessageKit

JulienKode · 2018-10-16T15:29:46Z

Adding other DetectorTypes such as:

.mention
.hashtag
.custom(regex: NSRegularExpression)

Others elements of the enum are made with NSDataDetector which inherit from NSRegularExpression

Theses three new element use only NSRegularExpression

We can discuss of the API or changes to make here

nathantannar4

We need some ample test cases to ensure the regex matches all cases for hashtag and mention

nathantannar4 · 2018-10-16T21:36:00Z

Please make sure your branch, JulienKode:CustomDetectorType is up to date with the latest development branch

SD10 · 2018-10-16T22:23:46Z

This looks promising ❤️ Thanks for working on this @JulienKode 🥇 💯

JulienKode · 2018-10-17T21:12:02Z

@nathantannar4 Thanks it's now sync with the development branch

@SD10 I've change it into a var

There now the testing to do I've see that there are already test for DetectorType in MessageLabelSpec but they're all commented. Anyone know the reason ?

If I uncomment theses tests they do not pass but they are right.
If any body know a limitation about that I'm interested
I'll try to fix that soon

talanov

Could you, please, squash commits into 1?

JulienKode · 2018-10-18T08:45:27Z

@talanov for sure

nathantannar4 · 2018-10-26T00:54:57Z

Very close! I'd like to see the Example updated so people can see these detectors in action 😊

…n pattern [DetectorType] Put mention and hashtag as var [CHANGELOG] Update [DetectorType] Update to have at least 4 characters and more [CHANGELOG] Update [DetectorType] [MessageLabel] run NSDataDetector once [MessageLabel] Run NSDataDetector once [Testing] Add testing and pass rangesOfDetectors as internal [Refactoring] [Filter] only .custom [Examples]

JulienKode · 2018-10-26T16:31:38Z

@nathantannar4 I’ve added examples

_{Sent with GitHawk}

sonnyio · 2018-10-31T19:17:42Z

Hey guys, in MessageLabel.swift, it's calling handleHashtag when it should be calling handleMention. Looking forward to this commit being pulled in!

MessageKit/Sources/Views/MessageLabel.swift

Line 493 in 77e9ccd

handleHashtag(match)

JulienKode · 2018-10-31T19:40:37Z

@strujillojr Thanks, It's fixed now

nathantannar4

@JulienKode can you please add the delegate methods in the Example so users know they exist?

func didSelectHashtag(_ hashtag: String) {
        print("Hashtag selected: \(hashtag)")
    }

    func didSelectMention(_ mention: String) {
        print("Mention selected: \(mention)")
    }

    func didSelectCustom(_ pattern: String, match: String?) {
        print("Custom data detector patter selected")
    }

nathantannar4 · 2018-10-31T22:44:44Z

After example changes it LGTM 💯 @SD10 Should we include this in 2.0.0?

…into CustomDetectorType

… into CustomDetectorType

JulienKode · 2018-11-01T10:47:54Z

@nathantannar4 I've update the branch to the latest development branch and updated the examples

SD10 · 2018-11-02T03:39:55Z

This can't be in 2.0 because we already did a beta. You can't have anything other than bugfixes in a beta or it defeats the purpose of the beta lol. This is an API breaking change so I changed it to be part of the 3.0 milestone.

SD10 · 2018-11-02T03:40:54Z

-
+        case Hashtag = 8
+        case Mention = 9
+        case Custom = 10


Let's make all these cases lowercased, it is not very Swifty 😆 If you could take care of all of them 💯

That’s true, It’s not really swift, may I changes all of them in this PR or create another one ? To change others

It can all be done in this PR 👍

SD10 · 2018-11-02T03:47:35Z

-        let detector = try? NSDataDetector(types: checkingTypes)
        let range = NSRange(location: 0, length: text.length)
-        let matches = detector?.matches(in: text.string, options: [], range: range) ?? []
+        var matches =  [NSTextCheckingResult]()


Nit: fix this double space please

I've did it, thanks

JulienKode · 2018-11-12T19:56:07Z

@SD10 @nathantannar4

I've made some benchmarks here is the results

Here is the configuration of the first test:

Here is the configuration of the second test

What do you think ?

nathantannar4 · 2018-11-12T20:06:06Z

What are the graphs showing? FPS? CPU %?

_{Sent with GitHawk}

JulienKode · 2018-11-12T20:48:26Z

@nathantannar4 FPS in purple
and upper it's the CPU

…orType

SD10 · 2018-11-28T02:48:37Z

@JulienKode It does look like adding those code has some major spikes in the CPU 😞

nathantannar4 · 2018-11-28T03:52:41Z

It is optional, personally in my apps I wouldn't use this, I would use attributed strings. But this is a nice to have feature for ease of use

_{Sent with GitHawk}

JulienKode · 2018-11-28T06:26:05Z

Ok, I understand, but It’s only in the worst case, if you just handle hashtag the perf is the same

I know it's a sensitive part of the library

If you have any idea of change to improve the performance. I want to know it

_{Sent with GitHawk}

nathantannar4 · 2019-03-01T03:24:42Z

Going to merge this into the 3.0.0-beta branch. While it does come at a performance hit I am going to leave it to the end user to decide if they want this since without using the new detectors there is no difference.

JulienKode · 2019-03-11T12:38:47Z

@nathantannar4 ok it makes sense, if someone have an idea to improve the speed, we also can improve it

JulienKode changed the title ~~[DetectorType][MessageLabel] Add custom detector type~~ [DetectorType][MessageLabel] Add custom, mention and hashtag detector type Oct 16, 2018

nathantannar4 requested changes Oct 16, 2018

View reviewed changes

Comment thread Sources/Models/DetectorType.swift Outdated

nathantannar4 changed the base branch from master to development October 16, 2018 21:35

SD10 reviewed Oct 16, 2018

View reviewed changes

Comment thread Sources/Models/DetectorType.swift Outdated

JulienKode force-pushed the CustomDetectorType branch from 2096ee1 to c71e599 Compare October 17, 2018 18:57

JulienKode changed the title ~~[DetectorType][MessageLabel] Add custom, mention and hashtag detector type~~ [DetectorType][MessageLabel] Add custom, mention and hashtag detector type 👻 Oct 17, 2018

JulienKode mentioned this pull request Oct 17, 2018

Is there a way to implement own detectors? #903

Closed

SD10 reviewed Oct 18, 2018

View reviewed changes

Comment thread CHANGELOG.md

talanov requested changes Oct 18, 2018

View reviewed changes

JulienKode force-pushed the CustomDetectorType branch from 701ba59 to fe11a31 Compare October 18, 2018 08:44

JulienKode force-pushed the CustomDetectorType branch from fe11a31 to 39bdde9 Compare October 18, 2018 09:16

SD10 reviewed Oct 19, 2018

View reviewed changes

Comment thread Sources/Models/DetectorType.swift Outdated

SD10 reviewed Oct 19, 2018

View reviewed changes

Comment thread CHANGELOG.md

Comment thread Sources/Models/DetectorType.swift Outdated

Comment thread Sources/Views/MessageLabel.swift Outdated

JulienKode force-pushed the CustomDetectorType branch from cc14069 to 1ae82ed Compare October 19, 2018 07:04

JulienKode commented Oct 19, 2018

View reviewed changes

Comment thread Sources/Views/MessageLabel.swift

JulienKode commented Oct 19, 2018

View reviewed changes

Comment thread Sources/Views/MessageLabel.swift

JulienKode commented Oct 25, 2018

View reviewed changes

Comment thread Sources/Views/MessageLabel.swift Outdated

nathantannar4 reviewed Oct 26, 2018

View reviewed changes

Comment thread Sources/Views/MessageLabel.swift Outdated

Comment thread Sources/Views/MessageLabel.swift Outdated

Comment thread Sources/Views/MessageLabel.swift Outdated

Comment thread Sources/Views/MessageLabel.swift Outdated

nathantannar4 added this to the 2.1 milestone Oct 26, 2018

nathantannar4 added enhancement under review labels Oct 26, 2018

JulienKode force-pushed the CustomDetectorType branch from c66ee49 to 77e9ccd Compare October 26, 2018 10:15

Fix handleMention call

8fb0600

nathantannar4 requested changes Oct 31, 2018

View reviewed changes

JulienKode added 3 commits November 1, 2018 10:46

Merge branch 'development' of https://github.com/MessageKit/MessageKit …

5593f5e

…into CustomDetectorType

[Examples] Add delegate methods

9a64ebf

Merge branch 'CustomDetectorType' of github.com:JulienKode/MessageKit…

cf11471

… into CustomDetectorType

SD10 modified the milestones: 2.1, 3.0 Nov 2, 2018

SD10 reviewed Nov 2, 2018

View reviewed changes

SD10 added the API breaking label Nov 2, 2018

JulienKode added 2 commits November 2, 2018 11:17

[Update]

d238e09

[Examples] Update MessageTypes enum

d7df6fd

Merge remote-tracking branch 'upstream/development' into CustomDetect…

60c10d6

…orType

nathantannar4 changed the base branch from development to 3.0.0-beta March 1, 2019 03:19

Merge branch '3.0.0-beta' into CustomDetectorType

7555e0a

nathantannar4 approved these changes Mar 1, 2019

View reviewed changes

nathantannar4 merged commit a465437 into MessageKit:3.0.0-beta Mar 25, 2019

Conversation

JulienKode commented Oct 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nathantannar4 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nathantannar4 commented Oct 16, 2018

Uh oh!

Uh oh!

SD10 commented Oct 16, 2018

Uh oh!

JulienKode commented Oct 17, 2018

Uh oh!

Uh oh!

talanov left a comment

Choose a reason for hiding this comment

Uh oh!

JulienKode commented Oct 18, 2018

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nathantannar4 commented Oct 26, 2018

Uh oh!

JulienKode commented Oct 26, 2018

Uh oh!

sonnyio commented Oct 31, 2018

Uh oh!

JulienKode commented Oct 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nathantannar4 left a comment

Choose a reason for hiding this comment

Uh oh!

nathantannar4 commented Oct 31, 2018

Uh oh!

JulienKode commented Nov 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SD10 commented Nov 2, 2018

Uh oh!

SD10 Nov 2, 2018

Choose a reason for hiding this comment

Uh oh!

JulienKode Nov 2, 2018

Choose a reason for hiding this comment

Uh oh!

SD10 Nov 2, 2018

Choose a reason for hiding this comment

Uh oh!

SD10 Nov 2, 2018

Choose a reason for hiding this comment

Uh oh!

JulienKode Nov 2, 2018

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JulienKode commented Nov 12, 2018

Uh oh!

nathantannar4 commented Nov 12, 2018

Uh oh!

JulienKode commented Nov 12, 2018

Uh oh!

SD10 commented Nov 28, 2018

Uh oh!

nathantannar4 commented Nov 28, 2018

Uh oh!

JulienKode commented Oct 16, 2018 •

edited

Loading

JulienKode commented Oct 31, 2018 •

edited

Loading

JulienKode commented Nov 1, 2018 •

edited

Loading

JulienKode commented Nov 28, 2018 •

edited

Loading

JulienKode commented Mar 11, 2019 •

edited

Loading