michaelbrusegard / tabline.wez

A versatile and easy to use retro tab bar plugin for the WezTerm terminal emulator created with the lualine.nvim configuration format
MIT License
55 stars 5 forks source link

feat: additional default process icon colors #15

Closed joncrangle closed 1 month ago

joncrangle commented 1 month ago

Would appreciate if you could take a look at the M.overwrite_icon changes to make sure I didn't cause an issue downstream. It looks like the 'battery' and 'datetime' modules call it, but they supply a string so wouldn't be affected by changes.

For the default icon colors, I referred to brand colors and picked the approximate ansi/bright to match.

michaelbrusegard commented 1 month ago

@joncrangle the opts.enable_icon_colors is not needed. Because the way I setup the colors is that if you specify your own color for the icons it will overwrite the colors. So you could do something like this:

tab_active = {
  { "process", icon = { color = { fg = '#ff0000' } } }
}

The way overwrite_icon is setup is that it will only overwrite the icon if it is not a table. If it is a table it will keep the properties (like padding or color) because when it is a table we know that they should be specified. In short:

This way it works with the same API as before -- {'branch', icon = { wezterm.nerdfonts.cod_terminal_tmux, align = 'right', color = { fg = '#00ff00' } } }

Now I feel like this could maybe be explained in the readme in a simple manner under the process component attributes. It should be possible to understand that it works like this from the readme already but I understand that it can be difficult to see. What do you think?

joncrangle commented 1 month ago

The addition in this PR adds an opt so that a user can disable all process icon colors just by adding enable_icon_colors = false within theprocess table. Otherwise, they would need to override all the icons with a color with nil individually.

Edit: I'm just unsure about the implementation of the overwrite icons function to not copy the colors part of the table while making sure to not break any existing functionality.

michaelbrusegard commented 1 month ago

That option is not needed. It is covered by this:

tab_active = {
  { "process", icon = { color = { fg = '#ff0000' } } }
}
  • If a user does not want custom colors they can specify their own color.

Whatever the color is set to will overwrite the color of all the icons

michaelbrusegard commented 1 month ago

Edit: I'm just unsure about the implementation of the overwrite icons function to not copy the colors part of the table while making sure to not break any existing functionality.

I explained it here:

The way overwrite_icon is setup is that it will only overwrite the icon if it is not a table. If it is a table it will keep the properties (like padding or color) because when it is a table we know that they should be specified.

My wording is probably a little bad, let me rephrase. If the icon property on "process" is a string: icon = wezterm.nerdfonts.icon then it will be overwritten by an icon from the process_to_icon table based on which process it is.

If the property process_to_icon on "process" is set to nil then the icon specified will be used and not overwritten.

Now if the icon property on "process" is a table. Something it only is when addition props are passed. Then the icon will still be overwritten (as long as process_to_icon is not set to nil). BUT the properties assigned to the icon will be placed on the icon obtained from process_to_icon instead. Regardless of which icon it is.

Here are some examples:

Now I think adding another prop is unnecessary since it is already handled by what we already have.

While writing this I also realised that you can use default colors (disable the custom icon colors) by doing this:

This took me a long time to write. I hope it clears things up for you. This could probably also be added to discussions

joncrangle commented 1 month ago

@michaelbrusegard - thanks for explaining and sorry that you had to. For some reason it didn't click in my head the first time you said explained. I reverted the PR to only update process icon colors.

michaelbrusegard commented 1 month ago

I think it is only a good thing. Most likely someone else is wondering the same thing. I found it a little hard myself to explain it and I am the one who wrote the code. Maybe it could be added as a GitHub discussion in case someone is wondering the same thing