Skip to content

feat: tab groups, live CWD tracking, and UX improvements (v1.8.8)#46

Open
ruscher wants to merge 1 commit intobig-comm:mainfrom
CommunityBigLinux:main
Open

feat: tab groups, live CWD tracking, and UX improvements (v1.8.8)#46
ruscher wants to merge 1 commit intobig-comm:mainfrom
CommunityBigLinux:main

Conversation

@ruscher
Copy link
Copy Markdown
Contributor

@ruscher 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
Copy link
Copy Markdown
Contributor

@talesam talesam left a comment

Choose a reason for hiding this comment

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

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 pytest

pytest 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.py bumps from 1.8.7 → 1.8.8, but the README in main already says 1.9.1
  • Note: main itself already has a version inconsistency (README = 1.9.1 vs config.py = 1.8.7). This PR makes it worse by setting both to 1.8.8
  • pyproject.toml is 0.1.0 everywhere 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_id on 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! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants