From c8f8a696011e51c8af70bc40d43780034abdbce5 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 10 Jun 2026 20:58:47 +0000 Subject: [PATCH 1/2] Fix v1.0 follow-up UI polish defects - About dialog: read MinVer's AssemblyInformationalVersion (stripping +commitsha build metadata) instead of the frozen MAJOR.0.0.0 AssemblyVersion, with fallback to the old value - Remove dead context-sensitive quick help feature: delete QuickHelpDialog, drop IKeybindingActions.ShowQuickHelp and its MainWindow implementation, and fix the stale HelpDialog tip text - Remove TrendPlot remnants from PR #164: enum value, switch arms, test assertion, and doc comment mention - Program.cs: validate config path before Application.Init so the error is visible, and dispose MainWindow in a try/finally - ScopeView: skip X-axis labels that cannot fit instead of crashing with ArgumentException in tiny terminals - LogView: unsubscribe Logger.LogAdded in Dispose - MainWindow: store the Application.SizeChanging handler in a field and unsubscribe it in Dispose - NodeDetailsView: guard against stale async attribute responses overwriting a newer selection - Remove unwired MainWindow.OnClientDisconnected and rename the File menu item "Start Recording..." to "Toggle Recording" (wired to the toggle handler to match Ctrl+R) https://claude.ai/code/session_012Vopnd9vWkzELveHRgZhie --- App/Dialogs/HelpDialog.cs | Bin 4917 -> 4914 bytes App/Dialogs/QuickHelpDialog.cs | 135 ------------------ App/Keybindings/DefaultKeybindings.cs | 1 - App/Keybindings/KeybindingContext.cs | 5 - App/Keybindings/KeybindingManager.cs | 4 +- App/MainWindow.cs | 51 ++++--- App/Views/LogView.cs | 4 + App/Views/NodeDetailsView.cs | 17 ++- App/Views/ScopeView.cs | 6 +- CLAUDE.md | 3 +- Configuration/Models/OpcilloscopeConfig.cs | 2 +- Program.cs | 39 +++-- .../App/Keybindings/KeybindingManagerTests.cs | 1 - 13 files changed, 80 insertions(+), 188 deletions(-) delete mode 100644 App/Dialogs/QuickHelpDialog.cs diff --git a/App/Dialogs/HelpDialog.cs b/App/Dialogs/HelpDialog.cs index d5517334d543ca0a6613853b39e7be8cc05ce8bf..d0dd562bda176044a7a4afb251eb7a4a047e898c 100644 GIT binary patch delta 53 zcmdn0wn=S655GcUi9%vtr9w$&ZmL2_zCv+Eez`(PMrN@>MruxhlBV_KKz@NoHB9LSboUa<)Q7YEHpq0|DvHNBHxY E0p7_HeE -/// Quick context-sensitive help overlay showing keybindings for the current context. -/// Inspired by lazygit's ? keybindings menu. -/// -public class QuickHelpDialog : Dialog -{ - private readonly KeybindingManager _keybindingManager; - - public QuickHelpDialog(KeybindingManager keybindingManager) - { - _keybindingManager = keybindingManager; - - var contextName = KeybindingManager.GetContextDisplayName(keybindingManager.CurrentContext); - Title = $" {contextName} - Keybindings "; - - // Calculate size based on content - var bindings = keybindingManager.GetActiveBindings().ToList(); - - int maxKeyWidth; - // Handle empty bindings case - if (bindings.Count == 0) - { - maxKeyWidth = 8; // Default width for "No keybindings" - Width = 44; - Height = 8; - } - else - { - maxKeyWidth = bindings.Max(b => b.KeyDisplay.Length); - var maxDescWidth = bindings.Max(b => b.Description.Length); - var contentWidth = Math.Max(maxKeyWidth + maxDescWidth + 6, 40); - - Width = Math.Min(contentWidth + 4, 60); - Height = Math.Min(bindings.Count + 6, 24); - } - - var theme = ThemeManager.Current; - - // Apply theme styling - ColorScheme = theme.MainColorScheme; - BorderStyle = theme.EmphasizedBorderStyle; - - // Create content with keybindings - var content = GenerateHelpContent(bindings, maxKeyWidth); - - var textView = new TextView - { - X = 1, - Y = 0, - Width = Dim.Fill(1), - Height = Dim.Fill(2), - ReadOnly = true, - WordWrap = false, - Text = content, - ColorScheme = new ColorScheme - { - Normal = new Attribute(theme.Foreground, theme.Background), - Focus = new Attribute(theme.Foreground, theme.Background), - HotNormal = new Attribute(theme.Foreground, theme.Background), - HotFocus = new Attribute(theme.Foreground, theme.Background), - Disabled = new Attribute(theme.MutedText, theme.Background) - } - }; - - // Close on any key - KeyDown += (_, e) => - { - if (e.KeyCode == KeyCode.Esc || e.KeyCode == (KeyCode)'?' || e.KeyCode == KeyCode.Enter) - { - RequestStop(); - e.Handled = true; - } - }; - - var closeButton = new Button - { - Text = "Close", - X = Pos.Center(), - Y = Pos.AnchorEnd(1), - IsDefault = true, - ColorScheme = theme.ButtonColorScheme - }; - closeButton.Accepting += (_, _) => RequestStop(); - - Add(textView); - Add(closeButton); - } - - private static string GenerateHelpContent(List bindings, int keyWidth) - { - var lines = new List(); - - if (bindings.Count == 0) - { - lines.Add(" No keybindings available for this context."); - } - else - { - // Group by category, deduping identical key rows (e.g. r/R variants - // where only the status-bar variant carries ShowInStatusBar). Ordering - // by priority keeps the status-bar variant. - var groups = bindings.GroupBy(b => b.Category); - - foreach (var group in groups) - { - var seen = new HashSet(); - foreach (var binding in group.OrderBy(b => b.StatusBarPriority)) - { - var rowKey = $"{binding.KeyDisplay} {binding.Description}"; - if (!seen.Add(rowKey)) - { - continue; - } - - var key = binding.KeyDisplay.PadRight(keyWidth + 2); - lines.Add($" {key}{binding.Description}"); - } - } - } - - // Add footer hint - lines.Add(""); - lines.Add(" Press ? or Esc to close"); - - return string.Join("\n", lines); - } -} diff --git a/App/Keybindings/DefaultKeybindings.cs b/App/Keybindings/DefaultKeybindings.cs index 8ee8649..3c480fb 100644 --- a/App/Keybindings/DefaultKeybindings.cs +++ b/App/Keybindings/DefaultKeybindings.cs @@ -16,7 +16,6 @@ public interface IKeybindingActions // Navigation void SwitchPane(); void ShowHelp(); - void ShowQuickHelp(); // Address Space void SubscribeSelected(); diff --git a/App/Keybindings/KeybindingContext.cs b/App/Keybindings/KeybindingContext.cs index 47d46fc..827d739 100644 --- a/App/Keybindings/KeybindingContext.cs +++ b/App/Keybindings/KeybindingContext.cs @@ -26,11 +26,6 @@ public enum KeybindingContext /// Scope, - /// - /// Keybindings active when TrendPlotView is displayed. - /// - TrendPlot, - /// /// Keybindings active in dialogs. /// diff --git a/App/Keybindings/KeybindingManager.cs b/App/Keybindings/KeybindingManager.cs index 3694b26..29f86cf 100644 --- a/App/Keybindings/KeybindingManager.cs +++ b/App/Keybindings/KeybindingManager.cs @@ -252,7 +252,6 @@ public static string GetContextDisplayName(KeybindingContext context) KeybindingContext.AddressSpace => "Address Space", KeybindingContext.MonitoredVariables => "Monitored Variables", KeybindingContext.Scope => "Scope View", - KeybindingContext.TrendPlot => "Trend Plot", KeybindingContext.Dialog => "Dialog", _ => context.ToString() }; @@ -279,8 +278,7 @@ private static int GetContextOrder(KeybindingContext context) KeybindingContext.AddressSpace => 1, KeybindingContext.MonitoredVariables => 2, KeybindingContext.Scope => 3, - KeybindingContext.TrendPlot => 4, - KeybindingContext.Dialog => 5, + KeybindingContext.Dialog => 4, _ => 99 }; } diff --git a/App/MainWindow.cs b/App/MainWindow.cs index efc81a5..40c09b4 100644 --- a/App/MainWindow.cs +++ b/App/MainWindow.cs @@ -51,6 +51,9 @@ public class MainWindow : Toplevel, DefaultKeybindings.IKeybindingActions private View? _focusedPanel; private FocusManager? _focusManager; + // Stored so it can be unsubscribed from the static Application event in Dispose + private readonly EventHandler _sizeChangingHandler; + // Lazygit-inspired keybinding system private readonly KeybindingManager _keybindingManager; @@ -223,7 +226,8 @@ public MainWindow() ApplyTheme(); // Handle window resize to update connection status label position - Application.SizeChanging += (s, e) => UiThread.Run(UpdateConnectionStatusLabelPosition); + _sizeChangingHandler = (s, e) => UiThread.Run(UpdateConnectionStatusLabelPosition); + Application.SizeChanging += _sizeChangingHandler; // Run status bar startup sequence RunStatusBarStartup(); @@ -295,7 +299,7 @@ private MenuBar CreateMenuBar() new MenuItem("_Save Config", "", SaveConfig, shortcutKey: Key.S.WithCtrl), new MenuItem("Save Config _As...", "", SaveConfigAs, shortcutKey: Key.S.WithCtrl.WithShift), null!, // Separator - new MenuItem("Start Recording...", "", () => OnRecordRequested(), shortcutKey: Key.R.WithCtrl), + new MenuItem("Toggle Recording", "", ToggleRecording, shortcutKey: Key.R.WithCtrl), new MenuItem("Stop Recording", "", () => OnStopRecordingRequested()), null!, // Separator new MenuItem("E_xit", "", () => RequestStop(), shortcutKey: Key.Q.WithCtrl) @@ -761,15 +765,6 @@ private void UpdateStatusBarShortcuts() _statusBar.SetNeedsLayout(); } - /// - /// Shows context-sensitive quick help overlay (lazygit-inspired ? menu). - /// - private void ShowQuickHelp() - { - using var dialog = new QuickHelpDialog(_keybindingManager); - Application.Run(dialog); - } - #endregion #region Global Keyboard Shortcuts @@ -867,14 +862,6 @@ private void OnConnectionStateChanged(ConnectionState state) }); } - private void OnClientDisconnected() - { - UiThread.Run(() => - { - UpdateConnectionStatus(isConnected: false); - }); - } - private void OnConnectionError(string message) { UiThread.Run(() => @@ -1111,7 +1098,7 @@ private void ShowHelp() private void ShowAbout() { - var version = Assembly.GetExecutingAssembly().GetName().Version?.ToString(3) ?? "0.0.0"; + var version = GetDisplayVersion(); var titleLine = $"opcilloscope v{version}"; var titlePadded = titleLine.PadLeft((38 + titleLine.Length) / 2).PadRight(38); @@ -1140,6 +1127,28 @@ industrial automation data in real-time. MessageBox.Query("About opcilloscope", about, "OK"); } + /// + /// Gets the application version for display. MinVer writes the full semver to + /// (AssemblyVersion is frozen at + /// MAJOR.0.0.0), so prefer that and strip any "+commitsha" build metadata. + /// + private static string GetDisplayVersion() + { + var assembly = Assembly.GetExecutingAssembly(); + var informational = assembly + .GetCustomAttribute()? + .InformationalVersion; + + if (!string.IsNullOrEmpty(informational)) + { + var metadataIndex = informational.IndexOf('+'); + return metadataIndex >= 0 ? informational[..metadataIndex] : informational; + } + + // Fall back to the assembly version if the attribute is missing + return assembly.GetName().Version?.ToString(3) ?? "0.0.0"; + } + #region Configuration File Handling /// @@ -1399,7 +1408,6 @@ public void LoadConfigFromCommandLine(string configPath) void DefaultKeybindings.IKeybindingActions.SwitchPane() => _focusManager?.FocusNext(); void DefaultKeybindings.IKeybindingActions.ShowHelp() => ShowHelp(); - void DefaultKeybindings.IKeybindingActions.ShowQuickHelp() => ShowQuickHelp(); void DefaultKeybindings.IKeybindingActions.SubscribeSelected() => SubscribeSelected(); void DefaultKeybindings.IKeybindingActions.RefreshTree() => RefreshTree(); void DefaultKeybindings.IKeybindingActions.UnsubscribeSelected() => UnsubscribeSelected(); @@ -1442,6 +1450,7 @@ protected override void Dispose(bool disposing) } Application.KeyDown -= OnApplicationKeyDown; + Application.SizeChanging -= _sizeChangingHandler; _connectionManager.Dispose(); } diff --git a/App/Views/LogView.cs b/App/Views/LogView.cs index dad77fa..6a57a96 100644 --- a/App/Views/LogView.cs +++ b/App/Views/LogView.cs @@ -153,6 +153,10 @@ protected override void Dispose(bool disposing) { if (disposing) { + if (_logger != null) + { + _logger.LogAdded -= OnLogAdded; + } _copyButton.Accepting -= OnCopyClicked; _listView.RowRender -= OnRowRender; ThemeManager.ThemeChanged -= OnThemeChanged; diff --git a/App/Views/NodeDetailsView.cs b/App/Views/NodeDetailsView.cs index 9678d64..9d7620a 100644 --- a/App/Views/NodeDetailsView.cs +++ b/App/Views/NodeDetailsView.cs @@ -120,6 +120,11 @@ public async Task ShowNodeByIdAsync(NodeId? nodeId) Application.Invoke(() => { + // Guard against stale responses: rapid selection changes can complete + // out of order, so only apply this result if it is still the current node. + if (!Equals(_currentNodeId, nodeId)) + return; + if (attrs == null) { _detailsLabel.Text = $"NodeId: {nodeId}\nFailed to read attributes"; @@ -165,14 +170,20 @@ public async Task ShowNodeAsync(BrowsedNode? node) return; } - _currentNodeId = node.NodeId; - var attrs = await _nodeBrowser.GetNodeAttributesAsync(node.NodeId); + var nodeId = node.NodeId; + _currentNodeId = nodeId; + var attrs = await _nodeBrowser.GetNodeAttributesAsync(nodeId); Application.Invoke(() => { + // Guard against stale responses: rapid selection changes can complete + // out of order, so only apply this result if it is still the current node. + if (!Equals(_currentNodeId, nodeId)) + return; + if (attrs == null) { - _detailsLabel.Text = $"NodeId: {node.NodeId}\nFailed to read attributes"; + _detailsLabel.Text = $"NodeId: {nodeId}\nFailed to read attributes"; _copyButton.Enabled = false; SetNormalColor(); return; diff --git a/App/Views/ScopeView.cs b/App/Views/ScopeView.cs index 17cedd1..f06a49e 100644 --- a/App/Views/ScopeView.cs +++ b/App/Views/ScopeView.cs @@ -619,7 +619,11 @@ private void DrawXAxisLabels(AppTheme theme, int plotLeft, int labelY, int x = plotLeft + (int)(fraction * (plotWidth - 1)); // Center the label around x int labelX = x - label.Length / 2; - labelX = Math.Clamp(labelX, plotLeft, plotLeft + plotWidth - label.Length); + int maxLabelX = plotLeft + plotWidth - label.Length; + if (maxLabelX < plotLeft) + continue; // Label wider than the plot area (tiny terminal) - skip it + + labelX = Math.Clamp(labelX, plotLeft, maxLabelX); Move(labelX, labelY); Driver!.AddStr(label); diff --git a/CLAUDE.md b/CLAUDE.md index 4858f26..deaf00f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -86,8 +86,7 @@ Opcilloscope/ │ │ ├── ScopeDialog.cs # Multi-signal scope dialog (up to 5 signals) │ │ ├── SaveConfigDialog.cs # Save configuration file dialog │ │ ├── SaveRecordingDialog.cs # Save CSV recording dialog -│ │ ├── HelpDialog.cs # Full help/documentation dialog -│ │ └── QuickHelpDialog.cs # Quick keyboard shortcuts reference +│ │ └── HelpDialog.cs # Full help/documentation dialog │ ├── Keybindings/ │ │ ├── Keybinding.cs # Keybinding model (key, action, context) │ │ ├── KeybindingContext.cs # Context enum (Global, AddressSpace, etc.) diff --git a/Configuration/Models/OpcilloscopeConfig.cs b/Configuration/Models/OpcilloscopeConfig.cs index be293a2..50715d9 100644 --- a/Configuration/Models/OpcilloscopeConfig.cs +++ b/Configuration/Models/OpcilloscopeConfig.cs @@ -76,7 +76,7 @@ public class SubscriptionSettings /// /// Publishing interval (in milliseconds) for the OPC UA subscription. /// Controls how often the server sends data-change notifications to the client. - /// This directly determines the data resolution for Scope, Trend Plot, and CSV recording: + /// This directly determines the data resolution for Scope and CSV recording: /// a 1000 ms interval means roughly one data point per second per variable. /// Valid range: 100-10000 ms (values outside this range will be clamped by SubscriptionManager). /// diff --git a/Program.cs b/Program.cs index a3ffcd1..acaab5b 100644 --- a/Program.cs +++ b/Program.cs @@ -57,33 +57,42 @@ static int Main(string[] args) } } + // Validate the config file path before initializing the terminal, so the error + // message is printed to the regular screen rather than being lost in the + // alternate screen buffer (same pattern as --help above). + if (!string.IsNullOrEmpty(configPath) && !File.Exists(configPath)) + { + Console.Error.WriteLine($"Error: Configuration file not found: {configPath}"); + return 1; + } + #pragma warning disable IL2026 // Terminal.Gui Application.Init uses reflection and is not AOT-compatible Application.Init(); #pragma warning restore IL2026 initialized = true; var mainWindow = new MainWindow(); - - // Load config file if specified (takes precedence over URL) - if (!string.IsNullOrEmpty(configPath)) + try { - if (!File.Exists(configPath)) + // Load config file if specified (takes precedence over URL) + if (!string.IsNullOrEmpty(configPath)) { - Console.Error.WriteLine($"Error: Configuration file not found: {configPath}"); - return 1; + mainWindow.LoadConfigFromCommandLine(configPath); } - mainWindow.LoadConfigFromCommandLine(configPath); + // Otherwise, if auto-connect URL provided, show warning (not yet implemented) + else if (!string.IsNullOrEmpty(autoConnectUrl)) + { + Console.Error.WriteLine( + $"Warning: Auto-connect via command-line URL ('{autoConnectUrl}') is not currently implemented. " + + "Please use a configuration file with an endpoint URL instead."); + } + + Application.Run(mainWindow); } - // Otherwise, if auto-connect URL provided, show warning (not yet implemented) - else if (!string.IsNullOrEmpty(autoConnectUrl)) + finally { - Console.Error.WriteLine( - $"Warning: Auto-connect via command-line URL ('{autoConnectUrl}') is not currently implemented. " + - "Please use a configuration file with an endpoint URL instead."); + mainWindow.Dispose(); } - - Application.Run(mainWindow); - mainWindow.Dispose(); } catch (Exception ex) { diff --git a/Tests/Opcilloscope.Tests/App/Keybindings/KeybindingManagerTests.cs b/Tests/Opcilloscope.Tests/App/Keybindings/KeybindingManagerTests.cs index 2070eb0..983843a 100644 --- a/Tests/Opcilloscope.Tests/App/Keybindings/KeybindingManagerTests.cs +++ b/Tests/Opcilloscope.Tests/App/Keybindings/KeybindingManagerTests.cs @@ -475,7 +475,6 @@ public void GetContextDisplayName_ReturnsHumanReadableNames() Assert.Equal("Address Space", KeybindingManager.GetContextDisplayName(KeybindingContext.AddressSpace)); Assert.Equal("Monitored Variables", KeybindingManager.GetContextDisplayName(KeybindingContext.MonitoredVariables)); Assert.Equal("Scope View", KeybindingManager.GetContextDisplayName(KeybindingContext.Scope)); - Assert.Equal("Trend Plot", KeybindingManager.GetContextDisplayName(KeybindingContext.TrendPlot)); Assert.Equal("Dialog", KeybindingManager.GetContextDisplayName(KeybindingContext.Dialog)); } From 525f77b283d72f2136fc8d0c1a621005a807ce2b Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 11 Jun 2026 02:54:29 +0000 Subject: [PATCH 2/2] Address review: NUL byte in HelpDialog.cs, redundant Stop Recording menu item - Replace a literal NUL byte in HelpDialog's dedup key with the \0 escape sequence (same runtime string) so git stops treating the file as binary - the NUL predates this branch but blocked diff review - Drop the File-menu 'Stop Recording' item: 'Toggle Recording' already stops an active recording, leaving the second item with no distinct purpose https://claude.ai/code/session_012Vopnd9vWkzELveHRgZhie --- App/Dialogs/HelpDialog.cs | Bin 4914 -> 4915 bytes App/MainWindow.cs | 1 - 2 files changed, 1 deletion(-) diff --git a/App/Dialogs/HelpDialog.cs b/App/Dialogs/HelpDialog.cs index d0dd562bda176044a7a4afb251eb7a4a047e898c..6b61b3466416bdfb5401c4a8d0ece700585fa3ff 100644 GIT binary patch delta 15 Wcmdm_wpne%PhO@NgU!EqEm#3G<^}Bl delta 14 Vcmdn2wn=TnPhLib&A)jqSOG291z`XH diff --git a/App/MainWindow.cs b/App/MainWindow.cs index 40c09b4..563228d 100644 --- a/App/MainWindow.cs +++ b/App/MainWindow.cs @@ -300,7 +300,6 @@ private MenuBar CreateMenuBar() new MenuItem("Save Config _As...", "", SaveConfigAs, shortcutKey: Key.S.WithCtrl.WithShift), null!, // Separator new MenuItem("Toggle Recording", "", ToggleRecording, shortcutKey: Key.R.WithCtrl), - new MenuItem("Stop Recording", "", () => OnStopRecordingRequested()), null!, // Separator new MenuItem("E_xit", "", () => RequestStop(), shortcutKey: Key.Q.WithCtrl) }),