Skip to content

fix/SaveLastOpenFilesList() - #692

Merged
Hirogen merged 4 commits into
LogExperts:Developmentfrom
Pr0metheus2:fix/SaveLastOpenFilesList()
Aug 24, 2026
Merged

fix/SaveLastOpenFilesList()#692
Hirogen merged 4 commits into
LogExperts:Developmentfrom
Pr0metheus2:fix/SaveLastOpenFilesList()

Conversation

@Pr0metheus2

@Pr0metheus2 Pr0metheus2 commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Fix for clearing "LastOpenFilesList" on app exit.
When app closes, it looks like it wipes the set of files to restore.

Before exit:
image

After exit:
image

Also fixes two other places (both places are affected by this fix and should now work correctly):

  • Close other tabs (LogTabWindow.cs:2326)
  • Session-save file list (LogTabWindow.cs:2410)

@Hirogen

Hirogen commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

looks good, but pls change your indent style from tab to spaces.
Pls also add a unit test for this case [TabControllerTests.cs] should be the right class

Also change the inner for to an foreach, the for is not needed

@Pr0metheus2

Copy link
Copy Markdown
Contributor Author

Ok

@Pr0metheus2

Copy link
Copy Markdown
Contributor Author

The indexed loop for DisplayingContents should be kept; it exposes displayed entries through its custom Count and indexer, while foreach uses the inherited ReadOnlyCollection enumerator and does not return the displayed tabs. The prior LINQ implementation had this failure and caused an empty LastOpenFilesList at shutdown.

@Hirogen Hirogen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

As a side note, pls also add to the pr description:
this fixes also 2 other places "Close other tabs" [LogTabWindow.cs:2326] and session-save file list [LogTabWindow.cs:2410], both places are affected by this fix and should now work correctly

Comment thread src/LogExpert.UI/Services/TabControllerService/TabController.cs
Comment thread src/LogExpert.UI/Services/TabControllerService/TabController.cs
Comment thread src/LogExpert.Tests/Services/TabControllerTests.cs Outdated
Comment thread src/LogExpert.Tests/Services/TabControllerTests.cs
Comment thread src/LogExpert.Tests/Services/TabControllerTests.cs
Comment thread src/LogExpert.Tests/Services/TabControllerTests.cs Outdated
Comment thread src/LogExpert.Tests/Services/TabControllerTests.cs Outdated
@Pr0metheus2

Copy link
Copy Markdown
Contributor Author

All should be fixed as required.

test: cover displayed tabs
fix/SaveLastOpenFilesList()

1. SaveLastOpenFilesList() clears LastOpenFilesList.
2. It receives an empty list from GetAllWindowsFromDockPanel().
3. Nothing is added back.
4. The empty list is saved to settings.json.
@Pr0metheus2
Pr0metheus2 force-pushed the fix/SaveLastOpenFilesList() branch from 0623ace to 5c3d5c9 Compare August 4, 2026 10:00
@Hirogen
Hirogen self-requested a review August 21, 2026 08:29
@Hirogen

Hirogen commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

Sorry been on holidays, here is another review, only /t tabs, pls use spaces

LogExpert.Tests.csproj

  • literal tab. +\t is tab-indented while the PackageReference line below it and every sibling use 4 spaces.

TabControllerTests.cs

  • literal tab + whitespace-only line. The #region GetAllWindows Tests line is re-emitted as +\t#region GetAllWindows Tests, preceded by a new line containing only four spaces. Same editorconfig rules; the tab also breaks alignment with all 30+ other #region lines in the file.
  • mixed indentation inside the initializer. In Setup() the two-line comment splits styles: // Match LogExpert's document style; … (spaces) then \t\t\t// Using DockingWindow both matches production behavior … (tabs). The second line is also ~180 chars where the first was wrapped.
  • "CreateLogWindow can now provide a non-null LogWindow when needed" describes a capability the test never exercises; it explains the helper, not the assertion.
  • #region Helpers is wedged between GetAllWindowsFromDockPanel Tests and GetAllWindows Tests, breaking the file's one-region-per-SUT-member ordering. Move helpers to the end.

TabController.cs

  • misaligned comment. The two // Use 'for' instead of 'foreach': … lines sit at 8 spaces inside a 12-space foreach body, directly above the for they document.

@Hirogen Hirogen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

see comment before

I alway forget you require spaces and not tab indent.
Spaces alignment
Spaces fix (again) and other requests.
@Pr0metheus2

Copy link
Copy Markdown
Contributor Author

Check please

@Hirogen
Hirogen merged commit 7982ba2 into LogExperts:Development Aug 24, 2026
2 checks passed
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