Skip to content

Fix: Preserve reading position after a small log-file shrink - #732

Closed
Pr0metheus2 wants to merge 1 commit into
LogExperts:Developmentfrom
Pr0metheus2:fix/preserve-reading-position-after-file-shrink
Closed

Pr0metheus2 wants to merge 1 commit into
LogExperts:Developmentfrom
Pr0metheus2:fix/preserve-reading-position-after-file-shrink

Conversation

@Pr0metheus2

Copy link
Copy Markdown
Contributor

This is a proposed fix for the issue Scrolling to the top of the file #639. I was able to replicate this behavior by removing only one character at the end of the tested file.

  • Current status:
    When a monitored log file shrinks, LogExpert reloads it as a new file. Even removing one trailing character could therefore move the view away from the line being read and turn "Follow tail" back on.

  • Fix:
    This change saves the visible position before an automatic reload and restores it when the reloaded file has the same number of lines or one fewer. A substantial drop in line count keeps the existing reset behavior for rotation.

Verification: The manual reproduction confirmed that the reading position is restored after removing the character.

When a monitored log file shrinks, LogExpert reloads it as a new file. Even removing one trailing character could therefore move the view away from the line being read and turn Follow tail back on.

This change saves the visible position before an automatic reload and restores it when the reloaded file has the same number of lines or one fewer. A substantial drop in line count keeps the existing reset behavior for rotation.

Verification: 40 related tests passed; the full suite passed with 1,014 passed and 7 skipped. The five-step manual reproduction confirmed that the reading position is restored after removing the character
@Hirogen

Hirogen commented Oct 12, 2026

Copy link
Copy Markdown
Member

can you verify this with a unit test as well?

@Hirogen

Hirogen commented Oct 12, 2026

Copy link
Copy Markdown
Member
  • "keeps scrolling to the top of the file while I'm reading in the middle … It seems to tail properly if I'm looking at the bottom." It never says the file shrinks. A growing log never reaches ReloadNewFile; only the shrink-or-delete check (newSize < FileSize || _isDeleted) in LogfileReader.FireChangeEvent (~L1606) does. Deleting a trailing character is a guess, not a proven one.
  • If the line count drops by two or more, the view still jumps back, even when the old position is still valid. A logger that truncates and rewrites in bursts would still show the bug.
  • "ArgumentException 'Filter string you provided is not valid…' at HighlightDialog.OnBtnExportGroupClick": the PR doesn't address this. The filter string looks correct on Development in the English, German and Chinese resource files (en, de, zh-CN), so it is probably already fixed. The PR should say so.
    - There is no regression test for the threshold rule.
  • Setting _reloadMemento during automatic reloads changes columnizer handling on rollover (same as Standards note 1). It now depends on whether follow tail is on.
  • Follow tail is forced off after a rollover whenever the new file happens to be about as long as the old one.
  • The "same or one fewer line" threshold is arbitrary. If a rotated or truncated file ends up with at least previous−1 lines, the old line number is applied to unrelated content and follow tail is switched off. That breaks the rollover case the PR says it keeps working.
  • Mismatched line counts. The display-row count is compared with the file-line count (same as Standards note 2). Both should come from _logFileReader.
  • Content is never compared. Nothing checks whether the lines before the cut are unchanged, so an edited tail on the same file can't be told apart from a brand-new file.
  • Fine as is: switching from the _reloadMemento field to the reloadMemento parameter is correct, and the new field is cleared properly.
  1. Biggest risk. A non-null _reloadMemento is also used as an "is this a manual reload" flag. When it is set, PreSelectColumnizerByName (~L2856) and CloneMemoryColumnizer (~L3164, 6015, 6040) are skipped. The PR now sets it during automatic reloads, so how the columnizer is chosen after a rollover changes. Unlike Reload(), the automatic path doesn't set _forcedColumnizerForLoading.
  2. Two different line counts are compared. The "before" count comes from _visibleRows.Map.LineCount, the "after" count from _logFileReader.LineCount. These may not match when some lines are hidden.
  3. FollowTailChanged(false, false) looks redundant. Follow tail was already off when the state was saved. If the call exists to undo LoadFile turning it back on, it needs a comment.
  4. Threading. Both fields are cleared on the reader thread, but they aren't volatile or locked. This matches the existing pattern for _reloadMemento.
  • Duplicated code. The new ReloadMemento { CurrentLine = CurrentLineNum, FirstDisplayedLine = … } added in ReloadNewFile repeats the one in Reload().
  • Data that belongs together is kept apart. _lineCountBeforeAutomaticReload always travels with _reloadMemento, and whether it is null secretly means "manual or automatic reload". It belongs on ReloadMemento (src/LogExpert.Core/Entities/ReloadMemento.cs). That would also remove the three separate places that set and clear it.
  • Unexplained number. The previousLineCount - 1 tolerance is only explained by a comment. A named constant or a small helper method would explain itself.

@Pr0metheus2

Copy link
Copy Markdown
Contributor Author

If this proposed change would make things worse, it's better to close PR.

@Hirogen

Hirogen commented Oct 12, 2026

Copy link
Copy Markdown
Member

If this proposed change would make things worse, it's better to close PR.

no, but we need a unit test as prove, otherwise we just fix things left and right without knowing if it really fixes a problem, and we also need to keep in mind that we don't break other things while fixing bugs.

@Pr0metheus2

Copy link
Copy Markdown
Contributor Author

The question is, if we really want to fix this issue (is it really issue?) - at least my replicated behavior by removing only one character at the end of the tested file. In real-world, log file should only increase, not decrease. If file shrinks, it must happen ONLY when a new rotating file is created. So either the ProfileX has different problem and he needs to replicate it to find a bug, or his log file shrinks by some bytes or lines - but in such case, this should not happen anyway and is not proper bahaviour of whatever application is creating his log.

@Hirogen

Hirogen commented Oct 14, 2026

Copy link
Copy Markdown
Member

you are right, deleting characters in a log file normally does not happen, so this is in my opinion a out of scope use case. So I wouldn't fix it, if it only happens when a character is deleted

@Pr0metheus2

Copy link
Copy Markdown
Contributor Author

Agreement

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