Skip to content

add support for custom emoji #3184

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

imorte
Copy link

@imorte imorte commented Mar 9, 2025

resolves #2624

@imorte
Copy link
Author

imorte commented Mar 9, 2025

@derailed Hello! May you check it please?

@derailed
Copy link
Owner

@imorte Thank you for this PR!

I am not super keen on the current approach for these reasons:

  1. I feel these should be part of a skin vs k9s config. As such it won't crowd up the main config and it's really a stylistic concern. Also folks could share their emojis using existing styles mechanics. So the styles could now reference a file i.e something like emojis/standard or emojis/custom-1 in their skin file and load up the respective emojiis.
  2. Emojis names should be namespaced i.ie something like prompt.filter, prompt.query, status.warn, status.info
  3. Not keen on defining each emoji kind as a new accessor. Better to use a generic call something like emojiFor(xxx). This allows to CRUD icons in the future with minimal effort.

What do you think?

@derailed derailed added enhancement New feature or request needs-tlc Pr needs additional updates question Further information is requested labels Mar 10, 2025
@imorte
Copy link
Author

imorte commented Mar 11, 2025

@derailed Thanks for the response! Yes, it all sounds reasonable. I will do it this way

@derailed
Copy link
Owner

@imorte Thank you!
Also... as I just got reminded... we need to make sure that none of these are activated when noIcons flag is enabled.

@imorte imorte force-pushed the add-configurable-emoji branch 2 times, most recently from 4d69b66 to 65dc856 Compare March 23, 2025 13:04
@imorte
Copy link
Author

imorte commented Mar 23, 2025

@derailed done, I hope I didn't forget anything

I haven't touched this func yet

name := "🦄 "

What do you think, is it worth spending time adding the config here now, or can we do it later?

@imorte imorte force-pushed the add-configurable-emoji branch from 65dc856 to 9300146 Compare March 23, 2025 13:39
@@ -80,7 +80,7 @@ type Prompt struct {

app *App
noIcons bool
icon rune
icon string
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

multiple code points support

@imorte imorte force-pushed the add-configurable-emoji branch from 9300146 to 30418e4 Compare March 23, 2025 13:56
@imorte
Copy link
Author

imorte commented Apr 9, 2025

@derailed Hi, what do you think about this PR? I can rebase and adapt it for the new version

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request needs-tlc Pr needs additional updates question Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make Icons / Emoji customizable
2 participants