Skip to content

chore: adopt derive_more to reduce boilerplate - #3573

Merged
taylordotfish merged 12 commits into
atuinsh:mainfrom
markovejnovic:chore/remove-boilerplate-with-derive-more
Jul 8, 2026
Merged

chore: adopt derive_more to reduce boilerplate#3573
taylordotfish merged 12 commits into
atuinsh:mainfrom
markovejnovic:chore/remove-boilerplate-with-derive-more

Conversation

@markovejnovic

@markovejnovic markovejnovic commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

This PR adds derive_more with the explicit aim of removing boilerplate from the code.

There is no obligation to merge this PR -- it's more-so just a suggestion. AI-generated, hand-reviewed.

Checks

  • I am happy for maintainers to push small adjustments to this PR, to speed up the review cycle
  • I have checked that there are no existing pull requests for the same thing

Replace manual Display, From, Into, AsRef, Deref, Debug, and
Constructor impls with derive_more derives where the implementation is
a straight delegation. Adds derive_more v2 as a workspace dependency
with per-feature opt-in to keep compile times minimal.

Touched crates: atuin-common, atuin-client, atuin-server-database,
atuin-server-sqlite, atuin-server-postgres, atuin-daemon, atuin-dotfiles,
atuin-kv, atuin-scripts, atuin (CLI).
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Fossier: Manual Review Requested

@markovejnovic is a new contributor. A maintainer should review this PR before merging.

Score Breakdown

Total Score: 50.1/100 | Confidence: 100% | Outcome: REVIEW

Signal Value Score Weight
account_age 3914 1.00 0.09
public_repos 85 1.00 0.05
contribution_history 96 0.48 0.05
follower_ratio 1.39 0.70 0.05
bot_signals False 0.50 0.04
open_prs_elsewhere 35 0.00 0.09
closed_prs_elsewhere 49 0.00 0.10
merged_prs_elsewhere 305 1.00 0.08
prior_interaction 0 0.00 0.08
activity_velocity 6 0.00 0.08
pr_content ... 1.00 0.08
commit_email marko@harmont.dev 0.80 0.04
pr_description ... 0.80 0.05
repo_stars 30470 0.30 0.04
org_membership 0 0.20 0.03
commit_verification ... 0.68 0.04
contributor_stars 25 0.50 0.04

Comment thread crates/atuin-common/src/shell.rs
… features

Switch derive_more from per-feature opt-in to features = ["full"].
Add Constructor derives to ControlService, Theme, PermissionRequest,
PermissionChecker, and ContextWindowBuilder. Add IsVariant derive to
ToolRenderData, replacing manual is_remote(). Add derive_more dep to
atuin-ai crate.
…nities

- DbUser/DbSession/DbHistory: add Into derives (sqlite + postgres)
- DbError: replace manual `impl Error` with derive_more::Error
- ClientToolCall: add From + IsVariant derives, simplify is_auto_approved
- AgentState: add IsVariant derive, simplify is_busy in driver
ToolState: is_resolved() now uses generated is_completed()/is_denied().
StreamPhase: cancel handler uses generated is_done().
@markovejnovic
markovejnovic marked this pull request as ready for review July 7, 2026 21:08
@greptile-apps

greptile-apps Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Adopts derive_more v2 as a workspace dependency to replace manual boilerplate Display, From, Into, AsRef, and Deref implementations across 11 crates with derive macros.

  • All existing manual impls are replaced with semantically equivalent derives; no logic changes to functionality.
  • DbError gains derive_more::Error (changing Error::source() from always-None to potentially returning the inner eyre::Report — discussed and accepted in prior thread).
  • AGENTS.md documents the new convention but contradicts itself: it says to use thiserror (not derive_more) for error types, while the PR applies derive_more::Error to DbError.

Confidence Score: 4/5

Safe to merge after resolving the AGENTS.md / DbError inconsistency.

All manual impl replacements are mechanically equivalent. The one gap is that AGENTS.md (updated in this PR) says to use thiserror for error types, yet DbError is migrated to derive_more::Error — either the code or the documented convention needs to be consistent.

AGENTS.md and crates/atuin-server-database/src/lib.rs — the stated convention and the implementation are in conflict.

Important Files Changed

Filename Overview
AGENTS.md Documents the new derive_more convention, but contradicts itself: says 'use thiserror not derive_more for error types' while the PR applies derive_more::Error to DbError
Cargo.toml Adds derive_more workspace dep with the error feature; that feature may be unneeded if the AGENTS.md thiserror-for-errors convention is enforced
crates/atuin-server-database/src/lib.rs Migrates DbError to derive_more::Error; semantically equivalent but contradicts the thiserror-for-errors convention documented in the same PR
crates/atuin-common/src/lib.rs Adds derive_more::Display, From, and Deref to all UUID newtype wrappers; fields were already pub so no type-safety regression
crates/atuin-common/src/shell.rs Replaces manual Display impl with per-variant display attributes; equivalent output
crates/atuin-server-postgres/src/wrappers.rs Derives Into for all Db* wrappers, removing manual From; Rust blanket From impl preserves the old interface
crates/atuin-server-sqlite/src/wrappers.rs Same Into-derive change as the postgres wrappers; equivalent behaviour
crates/atuin-client/src/settings.rs Replaces manual AsRef and Display impls on HubEndpoint and Timezone with derive_more equivalents; semantically identical

Reviews (3): Last reviewed commit: "Merge branch 'main' into chore/remove-bo..." | Re-trigger Greptile

Comment thread crates/atuin-server-database/src/lib.rs

@taylordotfish taylordotfish left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The cases where derive_more is being used to derive a trait like From or Display generally look good to me, but some of the Constructor and IsVariant uses are a little too magic for me and I could see them causing confusion.

Comment thread crates/atuin-ai/src/fsm/tools.rs Outdated
Comment thread crates/atuin-daemon/src/daemon.rs Outdated
Comment thread crates/atuin-ai/src/permissions/check.rs Outdated
Comment thread crates/atuin-common/src/shell.rs
Comment thread crates/atuin-dotfiles/src/store.rs
Comment thread crates/atuin-ai/src/fsm/mod.rs Outdated
…eatures

Remove derive_more::Constructor and derive_more::IsVariant usage,
restoring manual new() methods and matches!() patterns. Narrow
Cargo.toml features from "full" to only the derives still in use.
@markovejnovic
markovejnovic force-pushed the chore/remove-boilerplate-with-derive-more branch from 9d22fd5 to 2c28d45 Compare July 7, 2026 22:10
@socket-security

socket-security Bot commented Jul 7, 2026

Copy link
Copy Markdown

No dependency changes detected. Learn more about Socket for GitHub.

👍 No dependency changes detected in pull request

Comment thread crates/atuin-client/src/settings.rs Outdated
Comment thread crates/atuin-server-database/src/lib.rs
taylordotfish and others added 2 commits July 7, 2026 17:58
Avoid generating a blanket impl unless it's needed; Rust doesn't play
nice if you ever need two blanket impls.
…remove-boilerplate-with-derive-more

Use `as_ref(str)` instead of `as_ref(forward)`
@markovejnovic

Copy link
Copy Markdown
Contributor Author

ty for your pr @taylordotfish

@taylordotfish taylordotfish left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

looks good, thanks!

@taylordotfish
taylordotfish merged commit 1a379ad into atuinsh:main Jul 8, 2026
25 checks passed
taylordotfish pushed a commit that referenced this pull request Jul 9, 2026
### Bug Fixes

- *(ai)* Dispatch skills registered in the slash command registry
([#3593](#3593))
- *(ci)* Fossier install in scan workflow
([#3485](#3485))
- *(i18n)* Fix typos in Russian localization
([#3575](#3575))
- *(nu)* Use `char -u 1b` for ESC in OSC 133 sequences
([#3530](#3530))
- *(nu)* Suppress error when `ATUIN_HISTORY_ID` is missing in
`pre_prompt` hook
([#3587](#3587))
- *(pi)* Observe tool events instead of registering a bash tool
([#3557](#3557))
- *(pty-proxy)* Set `$SHELL` to the spawned shell
([#3548](#3548))
- *(search)* Fix terminal clearing with latest Ratatui
([#3578](#3578))
- *(sync)* Skip records that fail to decrypt or decode instead of
failing the whole store
([#3569](#3569))
- Atuin hangs when attempting to spawn daemon from Ctrl+R invocation
([#3502](#3502))
- Capture session ID from stream headers rather than final event
([#3531](#3531))
- Doctor resiliency fo runknown platforms + openbsd warning
([#3551](#3551))
- Double input on arrow keys in AI setup prompt on Windows
([#3552](#3552))
- Exclude AI agent commands from zsh-autosuggestions
([#3567](#3567))
- Silence shellcheck SC2016 on literal `$all-user` author filter
- Respect `store_failed` when using daemon
([#3571](#3571))


### Documentation

- Highlight `Ctrl-r` keybinding on docs page
([#3489](#3489))
- Document store purge workflow
([#3544](#3544))
- Fix command example typo in documentation
([#3536](#3536))
- Make commented-out lines in `config.toml` match real defaults
([#3583](#3583))
- Add fish shell cleanup step to uninstall instructions
([#3582](#3582))


### Features

- *(doctor)* Add whether daemon is enabled to `doctor` output
([#3572](#3572))
- *(pty-proxy)* Add `--shell` flag to override the spawned shell
([#3327](#3327))
- Setup fossier to stop bot slop prs
([#3482](#3482))
- Capture command output + expose to new `atuin_output` tool
([#3510](#3510))
- Cache user contexts on load until `/reload`
([#3525](#3525))
- Create database integration tests for atuin-server
([#3514](#3514))
- Add `/model` slash command for changing models
([#3576](#3576))
- Add mcp server for history tools and expand search filters
([#3581](#3581))
- Add status bar with model and usage information
([#3591](#3591))


### Miscellaneous Tasks

- *(rustdoc)* Fix Rustdoc warnings
([#3585](#3585))
- *(warnings)* Fix compile warnings with latest dependencies
([#3586](#3586))
- Vouch for all existing contributors
([#3486](#3486))
- Update GitHub app token format
- Update to Rust 1.96.1
([#3568](#3568))
- Adopt `derive_more` to reduce boilerplate across the codebase
([#3573](#3573))


### Performance

- *(search)* Scan history by recency until N unique
([#3553](#3553))
- Add `synchronous(Normal)` + `optimize_on_close` to record store SQLite
([#3577](#3577))
- Remove unnecessary clones in a hot path
([#3580](#3580))


### Refactor

- Implement `From<sqlx::Error>` and clean up `fix_error`
([#3484](#3484))
- Pull `fn into_utc` into `atuin-server-database` crate
([#3487](#3487))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants