WildcatIP / other.js

The Chatternet feature platform
GNU Affero General Public License v3.0
7 stars 1 forks source link

Move URL parsing into core.other.js #107

Open tonygentilcore opened 7 years ago

tonygentilcore commented 7 years ago

Thinking of a client side feature that:

  1. Listens to staged messages updates via existing SET_STAGED_MESSAGE.
  2. Runs it through https://github.com/markdown-it/linkify-it to find links (thanks @ruudderooij for finding this library).
  3. Creates attachments on the staged message for each link via existing UPDATE_STAGED_MESSAGE.

Shouldn't require any new events or anything and this looks a little more robust than our current URL detecting regexes. And, correct me if I'm mistaken @adamrothman, but I think that we should be able to just deploy this w/o coordinating w/ courier b/c it'll just see the URL attachments in the message and then it's detection logic won't take effect. Once both client have this feature, we can then remove the courier behavior.

adamrothman commented 7 years ago

Yup @tonygentilcore you have it right. If courier sees <+ebdd3692d15f4c648a3ac4b98fd0bdca> instead of a URL in the message body it will just move right along.

kharmabum commented 7 years ago

My 10th man is we should focus on more core capabilities of Other.js before adding in support for URL extraction, especially considering it doesn't seem to be a requirement for any of the features we want to add immediately (https://docs.google.com/document/d/1MX1zb2-cwnxrYHxQAFPEvJi7Zwdc4VbGiaQ4n6MrJI8/edit#). My recommendation is we punt this.

ruudderooij commented 7 years ago

Note that client-side link extraction is needed if we want to support links in end-to-end encrypted messages. In that case, it makes sense to centralize the link extraction into other.js, rather than let each client do it on its own.

tonygentilcore commented 7 years ago

@kharmabum So this came up b/c we keep hitting URL extraction bugs and we already have the core capabilities to do this in other.js. So I think it makes sense to just write a feature that wraps that library. It'll give us more experience pushing logic into other.js.