[Pr0metheus2]

This is a proposed fix for the Issue #735 and #185

Problem

The Max days setting only controlled how many previous calendar dates were searched when the MultiFile mask contained a $D date placeholder.

It did not check the last-modified date of files found using index patterns such as:

• *$J(.) → app.log, app.log.1, app.log.2
• *$J(.).log → app.log, app.1.log, app.2.log

As a result, all indexed rollover files were loaded regardless of their age.

Changes

• Added the file’s UTC last-write timestamp to ILogFileInfo.
• Implemented timestamp retrieval for:
  • Local files
  • SFTP files
• Applied the Max days limit when MultiFile files are discovered.
• The file explicitly opened by the user is always retained.
• MaxDayTry = 0 preserves the previous unlimited behavior.
• Filesystem plugins that cannot provide a timestamp retain the previous behavior and their files are not excluded.

Tests

Added coverage for both supported rollover filename layouts:

• app.log, app.log.1, app.log.2
• app.log, app.1.log, app.2.log

The tests verify that with MaxDayTry = 3:

• The active file is loaded.
• A file modified two days ago is loaded.
• A file modified four days ago is excluded.
• Setting MaxDayTry = 0 disables age filtering.

The existing $D date-placeholder rule remains working.

• added text hint (EN, DE CN)

[Hirogen]

hi, i added an analyses to the issue, its not really a bug

[Hirogen]

The PR doesn't do what the #735 spec asks. The spec wants a new, separate max-file-age option. The PR instead turns the existing MaxDayTry setting into an age limit. Because MaxDayTry still defaults to 3, every existing user with an index pattern like *$J(.) will silently lose rollover files older than 3 days. The spec lists exactly that change as out of scope.

Standards

Bugs

1. The "0 = unlimited" hint is wrong for $D date patterns. The date search still loops while (tryCounter < _options.MaxDayTry). With 0 that loop never runs, so no dated companion files load at all. The new hint text and the XML doc comment ("Zero disables the file-age limit") are only true for index patterns.
2. Existing users get a silent behaviour change. Index patterns had no age limit before. Now the default (and any saved MaxDayTry=3) drops app.log.N files older than 72 hours, and nothing tells the user.
3. Age is measured from now, not by calendar day. DateTime.UtcNow.AddDays(-MaxDayTry) is a rolling 72-hour window, but the date search counts calendar days. A file from day −3 written early in the morning is dropped even though the date search reached it.

Repo standards (judgement calls, none hard)

• Unrelated newline edits. A final newline is added to 9 .cs files the PR doesn't otherwise touch, and to Resources.zh-CN.resx. src/.editorconfig sets insert_final_newline = false for .cs, and .github/CLAUDE.md §3 says every changed line should trace to the request. The rule is a bit ambiguous, because the same editorconfig also sets resharper_csharp_insert_final_newline = true.
• Comment edits in MultiFileMaskDialog.Designer.cs. An existing // separator becomes //, and the new block doesn't match its neighbours. Same §3 rule.

Smells (judgement calls)

• One setting, two meanings. MaxDayTry now means both "missing days to skip" and "maximum file age", and its doc comment has to explain both.
• Duplicated code in SFTP. SftpLogFileInfo.LastWriteTimeUtc copies the try/_sftp.Get/catch shape of Length. It also adds an extra remote call for every file it checks.
• Over-broad catch. The SFTP getter catches ArgumentNullException, which Length next to it does not; catching it can hide bugs.
• Clock can't be faked. The code reads DateTime.UtcNow directly. The tests use real timestamps but leave a full day of margin, so they are stable enough.

Minor:

• TestData/app.log.2 ends with a trailing newline that app.log.1 doesn't have.
• The tests call Cleanup() inline, so a failing assert leaves the temp directory behind. The neighbouring tests do the same.
• The resource strings are consistent across all three languages and the designer file.

Spec (maintainer triage comment on #735)

Missing or partial

• No new option. The spec asks for "A new MultiFile option, max file age in days, with 0 meaning off". There's no new property; MaxDayTry is reused.
• Default isn't off. The spec says "0 is the default so existing behavior is unchanged". This is violated because the default stays at 3.
• No relabel. The spec asks to rename the label to something like "Max day gap" and to update the pattern-syntax help. The label still says "Max days:"; only a hint was added.
• Settings and Session File not touched. The spec asks for the option in Settings → MultiFile, and for a round-trip through SessionSnapshot, SessionFileComposer and PersistenceData, with old Session Files loading as 0. None of these files changed.
• No SDK version bump. The spec asks to "bump its own version in ColumnizerLib.csproj". The ILogFileInfo API changed, but the version didn't.
• Acceptance tests missing. There's no test for:
  • contiguity (an old app.log.1 followed by a new app.log.2)
  • the $D pattern being cut off by age
  • a file with no reported timestamp being included
  • the Session File round-trip
  • the setting being editable in both the dialog and Settings

Scope creep

• The spec lists "Changing what MaxDayTry does" as out of scope, and that is the core of the PR.

Implemented but wrong

• Clock not injectable. The spec says "Make 'now' injectable", but the code calls DateTime.UtcNow directly.
• Tests don't use fakes. The spec says tests should run "in Core against a fake file system and an injected clock, with no real file timestamps". The tests use real files, File.SetLastWriteTimeUtc and PluginRegistry.Instance.
• Stopping at the first old file only works for index patterns. The spec says the walk "stops at the first rollover file whose last-modified time is older". That holds for pure index patterns. In mixed patterns a too-old indexed file falls through to the date search, which keeps going. In date patterns a too-old file counts as a missing day.
• Done correctly: the new ILogFileInfo member defaults to null, a file with an unknown timestamp is included, and the local and SFTP versions are implemented.

Summary: 10 standards findings, the worst being the "0 = unlimited" hint, which actually loads no files for $D patterns. 12 spec findings, the worst being that the PR changes MaxDayTry instead of adding a separate option that defaults to off. The fix the spec points to is a new MaxFileAgeDays option (default 0) with an injected clock, carried through Settings and the Session File, plus a ColumnizerLib version bump.

[Pr0metheus2]

OK I will try to make your proposal as you require.

[Pr0metheus2]

Check, please

[Hirogen]

2 Bugs left:

• Overflow crash: a very large max file age crashes MultiFile loading.
• Language-dependent test: one UI test only passes on an English machine.

New:

• Bug: overflow crash. Both new up-downs allow values up to 2147483647. GetCandidateResult calls UtcDateTime.AddDays(-_options.MaxFileAgeDays), which throws ArgumentOutOfRangeException at about 740,000 days, and that breaks MultiFile loading. Fix: cap the up-downs (e.g. 3650) and/or clamp the value in code.
• Bug: test depends on UI language. src/LogExpert.UI.Tests/Dialogs/MultiFileAgeSettingsTests.cs:80 compares the label text, which is hard-coded in the Designer, against UIStrings.SettingsDialog_UI_Label_labelMaxDayGap. On a German or Chinese machine that fails. Fix: pin the culture, as the Highlight dialog tests do, or drop the assert.
• Judgement: unused resource keys. SettingsDialog never calls ApplyResources, so the three new SettingsDialog_UI_Label_* keys (in three languages) are never read; only the tooltips get localized. Either wire the labels up or don't add the keys.
• Judgement: nullable annotations. TimeProvider timeProvider = null should be TimeProvider?. TryGetFile(out SftpFile file) should be [NotNullWhen(true)] out SftpFile? file, like EncodingRegistry.TryGetEncoding.
• Smell: one concept, four names. MaxDayTry, MaxDayGap, MultiFileMaxDays and upDownMultifileDays all mean the same setting. The persisted names have to stay, so this is a judgement call.
• Smell: minor duplication. The two loops in GetNameList repeat the same Include/TooOld block. Tolerable at this size.
• Minor: SFTP cache. _lastWriteTimeUtc isn't cleared when TryGetFile fails. That's harmless for the short-lived info objects the rollover search uses.

Remaining:

• No test for global-default seeding. The spec says "the global default seeds new windows". The code does this at LogWindow.cs:2851 and :2901, but no test asserts it.
• Scope creep: SFTP error handling changed. With the shared TryGetFile, FileExists no longer catches ArgumentNullException and Length now also catches SshException. It's small and arguably an improvement.
• Scope creep: XML read of a new field. PersisterXML reads a maxFileAgeDays attribute that no XML writer ever produces. It's harmless dead code.

[Pr0metheus2]

• Overflow fixed: both controls cap at 3,650 days; persisted oversized values are clamped in dialogs and at runtime before the cutoff calculation.
• Added regression coverage for int.MaxValue age values.
• Removed the language-dependent Settings label assertion.
• Settings resource keys are actively applied by the existing ApplyTextResources mapping; they are not unused.
• Nullable annotations corrected for TimeProvider? and TryGetFile.
• SFTP now clears a stale cached timestamp after lookup failure, while preserving each property’s prior exception behavior.
• Removed the unused legacy-XML maxFileAgeDays reader.
• Added the missing test proving global MultiFile defaults seed a new LogWindow.
• Kept persisted legacy names (MaxDayTry, MultiFileMaxDays) for compatibility; no needless rename.
• Kept the small duplicated rollover-loop handling, as the maintainer marked it tolerable.

[Hirogen]

Thanks @Pr0metheus2 — everything from the last round is in. The overflow is capped and clamped, with int.MaxValue regression tests. The culture-dependent assert is gone. You were right about the Settings label keys: GenerateTextMapFromNaming picks them up by control name. The SFTP cache is cleared on failure, and each property keeps its old exception behaviour. The dead XML read is removed, and there's a seeding test. I also re-checked the spec from #735: default 0 leaves $D/MaxDayTry unchanged, the walk stops at the first file that's too old, and a null timestamp means the file is included. The clock is injected, the Session File round-trips with old files loading as 0, and the relabel is in all three languages. Functionally this is done.

A few small things before merge:

Description and title

• The description still describes the first version: "Applied the Max days limit… MaxDayTry = 0 preserves the previous unlimited behavior" and "with MaxDayTry = 3". Please rewrite it around the new MaxFileAgeDays option (default 0 = off, separate from "Max day gap").
• Title: "Enahancement" → e.g. "Add MultiFile max file age option".

Cleanup

• PersisterXML.cs: the only change left is an added final newline. Please revert the file. The same goes for the other trailing-newline-only edits, if your editor added them.
• ColumnizerLib.csproj: please drop <PackageVersion>1.22.0</PackageVersion>. My mistake in the spec: the build passes PackageVersion to the pack step itself (Build.cs), so this line is ignored in CI and only confuses local builds.
• MultifileOptions.cs:13-15: the new XML docs on the existing MaxDayTry aren't needed for this change.
• RolloverFilenameHandler.cs:147: the DateTime.MinValue underflow guard can't trigger once the value is clamped to 3650, so it can go.
• MultiFileMaskDialog.Designer.cs:114: a bare // instead of the generated // . Letting the designer regenerate the file will also fix the duplicate TabIndex values.

Optional

• MultiFileAgeSettingsTests pins the dialog maximum at MAX_FILE_AGE_DAYS. Doing the same for the Settings up-down would keep the two 3650 literals in sync.

Once the description is updated and PersisterXML.cs and the version line are reverted, this is good to merge. Thanks for sticking with it!

[Pr0metheus2]

Summary

Adds a separate MultiFile Max file age (days) option.

• Default value is 0, which disables the age limit and preserves existing behaviour.
• The value limits how old rollover files may be before they are excluded from MultiFile loading.
• The UI and reader clamp the value to 3650 days to prevent overflow.
• The global preference seeds new windows, and per-file settings are persisted separately.

This is independent of Max day gap (MaxDayTry), which remains responsible only for how many missing consecutive dates may be skipped when searching $D date-pattern files.

• removed final newlines

[Hirogen]

Thanks @Pr0metheus2 — the round-4 cleanup is all in: PersisterXML.cs and the PackageVersion line are reverted, the MaxDayTry docs and the MinValue guard are gone, the designer comments are fixed, and the Settings maximum is now pinned in a test too. I re-checked the #735 spec: default 0, stop at the first file that's too old, null timestamp means include, injected clock, the 3650 clamp, Session File round-trip, seeding, relabel, the opened file always kept, and mixed $D+$J. Everything holds. Build adds no warnings, and the Core/Persister/UI tests pass.

Two small things before merge:

1. PR description
The title is updated, but the description on GitHub is still the first version ("Applied the Max days limit…", "MaxDayTry = 0 preserves the previous unlimited behavior", "with MaxDayTry = 3"). The good summary you wrote is only in a comment. Please replace the description with it, describing the new MaxFileAgeDays option (default 0 = off, separate from "Max day gap"), the injected clock, the Settings/Session File round-trip and the ILogFileInfo.LastWriteTimeUtc addition.

2. Duplicate TabIndex values
These are still there (keyboard Tab order jumps around):

• MultiFileMaskDialog.Designer.cs: fileNamePatternTextBox moved to 2 (clashes with labelFileNamePattern), labelMaxFileAgeDays=6 clashes with Settings, labelMaxFileAgeDaysHint=7 clashes with syntaxHelpLabel.
• SettingsDialog.Designer.cs: upDownMultifileMaxFileAgeDays=2 clashes with labelPattern.

In VS: View → Tab Order on each dialog, click through the controls once, and save. That regenerates the values.

Optional, non-blocking

• Resources.Designer.cs looks hand-edited (blank lines without the generated indent, and the SettingsDialog_UI_NumericUpDown_ToolTip_* accessors out of order). If you run Custom Tool on Resources.resx, please use an English VS UI so the comments stay English.
• Math.Clamp(x, 0, MultiFileOptions.MAX_FILE_AGE_DAYS) appears three times (handler and both dialogs). A clamped getter on MultiFileOptions would keep that in one place.

With 1 and 2 done, this is good to merge. Thanks for sticking with it — it came together really nicely!