Icons were previously hand-added, so their provenance and licensing lived in
folklore and a mis-painted asset was invisible at runtime: GPUI's `svg()`
reduces a file to an alpha mask, and `img()` keeps its pixels, but nothing
complains when the two are swapped.
Assets are now described by `scripts/icons.manifest` and vendored by
`scripts/sync-icons.sh` from pinned upstream releases, keeping the tree
reproducible and reviewable. `icons/**` stays monochrome and `color/**` full
colour; `scripts/check-icon-references.sh` fails the build when a referenced
asset is missing or painted through the wrong element, and runs in CI.
The lookup tables move out of `formatting/` into a dedicated `features/icons`
module so a ~1000-line brand table is not rebuilt inside a render closure, and
saved connections gain `icon_auto_detect`, which fills in a blank icon from the
detected remote system without ever overwriting a deliberate choice.
Desktop dead-code warnings went 104 to 18. Name what the remaining 18
are, with the evidence that each is an unfinished feature rather than
cruft -- mostly tests covering code no UI reaches, and match arms that
still handle prompts nothing raises. This is a to-do list, not a
backlog of deletions.
ai.rs is down from 4,032 to 1,554 lines across providers, agent, risk
and settings, so drop it from the large-file table. Note why risk is
deliberately its own small module, and that serde default_* functions
constrain how far a settings type can move from them.
Grouping NyaTermApp fields into feature states did not move the methods,
so most desktop modules could still reach most desktop state through
self. Write down the rule the last four rounds followed, the two cases
it does not cover (render helpers and methods that also need a service),
and what the transfer browser round showed about how to spot a good
candidate.
storage.rs is down from 7,662 to 4,020 lines and transport/lib.rs from
8,418 to 4,423. Update the large-file table, the decisions section and
the suggested order to match, and say why domain cuts worked where the
earlier type-by-type extractions did not.
`storage.rs` was 7662 lines with one `impl ConnectionStore` spanning
three thousand of them. Earlier rounds extracted a few pure type modules,
which barely moved the count because the types were the small part.
These two go out as whole domains instead, each taking everything that
belongs to it: table constants and key prefixes, record types, the
`*_in_txn` helpers, and — for command history — the prompt-stripping
logic that only `sanitize_history_command` used.
`known_hosts` also took `storage.rs`'s only uses of `base64`, `hmac` and
`sha1`. Three crate dependencies leaving with one domain is the sign the
seam was in the right place; a type-by-type extraction would never have
surfaced that.
Table names, key layouts, record shapes, the legacy text-doc import and
the hashed-host matching rules are unchanged — this moves code, it does
not touch the persistence contract. 166 core tests still pass, including
the known-hosts structured/hashed/raw round-trip, the match/changed/
unknown distinction, the legacy import, and command-history
normalization.
storage.rs: 7662 -> 7077 lines.
Items 1, 3 and 4 are done, so the list was describing work that no longer
exists. It now records what is actually left, and two things worth
knowing before picking it up.
The `use super::*` item does not batch. Every file needs its own
dependency set resolved, so it is 355 small compiler-guided edits rather
than one sweep like the `#[path]` removal was.
And grouping `NyaTermApp` fields did not move the 236 `impl NyaTermApp`
blocks. The field count is down from 585 to 279, but most desktop modules
can still reach most desktop state through `self`; the methods are the
next structural question, not the remaining fields, which are a long tail
where the biggest domain is eighteen.
The remaining three snapshots turned out to be a closed loop, so the
whole publish path goes.
`WorkspaceSnapshot` and `SessionSnapshot` were read only by
`published_core_store_snapshots_are_current`, which decided whether to
republish them. `OverlaySnapshot` was a same-render round-trip: `Render`
published it in its prologue and `overlay_host` read it back a few calls
later, falling back — when the store was empty — to an expression that
recomputed every field from the same `self` fields. `overlay_host` now
evaluates those flags directly into a local `OverlayFlags`, which is
exactly what the fallback did.
None of it had an observer. `AppShell` deliberately does not observe the
stores; the comment there records that store-observe was amplifying each
publish into an extra shell paint. So every `cx.notify()` in the publish
path was landing on nothing.
Gone with it: `publish.rs`, the publish throttle
(`should_publish_store_snapshots`, `store_snapshot_publish_due`,
`STORE_SNAPSHOT_HEARTBEAT`, `last_store_snapshot_publish_at`),
`WorkspaceStore`, `SessionStore`, and the snapshot half of
`OverlayStore`. The runtime tick loses a per-tick snapshot build and
comparison; the two slow-tick diagnostic fields that reported on it are
dropped, and `output_pressure` is still computed for the rest.
The four surviving stores each own something `NyaTermApp` does not:
`Runtime` (app runtime and native services), `WindowRuntime` (the pump),
`StartupRestore` (the restore queue) and `Overlay` (quick switch state).
`entities/` drops from 959 to 412 lines and the projection layer is gone.
Ten entity tests covered only the deleted stores; 548 tests become 538.
While tracing consumers for the previous commit two things turned up that
the next person should not have to rediscover.
`OverlaySnapshot` is a same-render round-trip. `Render` publishes it in
its prologue and `overlay_host` reads it back a few calls later; when the
snapshot is absent, the fallback recomputes every field from the same
`self` fields it was published from. That fallback is proof the renderer
never needed the store. It is not a staleness bug today only because the
publish happens earlier in the same pass.
And `AppShell` deliberately does not observe the stores — there is a
comment explaining that store-observe was amplifying every publish into
an extra shell paint — so the `cx.notify()` inside each publish has no
subscriber.
Both point at the same conclusion for `Workspace`/`Session`/`Overlay`,
but that change lands in the render path and the publish throttle, so it
belongs in its own round after the current one is verified.
Tracing consumers settled the half-migrated Entity Store question. Six
of the stores — `Ai`, `CloudSync`, `Connections`, `RemoteOps`,
`Settings`, `Transfer` — were write-only. Outside `entities/` they
appeared only in `app_shell::new`, where they were constructed and
stashed in `UiStoreHandles`; nothing ever read their snapshots. Every
qualifying tick built six snapshot structs, compared them, and called
`cx.notify()` for a reader that does not exist.
They are deleted, along with the code that fed them: six accessors added
purely so the projection could read state through semantic methods, and
`SettingsTab::label`, which returned hardcoded English while the actual
UI uses `i18n_key`.
What remains has a reason to exist. `RuntimeStore`, `WindowRuntimeStore`
and `StartupRestoreStore` own real state. `OverlayStore` owns quick
switch authoritatively and its snapshot is read by `root.rs` when
rendering overlays.
`WorkspaceStore` and `SessionStore` stay for now, and the doc records why
they are the honest remaining question: their snapshots are read only by
`published_core_store_snapshots_are_current`, which decides whether to
republish them. That loop is self-referential, but it also gates the
overlay publish, so untangling it is a separate change rather than a
free deletion.
Three entity tests covered only the deleted stores and go with them;
552 tests become 548.
Twenty-four `NyaTermApp` fields belonged to the send-command bar. They
move into `SendCommandFeatureState`, split by the three phases the bar
actually has: `composer` holds the payload being written and where the
caret is, `options` holds how that payload is interpreted and delivered
along with the menus that set those, and `progress` holds the in-flight
send — cancellation flag and the counters shown while it runs.
The flat naming interleaved all three, so a field like
`send_command_progress_round` sat between menu-open booleans and hex
scroll offsets.
`app_state` no longer imports anything from `crate::send_command`. Field
count goes from 302 to 279.
Forty-seven `NyaTermApp` fields were terminal presentation state. They
move into `TerminalFeatureState`, split by what the code actually does
with them: `search`, the `view` runtime (live views, surfaces, frame
pipeline, scroll), `input` focus and IME, `selection` and mouse
reporting, painted `layout` geometry, `menus`, and the split/tab
`windows` tree.
This is presentation state only. Parsing, snapshots and the wire
protocol stay in `nyaterm-terminal` and `nyaterm-transport`, and nothing
here changes what is sent, decoded or drawn — the fields keep their
types and initial values, only their address changes.
`OverlaySnapshot` keeps its own `terminal_actions_open` and
`terminal_context_menu_open` projection fields with the old names; the
rewrite anchored on `self`/`this`/`app`, so `overlay.terminal_actions_open`
was left alone. Two accesses through `entity.read(cx)` needed fixing by
hand, which the compiler pointed out.
Field count goes from 348 to 302.
Seventy `NyaTermApp` fields carried the `ai_` prefix. They cover six
unrelated concerns, now one struct each in `AiFeatureState`: provider
`settings`, the `chat` composer and transcript, session `history`, model
`discovery`, the agent loop, and `panel` chrome.
Two details worth noting for review:
`SettingsDraftSnapshot` has its own `ai_settings`, `ai_model_draft`,
`ai_base_url_draft` and `ai_secret_draft` fields with exactly the names
being moved. They are deliberately unchanged — it is a separate snapshot
type — and the rewrite was anchored on `self`/`this` receivers so
`snapshot.ai_settings` was left alone while `self.ai_settings` became
`self.ai.settings.config`.
The constructor destructures a loaded-store tuple that also binds
`ai_settings`, `ai_session_count`, `ai_message_count` and
`ai_audit_count`. Those are local bindings, not struct fields; the field
sweep removed them and the compiler caught it immediately.
`app_state` drops eight more model imports. Field count goes from 417 to
348 — down from 585 when this series started.
Seventy-eight `NyaTermApp` fields carried the `transfer_` prefix, which
made them look like one feature. They are not. Grouping them into
`TransferFeatureState` separates five things that merely share a panel:
- `queue` upload/download jobs and their menus
- `browser` the SFTP listing, navigation history, selection and menus
- `file_ops` rename/move/delete/create/properties dialogs
- `editor` the built-in remote file editor workspace
- `external_sync` handing a file to an outside editor and syncing back
plus `paths` for the manual transfer endpoints and `panel` for focus
routing and height. Their lifetimes are unrelated — an open rename
dialog has nothing to do with a running upload — and the flat prefix
hid that completely.
`app_state/mod.rs` drops twenty-five transfer UI model imports, the
largest single reduction so far. Field count goes from 494 to 417.
977 accesses were rewritten. The first pass anchored on `self`/`this`/
`app` receivers; a second pass took the remaining seven, which reach the
app through `entity.read(cx)`, after confirming no method and no other
struct shares any of these names.
Twenty-three `NyaTermApp` fields were the security panel. They move into
`SecurityFeatureState`, split by what they actually are: the four editors
and their focus handles in `editors`, revealed passwords/credentials and
generated OTP codes in `revealed`, and the master password prompt in
`unlock`.
The `revealed` grouping is the useful part of this one. Values the user
has explicitly unmasked were previously three same-shaped maps sitting
among twenty other fields; collecting them makes it obvious that they
are display state with a shared lifetime, and gives a single place to
hang clearing behaviour later. Secrets themselves still live in
`nyaterm-core`; nothing about storage or decryption changes here.
One access came through `input_entity.read(cx)` rather than `self`, which
the receiver survey missed and the compiler caught.
Field count goes from 516 to 494.
Fifty-four `NyaTermApp` fields were the Remote page: Docker, the process
table and host stats, distinguished only by a `docker_` / `process_` /
`stats_` name prefix. They move into `RemoteOpsFeatureState` with one
struct per pane, named after the existing `RemoteOpsStore` projection.
Grouping them makes a symmetry visible that the flat naming hid: all
three panes carry the same refresh bookkeeping — job id, owning session
id, pending flag, consecutive failure streak and last refresh instant —
so `DockerPaneState`, `ProcessPaneState` and `StatsPaneState` now share
a recognisable shape. A future round can lift that into one struct.
The three job channels are created inside `RemoteOpsFeatureState::new`
rather than in `NyaTermApp::new`, since construction was the only place
that touched them. Two names read better in context: `processes` is
`process.items`, and `remote_stats` is `stats.data`.
`app_state/mod.rs` drops five more UI model imports. Field count goes
from 570 to 516.
The 486 rewritten accesses were anchored on `self.`/`this.` receivers
only, after confirming those are the sole receivers for every field, so
unrelated `.process_*` and `.stats*` calls on other types were left
alone. A second pass caught the accesses rustfmt had split across lines.
Twenty-seven of the fields on `NyaTermApp` were quick command panel,
overlay and editor state addressed as `self.quick_command_*`. They now
live in `QuickCommandFeatureState`, following the shape that
`ConnectionFeatureState` already validated: `list`, `editor`, `dialogs`,
`import` and `ai` sub-structs, built from a single
`QuickCommandFeatureFocus` bundle instead of seven separate
`cx.focus_handle()` fields at the app level.
The persisted collections stay where they were. `quick_commands` and
`quick_command_categories` are store-loaded data, not UI state, so they
remain on `NyaTermApp` exactly as the connections list does.
Two names get clearer in the move: the row overflow menu is `list.row_menu`
rather than a bare `quick_command_menu`, and the editor draft is
`editor.draft` rather than `quick_command_editor`, matching
`ConnectionEditorFeatureState`.
`app_state/mod.rs` no longer imports any of the eleven quick command UI
model types, which is the point: the app struct stops knowing how the
quick command panel represents its overlays. Field count drops from 585
to 570.
The 222 rewritten accesses were anchored on `.quick_command_<field>` with
a word boundary and applied longest-name-first, so helpers such as
`quick_command_category_label` and fields such as
`quick_command_editor_focus` were not caught by shorter prefixes.
The `pages` tree, `http/cloud_sync` and `models/workspace_tabs` were the
last pseudo-module roots. Each moves to `X/mod.rs` so its children
resolve by directory, and `pages/remote/docker` stops aliasing six
sibling files (`docker_containers.rs` as `mod containers`, and so on)
in favour of a normal `docker/` directory.
`nyaterm-terminal-gpui` had one redundant `#[path = "tests.rs"]` that
already pointed at the default location; it is dropped too.
`#[path = "..."]` is now absent from both crates, so the twenty-two
per-directory guards collapse into one crate-wide `check_no_matches`.
Module paths always match the directory layout again, which is what
makes `pub(in ...)` bounds mean something and what makes the next step,
replacing the `use super::*` chain with explicit imports, worth doing.
`layout`, `panels`, `inspector`, `formatting` and `view_widgets` were the
same pattern as the runtime areas: an `X.rs` pseudo-module root next to
an `X/` directory whose children were pulled in with `#[path]`.
Move each root to `X/mod.rs` and let the children resolve by directory,
including the nested `security_panel/panel`, `workspace/surface`,
`quick_commands_panel/panel`, `send_command_bar`, `tab_actions_overlay`
and `ai_widgets` subtrees. All five areas are guarded against new
`#[path]` declarations.
Desktop `#[path]` count drops from 178 to 90. What remains is the
`pages` tree plus two files under `models` and `http`.
The remaining twenty-one flattened declarations in `features/mod.rs`
covered the AI, commands, settings, sync, transfers, remote and
translation directories. Each becomes a real module with its own
`mod.rs`, and their `ai_runtime`, `command_runtime`,
`quick_command_runtime`, `security_runtime`, `settings_runtime`,
`cloud_sync_runtime`, `transfer_jobs` and `remote_runtime` subtrees
become directory modules as well, along with the deeper `ai_runtime/chat`,
`ai_runtime/settings` and `quick_command_runtime/import` trees.
`features/mod.rs` now contains zero `#[path]` declarations, which the
boundary script enforces, and every feature directory is guarded too.
As in the previous rounds, nesting surfaced real visibility: three
event-drain methods were `pub(super)` and only reachable because their
module used to be a flat sibling of the event pump. Ten more
`crate::features` level re-exports turned out to be unused once each
consumer sat inside the owning subtree, and were removed.
Desktop `#[path]` count drops from 247 to 178; what is left lives in the
view layer (`pages`, `panels`, `layout`, `inspector`) and in `models`.
Eight terminal modules were declared in `features/mod.rs` through
`#[path = "terminal/..."]`, and four of them declared their own children
the same way.
Make the whole area a real module tree:
- `mod terminal;` with `features/terminal/mod.rs` owning the children.
- `terminal_runtime`, `terminal_surface`, `terminal_selection_runtime`
and `terminal_context_menu_runtime` become directory modules,
removing seventeen further `#[path]` declarations.
- Terminal internals are addressed as
`crate::features::terminal::terminal_runtime` instead of a top-level
`crate::features::terminal_runtime`.
Nesting also showed that nine prompt and terminal symbols no longer
needed a `crate::features` level alias, because every consumer now
reaches them inside the owning subtree. Those re-exports are removed,
which keeps the warning count at the existing baseline.
`features/mod.rs` is down from 52 `#[path]` declarations to 21, and the
desktop total drops from 272 to 247.
Thirteen session modules were declared in `features/mod.rs` through
`#[path = "session/..."]`, so `features/session` was a directory with no
module of its own.
Make it a real module:
- `mod session;` with `features/session/mod.rs` owning the children.
- `session_runtime` becomes a directory module, removing its two
remaining `#[path]` declarations.
- Prompt and auth exports, plus the trzsz/zmodem session-state types
used by `app_state`, reach `crate::features` through explicit
re-exports instead of flattened module declarations.
Nesting immediately surfaced real visibility: four prompt-drain methods
were declared `pub(super)` and were only reachable from the event pump
because the module used to be a flat sibling. They are now
`pub(in crate::features)` on purpose rather than by accident.
`features/session` has zero `#[path]` declarations and is guarded.
Desktop `#[path]` count drops from 287 to 272.
`features/mod.rs` declared twelve shell modules through
`#[path = "shell/..."]`, so the directory looked layered while the module
tree stayed flat: every shell module was a direct child of
`crate::features` and `pub(in crate::features)` meant "visible to the
whole desktop feature crate".
Make `features/shell` a real module:
- `mod shell;` with `features/shell/mod.rs` owning the twelve children.
- `event_pump` and `keybinding_runtime` become directory modules, so
their own `#[path]` declarations go away too.
- Shell chrome exports reach the rest of `crate::features` through
explicit re-exports in `features/shell/mod.rs` instead of twelve
flattened module declarations.
`features/shell` now has zero `#[path]` declarations, and the boundary
script guards that instead of allowing a baseline of five. Desktop
`#[path]` count drops from 306 to 287.
The suggested order in the migration status doc is reworked to put the
module tree first: narrowing the shared prelude one symbol at a time
produced little real encapsulation while every feature directory was
still flattened into one namespace.
This commit introduces a new script, `check-architecture-boundaries.sh`, which enforces architectural boundaries within the project. The script checks for dependencies between different components, ensuring that low-level crates remain independent of higher-level presentation code. It also validates that certain fields and methods are accessed only through designated state management methods, preventing direct mutations that could lead to inconsistencies.
Key features of the script include:
- Dependency checks between crates.
- Validation of access patterns for state management.
- Enforcement of limits on specific code patterns to maintain architectural integrity.
- Reporting of violations with detailed output for easier debugging.
This addition aims to improve code quality and maintainability by enforcing architectural guidelines.
Kang