search

joshmedeski / tmux-nerd-font-window-name

Nerd Font icons for your tmux windows
145 stars 29 forks source link

use lowercase name for icon lookup #23

Closed wangyuehong closed 1 year ago

wangyuehong commented 1 year ago

got a pane_current_command like Emacs in my terminal environment, convert it to a lower string for icon lookup.

joshmedeski commented 1 year ago

Does yq allow for capitalized letters in keys? If so, it may make sense to just update your config to use capitalized letters.

My main concern is users will try and match their commands and not realize that they have to use all lowercase letters in the yaml file.

Let's discuss it before merging. Perhaps we mention something in the README if this ends up merging as-is.

wangyuehong commented 1 year ago

thanks for your great work and feedback.

Does yq allow for capitalized letters in keys? If so, it may make sense to just update your config to use capitalized letters.

yes, yq supports keys with capitalized letters, but users have to specify duplicate icon names in the configuration file, such as Emacs/Python(i got these command names in iterm2).

My main concern is users will try and match their commands and not realize that they have to use all lowercase letters in the yaml file.

I completely agree with your concerns. 👍

I submitted a new commit that allows users to specify whether to convert the window name to lowercase before icon lookup. this is a non-breaking change.

https://github.com/joshmedeski/tmux-nerd-font-window-name/pull/23/commits/8eb9d1a6ba74c3a50d39316e5258c0acf4efd63d

joshmedeski commented 1 year ago

Thanks for the update. Can you clarify what you mean about this?

users have to specify duplicate icon names in the configuration file

I'm not sure why I'd want to turn on this feature and use it, when I could just set:

icons:
  Python: 🐍
  Emacs: ✍️

So you want to be able to use lowercase letters in the keys? I don't want to add conditional logic unless it's necessary or serves a purpose. Thanks for your work so far.

wangyuehong commented 1 year ago

Thank you for your feedback. My original intention was that different environments or installation ways may result in variations in the capitalization of command name. 

icons:
  Python: 🐍
  python: 🐍
  Emacs: ✍️
  emacs: ✍️

To avoid duplicate definitions in the config file, we normalize the command name by converting it to lowercase before icon lookup, and we only need to use lowercase names to define icons in the config file.

icons:
  python: 🐍
  emacs: ✍️

I don't want to add conditional logic unless it's necessary or serves a purpose.

Indeed, this is just a special case, and having duplicate definitions in the config file is not a big issue. I will close this pull request. Thank you.

joshmedeski commented 1 year ago

Cool, thanks for working through this with me. I'm glad we're in agreement.

I appreciate your attention to detail on this plugin! Please keep up the good work 😄