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 mentions plugin #111

Merged
merged 3 commits into from Aug 16, 2016
Merged

Add mentions plugin #111

merged 3 commits into from Aug 16, 2016

Conversation

marksyzm
Copy link
Contributor

@marksyzm marksyzm commented Feb 4, 2016

Added mentions plugin, templates, tests

@marksyzm
Copy link
Contributor Author

marksyzm commented Feb 4, 2016

Do you need me to add the documentation as well? I can do that in a separate PR

@nfrasser
Copy link
Collaborator

Hey @marksyzm, thanks for adding this in, and my apologies for not getting to it sooner.

The main issue I have with this (it's actually more of a problem with* how Linkify is currently designed) is that these mentions don't allow characters other than those valid in domain names, which is represented by the DOMAIN text token. A username may include other non-domain characters like underscore. Other than that, these changes look good.

The plugin system hasn't been fully built out yet, and part of what has to be done is allowing hooks into the internal state machine to define new characters like _. Until then, we'd have to define the underscore (I'd be satisfied allowing at least _) explicitly here and add appropriate state transitions from domains sometime before this line

I'm going to be working on the plugin system next week, but you're welcome to try getting _ in there!

@marksyzm
Copy link
Contributor Author

Will do! What about utf8 characters? Might have to think up some sort of
configuration for it, maybe.

On Thu, 11 Feb 2016, 00:37 Nick Frasser notifications@github.com wrote:

Hey @marksyzm https://github.com/marksyzm, thanks for adding this in,
and my apologies for not getting to it sooner.

The main issue I have with this (it's actually more of a problem on how
Linkify is currently designed) is that these mentions don't allow characters
other than those valid in domain names
, which is represented by the
DOMAIN text token. A username may include other non-domain characters
like underscore. Other than that, these changes look good.

The plugin system hasn't been fully built out yet, and part of what has to
be done is allowing hooks into the internal state machine to define new
characters like _. Until then, we'd have to define the underscore (I'd be
satisfied allowing at least _) explicitly here
https://github.com/SoapBox/linkifyjs/blob/master/src/linkify/core/tokens.js
and add appropriate state transitions from domains sometime before this
line
https://github.com/SoapBox/linkifyjs/blob/233a6c0dc417d30fe434175b710d7e8037b6538f/src/linkify/core/scanner.js#L121

I'm going to be working on the plugin system next week, but you're welcome
to try getting _ in there!


Reply to this email directly or view it on GitHub
#111 (comment).

Regards,
Mark Elphinstone-Hoadley
www.oxfordsourceltd.com

@nfrasser
Copy link
Collaborator

@marksyzm I know it's been a while but I'd like to merge this soon.

Do you mind rebasing?

@marksyzm
Copy link
Contributor Author

I did a pull and merge from yours as a rebase would likely just make a mess of the commits - all in now

@nfrasser nfrasser merged commit 9c200b1 into Hypercontext:master Aug 16, 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