Skip to content

Simplify the history package#2567

Merged
dgageot merged 3 commits intodocker:mainfrom
dgageot:board/simplifying-history-package-code-ffaadbd2
Apr 28, 2026
Merged

Simplify the history package#2567
dgageot merged 3 commits intodocker:mainfrom
dgageot:board/simplifying-history-package-code-ffaadbd2

Conversation

@dgageot
Copy link
Copy Markdown
Member

@dgageot dgageot commented Apr 28, 2026

Tidy up pkg/history so it's easier to read and maintain, without
changing any user-facing behaviour or the on-disk file format.

What changed

  • load(): replaced the manual byte counting / line splitting /
    strconv.Unquote parser with bytes.Lines + json.Unmarshal,
    symmetric with how entries are written.
  • Shared dedup helper: extracted addInMemory so Add and load
    share the "remove prior occurrence + append" logic instead of
    carrying two different implementations.
  • LatestMatch: uses slices.Backward for cleaner backward
    iteration.
  • Dropped the options pattern: New(opts ...Opt) + Opt +
    WithBaseDir + private options struct collapsed into a single
    New(baseDir string). Empty string falls back to the user's home
    directory.
  • Dropped the current == -1 cursor sentinel: the cursor is now
    always a valid index in [0, len(Messages)], which kills a special
    case in Previous().
  • SetCurrent clamps to [0, len(Messages)], eliminating latent
    out-of-bounds panics in Previous/Next if a caller ever passed an
    invalid index.
  • Reordered the file so the public navigation/search API is at the top
    and the persistence/migration helpers are grouped at the bottom.
  • Removed the unused json:"messages" struct tag (the file format is
    one JSON-encoded string per line, the struct itself is never
    marshalled).
  • Tightened doc comments and added a test for SetCurrent with
    out-of-range values.

@dgageot dgageot requested a review from a team as a code owner April 28, 2026 09:20
@dgageot dgageot merged commit 6a124b2 into docker:main Apr 28, 2026
9 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