Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
230 changes: 203 additions & 27 deletions logbar/columns.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from functools import wraps
from typing import Any, Callable, Dict, Iterable, List, Optional, Sequence, Tuple, Union

from .drawing import iter_display_atoms as _iter_display_atoms
from .drawing import strip_ansi as _strip_ansi
from .drawing import visible_length as _visible_length
from .terminal import terminal_size
Expand All @@ -24,6 +25,46 @@ def _pad_visible(text: str, target: int) -> str:
return f"{text}{' ' * (target - current)}"


def _fit_visible(text: str, target: int, placeholder: str = "...") -> str:
"""Pad or truncate `text` so its rendered width is exactly `target`."""

if target <= 0:
return ""

current = _visible_length(text)
if current < target:
return f"{text}{' ' * (target - current)}"
if current == target:
return text

result: List[str] = []
width = 0
ph_width = _visible_length(placeholder)
budget = target - ph_width if target >= ph_width else target

for is_ansi, token, token_width in _iter_display_atoms(text):
if is_ansi:
result.append(token)
continue
if width + token_width > budget:
break
result.append(token)
width += token_width

if target >= ph_width:
result.append(placeholder)
width += ph_width

# Stop any active ANSI style from bleeding into trailing padding.
if "\x1b" in text:
result.append("\033[0m")

if width < target:
result.append(" " * (target - width))

return "".join(result)
Comment on lines +40 to +65

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Truncated cells containing wide characters come out one column too narrow, misaligning the table

When a cell value is shortened to fit its column (_fit_visible at logbar/columns.py:40-61) and the text contains a full-width character (e.g. CJK or emoji) right at the truncation boundary, the truncated text plus the ... marker ends up one column short of the intended width, so that row is drawn narrower than its border and the table stops lining up.

Impact: Rows that hold wide (double-width) characters render slightly misaligned against the surrounding borders, breaking the clean grid the fix is meant to guarantee.

Why the width falls short during truncation

In the truncation path, budget = target - ph_width and the loop stops adding clusters once the next cluster would exceed budget (logbar/columns.py:49). A double-width cluster (width 2) can leave the accumulated width at budget - 1 because adding it would overflow. The code then appends the placeholder (ph_width) giving a total visible width of target - 1, but it never pads the shortfall. Compare with _pad_visible/the current < target branch which always reaches exactly target. The border segment in _emit_border (logbar/columns.py:844-850) is sized from self._slot_widths[idx] (the full target), so the content cell is one column narrower than its border. Single-width text always fills budget exactly, so this only manifests with wide clusters.

Suggested change
result: List[str] = []
width = 0
ph_width = _visible_length(placeholder)
budget = target - ph_width if target >= ph_width else target
for is_ansi, token, token_width in _iter_display_atoms(text):
if is_ansi:
result.append(token)
continue
if width + token_width > budget:
break
result.append(token)
width += token_width
if target >= ph_width:
result.append(placeholder)
# Stop any active ANSI style from bleeding into trailing padding.
if "\x1b" in text:
result.append("\033[0m")
return "".join(result)
result: List[str] = []
width = 0
ph_width = _visible_length(placeholder)
budget = target - ph_width if target >= ph_width else target
for is_ansi, token, token_width in _iter_display_atoms(text):
if is_ansi:
result.append(token)
continue
if width + token_width > budget:
break
result.append(token)
width += token_width
if target >= ph_width:
result.append(placeholder)
width += ph_width
# Stop any active ANSI style from bleeding into trailing padding.
if "\x1b" in text:
result.append("\033[0m")
if width < target:
result.append(" " * (target - width))
return "".join(result)
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — _fit_visible now pads after the placeholder so the returned string always matches the target visible width, even when a double-width character falls on the truncation boundary. Added test_columns_fit_visible_exact_width_for_wide_chars to cover CJK cells.



def _columns_locked(method):
"""Serialize one `ColumnsPrinter` API call through its instance lock."""

Expand Down Expand Up @@ -109,6 +150,8 @@ def __init__(
self._terminal_size = terminal_size_provider or terminal_size
self._level_proxies: Dict[Any, ColumnsPrinter._LevelProxy] = {}
self._lock = threading.RLock()
self._slot_min_widths: List[int] = []
self._fixed_slots: set = set()

if headers:
self._set_columns(headers)
Expand Down Expand Up @@ -321,6 +364,13 @@ def _recompute_layout(self) -> None:
elif len(self._slot_padding) > slot_count:
self._slot_padding = self._slot_padding[:slot_count]

if len(self._slot_min_widths) < slot_count:
self._slot_min_widths.extend([1] * (slot_count - len(self._slot_min_widths)))
elif len(self._slot_min_widths) > slot_count:
self._slot_min_widths = self._slot_min_widths[:slot_count]

self._fixed_slots = set()

def _parse_width_hint(self, value: Optional[Union[str, int, float]]) -> Optional[Tuple[str, float]]:
"""Normalize width hints such as `fit`, percentages, or fixed chars."""

Expand Down Expand Up @@ -406,7 +456,9 @@ def _get_target_width(self) -> int:
minimal_cells = minimal_total
target_cells = target_total

return max(target_cells, minimal_cells)
desired = max(target_cells, minimal_cells)
content_budget = max(0, available_total - separator_count)
return min(desired, content_budget)
Comment on lines +459 to +461

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 _get_target_width now clamps below minimal header width

The new return min(desired, content_budget) at logbar/columns.py:455-457 can return a content width smaller than minimal_cells (the width needed to fit header labels) when the terminal is narrower than the headers. Previously the function guaranteed at least minimal_cells. This is intentional per the PR (headers are now truncated with ... rather than overflowing), but it is a genuine behavioral change: any caller/test relying on the minimal-width guarantee will now see truncation instead. Worth confirming no downstream code assumes headers always fit.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed as intentional. The whole point of this PR is to prevent headers from forcing the table wider than the terminal, so headers are now truncated with ... when needed. Existing tests still pass and the new test_columns_clamp_wide_rows_to_terminal_width explicitly checks that the rendered table never exceeds the terminal width even with many long headers.


def _apply_initial_widths(self) -> None:
"""Seed slot widths from hints, then distribute any remaining space."""
Expand All @@ -418,26 +470,33 @@ def _apply_initial_widths(self) -> None:
self._slot_widths = [1] * slot_count
self._slot_padding = [self._padding] * slot_count

total_width = self._get_target_width()
max_content = self._get_target_width()
column_count = len(self._columns)

for col_idx, spec in enumerate(self._columns):
if spec.width is None:
continue
if spec.width[0] == "fit":
continue
target = self._resolve_width_hint(spec.width, total_width)
target = self._resolve_width_hint(spec.width, max_content)
self._configure_column_width(col_idx, target)

current_total = sum(self._column_total_width(idx) for idx in range(column_count))
has_percent = any(spec.width and spec.width[0] == "percent" for spec in self._columns)
if current_total > total_width:
total_width = current_total

if not self._target_width_hint and not has_percent:
total_width = current_total
if self._target_width_hint or has_percent:
desired_content = max_content
else:
desired_content = min(current_total, max_content)

if current_total > desired_content:
full_width = desired_content + (slot_count + 1)
self._clamp_total_width(full_width)
current_total = sum(self._column_total_width(idx) for idx in range(column_count))
remaining = 0
else:
remaining = max(0, desired_content - current_total)

remaining = max(0, total_width - current_total)
expandable = [idx for idx, spec in enumerate(self._columns) if spec.width is None]
if not expandable:
expandable = [
Expand All @@ -446,25 +505,23 @@ def _apply_initial_widths(self) -> None:
if spec.width is None or (spec.width and spec.width[0] != "fit")
]

if remaining > 0 and expandable:
while remaining > 0:
progressed = False
for col_idx in expandable:
if remaining <= 0:
break
spec = self._columns[col_idx]
if spec.width and spec.width[0] == "fit":
continue
self._grow_column(col_idx, 1)
remaining -= 1
progressed = True
if not progressed:
while remaining > 0:
progressed = False
for col_idx in expandable:
if remaining <= 0:
break
spec = self._columns[col_idx]
if spec.width and spec.width[0] == "fit":
continue
self._grow_column(col_idx, 1)
remaining -= 1
progressed = True
if not progressed:
break

slot_count = self._slot_count()
separator_count = slot_count + 1 if slot_count else 0
current_total = sum(self._column_total_width(idx) for idx in range(column_count))
self._current_total_width = current_total + separator_count
full_width = max_content + (slot_count + 1)
self._clamp_total_width(full_width)
self._update_current_total_width()

def _resolve_width_hint(self, hint: Optional[Tuple[str, float]], total_width: int) -> Optional[int]:
"""Convert a parsed width hint into an absolute character width."""
Expand Down Expand Up @@ -503,6 +560,10 @@ def _configure_column_width(self, col_idx: int, target: Optional[int]) -> None:
if extra <= 0:
break

for idx in slot_indices:
self._slot_min_widths[idx] = self._slot_widths[idx]
self._fixed_slots.add(idx)

def _grow_column(self, col_idx: int, amount: int) -> None:
"""Expand a column span by distributing width across its slots."""

Expand Down Expand Up @@ -534,6 +595,112 @@ def _column_total_width(self, col_idx: int) -> int:
total += max(0, span - 1)
return total

def _row_width_budget(self) -> int:
"""Return the maximum bordered row width that fits in the terminal."""

term_cols, _ = self._terminal_size()
if term_cols <= 0:
term_cols = 80
return max(0, term_cols - (self._level_max_length + 2))

def _clamp_total_width(self, max_total: Optional[int] = None) -> None:
"""Reduce slot widths so the full bordered row does not exceed `max_total`."""

slot_count = self._slot_count()
if slot_count == 0:
return

if max_total is None:
max_total = self._row_width_budget()
if max_total <= 0:
return

separators = slot_count + 1
padding_total = sum((self._slot_padding[idx] * 2) for idx in range(slot_count))
max_content = max(0, max_total - separators - padding_total)
total = sum(self._slot_widths)
if total <= max_content:
self._update_current_total_width()
return

min_widths = self._slot_min_widths

while total > max_content:
reducible = [i for i in range(slot_count) if self._slot_widths[i] > min_widths[i]]
if not reducible:
break

max_w = max(self._slot_widths[i] for i in reducible)
count_max = sum(1 for i in reducible if self._slot_widths[i] == max_w)
next_candidates = [self._slot_widths[i] for i in reducible if self._slot_widths[i] < max_w]
min_at_max = max(min_widths[i] for i in reducible if self._slot_widths[i] == max_w)
next_candidates.append(min_at_max)
next_w = max(next_candidates)

available = (max_w - next_w) * count_max
need = total - max_content
reduce_total = min(available, need)
if reduce_total <= 0:
break

per_slot = reduce_total // count_max
extra = reduce_total % count_max
for idx in reducible:
if self._slot_widths[idx] == max_w:
self._slot_widths[idx] -= per_slot
if extra > 0:
self._slot_widths[idx] -= 1
extra -= 1
if self._slot_widths[idx] < min_widths[idx]:
self._slot_widths[idx] = min_widths[idx]
total = sum(self._slot_widths)

if total > max_content:
# Fallback: reduce padding and scale proportionally when minimums are infeasible.
room = max_total - separators - slot_count
new_pad = 0
if room > 0:
new_pad = min(self._padding, room // (2 * slot_count))
for idx in range(slot_count):
self._slot_padding[idx] = new_pad

padding_total = 2 * new_pad * slot_count
max_content = max(0, max_total - separators - padding_total)

if max_content < slot_count:
for idx in range(slot_count):
self._slot_widths[idx] = 1
total = slot_count
else:
factor = max_content / total
scaled = [max(1, int(self._slot_widths[idx] * factor)) for idx in range(slot_count)]
leftover = max_content - sum(scaled)
if leftover > 0:
fracs = sorted(
(
(self._slot_widths[idx] * factor) - int(self._slot_widths[idx] * factor),
idx,
)
for idx in range(slot_count)
)
for _, idx in fracs[:leftover]:
scaled[idx] += 1
self._slot_widths = scaled
total = sum(self._slot_widths)

self._update_current_total_width()

def _update_current_total_width(self) -> None:
"""Store the full bordered width implied by the current slot widths."""

slot_count = self._slot_count()
if slot_count == 0:
self._current_total_width = 0
return

current_total = sum(self._column_total_width(idx) for idx in range(len(self._columns)))
self._current_total_width = current_total + (slot_count + 1)

def _slot_count(self) -> int:
"""Return the number of physical slots in the current layout."""

Expand Down Expand Up @@ -568,6 +735,8 @@ def _apply_header_widths(self) -> None:
continue
if start >= len(self._slot_widths):
continue
if start in self._fixed_slots:
continue

total_slot_width = 0
for offset in range(span):
Expand All @@ -588,6 +757,9 @@ def _apply_header_widths(self) -> None:
if inner_width < label_len:
deficit = label_len - inner_width
self._slot_widths[start] += deficit
self._slot_min_widths[start] = max(self._slot_min_widths[start], label_len)

self._clamp_total_width()

def _prepare_values(self, values: Iterable) -> List[str]:
"""Normalize row values to strings and match the active slot count."""
Expand All @@ -607,10 +779,14 @@ def _update_slot_widths(self, values: Iterable[str]) -> None:
for idx, value in enumerate(values):
if idx >= len(self._slot_widths):
break
if idx in self._fixed_slots:
continue
current = _visible_length(value)
if current > self._slot_widths[idx]:
self._slot_widths[idx] = current

self._clamp_total_width()

def _render_header(self) -> str:
"""Build the visible header row with current widths and padding."""

Expand All @@ -628,7 +804,7 @@ def _render_header(self) -> str:
inner_width = max(0, total_width - left_pad_val - right_pad_val)
pad_left = " " * left_pad_val
pad_right = " " * right_pad_val
content = _pad_visible(spec.label, inner_width)
content = _fit_visible(spec.label, inner_width)
cells.append(f"{pad_left}{content}{pad_right}")

return "|" + "|".join(cells) + "|"
Expand All @@ -648,7 +824,7 @@ def _render_row(self, values: Iterable[str]) -> str:
width = _visible_length(text)
pad_width = self._slot_padding[idx] if idx < len(self._slot_padding) else self._padding
pad = " " * pad_width
padded_text = _pad_visible(text, width)
padded_text = _fit_visible(text, width)
cell = f"{pad}{padded_text}{pad}"
cells.append(cell)

Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"

[project]
name = "LogBar"
version = "0.4.9"
version = "0.4.10"
description = "A unified Logger and ProgressBar util with zero dependencies."
readme = "README.md"
requires-python = ">=3"
Expand Down
Loading