feat: tab groups, live CWD tracking, and UX improvements (v1.8.8)#46
feat: tab groups, live CWD tracking, and UX improvements (v1.8.8)#46ruscher wants to merge 1 commit intobig-comm:mainfrom
Conversation
ruscher
commented
Apr 8, 2026
- Add Chrome-style tab groups with color-coded chips and alphabetical naming (A-Z, 1A-1Z...)
- Add /proc//cwd polling for real-time tab title updates (1s interval)
- Add VTE_VERSION env var for OSC7 shell integration activation
- Add full path tooltip on tab hover
- Fix tab title reset to 'Local' on focus/click (fallback to last polled CWD)
- Fix tab move auto-join for right side of last group tab
- Fix tab resizing during move mode (use opacity instead of visibility)
- Add tab group CSS styles and chip/collapse UI
- Add group move, rename, ungroup, and context menu actions
- Update README with tab groups and live directory tracking features
- Add Chrome-style tab groups with color-coded chips and alphabetical naming (A-Z, 1A-1Z...) - Add /proc/<pid>/cwd polling for real-time tab title updates (1s interval) - Add VTE_VERSION env var for OSC7 shell integration activation - Add full path tooltip on tab hover - Fix tab title reset to 'Local' on focus/click (fallback to last polled CWD) - Fix tab move auto-join for right side of last group tab - Fix tab resizing during move mode (use opacity instead of visibility) - Add tab group CSS styles and chip/collapse UI - Add group move, rename, ungroup, and context menu actions - Update README with tab groups and live directory tracking features
talesam
left a comment
There was a problem hiding this comment.
PR #46 Review — Tab Groups, Live CWD Tracking, UX Improvements
Hey @ruscher, thanks for the PR! The feature set is solid — tab groups, live CWD tracking, and the Chrome-style UX are great additions. I ran a thorough review and found a few items that need fixing before merge.
Must Fix Before Merge
1. Docstring vs Code Mismatch in manager.py
File: src/ashyterm/terminal/manager.py, method _periodic_process_check()
The docstring says:
This runs every 3 seconds and checks for:
But the timer was changed to 1 second (GLib.timeout_add(1000, ...)).
The inline comment at initialization (line ~94-95) is correct ("runs every second"), but the docstring in the method itself is stale. Please update to match.
2. Accessing Private Internals of ProcessTracker
File: src/ashyterm/terminal/manager.py, method _poll_terminal_cwd()
tracker = self.spawner.process_tracker
with tracker._lock:
processes = dict(tracker._processes)_lock and _processes are private attributes of ProcessTracker. Accessing them directly breaks encapsulation and will break if ProcessTracker internals change.
Suggested fix: Add a public method to ProcessTracker, e.g.:
def get_processes_snapshot(self) -> Dict[int, Dict[str, Any]]:
"""Return a thread-safe snapshot of tracked processes."""
with self._lock:
return dict(self._processes)Then in _poll_terminal_cwd():
processes = tracker.get_processes_snapshot()3. Unused Import in test_tab_groups.py
File: tests/test_tab_groups.py, line 3
import pytestpytest is imported but never used — no pytest.raises, pytest.mark, pytest.fixture, etc. Please remove it.
4. Version Mismatch — Critical
The PR introduces version conflicts with the current main branch:
| File | PR #46 | Current main |
|---|---|---|
README.md |
1.8.8 | 1.9.1 |
config.py |
1.8.8 | 1.8.7 |
pyproject.toml |
0.1.0 | 0.1.0 |
Issues:
- The README badge is downgraded from 1.9.1 → 1.8.8
config.pybumps from 1.8.7 → 1.8.8, but the README inmainalready says 1.9.1- Note:
mainitself already has a version inconsistency (README= 1.9.1 vsconfig.py= 1.8.7). This PR makes it worse by setting both to 1.8.8 pyproject.tomlis0.1.0everywhere and has never been updated
Required: Decide on the correct version number for this release (should be ≥ 1.9.2 since main README already shows 1.9.1), and align all three files (README.md, config.py, pyproject.toml).
Should Fix Before Merge
5. Integration Tests for Save/Restore
The unit tests for TabGroupManager are good and cover CRUD well. However, there are no integration tests for:
- Save/restore of groups via
WindowStateManager - Serialization of
group_idon tabs - Rebuild of tab bar after restore
- Basic state roundtrip
At least 1-2 integration tests for the restore flow would increase confidence before merge.
Can Be Follow-Up
6. id(tab_widget) as Tab Identifier
get_tab_id() returns str(id(tab_widget)) — this is the Python object memory address. It:
- Changes between sessions (not stable across restart)
- Can be reused if a widget is destroyed and a new one is allocated at the same address
The restore code works around this by using positional matching (last-created tab), so it's functional. But for long-term robustness, consider switching to UUID or an incremental counter. This can be a follow-up PR.
7. Empty Callback _on_setting_changed in tabs.py
This method is just pass — but it already existed in main before this PR, so it's not your issue. Just flagging it for awareness.
8. Planning Docs in Repo
The PR adds docs/chrome_tab_groups_reference.md and docs/tab_groups_plan.md. These are useful for development context but may not belong in the production repo long-term. Consider moving them to the wiki or issues after the feature stabilizes.
Summary
| Priority | Items |
|---|---|
| Must fix | Docstring mismatch, tracker encapsulation, unused import, version alignment |
| Should fix | 1-2 integration tests for restore flow |
| Follow-up | Replace id(tab_widget), empty callback, planning docs location |
Overall the implementation is clean and well-structured. The TabGroupManager is nicely decoupled from UI, the CSS is proper, and the Chrome-style UX is a good model. Just need these fixes and we're good to merge.
Looking forward to the updated version! 🚀