Conversation
# Conflicts: # CHANGELOG.md # src/config.rs # src/main.rs # src/ollama/launch.rs # src/ollama/mod.rs # ui/app.slint
… main, mod, and model files
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the application into an MVC-style architecture (model/view/controller + repository), adds a settings panel concept for terminal selection, and updates the Slint UI to use a new vector icon component and a revised layout (including working-directory display and increased window height). It also bumps the crate version to 0.6.1 and documents the release in the changelog.
Changes:
- Introduce MVC modules (
model,view,controller,repository) and reworkmain.rsinto a composition root. - Add terminal selection infrastructure (Rust
terminalmodule + SlintTerminalSelect+ prefs field), plus provider/agent logo mapping utilities. - Update Slint UI styling and structure (vector
Icon, updated badges, footer working-dir display, window height increase) and bump version/changelog.
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 17 comments.
Show a summary per file
| File | Description |
|---|---|
| ui/app.slint.bak | Adds an unused backup copy of the UI (not compiled). |
| ui/app.slint | Adds vector icons, terminal dropdown + settings panel, layout tweaks (height/footer/working-dir row). |
| src/main.rs | Composition root wiring for MVC + Slint timer tick. |
| src/view.rs | New view layer: drains ViewCommands, applies snapshots, detects user changes. |
| src/controller.rs | New controller: intent handlers + poller + model→view mirror, with unit tests. |
| src/model.rs | New model: canonical state + persistence + refresh/test/launch intents, with unit tests. |
| src/repository.rs | Repository trait + production OllamaRepository implementation. |
| src/terminal.rs | Terminal detection/spawn logic + persistence keys/labels, with tests. |
| src/test_util.rs | Test-only utilities for isolating $HOME in tests. |
| src/slint_generated.rs | Centralized slint::include_modules!() re-export module. |
| src/config.rs | Adds persisted terminal preference field. |
| src/ollama/mod.rs | Exposes provider mapping and Model type; adds logos module. |
| src/ollama/logos.rs | New mapping from model name → provider slug. |
| src/ollama/launch.rs | Extends launch to accept terminal selection; adjusts picker/restore/spawn behavior. |
| src/ollama/agents.rs | Adds logo field and mapping for agent logos. |
| CHANGELOG.md | Adds 0.6.1 entry (currently dated in the future). |
| Cargo.toml / Cargo.lock | Bump version to 0.6.1 and add async-trait dependency. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
213
to
215
| pub fn pick_directory(start_dir: Option<&str>) -> Option<String> { | ||
| let start = start_dir.filter(|d| !d.is_empty()); | ||
| let _start = start_dir.filter(|d| !d.is_empty()); | ||
|
|
Comment on lines
+1093
to
+1096
| terminals: root.terminals; | ||
| sel-terminal-index <=> root.sel-terminal-index; | ||
| sel-terminal-key <=> root.sel-terminal-key; | ||
| select-terminal(key) => { root.select-terminal(key); } |
Comment on lines
+883
to
+891
| // Settings gear is hidden in this interim release while | ||
| // the new settings UI is still being designed. Restore | ||
| // by removing this comment block + uncommenting the | ||
| // IconBtn below. | ||
| // IconBtn { | ||
| // name: "gear"; | ||
| // active: root.settings-open; | ||
| // clicked => { root.toggle-settings(); } | ||
| // } |
Comment on lines
+148
to
+152
| // ---- plain string dropdown used for the terminal list ---- | ||
| component TerminalSelect { | ||
| in property <[TerminalItem]> items; | ||
| in-out property <int> current-index; | ||
| min-width: 200px; |
Comment on lines
+231
to
+235
| row-ta := TouchArea { | ||
| clicked => { | ||
| root.current-index = idx; | ||
| popup.close(); | ||
| } |
Comment on lines
+22
to
+24
| mod terminal; | ||
| mod test_util; | ||
| mod view; |
Comment on lines
+26
to
+35
| use crate::ollama::logos::provider_for_model; | ||
| use crate::ollama::Agent; | ||
| use crate::slint_generated::{AgentItem, ModelItem, TerminalItem}; | ||
| use crate::terminal::Terminal; | ||
| use crate::view::SlintAppView; | ||
| use controller::AppController; | ||
| use model::AppModel; | ||
| use repository::OllamaRepository; | ||
| use slint::{Model, ModelRc, SharedString, Timer, TimerMode, VecModel}; | ||
| use std::cell::RefCell; |
Comment on lines
+3
to
+6
| //! `slint::include_modules!()` can only be called from one site in the | ||
| //! crate. Centralising it here lets every module refer to `AppWindow`, | ||
| //! `AgentItem`, and `ModelItem` by name. | ||
|
|
Comment on lines
+568
to
+571
| // from_key. The is_installed() check itself depends on the | ||
| // real filesystem. | ||
| assert!(Terminal::Warp.is_installed() || !Terminal::Warp.is_installed()); | ||
| // But on a macOS host with Warp.app in /Applications, |
| let rig = make_rig(world(vec![]), Prefs::default(), "http://h"); | ||
| rig.controller.on_selection_changed(Some("codex-app".into()), Some("glm-4.6:cloud".into())); | ||
| let prefs = crate::config::load(); | ||
| eprintln!("DEBUG: home={:?} agent={:?} model={:?}", std::env::var("HOME"), prefs.agent, prefs.model); |
Comment on lines
+1
to
+5
| import { VerticalBox, HorizontalBox, ScrollView } from "std-widgets.slint"; | ||
|
|
||
| export struct AgentItem { | ||
| name: string, // ollama launch token (e.g. "codex-app") | ||
| display: string, // label (e.g. "Codex App") |
Comment on lines
+149
to
+156
| component TerminalSelect { | ||
| in property <[TerminalItem]> items; | ||
| in-out property <int> current-index; | ||
| min-width: 200px; | ||
| height: 34px; | ||
| property <length> vis: min(items.length * 36px, 200px); | ||
| property <int> open-tick: 0; | ||
|
|
Comment on lines
+231
to
+236
| row-ta := TouchArea { | ||
| clicked => { | ||
| root.current-index = idx; | ||
| popup.close(); | ||
| } | ||
| } |
Comment on lines
+735
to
+739
| terminal-select := TerminalSelect { | ||
| items: root.terminals; | ||
| current-index <=> root.sel-terminal-index; | ||
| horizontal-stretch: 1; | ||
| } |
Comment on lines
+1093
to
+1096
| terminals: root.terminals; | ||
| sel-terminal-index <=> root.sel-terminal-index; | ||
| sel-terminal-key <=> root.sel-terminal-key; | ||
| select-terminal(key) => { root.select-terminal(key); } |
Comment on lines
+883
to
+891
| // Settings gear is hidden in this interim release while | ||
| // the new settings UI is still being designed. Restore | ||
| // by removing this comment block + uncommenting the | ||
| // IconBtn below. | ||
| // IconBtn { | ||
| // name: "gear"; | ||
| // active: root.settings-open; | ||
| // clicked => { root.toggle-settings(); } | ||
| // } |
Comment on lines
+453
to
+456
| // ---- simple props ---- | ||
| if prev.as_ref().map(|p| p.ollama_host.as_str()) != Some(snap.ollama_host.as_str()) { | ||
| self.ui.set_ollama_host(snap.ollama_host.clone().into()); | ||
| } |
Comment on lines
+87
to
+100
| let host = self.view_state.ollama_host(); | ||
| let terminal = crate::terminal::Terminal::from_key( | ||
| &self.view_state.selected_terminal_key(), | ||
| ); | ||
| // Snapshot the working dir into an owned String so the borrow | ||
| // survives the async move into the tokio task. | ||
| let working_dir: String = self.view_state.working_dir(); | ||
| self.model.record_launch(agent.name.clone(), model.clone()); | ||
| let m = self.model.clone(); | ||
| let sink = self.sink.clone(); | ||
| tokio::spawn(async move { | ||
| let dir_opt = if working_dir.is_empty() { None } else { Some(working_dir.as_str()) }; | ||
| let res = m.launch(agent.clone(), model.clone(), Some(host), dir_opt, terminal).await; | ||
| let (msg, kind) = match res { |
Comment on lines
631
to
632
| cmd.status().context("failed to run `ollama launch --restore`")?; | ||
| Ok(()) |
Comment on lines
200
to
207
| /// Strip everything that is not safe inside a double-quoted shell string. We | ||
| /// use this for the `cd "<dir>"` prefix we hand to Terminal.app on macOS, so a | ||
| /// directory with `\"` or `;` can't break out of the quoting. | ||
| fn shell_safe_dir(dir: &str) -> String { | ||
| dir.chars() | ||
| .filter(|c| !"\"`$\\\n\r".contains(*c)) | ||
| .filter(|c| c.is_ascii_alphanumeric() || "/._- ".contains(*c)) | ||
| .collect() | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.