refactor(tab_strip): improve structure and readability#218
refactor(tab_strip): improve structure and readability#218Gitkbc wants to merge 1 commit intolunarmodules:mainfrom
Conversation
| -- Process and validate an items array into the internal format. | ||
| -- @tparam table|nil items Raw items array from caller. | ||
| -- @treturn table Processed items with validated labels and default ids. | ||
| local function process_items(items) |
There was a problem hiding this comment.
the name doesn't describe what it does, it is overly generic. maybe validate_items? Also the parameter description is waste of bytes. This is what AI generated slob looks like.
The implementation is not lua-ish.
| self._cache_valid = false | ||
| end | ||
|
|
||
|
|
There was a problem hiding this comment.
why the whitespace changes? this messes with git history, try to avoid changes like this.
|
|
||
| -- Overflow indicator used when tabs exceed available width. | ||
| local ELLIPSIS = "…" | ||
| local ELLIPSIS_WIDTH = width.utf8swidth(ELLIPSIS) |
There was a problem hiding this comment.
this is not ok. You initialize the value here during loading the module, before the user has even initialized the terminal. During initialization the ambiguous-width might be configured differently and this value generated here might be wrong.
|
|
||
| -- Find the index of the currently selected tab, or nil if not found. | ||
| -- @treturn number|nil The 1-based index into self.items. | ||
| function TabStrip:_find_selected_index() |
There was a problem hiding this comment.
this method is rather similar to the resolve_initial_selection above. Yet that one is implemented as a function and this one as a method. That is inconsistent.
| local visible_end = visible_start + effective_width | ||
| local rendered_width = has_left_overflow and ellipsis_width or 0 | ||
| local visible_start = self._viewport_offset | ||
| local visible_end = visible_start + effective_width |
There was a problem hiding this comment.
more whitespace changes that convolute the git history
|
|
||
| return TabStrip | ||
|
|
||
| return TabStrip No newline at end of file |
There was a problem hiding this comment.
whitespace; the final newline was removed. fyi; you can configure your editor most likely to honor the settings in .editorconfig
| -- @tparam table processed_items Validated items array. | ||
| -- @tparam any|nil requested_id The caller-supplied initial selection id. | ||
| -- @treturn any|nil The resolved tab id, or nil if no items exist. | ||
| local function resolve_initial_selection(processed_items, requested_id) |
There was a problem hiding this comment.
some of these function have but a single caller. In that case the scope can be reduced by wrapping the function and its caller in a do...end block
Gitkbc
force-pushed
the
refactor/tab-strip-cleanup
branch
from
b47a47d to
c890b14
Compare
2 participants
Summary
Refactors
tab_strip.luato improve readability and maintainability by removing duplicated logic and extracting helper functions.Changes
process_items.resolve_initial_selectionand introduced_find_selected_indexto remove repeated tab lookup logic._render_tab_sliceand_render_padding_sliceto simplify_build_tab_line.ELLIPSIS) and improved variable naming (e.g.tab_index) for clarity.All tests are passed. Please let me know if any further improvements are required — happy to update.
Fixes #217