fix: five review findings on the tabbed-sidebar commit

Finding 2 — four builder ids had no null check. `side_stack`,
`side_switcher`, `measure_page` and `signals_page` were resolved at
lines 4709-4712, well past the skeleton null-guard. A typo in the `.ui`
would have been a silent segfault.  The four resolutions are moved to
the guard block and added to its condition, in the same form the guard
already uses.

Finding 3 — the 'editable focused: m refused' leg of keys_probe was
deleted instead of ported.  It is restored, adapted for the new
behaviour: 'm' now selects the Measure tab instead of toggling a pane,
so the refusal is observed as the side_tab name being unchanged across
the synthesised key, not as a toggle that did not flip.  The dump line
keeps its existing name and `refused=` field so probe-readings.md:601
still describes it.

Finding 4 — `exit_probe_step3` assigned `sidebar_shown` to both
`dbg_side_visible` and `dbg_side_column_after_hide`, and the panels:
dump line read the wrong one (dbg_side_visible) for its `(visible=)`
field.  Two readings that cannot disagree diagnose nothing.  Resolved
by making `panels:` read `dbg_side_column_after_hide` and removing the
redundant `dbg_side_visible` field.

Finding 5 — the three-line rationale comment above `reload_file_view`
in `apply_math_channels` was removed by the commit under review.
Nothing it said became untrue.  Restored verbatim from e10a92f.

Finding 6 — three comments the change made false:
- `on_activate`: 'the sidebar's scroller' replaced with 'the sidebar's
  tab switcher and two-page stack', matching the actual ui file.
- `update_side_visibility`: 'Hiding the scroller instead left an empty
  column where the panels should be' removed; the design it described
  no longer exists.
- `header-bar.ui`: 'two of them are toggles' updated to 'one of them
  is a toggle'; after signal_toggle's deletion the overflow menu holds
  one toggle (layout).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Martino FerrariandClaude Opus 4.6 committed 2026-10-14 11:45:00 +02:00
1 parent f224f56c4c
commit 619b5e356c
5 files changed
+35 -16

No files matched your search

+15 -12
View File
@@ -2854,8 +2854,7 @@ void update_side_visibility(App& app) {
gtk_widget_set_sensitive(app.sidebar_toggle, have_signals);
const bool want_column = have_signals && app.sidebar_wanted;
// The split view owns the sidebar's visibility: it animates it, and when the window is too narrow it
// shows it as an overlay rather than letting the column be squeezed to nothing. Hiding the scroller
// instead left an empty column where the panels should be.
// shows it as an overlay rather than letting the column be squeezed to nothing.
if (app.side_view != nullptr) {
app.sidebar_syncing = true;
adw_overlay_split_view_set_show_sidebar(ADW_OVERLAY_SPLIT_VIEW(app.side_view), want_column);
@@ -4551,8 +4550,9 @@ void on_activate(GtkApplication* application, gpointer user) {
install_css();
// The window's skeleton is a `.ui` file: the window, its two panes, the plot column with the status
// line under it, and the sidebar's scroller. Everything that depends on the stream -- the events
// pane, the two plot areas, the sidebar's panels -- is built below and put into the slots by id.
// line under it, and the sidebar's tab switcher and two-page stack. Everything that depends on the
// stream -- the events pane, the two plot areas, the sidebar's panels -- is built below and put
// into the slots by id.
g_clear_object(&app->window_ui);
app->window_ui = gtk_builder_new_from_resource("/org/udpscope/ui/main-window.ui");
if (app->window_ui == nullptr) return;
@@ -4566,12 +4566,17 @@ void on_activate(GtkApplication* application, gpointer user) {
app->status_label = GTK_WIDGET(gtk_builder_get_object(app->window_ui, "status_label"));
app->status_info_button = GTK_WIDGET(gtk_builder_get_object(app->window_ui, "status_info"));
GtkWidget* side = GTK_WIDGET(gtk_builder_get_object(app->window_ui, "side_column"));
GtkWidget* side_stack = GTK_WIDGET(gtk_builder_get_object(app->window_ui, "side_stack"));
GtkWidget* side_switcher = GTK_WIDGET(gtk_builder_get_object(app->window_ui, "side_switcher"));
GtkWidget* measure_page = GTK_WIDGET(gtk_builder_get_object(app->window_ui, "measure_page"));
GtkWidget* signals_page = GTK_WIDGET(gtk_builder_get_object(app->window_ui, "signals_page"));
// A missing id is the one mistake neither the build nor `GtkBuilder` sees, and every use below would
// be a null pointer.
if (app->window == nullptr || side_view == nullptr || events_divider == nullptr ||
app->plot_box == nullptr || app->plot_split == nullptr || app->status_badge == nullptr ||
app->status_label == nullptr || app->status_info_button == nullptr ||
side == nullptr) {
side == nullptr || side_stack == nullptr || side_switcher == nullptr ||
measure_page == nullptr || signals_page == nullptr) {
return;
}
// The window's actions live on the application, so it has to know which one it belongs to.
@@ -4703,13 +4708,11 @@ void on_activate(GtkApplication* application, gpointer user) {
// no section competes with four others for height. Each page scrolls on its own -- the two are
// different heights, and one shared scroller would leave a switch scrolled where the new page
// never asked to be.
// `side` is `side_column` from the `.ui`, resolved above with the rest of the skeleton; the app
// keeps it because `update_side_visibility()` still asks about the column as a whole.
// `side` and `side_stack`/`side_switcher`/`measure_page`/`signals_page` are resolved above with
// the rest of the skeleton and checked by the null guard; assign the members here.
app->side_box = side;
app->side_stack = GTK_WIDGET(gtk_builder_get_object(app->window_ui, "side_stack"));
app->side_switcher = GTK_WIDGET(gtk_builder_get_object(app->window_ui, "side_switcher"));
GtkWidget* measure_page = GTK_WIDGET(gtk_builder_get_object(app->window_ui, "measure_page"));
GtkWidget* signals_page = GTK_WIDGET(gtk_builder_get_object(app->window_ui, "signals_page"));
app->side_stack = side_stack;
app->side_switcher = side_switcher;
// Measure: what the cursors bracket, and the figures derived from it.
app->cursors_panel = build_cursors_panel(*app);
@@ -5642,7 +5645,7 @@ int run_ui(int argc, char** argv, const Config& cfg, ScopeModel& model, Acquisit
std::printf("[udpscope] panels: side=%dx%d (visible=%d) signals=%dx%d row_h=%d "
"(page_current=%d) trigger=%dx%d (sensitive=%d) switcher=%d cursors=%d window=%.3fs "
"window_cap=%.1fs\n",
app.dbg_side_w, app.dbg_side_h, app.dbg_side_visible, app.dbg_signal_w,
app.dbg_side_w, app.dbg_side_h, app.dbg_side_column_after_hide, app.dbg_signal_w,
app.dbg_signal_h, app.dbg_signal_row_h, app.dbg_signal_page_current,
app.dbg_trigger_w, app.dbg_trigger_h,
app.dbg_trigger_sensitive, app.dbg_switcher_visible,
+1 -1
View File
@@ -6,7 +6,7 @@
marks where the *view's* controls end, and the three at the far end.
The bar carries only the controls you reach for *while watching a signal*; everything you set up once lives
in the overflow menu, which is code — its rows act on the `RowSet` directly and two of them are toggles
in the overflow menu, which is code — its rows act on the `RowSet` directly and one of them is a toggle
whose state the rest of the interface reads, so a `GMenu` model would only add a second copy of that state.
The order is the file's, and it is the one thing here that is not self-evident: `pack_end()` lays its
-1
View File
@@ -737,7 +737,6 @@ struct App {
int dbg_trigger_h = 0;
int dbg_signal_page_current = 0;
int dbg_trigger_sensitive = 0;
int dbg_side_visible = 0;
int dbg_trigger_items = 0;
int dbg_trigger_selected = -1;
int dbg_side_column_after_hide = 1;
+3
View File
@@ -18,6 +18,9 @@ void apply_math_channels(App& app) {
// `set_math_channels()` takes the model's lock itself -- holding it here is a self-deadlock on a
// non-recursive mutex, and it is exactly how the first version of the probe hung the app.
app.model->set_math_channels(app.math);
// The structural rebuild above replaced the buffer and dropped its history. A stream's producer
// refills that; a file's nothing does, so the view is re-read -- and with the math set now in place
// the reload computes the derived channels over the file as it goes.
reload_file_view(app);
rebuild_math_pane(app);
// The math channels are rows of the signal list too -- `append_math_signals_locked()` puts them
+16 -2
View File
@@ -103,7 +103,6 @@ gboolean exit_probe_step3(gpointer user) {
a->dbg_side_column_after_hide = sidebar_shown;
a->dbg_switcher_visible =
a->side_switcher != nullptr && gtk_widget_get_visible(a->side_switcher) ? 1 : 0;
a->dbg_side_visible = sidebar_shown;
if (a->trigger_signal_drop != nullptr) {
GListModel* model = gtk_drop_down_get_model(GTK_DROP_DOWN(a->trigger_signal_drop));
a->dbg_trigger_items = model != nullptr ? static_cast<int>(g_list_model_get_n_items(model)) : 0;
@@ -4051,7 +4050,22 @@ gboolean keys_probe(gpointer user) {
before_after("l: layout", a->layout_toggle, AppAction::LayoutEditor);
before_after("s: side panel", a->sidebar_toggle, AppAction::SidePanel);
line += "\n";
// An accel must be refused while text is being typed. Focus the window entry, send 'm'
// through the real handler, and require the side tab not to change.
int refused = -1;
if (a->window_entry != nullptr) {
gtk_menu_button_popup(GTK_MENU_BUTTON(a->window_button));
gtk_widget_grab_focus(a->window_entry);
const std::string before_tab = a->side_tab;
const gboolean handled = on_app_key_pressed(nullptr, GDK_KEY_m, 0,
static_cast<GdkModifierType>(0), a);
const std::string after_tab = a->side_tab;
refused = (handled == FALSE && before_tab == after_tab) ? 1 : 0;
gtk_menu_button_popdown(GTK_MENU_BUTTON(a->window_button));
}
char tail[64];
std::snprintf(tail, sizeof(tail), " editable focused: 'm' refused=%d\n", refused);
line += tail;
std::fprintf(stderr, "%s", line.c_str());
std::fflush(stderr);