style: apply cargo fmt to browser + theme-import files#157
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to apply cargo fmt to a small set of Rust UI/browser files that were previously unformatted on main, unblocking the Formatting + Clippy CI jobs after the runner-label fix in #155.
Changes:
- Reflowed match arms and closures in
BrowserView(find overlay handlers, zoom-step access). - Reformatted constructor/signature layout in
BrowserPane. - Minor formatting normalization in the theme import modal status/save paths.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| app/src/settings_view/import_theme_modal.rs | Small formatting-only changes to error assignment and idle-state tuple formatting. |
| app/src/pane_group/pane/browser_pane.rs | Reformatting of function signatures and a view-construction statement. |
| app/src/browser/browser_view.rs | Reformatting around editor subscriptions, zoom-step retrieval, and action match arms. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| session_id: String, | ||
| ctx: &mut ViewContext<V>, | ||
| ) -> Self { | ||
| pub fn new<V: View>(url: Option<String>, session_id: String, ctx: &mut ViewContext<V>) -> Self { |
| visible: bool, | ||
| ctx: &mut ViewContext<PaneGroup>, | ||
| ) { | ||
| fn on_workspace_tab_visibility_changed(&self, visible: bool, ctx: &mut ViewContext<PaneGroup>) { |
Comment on lines
1477
to
1481
| BrowserViewAction::ToggleFind => self.toggle_find(ctx), | ||
| BrowserViewAction::CloseFind => self.close_find(ctx), | ||
| BrowserViewAction::FindNext => { | ||
| BrowserViewAction::FindNext => | ||
| { | ||
| #[cfg(not(target_family = "wasm"))] |
Comment on lines
1484
to
1488
| } | ||
| } | ||
| BrowserViewAction::FindPrev => { | ||
| BrowserViewAction::FindPrev => | ||
| { | ||
| #[cfg(not(target_family = "wasm"))] |
main carries three unformatted files inherited from feature commits that landed while heavy CI jobs (Formatting + Clippy) were stuck queueing on unprovisioned warpdotdev runner labels. Now that PR #155 switches to stock GitHub runners, those jobs actually execute and fail the fmt check, blocking every PR. Apply `cargo fmt` to the three files cargo flagged: - app/src/browser/browser_view.rs (Layer A + B) - app/src/pane_group/pane/browser_pane.rs (Layer A + B) - app/src/settings_view/import_theme_modal.rs (theme import) No behavioral changes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
f308922 to
02dc226
Compare
Now that PR #155 made the heavy CI jobs actually execute (stock GitHub runners instead of unprovisioned warpdotdev SKUs), the clippy half of the Formatting + Clippy job surfaces 40+ preexisting `-D warnings` violations on main. This commit lands the mechanical / cheap ones: - `items_after_test_module`: move test mod to end of file in workspace/view.rs and settings_view/main_page.rs. - `disallowed_types`: swap `std::process::Command` for the Windows-friendly `command::blocking::Command` wrapper inside util/git_tests.rs (1) and util/worktree_tests.rs (5). - `dead_code` in scaffolding-only browser modules (downloads.rs, popup_policy.rs): allow at module level. Both modules are consumed only by browser/webview_host.rs's in-progress popup + download wiring; deleting them would tear out the follow-up work. Module-level allow keeps the call graph intact until the wry handlers land. No behavioral changes. Remaining clippy fallout (mostly more dead_code in browser + cli_chat scaffolding plus a handful of one-off lints) tracked in follow-up commits. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Round 2 of the heavy-CI fallout. With #155 making jobs actually run, clippy on Linux flagged 30+ more violations. Split between dead_code allows on in-progress scaffolding and one-off lint fixes: Browser pane (in-progress wry wiring, currently macOS-only): - webview_host.rs: gate `FindResultsMessage` and `popup_policy::*` imports to target_os = "macos" so non-macOS builds don't pull them in just to fail dead_code; add struct-level + enum-level `#[allow(dead_code)]` to `NativeBrowserWebView` and `NativeWebViewEvent` since their fields/variants are constructed only by macOS handlers. - browser_model.rs: allow pinned/favicon accessors + mutators (tab strip UI scaffolding). - find.rs: allow `FindResultsMessage` (deserialized only inside the macOS-gated IPC handler). - browser_view.rs: allow `too_many_arguments` on the two render helpers; drop a useless `.into()` on `chip_text_color`. CLI chat (Phase 5/6 scaffolding): - cli_chat/mod.rs: tag each `pub mod` declaration with `#[allow(dead_code)]` since the layers compile but no caller binds them yet. - empty_state.rs: allow `enum_variant_names` (all "No…" prefixes are intentional UX strings, not naming churn). - model_picker.rs: drop `&` on a `&&ChatModel` arg. - store.rs: replace `|row| row_to_conversation(row)` closure with the function pointer (`redundant_closure`). - store_schema.rs: rewrite the migrations loop to use `iter().enumerate().skip(...)` instead of indexing (`needless_range_loop`). Cross-cutting one-offs: - claude_transcript.rs: drop a stray blank line between the doc and `validated_stem`. - coven_brand.rs: allow `OPENCOVEN_MUTED` (kept for follow-up UI use). - server_api.rs: allow `AccessTokenRefreshed::token` field on all platforms (was already wasm-only). - settings/import/config.rs: allow `large_enum_variant` on `ThemeType` (cold path, one allocation per import). - terminal/view/pane_impl.rs: collapse identical `if/else` returning the same string; rename the parameter to `_` and document the follow-up that distinguishes ambient vs regular labels. - themes/tweakcn_import.rs: drop a `format!("{}", selector)` that's literally the identity, and factor the heavy fn-pointer type into a `UiTokenSetter` alias to satisfy `type_complexity`. No behavioural changes. Remaining: the `disallowed_type std::process::Command` errors landed in commit 3315e06; this commit covers the lib lints. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
BunsDev
added a commit
that referenced
this pull request
May 27, 2026
PR #157's first heavy CI run validated the Linux clippy fix (passes green) but surfaced two additional platform-specific lints that Linux didn't trigger: - cast_agent/src/comux.rs: gate `ListPanesRequest` and `ListPanesResponse` to `#[cfg(unix)]` to match the only call site (the unix-only block at line 96). On Windows the structs compiled but no constructor existed, so dead_code fired. - warp_cli/src/lib.rs: split the `std::ffi::OsString` import out of the unconditional `use std::{env, ffi::OsString, ...}` line and gate it to `#[cfg(not(target_family = "wasm"))]`, matching the three OsString-using helpers that are already wasm-gated. No behavioural changes; just import + cfg housekeeping. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Member
Author
|
Pushed |
3 tasks
BunsDev
added a commit
that referenced
this pull request
May 27, 2026
Two more wasm-only clippy errors surfaced once #157's main-side fmt fixes shipped: - crates/lsp/src/model.rs: split `LspStartupError` out of the unconditional `use ... supported_servers::{...}` group. The only usage (the downcast inside `LspServerModel::start`) is already `#[cfg(not(target_arch = "wasm32"))]`; the import is now too. - crates/lsp/src/supported_servers.rs: gate `LspStartupError::missing_binary` to non-wasm too. It's the only constructor of `LspStartupError`, and only `start` calls it, so on wasm the function was unused. No behavioural changes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
BunsDev
added a commit
that referenced
this pull request
May 27, 2026
PR #157's first heavy CI run validated the Linux clippy fix (passes green) but surfaced two additional platform-specific lints that Linux didn't trigger: - cast_agent/src/comux.rs: gate `ListPanesRequest` and `ListPanesResponse` to `#[cfg(unix)]` to match the only call site (the unix-only block at line 96). On Windows the structs compiled but no constructor existed, so dead_code fired. - warp_cli/src/lib.rs: split the `std::ffi::OsString` import out of the unconditional `use std::{env, ffi::OsString, ...}` line and gate it to `#[cfg(not(target_family = "wasm"))]`, matching the three OsString-using helpers that are already wasm-gated. No behavioural changes; just import + cfg housekeeping. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
BunsDev
added a commit
that referenced
this pull request
May 27, 2026
Two more wasm-only clippy errors surfaced once #157's main-side fmt fixes shipped: - crates/lsp/src/model.rs: split `LspStartupError` out of the unconditional `use ... supported_servers::{...}` group. The only usage (the downcast inside `LspServerModel::start`) is already `#[cfg(not(target_arch = "wasm32"))]`; the import is now too. - crates/lsp/src/supported_servers.rs: gate `LspStartupError::missing_binary` to non-wasm too. It's the only constructor of `LspStartupError`, and only `start` calls it, so on wasm the function was unused. No behavioural changes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Summary
cargo fmtto three files cargo flagged onmain:browser_view.rs,browser_pane.rs(Layer A + B), andimport_theme_modal.rs(theme import)Formatting + Clippyjobs that started actually executing once ci: use stock github-hosted runners instead of warpdotdev SKUs #155 switched to stock GitHub runnersWhy now
The unformatted code has been on
mainfor several commits, but theFormatting + Clippyjobs never caught it because they were stuck queued indefinitely on the unprovisionedubuntu-latest-large/windows-latest-large/namespace-profile-mac-cirunner labels. PR #155 fixes the runner labels; this PR fixes the fmt fallout so the jobs go green.Test plan
Formatting + Clippy (Linux/Windows/wasm)jobs passcargo fmt --checkruns clean on the merged result🤖 Generated with Claude Code