Closed jonnybel closed 7 years ago
I think this can be achieved by writing this option to a file (which to me sounds a bit overkill, for saving only one numeric value)
It might be overkill, but it's a one time thing for us, but will save users lots of time. Otherwise they'll have to deal with setting an environment variable or shell alias, not all users of emoj
even know how to do that. You can achieve the saving in a couple of lines with https://github.com/sindresorhus/conf
I understand and agree, I'll make the changes.
You can achieve the saving in a couple of lines with https://github.com/sindresorhus/conf
This looks useful, thanks!
This looks very good @jonnybel. Nice work! 👌✨
@sindresorhus Thanks 😃 Happy to contribute!
And congrats on your first pull request. My first pull request was fixing a typo.
I discovered a tiny issue right after merging https://github.com/sindresorhus/emoj/commit/8376b36756868d97ccdba0f2cf86cbe0d614b3b6, but was easy to fix.
Fixes #4
It implements the Up/Down key presses to change the skin tone in interactive mode, and the
--skin-tone
flag which affects both modes.Two considerations:
Persistence: I understand the
--skin-tone
flag should make the setting persistent across different program runs. I think this can be achieved by writing this option to a file (which to me sounds a bit overkill, for saving only one numeric value), or by setting/accessing it through an Enviroment Variable. However, I think the same effect could easily be achieved if a user just creates his own alias for the program with the flag set to his preference. What do you think?Tests: I had added a very simple ava test for testing the new flag, but the output of toned-emojis is different in a Linux terminal - it shows 2 characters: the original emoji followed by a greyscale square, which differs from the expected output of a single toned emoji character -- even though the output is actually correct: the two characters combined form the intended toned emoji if you paste them together in a text-editor or browser. I suspect the test would pass in a Mac, but I am unable to do so, and because of the discrepancy I have decided to remove the test altogether. Also, since the output always comes from the Dango neural network API, it is never guaranteed that an output remains the same given the same query in different instances, so this kind of test is inconsistent.