Skip to content

Opening a single log file silently loads its rotated siblings — LogfileReader.IsMultiFile is always true #679

Description

@Hirogen

Category: bug
Area: log reading / multi-file
Carved out of: #444 (which keeps the design questions — see Out of scope)
Found: while removing the dead memory-mapped reader on branch fix/configured-encoding-registry

What happens

Open app.log in a directory that also contains app.log.1. Both files are loaded into the tab as one
logical stream, even though multi-file mode is off — File → Multi-file is unchecked and nothing in the
UI indicates that a second file was pulled in.

Cause

LogfileReader's single private constructor does:

IsMultiFile = multiFile || fileNames.Length == 1;

Both public constructors funnel into it. The single-file overload passes [fileName], so
fileNames.Length == 1 and IsMultiFile becomes true. The multi-file overload hard-codes
multiFile: true. IsMultiFile is therefore true in every reachable configuration and the
multiFile parameter never changes the outcome.

Because it is true, the constructor resolves names through
new RolloverFilenameHandler(GetLogFileInfo(_fileName), _multiFileOptions).GetNameList(...) instead of
[_fileName]. The default MultiFileOptions.FormatPattern is *$J(.) — an index pattern — so the handler
probes app.log.1, app.log.2, … on disk and adds every one that exists. _fileName is then reassigned to
fileInfo.FullName of the last file found.

Note that LogWindow.IsMultiFile (the window flag, the one written to persistence) is a separate field
that correctly defaults to false. So the window says "single file" while its reader says "multi file".
Persisted .lxp sessions are not polluted by this and need no migration.

Reproduce

  1. Create app.log with 10 lines and app.log.1 with 10 lines in the same directory.
  2. Open only app.log in LogExpert.
  3. The tab shows 20 lines. File → Multi-file is unchecked.

The fix

IsMultiFile = multiFile;

Multi-file is an explicit caller decision and is never inferred from the array length. In particular the
multi-file overload keeps hard-coding true: RolloverFilenameHandler expands one name into many, so
"one filename + a rollover pattern" is a legitimate multi-file case, not a contradiction. #444's framing
that "it's only multifile if there are 2 or more files in the array" does not survive contact with the code
BufferShiftTest constructs the reader with a single filename and multiFile: true, and that is exactly
the rollover scenario the feature exists for.

Blast radius — three dead branches come back to life

Because IsMultiFile has been unconditionally true, three code paths have never executed in any recent
release. The fix turns all three on for single-file tabs. This is the part that needs care in review:

  1. names = [_fileName] in the constructor. Trivially correct; this is the fix's whole point.
  2. The else branch of FireChangeEvent. Today, when a watched file shrinks or is deleted
    (if (newSize < FileSize || _isDeleted)), a single-file tab runs ShiftBuffers() and reports
    IsRollover = true. After the fix it calls _progressReporter.ReportNewFile(...).
  3. LogWindow's NewFile handling. ReportNewFile is the only producer of LoadFileEventArgs.NewFile,
    so LogWindow's if (e.NewFile) → ReloadNewFile() is dead today too. ReloadNewFile does
    SavePersistenceData → full LoadFileClearBookmarkList()SavePersistenceData, and
    ClearBookmarkList calls _bookmarkProvider.ClearAllBookmarks()manual bookmarks included, then
    persists the empty list.

Accepted consequence: truncating or deleting a tailed single-file log now reloads the tab and clears all
its bookmarks. That is the intended single-file semantic — the content those bookmarks pointed at is gone.
Deletion and in-place truncation (newSize < FileSize, e.g. a writer that reopens the same file with
truncate) are deliberately treated the same way; the line numbers are equally meaningless in both cases.
ReloadNewFile itself is not to be modified under this issue.

Out of scope — all of this stays with #444

  • FileOperationService.LoadFilesWithOption's fileNames.Length == 1 short-circuit, which means a single
    file can never enter multi-file mode from drag/drop even with Shift held. That is Multi File Behaviour #444's "opening a file
    from the open menu should have a multi-file option".
  • Moving the flag out of the constructor. Multi File Behaviour #444 says it "should be removed from the constructor … it has to
    find its way into the class from the UI". Keep both public constructors and the bool multiFile
    parameter exactly as they are.
  • SwitchMultiFile semantics on an open tab. This fix makes File → Multi-file off do something for the
    first time (today toggling off still re-expands rollovers); refining what it should do — reactive vs
    observable — is Multi File Behaviour #444's.
  • ReloadNewFile / bookmark clearing, per above.
  • Wiki updates.

Acceptance criteria

Tests 1–3 go in a new file src/LogExpert.Tests/StreamReaderTests/LogfileReaderMultiFileFlagTests.cs, each
building a temp directory containing both app.log and app.log.1 so the default *$J(.) pattern has
something to find. GetLogFileInfoList() is public and is the assertion surface:

  1. SingleFileCtor_MultiFileFalse_DoesNotExpandRollovermultiFile: falseIsMultiFile == false,
    GetLogFileInfoList() has exactly one entry, LineCount equals only app.log's line count.
    This test fails before the fix.
  2. SingleFileCtor_MultiFileTrue_ExpandsRollovermultiFile: trueIsMultiFile == true, two entries.
    Passes today and must keep passing; this is the "one name + pattern" feature guard.
  3. MultiFileCtor_AlwaysMultiFile — the array overload with a single-element array ⇒
    IsMultiFile == true. Pins the decision so nobody later adds && fileNames.Length > 1.
  4. SingleFileReader_OnTruncation_ReportsNewFileNotRolloverplaced in
    src/LogExpert.Tests/Buffers/ beside BufferShiftTest
    , not in the new file: it is a monitoring test,
    and BufferShiftTest already solves the watch-and-mutate timing problem, so reuse its helpers. Start
    monitoring a single-file reader with multiFile: false, truncate the file, assert a NewFile report
    arrives and no IsRollover event does. Use a fake ILoadProgressReporter. Give it a generous timeout so
    a flake is obvious rather than mysterious.
  5. BufferShiftTest passes unmodified. Do not edit it. It is the proof that rollover still works.

Verification

dotnet test src/LogExpert.Tests/LogExpert.Tests.csproj
dotnet test src/LogExpert.Tests/LogExpert.Tests.csproj --filter BufferShiftTest

Optional manual check with src/tools/LogRotator/ while watching the files in LogExpert.

Definition of done

Branch fix/single-file-multifile-flag off Development, conventional-commit subject, no Co-Authored-By
trailer. Do not open the PR — commit, run the suite, and leave a drafted PR title/body for the
maintainer to post.

Files: src/LogExpert.Core/Classes/Log/LogfileReader.cs (private constructor, line ~153) plus the two test
files above. No production file other than LogfileReader.cs should change.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugPesky little gritter, needs squashing

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions