Port instance display names + root-color highlighting onto nav-rail UI#51
Merged
Merged
Conversation
#47) #47 (display_name config parsing, root-status color highlighting, and a scroll area for large instance lists) predates #45's rewrite into the views/ package and no longer applies to the old main.py it was written against. This ports the same feature onto views/instances_page.py and views/main_window.py: - constants.py / config_handler.py: unchanged from #47's approach -- DISPLAY_NAME_KEY + display_names dict parsed alongside instance_statuses. Also fixed the isfile()-false early return to include the display_names key (matching the exception-handler fix #47 already had), so both empty paths agree on shape. - views/instances_page.py: display-name column, green highlight on rooted rows, and the QScrollArea from #47 (for Chris's 20+ instance setup) wrapping the grid so a large instance count doesn't force the window taller than the screen. - views/main_window.py: wires display_names through update_instance_data. All credit to Chris Borneman (@Hobbes4Pres) -- the feature, the regex approach, and the scroll-area idea are his from #47. This is that PR ported onto the new file layout plus the test coverage #47 didn't have. Testing - New tests/test_config_handler.py (display_names parsing, both empty- result paths) and 4 new tests in tests/test_instances_page.py (display name rendering + fallback, root-color highlighting, no widget leak on refresh). All 5 confirmed to fail against pre-fix code. - Live-smoke-tested through the real MainWindow with 25 synthetic instances (mixed display names / rooted states): correct row count, correct highlight count, scroll area actually engaged, stable across a simulated refresh. - Full suite: 81 passed. ruff + compileall clean (same gates CI runs). Co-authored-by: Chris Borneman <75288580+Hobbes4Pres@users.noreply.github.com>
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Closes #47 by porting it forward. #47 (display-name column, root-status color highlighting, scroll area for large instance lists) was written against the old single-file
main.py. #45 merged first and replaced that file with the nav-railviews/layout, so #47 no longer applies and can't be rebased cleanly — the UI it touched doesn't exist in that shape anymore.All credit to Chris Borneman (@Hobbes4Pres) — the feature, the config-parsing approach, and the scroll-area fix for his 20+-instance setup are his, from #47. This PR is that work carried onto the new file layout, plus the test coverage the original didn't have.
Changes
constants.py/config_handler.py: same approach as Instance display name and root color highlighting #47 —DISPLAY_NAME_KEY,display_namesdict parsed alongsideinstance_statuses. Also closed a small gap Instance display name and root color highlighting #47 had: theisfile()-false early return was missing thedisplay_nameskey that the exception-handler path got (per the Gemini review on Instance display name and root color highlighting #47) — now both empty-result paths agree on shape.views/instances_page.py: display-name column, green highlight on rooted rows,QScrollAreawrapping the grid (so 20+ instances don't push the window past screen height).views/main_window.py: wiresdisplay_namesthroughupdate_instance_data.Testing
tests/test_config_handler.py+ 4 new tests intests/test_instances_page.py(display name + fallback, root-color highlighting, no widget leak across refresh). Confirmed each fails against pre-fix code before the change, passes after.MainWindowwith 25 synthetic instances (mixed display names / rooted states): correct row count, correct highlight count (9/25 green), scroll area actually engaged (737px content in a fixed-height window), stable across a simulated refresh.ruff check --select E9,F63,F7,F82,F401 .andpython -m compileall -q .clean.Closes #47.