Change ui_window_style_set and change the way cursorline is drawn #1229
Conversation
|
Hi! I'm not sure I understand the point? CURSOR_LINE already doesn't override non-default styles if the user didn't request it. For example the zenburn theme only specifies the background for CURSOR_LINE so whatever foreground was set by the other rules remains unchanged when CURSOR_LINE is applied. |
If other styles on the same line have non-default background, then CURSOR_LINE will override them. That's is kind of the problem when using Before:
After:
|
| static bool is_default_fg(CellColor c) { | ||
| return is_default_color(c); | ||
| static bool is_default_fg(CellColor c, CellColor default_fg) { | ||
| return is_default_color(c) || cell_color_equal(c, default_fg); | ||
| } | ||
|
|
||
| static bool is_default_bg(CellColor c) { | ||
| return is_default_color(c); | ||
| static bool is_default_bg(CellColor c, CellColor default_bg) { | ||
| return is_default_color(c) || cell_color_equal(c, default_bg); | ||
| } |
There was a problem hiding this comment.
Please do the cell_color_equal() inline where its needed. The current change makes it very hard to understand what the is_default_* functions are checking.
| @@ -269,25 +269,29 @@ static void ui_window_draw(Win *win) { | |||
| } | |||
| } | |||
|
|
|||
| void ui_window_style_set(Ui *tui, int win_id, Cell *cell, enum UiStyle id) { | |||
| void ui_window_style_set(Ui *tui, int win_id, Cell *cell, enum UiStyle id, bool keep) { | |||
There was a problem hiding this comment.
Please use a more descriptive name than keep. What is being kept? It should be immediately clear.
| set.bg = is_default_bg(set.bg)? cell->style.bg : set.bg; | ||
| CellStyle def = tui->styles[win_id * UI_STYLE_MAX + UI_STYLE_DEFAULT]; | ||
| if (keep) { | ||
| set.fg = is_default_fg(cell->style.fg, def.fg) ? set.fg : cell->style.fg; |
There was a problem hiding this comment.
| set.fg = is_default_fg(cell->style.fg, def.fg) ? set.fg : cell->style.fg; | |
| if (!is_default_fg(cell->style.fg) && !cell_color_equal(cell->style.fg, def.fg)) | |
| set.fg = cell->style.fg; |
These ternary operations are too difficult to follow with the proposed changes.
| @@ -2038,7 +2039,8 @@ static int window_style(lua_State *L) { | |||
| enum UiStyle style = luaL_checkunsigned(L, 2); | |||
| size_t start = checkpos(L, 3); | |||
| size_t end = checkpos(L, 4); | |||
| win_style(win, style, start, end); | |||
| bool keep = lua_isboolean(L, 5) && lua_toboolean(L, 5); | |||
There was a problem hiding this comment.
| bool keep = lua_isboolean(L, 5) && lua_toboolean(L, 5); | |
| if (!lua_isboolean(L, 5)) | |
| return luaL_argerror(L, 5, "boolean expected"); |
I think we should raise an error rather than assume false when the caller passes something weird.
| @@ -2063,7 +2066,8 @@ static int window_style_pos(lua_State *L) { | |||
| enum UiStyle style = luaL_checkunsigned(L, 2); | |||
| size_t x = checkpos(L, 3); | |||
| size_t y = checkpos(L, 4); | |||
| bool ret = ui_window_style_set_pos(win, (int)x, (int)y, style); | |||
| bool keep = lua_isboolean(L, 5) && lua_toboolean(L, 5); | |||
Changes:
ui_window_style_setnow acceptskeepflag, that allows to not override non-default style values (fg and bg);ui_window_style_set_posandwin_stylealso accept the same flag;win:styleandwin:style_posboth accept the samekeepflag, but it is optional;This was mainly done so that CURSOR_LINE wouldn't override other styles, but plugins can potentially benefit from this as well.