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 Emojilator for embedded short names #14

Merged
merged 2 commits into from
Dec 4, 2017

Conversation

som-snytt
Copy link
Contributor

As requested on gitter.

@SethTisue
Copy link
Contributor

LGTM, but would need accompanying doc in README.md. @wsargent, what do you think?

@wsargent
Copy link
Contributor

"colonoscope" really?

@som-snytt
Copy link
Contributor Author

I'll update it to "Colonus", as in Oedipus blind and near death.

I'll also revise the test to I ❤️ 👵, 👀 :hurtrealbad: like 🤕

@SethTisue
Copy link
Contributor

keeping it family friendly aids enterprise uptake

and who is the poster child for "family friendly" if not Oedipus?

@som-snytt
Copy link
Contributor Author

Cleaned it up.

@SethTisue
Copy link
Contributor

SethTisue commented Dec 1, 2017

👀 LGTM, but still needs doc 📖 in README.md 😛

@wsargent wsargent merged commit 038ae46 into lightbend-labs:master Dec 4, 2017
@SethTisue
Copy link
Contributor

🏁

@som-snytt
Copy link
Contributor Author

I don't mind going the distance. Thanks again for your attention to detail and keeping me from doing something embarrassing. Any remaining embarrassments are, of course, my own.

I'll create an issue to support e"\:" instead of double colon; I was too lazy, and StringContext doesn't make it trivial. I considered catching InvalidEscapeException and fixing escaped colon that way. That would be embarrassing, but maybe less than e"::".

The other desirable feature would be to handle emoticons, e":)". That was too much for a weekday and might require a SIP or something.

@SethTisue
Copy link
Contributor

That was too much for a weekday and might require a SIP or something

alternative plan: ⛱🍹

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants