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

[FEATURE REQUEST] Lenny faces ( ͡° ͜ʖ ͡°) #162

Closed
grappas opened this issue Oct 10, 2023 · 11 comments
Closed

[FEATURE REQUEST] Lenny faces ( ͡° ͜ʖ ͡°) #162

grappas opened this issue Oct 10, 2023 · 11 comments

Comments

@grappas
Copy link

grappas commented Oct 10, 2023

Could you add lenny faces, or at least possibility to plug your own emotes?

https://raw.githubusercontent.com/cspeterson/splatmoji/master/data/emoticons/emoticons.tsv

@fdw
Copy link
Owner

fdw commented Oct 11, 2023

If you have a good source for them (with an appropriate license), then it should be easy to add. You can have a look at the existing extractors for examples.

And it's already possible to add your own characters: https://github.com/fdw/rofimoji#custom-character-files-and-descriptions

@fdw
Copy link
Owner

fdw commented Dec 5, 2023

I'll close this issue for now. Please re-open if you have a good source for them. Or open a PR 😉

@fdw fdw closed this as completed Dec 5, 2023
@grappas
Copy link
Author

grappas commented Dec 5, 2023

I'll close this issue for now. Please re-open if you have a good source for them. Or open a PR 😉

@fdw isn't good what I provided? Also I can't reopen anything closed by owner.

@fdw
Copy link
Owner

fdw commented Dec 6, 2023

There is already a way to add your own characters of any kind, and I already linked the relevant readme section. If you have questions about it, please ask them 🙂

Also, I'd look at a PR to add an extractor, if you want to write one. Please make sure that the license for the data is permissive enough. In this case, it seems the original source is https://github.com/w33ble/emoticon-data , which has a good license and should be easy to use, given that it's json.

Also I can't reopen anything closed by owner.

Interesting, I didn't know that. Seems to be a GitHub setting and not something I can change.
However, you see that I also react to comments on closed issues 😉

@grappas
Copy link
Author

grappas commented Dec 6, 2023

I wouldn't ask if it was not coming with its own issues. Each character of lenny face is treated as separate character. There's no way of quoting whole string. Test it.

@fdw
Copy link
Owner

fdw commented Dec 6, 2023

Then please describe your issue! In your original post, you just asked

Could you add lenny faces, or at least possibility to plug your own emotes?

and I answered both questions. How can I know that you tried it and had problems?

In the original file you linked, the faces are separated from their description with tabs. However, rofimoji expects spaces. If I change that in the file, it works fine for me.
However, as some faces already contain spaces, that will not work. Unfortunately, supporting that is probably a very large and breaking change.

@fdw fdw reopened this Dec 6, 2023
@grappas
Copy link
Author

grappas commented Dec 6, 2023

and I answered both questions. How can I know that you tried it and had problems?

rofimoji.mp4

lennies.additional.csv

In theory quotations of objects and separating them with commas or semicolons would suffice.

supporting that is probably a very large and breaking change.

Large? Agreed. But breaking? nah... Just make it backward compatible with old files.

@fdw
Copy link
Owner

fdw commented Dec 6, 2023

The problem seems to be exactly what I said: The file you linked still separates characters from descriptions with tabs, not spaces. Additionally, the one you used in the video has a space directly after the opening parens - that's why rofimoji only treats the ( as the character, because it assumes that space is the separator.

And there is currently no way of using something else as a separator.

In theory quotations of objects and separating them with commas or semicolons would suffice.

Whatever character is chosen cannot be used anywhere else, especially not as a character itself (which is problematic, as rofimoji can do all of Unicode).
And it's not just about parsing the file; the separator is also used when communicating with the selector. I cannot see anything but whitespace working here.

Large? Agreed. But breaking? nah... Just make it backward compatible with old files.

You're invited to try the change, but I fail to see how rofimoji can automatically determine the correct separator for a file. In my eyes, there is no way to make this backwards compatible. (And even if there is some way, the additional complexity in the code must also be worth it.)

@grappas
Copy link
Author

grappas commented Dec 6, 2023

At least make space escapeable... with \

@fdw
Copy link
Owner

fdw commented Dec 7, 2023

As I said, it's not just about parsing the file, but also about the communication with rofi. Even if I add escaping for spaces, that would be visible in rofi.

I'll think about it, but don't get your hopes up.

@fdw
Copy link
Owner

fdw commented Mar 29, 2024

I've finally tried to use a real csv format for the data files, which would be the first step to enable characters with spaces. However, the performance is much worse than with the simpler format currently used - loading a file now takes five times as long as before. If you try to open many --files, it's noticeable slower and worse. For this reason, I don't think I'll continue on this path.

Maybe I'll change my opinion at some point in the future, if there's a compelling reason. And I'm still willing to look at PRs.

@fdw fdw closed this as completed Mar 29, 2024
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

No branches or pull requests

2 participants