Skip to content

Builder interface for History objects#933

Merged
conradludgate merged 6 commits intoatuinsh:mainfrom
utterstep:history-builder-interface
Jun 15, 2023
Merged

Builder interface for History objects#933
conradludgate merged 6 commits intoatuinsh:mainfrom
utterstep:history-builder-interface

Conversation

@utterstep
Copy link
Copy Markdown
Contributor

@utterstep utterstep commented May 3, 2023

Also part of #882

Allow History to be created through typed builder interface, powered by typed-builder crate.

This allows to specify different required fields for a different scenarios:

  1. entry, imported from shell history should have at least command and timestamp and may have some other fields specified, if shell history stores them
  2. entry, caught by hook – has timestamp, command and cwd. These are only fields accessible at hook's start phase, all the other ones (duration, exit and such) are added after command is executed, on an instance, queried from DB by id.
  3. entry, queried from DB. In this case all fields are required, as we're querying the whole object as stored from DB.

I've also added documentation for History and provided builders, and added _seal field to the History to ensure that it could not be erroneously constructed from outside without going through the builder, which enforces required constraints.

@vercel
Copy link
Copy Markdown

vercel bot commented May 3, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
atuin ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 15, 2023 10:20am

@utterstep
Copy link
Copy Markdown
Contributor Author

I'll create PRs with other changes from #882 after this PR is resolved, because they depend on changes from this one

Comment thread atuin-client/src/history.rs Outdated
Comment thread atuin-client/src/history.rs
Comment thread atuin-client/src/history.rs Outdated
Comment thread atuin-client/src/history.rs
Comment thread atuin-client/src/history.rs
Comment thread atuin-client/src/history.rs
Comment thread atuin-client/src/history.rs Outdated
Comment thread atuin-client/src/history.rs Outdated
Comment thread atuin-client/src/history.rs Outdated
Comment thread atuin-client/src/import/resh.rs
Comment thread atuin/src/command/client/history.rs
utterstep and others added 5 commits June 15, 2023 11:16
WIP: remove `HistoryWithoutDelete`, add some docstrings, tests
Assure in compile-time that all required fields
are set for the given construction scenario
Copy link
Copy Markdown
Collaborator

@conradludgate conradludgate left a comment

Choose a reason for hiding this comment

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

Thanks, and sorry this took so long. I made some changes that would have been too complicated to try and explain.

I removed the HistorySeal as it's unnecessary pedantry, although I admire the effort for strict type safety

@conradludgate conradludgate enabled auto-merge (squash) June 15, 2023 10:25
@conradludgate conradludgate merged commit 4077c33 into atuinsh:main Jun 15, 2023
@utterstep
Copy link
Copy Markdown
Contributor Author

Thanks for your help and attention :)

@utterstep utterstep deleted the history-builder-interface branch June 16, 2023 12:51
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.

3 participants