-
Notifications
You must be signed in to change notification settings - Fork 15
refactor: simplify terminal.text.width using luasystem 0.7.0 (#204) #206
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
base: main
Are you sure you want to change the base?
Changes from all commits
bbaba02
8ce5d76
57588d8
50c239d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -109,7 +109,6 @@ The main entry point is `src/terminal/init.lua`, which exposes the `terminal` mo | |
| - Holds version metadata and high-level helpers: | ||
| - `terminal.size()` – wrapper around `system.termsize`. | ||
| - `terminal.bell()` / `terminal.bell_seq()` – terminal bell. | ||
| - `terminal.preload_widths()` – preloads characters into the width cache for box drawing and progress spinners. | ||
| - Manages initialization/shutdown and integration with `system`: | ||
| - Console flags, non-blocking input, code page, alternate screen buffer. | ||
| - Sleep function wiring for async usage. | ||
|
|
@@ -234,18 +233,24 @@ This is encapsulated by **`terminal.input`** (e.g. `preread` and `read_query_ans | |
| ## 5. Text handling in the UI | ||
|
|
||
| Terminal UI must align and truncate text by **display columns**, not by bytes or UTF-8 character count. Characters can be one or two columns wide (e.g. CJK, emojis), and some have ambiguous width. This section describes how to handle width, substrings, and formatted display so text renders correctly. | ||
|
|
||
| ### 5.1 Display width | ||
|
|
||
| - **`terminal.text.width`** provides the width primitives: | ||
| - **`utf8cwidth(char)`** – width in columns of a single character (string or codepoint). Uses a cache when available; otherwise falls back to `system.utf8cwidth`. | ||
| - **`utf8swidth(str)`** – total display width of a string in columns. | ||
| - **Width cache:** Not all characters have a fixed width (e.g. East Asian ambiguous). The library maintains a cache of **tested** widths. To populate it: | ||
| - **`terminal.text.width.test(str)`** – writes characters invisibly, measures cursor movement, and records each character’s width. Call during startup or when you first display unknown glyphs. | ||
| - **`terminal.preload_widths(str)`** – convenience that tests the library’s own box-drawing and progress characters plus any optional `str`. Call once after `terminal.initialize` if you use `terminal.draw` or `terminal.progress`. | ||
| - Use **`terminal.size()`** to get terminal dimensions (rows × columns) so you can fit text to the visible area. | ||
| - **`utf8cwidth(char[, ambiguous_width])`** – returns the display width in columns of a single UTF-8 character (string or codepoint). | ||
| - **`utf8swidth(str[, ambiguous_width])`** – returns the total display width in columns of a UTF-8 string. | ||
|
|
||
| Width calculation is delegated to the underlying `system.utf8cwidth` | ||
| and `system.utf8swidth` functions provided by `luasystem`. | ||
|
|
||
| Ambiguous-width characters default to a width of 1 column. A different | ||
| width (1 or 2) can be specified explicitly via the optional | ||
| `ambiguous_width` parameter. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. wrong, this should probably mention the calibration option, or the setter that explicitly sets the value |
||
|
|
||
| Use **`terminal.size()`** to obtain terminal dimensions (rows × columns) | ||
| so text can be laid out to fit the visible area. | ||
|
|
||
| **Rule of thumb:** For correct alignment and truncation, always reason in **columns**. Use `utf8swidth` to measure strings and `utf8cwidth` for per-character width when implementing substrings or cursors. | ||
| **Rule of thumb:** For correct alignment and truncation, always reason in | ||
| **display columns**, not bytes or character count. | ||
|
|
||
| ### 5.2 Substrings by characters vs columns | ||
|
|
||
|
|
@@ -289,7 +294,8 @@ Key methods for display and layout: | |
|
|
||
| - **Simple truncation or fixed-width slice:** use **`utils.utf8sub_col(str, 1, max_col)`** (and optionally ellipsis). | ||
| - **Editable single/multi-line text with cursor and word wrap:** use **EditLine** and **`EditLine:format(...)`**. | ||
| - **Measuring or testing width:** use **`terminal.text.width.utf8swidth`** / **`utf8cwidth`** and **`terminal.text.width.test`** / **`terminal.preload_widths`** as above. | ||
| - Measuring display width: use `terminal.text.width.utf8swidth` | ||
| or `utf8cwidth`. | ||
|
|
||
| All terminal output must go through **`terminal.output`** (e.g. `terminal.output.write`), not raw `print` or `io.write`, so that the library’s stream and any patching behave correctly. | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,8 +32,13 @@ The scope of what is covered by the version number excludes: | |
|
|
||
| ### Version X.Y.Z, unreleased | ||
|
|
||
| - a fix | ||
| - a change | ||
| - refactor: simplify `terminal.text.width` with `luasystem` (>= 0.7.0). | ||
| Removed per-character width cache and now calibrate one ambiguous-width | ||
| character during initialization, reused for all width calculations. | ||
| `test` and `test_write` are preserved for API compatibility. | ||
| - feat(progress): account for ANSI escape sequences in sprite width math. | ||
| Spinner frames and done sprites can now include color/style sequences | ||
| without breaking cursor rewind behavior. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no need to add a change entry, there is no release yet |
||
|
|
||
| ### Version 0.1.0, released 01-Jan-2022 | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,87 @@ | ||
| local helpers = require "spec.helpers" | ||
|
|
||
|
|
||
| describe("terminal.progress", function() | ||
|
|
||
| local terminal | ||
| local progress | ||
|
|
||
| setup(function() | ||
| terminal = helpers.load() | ||
| progress = require("terminal.progress") | ||
| end) | ||
|
|
||
|
|
||
| teardown(function() | ||
| progress = nil | ||
| terminal = nil | ||
| helpers.unload() | ||
| end) | ||
|
|
||
|
|
||
|
|
||
| describe("spinner()", function() | ||
|
|
||
| before_each(function() | ||
| helpers.clear_output() | ||
| end) | ||
|
|
||
|
|
||
|
|
||
| it("uses visible width for ANSI-styled single-width sprites", function() | ||
| local spinner = progress.spinner({ | ||
| sprites = { | ||
| [0] = "", | ||
| "\27[31mX\27[0m", | ||
| }, | ||
| stepsize = 10, | ||
| }) | ||
|
|
||
| spinner(false) | ||
|
|
||
| assert.are.equal( | ||
| "\27[31mX\27[0m" .. terminal.cursor.position.left_seq(1), | ||
| helpers.get_output() | ||
| ) | ||
| end) | ||
|
|
||
|
|
||
| it("uses visible width for ANSI-styled double-width sprites", function() | ||
| local spinner = progress.spinner({ | ||
| sprites = { | ||
| [0] = "", | ||
| "\27[31m界\27[0m", | ||
| }, | ||
| stepsize = 10, | ||
| }) | ||
|
|
||
| spinner(false) | ||
|
|
||
| assert.are.equal( | ||
| "\27[31m界\27[0m" .. terminal.cursor.position.left_seq(2), | ||
| helpers.get_output() | ||
| ) | ||
| end) | ||
|
|
||
|
|
||
| it("uses visible width for ANSI-styled done_sprite", function() | ||
| local spinner = progress.spinner({ | ||
| sprites = { | ||
| [0] = "x", | ||
| "x", | ||
| }, | ||
| done_sprite = "\27[32mOK\27[0m", | ||
| stepsize = 10, | ||
| }) | ||
|
|
||
| spinner(true) | ||
|
|
||
| assert.are.equal( | ||
| "\27[32mOK\27[0m" .. terminal.cursor.position.left_seq(2), | ||
| helpers.get_output() | ||
| ) | ||
| end) | ||
|
|
||
| end) | ||
|
|
||
| end) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -76,6 +76,8 @@ end | |
| --- Creates a sequence to draw a horizontal line with a title centered in it without writing it to the terminal. | ||
| -- Line is drawn left to right. If the width is too small for the title, the title is truncated. | ||
| -- If less than 4 characters are available for the title, the title is omitted altogether. | ||
| -- ANSI escape sequences in title/prefix/postfix are ignored for width calculations. | ||
| -- If truncation is needed, the rendered title uses plain text. | ||
| -- @tparam number width the total width of the line in columns | ||
| -- @tparam[opt=""] string title the title to draw (if empty or nil, only the line is drawn) | ||
| -- @tparam[opt="─"] string char the line-character to use | ||
|
|
@@ -92,11 +94,20 @@ function M.title_seq(width, title, char, pre, post, type, title_attr) | |
|
|
||
| pre = pre or "" | ||
| post = post or "" | ||
| local pre_w = text.width.utf8swidth(pre) | ||
| local post_w = text.width.utf8swidth(post) | ||
| local pre_w = text.width.utf8swidth(utils.strip_ansi(pre)) | ||
| local post_w = text.width.utf8swidth(utils.strip_ansi(post)) | ||
| local w_for_title = width - pre_w - post_w | ||
|
|
||
| local title, title_w = utils.truncate_ellipsis(w_for_title, title, type) | ||
| local stripped_title = utils.strip_ansi(title) | ||
| local stripped_title_w = text.width.utf8swidth(stripped_title) | ||
| local title_w | ||
| if stripped_title_w <= w_for_title then | ||
| title_w = stripped_title_w | ||
| else | ||
| stripped_title, title_w = utils.truncate_ellipsis(w_for_title, stripped_title, type) | ||
| title = stripped_title | ||
| end | ||
|
|
||
|
Comment on lines
+101
to
+110
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. shouldn't most of this logic actually go into the |
||
| if title_w == 0 then | ||
| return M.horizontal_seq(width, char) | ||
| end | ||
|
|
@@ -139,4 +150,4 @@ end | |
|
|
||
|
|
||
|
|
||
| return M | ||
| return M | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nitpick; this is an unwanted whitespace change. you can configure you edito probably to honor the settings in |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is incorrect, the optional parameter doesn't exist