Persistence fix on app exit/start + Highlight dialog Import/Export error fix by Pr0metheus2 · Pull Request #662 · LogExperts/LogExpert

Pr0metheus2 · 2026-07-23T10:06:52Z

Bug Fix :

.lxp persistence on exit when automatic saving and filter-tab restoration are enabled.
Save the live filter UI state before shutdown teardown, so FilterParamsList is no longer written empty or stale.
Prevent close-time tab disposal from overwriting the already saved snapshot.
Added:
a regression test covering the snapshot behavior.

Bug Fix : - .lxp persistence on exit when automatic saving and filter-tab restoration are enabled. - Save the live filter UI state before shutdown teardown, so FilterParamsList is no longer written empty or stale. - Prevent close-time tab disposal from overwriting the already saved snapshot. Added: - a regression test covering the snapshot behavior.

- Fix the Highlight dialog export button by correcting the SaveFileDialog.Filter resource string. - The filter now uses valid description|pattern pairs, so exporting highlight groups no longer throws an ArgumentException. - Update the localized resources consistently so the export dialog works in all supported languages.

Pr0metheus2 · 2026-07-24T07:38:21Z

Seems error on GitHub side? Or how fix it?

Hirogen · 2026-07-24T07:48:08Z

        return snapshot;
    }

+    private FilterParams CaptureCurrentFilterParams ()


Duplicated Code — CaptureCurrentFilterParams() repeats the controls→FilterParams copy shape already present at ~4420–4440 (FilterSearch) and again in the dirty-check at ~4770–4795. Three sites now share the shape; one helper is warranted — though "Surgical Changes" partially excuses not touching the pre-existing sites.

Thanks. I agree the duplication is real, but I kept this commit "surgical" and only touched the new capture path.

Hirogen · 2026-07-24T07:49:24Z

+        GC.SuppressFinalize(this);
+    }
+
+    private void Dispose (bool disposing)


Duplicated Code / Speculative Generality (tests) — the test fixture implements a full Dispose(bool)/_disposed pattern that duplicates its own TearDown; possibly analyzer-driven (NUnit1032), but the guard-flag pattern is overkill either way.

Yes, mostly style noise, but it is not a product bug.

Hirogen · 2026-07-24T07:50:17Z

+    {
+        SetText("filterComboBox", "LIVE_FILTER_TEXT");
+        SetText("filterRangeComboBox", "LIVE_RANGE_TEXT");
+        SetChecked("filterCaseSensitiveCheckBox", true);


Speculative Generality (tests) — SetChecked/SetValue use reflection (GetProperty("Checked")) on controls that could be typed directly.

Mostly style noise, not a correctness issue.

Hirogen · 2026-07-24T07:51:13Z

+    /// </summary>
+    [Browsable(false)]
+    [DesignerSerializationVisibility(DesignerSerializationVisibility.Hidden)]
+    internal bool SkipPersistenceSaveOnClose { get; set; }


Control-flag coupling — LogTabWindow mutates SkipPersistenceSaveOnClose on children before closing them; a Close(savePersistence: false) path would avoid the mutable flag.

Agree that a Close(savePersistence: false) would be cleaner. I used the existing flag here to avoid widening the close-path change in this commit and to keep the behavior isolated.

Hirogen · 2026-07-24T07:52:36Z

@@ -0,0 +1,166 @@
+using System.Reflection;


"Added: a regression test covering the snapshot behavior." — Partial. The test covers only the live-UI capture in GatherSessionSnapshot. Neither of the other two claims is tested: nothing exercises "Save the live filter UI state before shutdown teardown" (the ordering in OnLogTabWindowFormClosing), and SkipPersistenceSaveOnClose is never touched by any test.
".lxp persistence on exit/start" — the "start" half has no changes; the restore path (RestoreFilters, [LogWindow.cs:2677] already used FilterParamsList[0], so exit-side changes alone plausibly complete the fix.

You are right, “exit/start” is only partially backed by tests. The fix is primarily on the save/exit side, not on restore logic.

Hirogen · 2026-07-24T07:53:29Z

    </data>
  <data name="HighlightDialog_UI_Export_Filter" xml:space="preserve">
-        <value>Settings (*.json)|*.json|All files (*.*)</value>
+        <value>Settings (*.json)|*.json|All files (*.*)|*.*</value>


This is a fix for export, not import! Import already has it.

Hirogen · 2026-07-24T07:56:56Z

        }

-        SavePersistenceData(false);
+        if (!SkipPersistenceSaveOnClose)


"Prevent close-time tab disposal from overwriting the already saved snapshot." — Works for the guarded path (pre-save runs before RemoveAndDisposeLogWindow, and OnLogWindowClosing at [LogWindow.cs:917] (skips the re-save). But the flag is set-only and never reset — if FormClosing is ever cancelled after the handler runs, surviving windows silently skip persistence on all later manual tab closes. Also Reload() ([LogWindow.cs:7420] and ReloadNewFile() call SavePersistenceData(false) unguarded — a rollover firing mid-shutdown could still overwrite the snapshot. Narrow races, worth noting.

Could be a concern. The flag is only set in the app shutdown cascade, immediately before the pre-save and disposal sequence. I don’t see a normal path where it remains active after that close flow completes.
This is relevant if the close cascade aborts or throws before disposal completes. LogTabWindow does swallow these exceptions in its close handler.

Hirogen · 2026-07-24T07:57:51Z

+    /// snapshot after child filter tabs have already been torn down.
+    /// </summary>
+    [Browsable(false)]
+    [DesignerSerializationVisibility(DesignerSerializationVisibility.Hidden)]


[Browsable(false)]/[DesignerSerializationVisibility] on SkipPersistenceSaveOnClose are speculative decoration; LogWindow is never designer-hosted with this property.

It looks unnecessary because this is not a designer-driven surface, but it is harmless metadata, not a correctness issue. I would treat it as cleanup, not a defect. They are not behavior-changing.

Hirogen · 2026-07-24T07:59:23Z

Seems error on GitHub side? Or how fix it?

probably because the github actions cant write on the branch, development branches are normally protected, thats why a separate issue branch should be created and then a PR into the dev branch, to avoid it.

Pr0metheus2 · 2026-07-24T10:21:29Z

If you recommend, I can close this pull request and start over again with small individual steps.

Pr0metheus2 force-pushed the Development branch from 7335327 to a190451 Compare July 23, 2026 10:58

Pr0metheus2 changed the title ~~Persistence fix on app exit/start~~ Persistence fix on app exit/start + Highlight dialog Import/Export error fix Jul 23, 2026

Hirogen requested changes Jul 24, 2026

View reviewed changes

Pr0metheus2 closed this Jul 24, 2026

Conversation

Pr0metheus2 commented Jul 23, 2026

Uh oh!

Pr0metheus2 commented Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Hirogen commented Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Pr0metheus2 commented Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Pr0metheus2 commented Jul 24, 2026 •

edited

Loading

Hirogen commented Jul 24, 2026 •

edited

Loading

Pr0metheus2 commented Jul 24, 2026 •

edited

Loading