cross storage docs + mouse support on the client and in the tui-pages example
This commit is contained in:
1
Cargo.lock
generated
1
Cargo.lock
generated
@@ -987,6 +987,7 @@ dependencies = [
|
||||
"nucleo",
|
||||
"prost",
|
||||
"prost-types",
|
||||
"quick-xml",
|
||||
"ratatui",
|
||||
"regex",
|
||||
"rstest",
|
||||
|
||||
2
client
2
client
Submodule client updated: fb149f196b...31ed0b28f8
171
crossplatform_storage.md
Normal file
171
crossplatform_storage.md
Normal file
@@ -0,0 +1,171 @@
|
||||
# Cross-platform storage for the client (Windows / macOS / Linux)
|
||||
|
||||
This document covers two things:
|
||||
|
||||
1. Where to store **app-internal state** (the auto-login token, config, caches).
|
||||
2. Where to store **user-facing output files** (exports).
|
||||
|
||||
And which crate to use so you never hand-roll platform paths.
|
||||
|
||||
---
|
||||
|
||||
## 1. The shortcut crate: `directories`
|
||||
|
||||
Yes, there is a crate for exactly this: [`directories`](https://crates.io/crates/directories).
|
||||
You already depend on its little sibling `dirs = "6.0.0"`, which gives you the *base*
|
||||
folders (`home_dir`, `data_dir`, `download_dir`, …). `directories` builds on the same
|
||||
logic but adds `ProjectDirs`, which computes the **per-application** subdirectory
|
||||
following each OS's conventions in one call:
|
||||
|
||||
```toml
|
||||
# client/Cargo.toml
|
||||
[dependencies]
|
||||
directories = "6"
|
||||
```
|
||||
|
||||
```rust
|
||||
use directories::ProjectDirs;
|
||||
|
||||
// (qualifier, organization, application)
|
||||
let proj = ProjectDirs::from("com", "komp-ac", "komp_ac_client")
|
||||
.ok_or_else(|| anyhow::anyhow!("no home directory available"))?;
|
||||
|
||||
proj.config_dir(); // settings the user may edit
|
||||
proj.data_dir(); // app-owned data (safe default for the token too)
|
||||
proj.cache_dir(); // re-creatable stuff, OK to delete
|
||||
proj.state_dir(); // Option<&Path> — logs, session state; Linux-only concept
|
||||
```
|
||||
|
||||
What that resolves to:
|
||||
|
||||
| Call | Linux | macOS | Windows |
|
||||
|----------------|------------------------------------|---------------------------------------------------|--------------------------------------------------|
|
||||
| `config_dir()` | `~/.config/komp_ac_client` | `~/Library/Application Support/com.komp-ac.komp_ac_client` | `%APPDATA%\komp-ac\komp_ac_client\config` |
|
||||
| `data_dir()` | `~/.local/share/komp_ac_client` | `~/Library/Application Support/com.komp-ac.komp_ac_client` | `%APPDATA%\komp-ac\komp_ac_client\data` |
|
||||
| `cache_dir()` | `~/.cache/komp_ac_client` | `~/Library/Caches/com.komp-ac.komp_ac_client` | `%LOCALAPPDATA%\komp-ac\komp_ac_client\cache` |
|
||||
| `state_dir()` | `~/.local/state/komp_ac_client` | `None` | `None` |
|
||||
|
||||
> Alternative: keep only `dirs` and compose paths yourself (`dirs::data_dir()?.join(APP_NAME)`).
|
||||
> That works, but `ProjectDirs` removes the per-OS naming decisions entirely, so prefer it.
|
||||
|
||||
---
|
||||
|
||||
## 2. Fixing the auto-login token path (the current bug)
|
||||
|
||||
`client/src/config/storage/storage.rs:34` currently does:
|
||||
|
||||
```rust
|
||||
let state_dir = dirs::state_dir()
|
||||
.or_else(|| dirs::home_dir().map(|home| home.join(".local").join("state")))
|
||||
```
|
||||
|
||||
`dirs::state_dir()` returns `Some` **only on Linux**. On Windows and macOS it is
|
||||
`None`, so the fallback kicks in and you get `C:\Users\x\.local\state\...` and
|
||||
`/Users/x/.local/state/...` — it *works*, but it's a hidden Unix-style litter
|
||||
directory that no Windows/macOS user or uninstaller will ever find.
|
||||
|
||||
Cross-platform fix — fall back to the **data dir**, which exists everywhere:
|
||||
|
||||
```rust
|
||||
pub fn get_token_storage_path() -> Result<PathBuf> {
|
||||
let proj = directories::ProjectDirs::from("com", "komp-ac", APP_NAME)
|
||||
.ok_or_else(|| anyhow::anyhow!("Could not determine app directories"))?;
|
||||
|
||||
// state_dir is the right place on Linux; data_dir is the right place elsewhere.
|
||||
let dir = proj.state_dir().unwrap_or_else(|| proj.data_dir()).to_path_buf();
|
||||
|
||||
fs::create_dir_all(&dir)
|
||||
.with_context(|| format!("Failed to create app state directory at {dir:?}"))?;
|
||||
|
||||
Ok(dir.join(TOKEN_FILE_NAME))
|
||||
}
|
||||
```
|
||||
|
||||
**Migration:** existing Linux users keep the same path (`~/.local/state/komp_ac_client`),
|
||||
so nothing breaks there. If you care about existing Windows/macOS installs that already
|
||||
wrote to `~/.local/state`, add a one-time check: if the new path doesn't exist but the
|
||||
old one does, move the file.
|
||||
|
||||
### Token file security per platform
|
||||
|
||||
- **Unix:** you already `chmod 600` — keep the `#[cfg(unix)]` block as is.
|
||||
- **Windows:** there is no mode-bit equivalent, but `%APPDATA%` is already
|
||||
ACL-restricted to the current user, so a plain file there is the accepted baseline.
|
||||
No extra code needed.
|
||||
- **Optional upgrade:** the [`keyring`](https://crates.io/crates/keyring) crate stores
|
||||
the token in the OS credential store (Windows Credential Manager, macOS Keychain,
|
||||
Linux Secret Service). More secure, but adds a D-Bus/secret-service requirement on
|
||||
Linux and Keychain prompts on macOS. For a TUI client, the ACL'd file is a fine
|
||||
default; consider `keyring` only for the `access_token` field if security review
|
||||
demands it.
|
||||
|
||||
---
|
||||
|
||||
## 3. Where output/export files go
|
||||
|
||||
Rule of thumb: **files the user asked for go in user-visible folders; files the app
|
||||
needs go in `ProjectDirs`.** Never write outputs next to the executable (unwritable
|
||||
under `Program Files` / `/usr/bin`) and never default to the current working directory
|
||||
(unpredictable for a GUI/TUI launched from a launcher).
|
||||
|
||||
Your export logic (`client/src/pages/import_export/export/logic.rs:70`) already does
|
||||
the right first step:
|
||||
|
||||
```rust
|
||||
dirs::download_dir().unwrap_or_else(std::env::temp_dir)
|
||||
```
|
||||
|
||||
Two improvements:
|
||||
|
||||
1. `download_dir()` can be `None` on minimal Linux setups (no `xdg-user-dirs`
|
||||
installed → no `~/Downloads` mapping). Falling all the way to `temp_dir()` means
|
||||
exports silently land in `/tmp` and get wiped on reboot. Prefer a chain:
|
||||
|
||||
```rust
|
||||
let out_dir = dirs::download_dir()
|
||||
.or_else(dirs::document_dir)
|
||||
.or_else(dirs::home_dir)
|
||||
.unwrap_or_else(std::env::temp_dir)
|
||||
.join(APP_NAME); // keep exports grouped in one folder
|
||||
```
|
||||
|
||||
2. Always **show the resolved absolute path in the status line** after export (you
|
||||
already do this via `last_export_path`) — on three platforms the default location
|
||||
differs, so telling the user where the file went matters more than which folder
|
||||
you picked.
|
||||
|
||||
For program outputs that are *not* user-requested (logs, generated caches, import
|
||||
scratch files): use `proj.state_dir()/data_dir()` for logs, `proj.cache_dir()` for
|
||||
scratch. Same `ProjectDirs` instance, no new decisions.
|
||||
|
||||
---
|
||||
|
||||
## 4. General cross-platform hygiene checklist
|
||||
|
||||
- Build every path with `PathBuf::join`, never string concatenation with `/` or `\\`.
|
||||
(The codebase already does this — keep it that way.)
|
||||
- Always `fs::create_dir_all` before the first write; none of these directories are
|
||||
guaranteed to exist, especially on a fresh Windows profile.
|
||||
- Don't read `$HOME` / `%USERPROFILE%` env vars directly — that's exactly what
|
||||
`dirs`/`directories` abstracts (including Windows Known Folders redirection).
|
||||
- Handle the `None` case from every `dirs::*` call (headless CI, service accounts,
|
||||
containers have no home). Return an error or fall back to `temp_dir` explicitly.
|
||||
- Filenames typed by the user: reject `/`, `\`, and reserved Windows names
|
||||
(`CON`, `NUL`, `COM1`, trailing dots/spaces) before creating export files, or the
|
||||
export will fail only on Windows.
|
||||
- Test matrix: the cheap smoke test is `cargo check --target x86_64-pc-windows-gnu`
|
||||
plus running the path-resolution unit tests with the `dirs` values mocked; the real
|
||||
test is running the client once per OS and checking where `auth.token` and one
|
||||
export actually land.
|
||||
|
||||
---
|
||||
|
||||
## 5. Summary of concrete steps
|
||||
|
||||
1. Add `directories = "6"` to `client/Cargo.toml` (keep `dirs` for `download_dir` etc.).
|
||||
2. Create one `ProjectDirs::from("com", "komp-ac", "komp_ac_client")` helper (e.g. in
|
||||
`client/src/config/storage/`) and route all internal paths through it.
|
||||
3. Rewrite `get_token_storage_path()` as in §2 → auto-login token lands in the
|
||||
idiomatic per-OS location; Linux path unchanged.
|
||||
4. Improve the export default-directory chain as in §3 (`download → documents → home → temp`).
|
||||
5. (Optional) move the raw `access_token` into `keyring` later.
|
||||
Submodule tui-pages updated: 981a77c863...750d357d9c
Reference in New Issue
Block a user