diff --git a/Cargo.lock b/Cargo.lock index 6a87905c3..02bb0950e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4374,6 +4374,7 @@ dependencies = [ "ide-jsonrpc", "ide-language", "ide-text", + "ide-vfs", "ignore", "lsp-types", "parking_lot", diff --git a/crates/ide-core/src/editor_settings.rs b/crates/ide-core/src/editor_settings.rs index a478558c6..4806bf2cd 100644 --- a/crates/ide-core/src/editor_settings.rs +++ b/crates/ide-core/src/editor_settings.rs @@ -371,6 +371,12 @@ pub struct TextAttributesOverride { #[derive(Clone, Debug, Default, PartialEq, Serialize, Deserialize, Type)] #[serde(default, rename_all = "camelCase")] pub struct LanguageCodeStyle { + /// Java formatter: `default` | `intellij` | `custom` (other languages ignore it). + #[serde(skip_serializing_if = "Option::is_none")] + pub formatter: Option, + /// Eclipse formatter XML; relative paths start at the project root. + #[serde(skip_serializing_if = "Option::is_none")] + pub formatter_path: Option, #[serde(skip_serializing_if = "Option::is_none")] pub use_tab: Option, #[serde(skip_serializing_if = "Option::is_none")] diff --git a/crates/ide-host/src/ops/project.rs b/crates/ide-host/src/ops/project.rs index 4c17cdde1..e6632c1d1 100644 --- a/crates/ide-host/src/ops/project.rs +++ b/crates/ide-host/src/ops/project.rs @@ -172,12 +172,14 @@ pub fn set_settings( host.apply_proxy(); } // A new interpreter / toolchain / server executable restarts the language servers it affects. - if ["languages", "tools", "languageServers", "projectStructure"] + let code_style = key == "editor" || key == "editor.codeStyle" || key.starts_with("editor.codeStyle."); + let toolchain = ["languages", "tools", "languageServers", "projectStructure"] .iter() - .any(|k| key == *k || key.starts_with(&format!("{k}."))) - { + .any(|k| key == *k || key.starts_with(&format!("{k}."))); + // Code style only reaches servers as settings (e.g. jdtls' formatter profile); builds don't depend on it. + if code_style || toolchain { for project in affected() { - if let Some(bridge) = &project.kotlin_bridge { + if toolchain && let Some(bridge) = &project.kotlin_bridge { bridge.schedule(); } let lsp = project.lsp.clone(); diff --git a/crates/ide-host/src/project.rs b/crates/ide-host/src/project.rs index f28c7cffe..baf7e10ea 100644 --- a/crates/ide-host/src/project.rs +++ b/crates/ide-host/src/project.rs @@ -129,6 +129,8 @@ impl Host { let root = root.clone(); let data_dir = self.data_dir.clone(); let config_dir = self.config_dir.clone(); + let sink = sink.clone(); + let formatter_problem = std::sync::Mutex::new(None); lsp.set_config_hook(Box::new(move |cfg, server_root| { let member = ide_workspace::workspace::Workspace::load(&root).ok().flatten().and_then(|w| { w.projects.into_iter().filter(|p| server_root.starts_with(p)).max_by_key(|p| p.components().count()) @@ -142,6 +144,27 @@ impl Host { ide_sdk::apply::configure_server(cfg, root, server_root, &effective); ide_sdk::java::cap_java_home(cfg, &ide_sdk::java::installed_jdks()); if cfg.name == "jdtls" { + let problem = ide_lsp::jdtls::configure_formatter( + cfg, + effective.editor.code_style.languages.get("java"), + root, + &data_dir, + effective.language_servers.get("jdtls"), + ) + .err() + .map(|e| e.to_string()); + let mut previous = formatter_problem.lock().unwrap(); + if problem != *previous { + if let Some(body) = &problem { + sink.emit(IdeEvent::Notify { + level: ide_core::event::NotifyLevel::Warning, + title: "Java formatter configuration unavailable".into(), + body: Some(body.clone()), + }); + } + *previous = problem; + } + drop(previous); ide_lsp::jdtls::configure_source_paths( cfg, ide_workspace::roots::unmanaged_java_source_paths( diff --git a/crates/ide-lsp/Cargo.toml b/crates/ide-lsp/Cargo.toml index 8b011c795..81cd83810 100644 --- a/crates/ide-lsp/Cargo.toml +++ b/crates/ide-lsp/Cargo.toml @@ -7,6 +7,7 @@ license.workspace = true [dependencies] ide-core.workspace = true +ide-vfs.workspace = true dunce.workspace = true ide-text.workspace = true ide-language.workspace = true diff --git a/crates/ide-lsp/src/jdtls.rs b/crates/ide-lsp/src/jdtls.rs index f2b79e0c1..82a52ef41 100644 --- a/crates/ide-lsp/src/jdtls.rs +++ b/crates/ide-lsp/src/jdtls.rs @@ -17,6 +17,9 @@ use std::sync::Arc; use std::time::{Duration, Instant}; const SERVER: &str = "jdtls"; + +mod formatter; +pub use formatter::configure_formatter; /// First start imports the Maven / Gradle model, which can take minutes. const READY_TIMEOUT: Duration = Duration::from_secs(240); /// A workspace build of a large multi-module project can take far longer than diff --git a/crates/ide-lsp/src/jdtls/formatter.rs b/crates/ide-lsp/src/jdtls/formatter.rs new file mode 100644 index 000000000..f7c19dea6 --- /dev/null +++ b/crates/ide-lsp/src/jdtls/formatter.rs @@ -0,0 +1,224 @@ +use ide_core::IdeResult; +use ide_core::editor_settings::LanguageCodeStyle; +use ide_core::settings::LanguageServerSettings; +use ide_language::LanguageServerConfig; +use serde_json::json; +use std::path::Path; + +const PROFILE: &str = "OpenIDEA IntelliJ IDEA"; +const XML: &str = include_str!("intellij.xml"); + +/// Protocol settings only: switching profiles reaches a running jdtls through +/// didChangeConfiguration. Explicit language-server settings are applied later. +pub fn configure_formatter( + cfg: &mut LanguageServerConfig, + style: Option<&LanguageCodeStyle>, + project_root: &Path, + data_dir: &Path, + overrides: Option<&LanguageServerSettings>, +) -> IdeResult<()> { + if cfg.name != "jdtls" { + return Ok(()); + } + // jdtls retains omitted preferences. Send empty values when returning to + // Default, while preserving any profile from the base server configuration. + let inherited = crate::handler::all_settings(cfg); + let defaults = json!({ + "url": inherited.pointer("/java/format/settings/url").cloned().unwrap_or(json!("")), + "profile": inherited.pointer("/java/format/settings/profile").cloned().unwrap_or(json!("")), + }); + let mut settings = cfg.settings.take().unwrap_or_else(|| json!({})); + settings["java"]["format"]["settings"] = formatter_overrides(defaults, overrides); + cfg.settings = Some(settings); + let Some(style) = style else { return Ok(()) }; + let (path, profile) = match style.formatter.as_deref() { + Some("intellij") => { + let path = data_dir.join("lsp/jdtls/intellij.xml"); + std::fs::create_dir_all(path.parent().unwrap())?; + if std::fs::read_to_string(&path).ok().as_deref() != Some(XML) { + ide_vfs::write_atomic(&path, XML.as_bytes())?; + } + (path, PROFILE) + } + Some("custom") => { + let value = style.formatter_path.as_deref().unwrap_or_default().trim(); + if value.is_empty() { + return Err(ide_core::IdeError::invalid("Choose an Eclipse formatter XML file.")); + } + let path = project_root.join(value); + // Fail without poisoning the server launch; code intelligence remains available. + std::fs::File::open(&path).map_err(|error| { + let path = path.display(); + ide_core::IdeError::invalid(format!("Cannot read Java formatter XML {path}: {error}")) + })?; + if !path.is_file() { + return Err(ide_core::IdeError::invalid(format!("Not a formatter XML file: {}", path.display()))); + } + (path, "") + } + _ => return Ok(()), + }; + let mut settings = cfg.settings.take().unwrap_or_else(|| json!({})); + settings["java"]["format"]["settings"] = formatter_overrides( + json!({ + "url": crate::uri::path_to_uri_string(&path), + "profile": profile, + }), + overrides, + ); + cfg.settings = Some(settings); + Ok(()) +} + +fn formatter_overrides(mut format: serde_json::Value, overrides: Option<&LanguageServerSettings>) -> serde_json::Value { + if let Some(overrides) = overrides { + // all_settings merges protocol settings over initializationOptions. + // Mirror explicit formatter keys so generated protocol values cannot + // accidentally mask an advanced initializationOptions override. + for layer in [ + overrides.initialization_options.as_ref().and_then(|v| v.pointer("/settings/java/format/settings")), + overrides.settings.as_ref().and_then(|v| v.pointer("/settings/java/format/settings")), + overrides.settings.as_ref().and_then(|v| v.pointer("/java/format/settings")), + ] + .into_iter() + .flatten() + { + for key in ["url", "profile"] { + if let Some(value) = layer.get(key) { + format[key] = value.clone(); + } + } + } + } + format +} + +#[cfg(test)] +mod tests { + use super::*; + + fn config(name: &str) -> LanguageServerConfig { + serde_json::from_value(json!({"name": name, "command": name})).unwrap() + } + + #[test] + fn profiles_resolve_paths_preserve_settings_and_leave_other_servers_alone() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path().join("project with 空格"); + let data = dir.path().join("data"); + std::fs::create_dir_all(&root).unwrap(); + let custom = root.join("style.xml"); + std::fs::write(&custom, XML).unwrap(); + for name in ["jdtls", "rust-analyzer"] { + for mode in ["default", "intellij", "custom", "unknown"] { + let mut cfg = LanguageServerConfig { + name: name.into(), + settings: Some(json!({"java": {"autobuild": {"enabled": false}}})), + ..config(name) + }; + let initial = cfg.clone(); + let style = LanguageCodeStyle { + formatter: Some(mode.into()), + formatter_path: Some("style.xml".into()), + ..Default::default() + }; + configure_formatter(&mut cfg, Some(&style), &root, &data, None).unwrap(); + if name != "jdtls" { + assert_eq!(cfg, initial); + continue; + } + let settings = crate::handler::all_settings(&cfg); + if !["intellij", "custom"].contains(&mode) { + assert_eq!(settings["java"]["format"]["settings"], json!({"url": "", "profile": ""})); + continue; + } + let path = if mode == "custom" { custom.clone() } else { data.join("lsp/jdtls/intellij.xml") }; + assert_eq!(settings["java"]["format"]["settings"]["url"], crate::uri::path_to_uri_string(&path)); + assert_eq!( + settings["java"]["format"]["settings"]["profile"], + if mode == "custom" { "" } else { PROFILE } + ); + assert_eq!(settings["java"]["autobuild"]["enabled"], false); + assert_eq!(std::fs::read_to_string(path).unwrap(), XML); + } + } + } + + #[test] + fn unavailable_custom_paths_are_nonfatal_and_do_not_set_launch_error() { + let dir = tempfile::tempdir().unwrap(); + for path in ["", "missing.xml", "."] { + let mut cfg = config("jdtls"); + let style = LanguageCodeStyle { + formatter: Some("custom".into()), + formatter_path: Some(path.into()), + ..Default::default() + }; + assert!(configure_formatter(&mut cfg, Some(&style), dir.path(), dir.path(), None).is_err()); + assert!(cfg.launch_error.is_none()); + assert_eq!(crate::handler::all_settings(&cfg)["java"]["format"]["settings"]["url"], ""); + } + } + + #[test] + fn default_keeps_explicit_server_configuration_and_custom_accepts_absolute_paths() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("custom.xml"); + std::fs::write(&path, XML).unwrap(); + let mut cfg = config("jdtls"); + cfg.initialization_options = Some( + toml::Value::try_from(json!({ + "settings": {"java": {"format": {"settings": {"url": "existing.xml", "profile": "Existing"}}}} + })) + .unwrap(), + ); + configure_formatter(&mut cfg, None, dir.path(), dir.path(), None).unwrap(); + assert_eq!( + crate::handler::all_settings(&cfg)["java"]["format"]["settings"], + json!({"url": "existing.xml", "profile": "Existing"}) + ); + let style = LanguageCodeStyle { + formatter: Some("custom".into()), + formatter_path: Some(path.to_string_lossy().into_owned()), + ..Default::default() + }; + configure_formatter(&mut cfg, Some(&style), &dir.path().join("different-root"), dir.path(), None).unwrap(); + assert_eq!( + crate::handler::all_settings(&cfg)["java"]["format"]["settings"]["url"], + crate::uri::path_to_uri_string(&path) + ); + } + #[test] + fn advanced_formatter_options_win_for_each_profile_and_protocol_layer() { + let dir = tempfile::tempdir().unwrap(); + std::fs::write(dir.path().join("custom.xml"), XML).unwrap(); + for mode in ["default", "intellij", "custom"] { + for layer in ["initialization", "wrapped", "direct"] { + let mut overrides = LanguageServerSettings { + initialization_options: Some(json!({"settings": {"java": {"format": {"settings": { + "url": "advanced.xml", "profile": "Advanced" + }}}}})), + ..Default::default() + }; + if layer != "initialization" { + let value = json!({"java": {"format": {"settings": {"profile": "Protocol"}}}}); + overrides.settings = Some(if layer == "wrapped" { json!({"settings": value}) } else { value }); + } + let mut cfg = config("jdtls"); + let style = LanguageCodeStyle { + formatter: Some(mode.into()), + formatter_path: Some("custom.xml".into()), + ..Default::default() + }; + configure_formatter(&mut cfg, Some(&style), dir.path(), dir.path(), Some(&overrides)).unwrap(); + assert_eq!( + crate::handler::all_settings(&cfg)["java"]["format"]["settings"], + json!({ + "url": "advanced.xml", + "profile": if layer == "initialization" { "Advanced" } else { "Protocol" }, + }) + ); + } + } + } +} diff --git a/crates/ide-lsp/src/jdtls/intellij.xml b/crates/ide-lsp/src/jdtls/intellij.xml new file mode 100644 index 000000000..18b3cf8de --- /dev/null +++ b/crates/ide-lsp/src/jdtls/intellij.xml @@ -0,0 +1,235 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/crates/ide-lsp/tests/fixtures/FormatterSample.input.java b/crates/ide-lsp/tests/fixtures/FormatterSample.input.java new file mode 100644 index 000000000..6c7121a84 --- /dev/null +++ b/crates/ide-lsp/tests/fixtures/FormatterSample.input.java @@ -0,0 +1,25 @@ +package demo; +import java.util.List; +import java.util.function.Function; +@Deprecated @SuppressWarnings("unused") public class FormatterSample { +@Deprecated @SuppressWarnings("unused") private String first="first"; +private String second="second"; +/** A description which remains on a very long single line to check whether formatting introduces wrapping when the right margin is exceeded with ordinary words in a JavaDoc comment. */ +@Deprecated @SuppressWarnings("unused") public String combine(@Deprecated @SuppressWarnings("unused") String firstParameterWithAVeryLongName, String secondParameterWithAVeryLongName, String thirdParameterWithAVeryLongName){if(firstParameterWithAVeryLongName==null){return "none";}else{try{return List.of(firstParameterWithAVeryLongName,secondParameterWithAVeryLongName,thirdParameterWithAVeryLongName).stream().map(value->value.trim()).filter(value->!value.isEmpty()).findFirst().orElse("none");}catch(RuntimeException problem){throw problem;}finally{System.out.println("done");}}} +public void multiline(String firstParameter, +String secondParameter, +String thirdParameter){ +combine(firstParameter, +secondParameter, +thirdParameter); +List.of(firstParameter, secondParameter) +.stream() +.map(value->value.trim()) +.forEach(System.out::println); +} +public void retained(String firstParameter, // retain declaration break +String secondParameter){combine(firstParameter, // retain call break +secondParameter, "third");} +public void arrays(){int[] values=new int[]{1,2,3};int[] empty=new int[]{};Function clean=value->{return value.trim();};Runnable run=()->{System.out.println(values.length);};} +// A comment which intentionally exceeds the right margin and should remain unwrapped with the built-in profile even when it contains many ordinary words and spaces to split on. +} diff --git a/crates/ide-lsp/tests/fixtures/FormatterSample.intellij.java b/crates/ide-lsp/tests/fixtures/FormatterSample.intellij.java new file mode 100644 index 000000000..f1211e24c --- /dev/null +++ b/crates/ide-lsp/tests/fixtures/FormatterSample.intellij.java @@ -0,0 +1,65 @@ +package demo; + +import java.util.List; +import java.util.function.Function; + +@Deprecated +@SuppressWarnings("unused") +public class FormatterSample { + @Deprecated + @SuppressWarnings("unused") + private String first = "first"; + private String second = "second"; + + /** A description which remains on a very long single line to check whether formatting introduces wrapping when the right margin is exceeded with ordinary words in a JavaDoc comment. */ + @Deprecated + @SuppressWarnings("unused") + public String combine(@Deprecated @SuppressWarnings("unused") String firstParameterWithAVeryLongName, + String secondParameterWithAVeryLongName, String thirdParameterWithAVeryLongName) { + if (firstParameterWithAVeryLongName == null) { + return "none"; + } else { + try { + return List + .of(firstParameterWithAVeryLongName, secondParameterWithAVeryLongName, + thirdParameterWithAVeryLongName) + .stream().map(value -> value.trim()).filter(value -> !value.isEmpty()).findFirst() + .orElse("none"); + } catch (RuntimeException problem) { + throw problem; + } finally { + System.out.println("done"); + } + } + } + + public void multiline(String firstParameter, + String secondParameter, + String thirdParameter) { + combine(firstParameter, + secondParameter, + thirdParameter); + List.of(firstParameter, secondParameter) + .stream() + .map(value -> value.trim()) + .forEach(System.out::println); + } + + public void retained(String firstParameter, // retain declaration break + String secondParameter) { + combine(firstParameter, // retain call break + secondParameter, "third"); + } + + public void arrays() { + int[] values = new int[]{1, 2, 3}; + int[] empty = new int[]{}; + Function clean = value -> { + return value.trim(); + }; + Runnable run = () -> { + System.out.println(values.length); + }; + } + // A comment which intentionally exceeds the right margin and should remain unwrapped with the built-in profile even when it contains many ordinary words and spaces to split on. +} diff --git a/crates/ide-lsp/tests/mock_server.rs b/crates/ide-lsp/tests/mock_server.rs index a636e5e4d..7bfb45ccd 100644 --- a/crates/ide-lsp/tests/mock_server.rs +++ b/crates/ide-lsp/tests/mock_server.rs @@ -1017,6 +1017,70 @@ async fn settings_changes_reach_servers_without_restarts() { assert_eq!(s.launches.load(Ordering::SeqCst), 1); } +#[tokio::test(flavor = "multi_thread")] +async fn java_formatter_profiles_reach_initialization_and_live_configuration_without_restart() { + let h = harness_configured("java", "java", &[("jdtls", incremental_caps())], |cfg| { + cfg.initialization_options = Some( + toml::Value::try_from(json!({ + "settings": {"java": {"autobuild": {"enabled": false}}} + })) + .unwrap(), + ); + }); + let data = tempfile::tempdir().unwrap(); + std::fs::write(h.root.join("custom.xml"), "").unwrap(); + let style = Arc::new(Mutex::new(ide_core::editor_settings::LanguageCodeStyle { + formatter: Some("intellij".into()), + formatter_path: Some("custom.xml".into()), + ..Default::default() + })); + h.mgr.set_config_hook(Box::new({ + let style = style.clone(); + let root = h.root.clone(); + let data = data.path().to_path_buf(); + move |cfg, _| ide_lsp::jdtls::configure_formatter(cfg, Some(&style.lock()), &root, &data, None).unwrap() + })); + h.open("Main.java", "public class Main {}"); + h.wait_status("jdtls", ServerStatus::Ready).await; + let state = h.launcher.state("jdtls"); + let init = state.requested("initialize")[0].clone(); + assert_eq!( + init["initializationOptions"]["settings"]["java"]["format"]["settings"]["profile"], + "OpenIDEA IntelliJ IDEA" + ); + for mode in ["custom", "default", "intellij", "default"] { + style.lock().formatter = Some(mode.into()); + h.mgr.reconfigure().await; + let expected = match mode { + "custom" => uri_of(&h.root.join("custom.xml")), + "intellij" => uri_of(&data.path().join("lsp/jdtls/intellij.xml")), + _ => String::new(), + }; + eventually(|| { + state + .notified("workspace/didChangeConfiguration") + .last() + .is_some_and(|p| p["settings"]["java"]["format"]["settings"]["url"] == expected) + }) + .await; + let result: Value = state + .peer() + .request( + "workspace/configuration", + json!({ + "items": [{"section": "java.format.settings"}, {"section": "java.autobuild.enabled"}] + }), + None, + ) + .await + .unwrap(); + assert_eq!(result[0]["url"], expected); + assert_eq!(result[0]["profile"], if mode == "intellij" { "OpenIDEA IntelliJ IDEA" } else { "" }); + assert_eq!(result[1], false); + assert_eq!(state.launches.load(Ordering::SeqCst), 1); + } +} + #[tokio::test(flavor = "multi_thread")] async fn java_source_paths_reach_initialization_live_configuration_and_builds() { let h = harness_configured("java", "java", &[("jdtls", incremental_caps())], |cfg| { diff --git a/crates/ide-lsp/tests/real_servers.rs b/crates/ide-lsp/tests/real_servers.rs index 43c250a20..f25f77745 100644 --- a/crates/ide-lsp/tests/real_servers.rs +++ b/crates/ide-lsp/tests/real_servers.rs @@ -62,6 +62,53 @@ fn lang(id: &str, ext: &str, server: LanguageServerConfig) -> LanguageConfig { } } +#[tokio::test(flavor = "multi_thread")] +#[ignore = "requires jdtls on PATH and a compatible JDK"] +async fn intellij_java_formatter_preserves_manual_breaks_and_wraps_long_lines() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path().canonicalize().unwrap(); + let source = include_str!("fixtures/FormatterSample.input.java"); + let expected = include_str!("fixtures/FormatterSample.intellij.java"); + let path = root.join("src/demo/FormatterSample.java"); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write(&path, source).unwrap(); + let mut java = LanguageRegistry::load(None).unwrap().get(&LanguageId::new("java")).unwrap(); + let server = &mut java.language_servers[0]; + if let Ok(bin) = std::env::var("OPENIDEA_TEST_JDTLS") { + server.command = bin; + } + server.args.extend(["-data".into(), root.join("jdtls-workspace").to_string_lossy().into_owned()]); + server.settings = Some(serde_json::json!({"java": {"autobuild": {"enabled": false}}})); + let (mgr, events) = manager(&root, LanguageRegistry::from_configs(vec![java])); + mgr.set_config_hook(Box::new({ + let root = root.clone(); + move |cfg, _| { + ide_lsp::jdtls::configure_source_paths(cfg, Some(vec!["src".into()])); + let style = ide_core::editor_settings::LanguageCodeStyle { + formatter: Some("intellij".into()), + ..Default::default() + }; + ide_lsp::jdtls::configure_formatter(cfg, Some(&style), &root, &root.join("data"), None).unwrap(); + } + })); + let doc = DocId::next(); + mgr.did_open(doc, &path, &LanguageId::new("java"), 1, source); + retry("jdtls ready", Duration::from_secs(120), || { + let status = last_status(&events, "jdtls"); + if let Some((ServerStatus::Crashed | ServerStatus::Missing | ServerStatus::Misconfigured, msg)) = &status { + panic!("jdtls failed: {msg:?}"); + } + async move { matches!(status, Some((ServerStatus::Ready | ServerStatus::Busy, _))).then_some(()) } + }) + .await; + let edits = + mgr.format(doc, None, ide_lsp::types::FormatOptions { tab_size: 4, insert_spaces: true, on_save: false }).await; + tokio::time::timeout(Duration::from_secs(10), mgr.shutdown_all()).await.unwrap(); + let mut document = ide_text::Document::new(source); + document.apply_edits(&edits.unwrap()).unwrap(); + assert_eq!(document.text(), expected); +} + #[tokio::test(flavor = "multi_thread")] async fn missing_binary_is_reported_with_install_hint() { let dir = tempfile::tempdir().unwrap(); diff --git a/crates/ide-tools/src/process.rs b/crates/ide-tools/src/process.rs index fad1f755d..11bc37b45 100644 --- a/crates/ide-tools/src/process.rs +++ b/crates/ide-tools/src/process.rs @@ -348,6 +348,89 @@ pub fn progress_event(bytes: u64, size: Option) -> InstallEvent { } } +/// Runs a formatter with separate pipes so diagnostics can never become document text. +pub(crate) fn run_input( + cmd: &mut Command, + input: &str, + timeout: Duration, + cancel: &CancellationToken, +) -> IdeResult { + use std::io::Write; + if cancel.is_cancelled() { + return Err(IdeError::cancelled()); + } + let mut child = own_group(cmd).stdin(Stdio::piped()).stdout(Stdio::piped()).stderr(Stdio::piped()).spawn()?; + let mut stdin = child.stdin.take().unwrap(); + let input = input.to_owned(); + // All pipes drain concurrently, including stdin: large documents cannot deadlock or bypass the timeout. + enum Pipe { + Written(std::io::Result<()>), + Read(bool, std::io::Result>), + } + let (tx, rx) = mpsc::channel(); + let writer = tx.clone(); + std::thread::spawn(move || { + let _ = writer.send(Pipe::Written(stdin.write_all(input.as_bytes()))); + }); + for (is_stderr, mut pipe) in [ + (false, Box::new(child.stdout.take().unwrap()) as Box), + (true, Box::new(child.stderr.take().unwrap()) as Box), + ] { + let tx = tx.clone(); + std::thread::spawn(move || { + let mut bytes = Vec::new(); + let _ = tx.send(Pipe::Read(is_stderr, pipe.read_to_end(&mut bytes).map(|_| bytes))); + }); + } + drop(tx); + let started = Instant::now(); + let (mut status, mut output, mut errors, mut written) = (None, None, None, None); + let result = loop { + if cancel.is_cancelled() { + break Err(IdeError::cancelled()); + } + if started.elapsed() >= timeout { + break Err(IdeError::new(ErrorKind::TimedOut, "Formatting timed out")); + } + if status.is_none() { + match child.try_wait() { + Ok(s) => status = s, + Err(e) => break Err(e.into()), + } + } + if status.is_some() && output.is_some() && errors.is_some() && written.is_some() { + break Ok(()); + } + match rx.recv_timeout(Duration::from_millis(10)) { + Ok(Pipe::Written(result)) => written = Some(result), + Ok(Pipe::Read(true, result)) => errors = Some(result), + Ok(Pipe::Read(false, result)) => output = Some(result), + Err(mpsc::RecvTimeoutError::Disconnected) if output.is_some() && errors.is_some() && written.is_some() => { + std::thread::sleep(Duration::from_millis(10)); + } + Err(mpsc::RecvTimeoutError::Disconnected) => break Err(IdeError::internal("formatter pipe reader failed")), + Err(mpsc::RecvTimeoutError::Timeout) => {} + } + }; + if result.is_err() { + kill(&mut child); + } + result?; + let output = output.unwrap()?; + let errors = errors.unwrap()?; + let status = status.unwrap(); + if !status.success() { + let diagnostics: String = String::from_utf8_lossy(&errors).into_owned(); + return Err(IdeError::invalid(if diagnostics.trim().is_empty() { + format!("Formatter exited with {status}") + } else { + diagnostics + })); + } + written.unwrap()?; + String::from_utf8(output).map_err(|e| IdeError::invalid(e.to_string())) +} + #[cfg(test)] mod tests { use super::*; @@ -465,89 +548,6 @@ mod tests { } } -/// Runs a formatter with separate pipes so diagnostics can never become document text. -pub(crate) fn run_input( - cmd: &mut Command, - input: &str, - timeout: Duration, - cancel: &CancellationToken, -) -> IdeResult { - use std::io::Write; - if cancel.is_cancelled() { - return Err(IdeError::cancelled()); - } - let mut child = own_group(cmd).stdin(Stdio::piped()).stdout(Stdio::piped()).stderr(Stdio::piped()).spawn()?; - let mut stdin = child.stdin.take().unwrap(); - let input = input.to_owned(); - // All pipes drain concurrently, including stdin: large documents cannot deadlock or bypass the timeout. - enum Pipe { - Written(std::io::Result<()>), - Read(bool, std::io::Result>), - } - let (tx, rx) = mpsc::channel(); - let writer = tx.clone(); - std::thread::spawn(move || { - let _ = writer.send(Pipe::Written(stdin.write_all(input.as_bytes()))); - }); - for (is_stderr, mut pipe) in [ - (false, Box::new(child.stdout.take().unwrap()) as Box), - (true, Box::new(child.stderr.take().unwrap()) as Box), - ] { - let tx = tx.clone(); - std::thread::spawn(move || { - let mut bytes = Vec::new(); - let _ = tx.send(Pipe::Read(is_stderr, pipe.read_to_end(&mut bytes).map(|_| bytes))); - }); - } - drop(tx); - let started = Instant::now(); - let (mut status, mut output, mut errors, mut written) = (None, None, None, None); - let result = loop { - if cancel.is_cancelled() { - break Err(IdeError::cancelled()); - } - if started.elapsed() >= timeout { - break Err(IdeError::new(ErrorKind::TimedOut, "Formatting timed out")); - } - if status.is_none() { - match child.try_wait() { - Ok(s) => status = s, - Err(e) => break Err(e.into()), - } - } - if status.is_some() && output.is_some() && errors.is_some() && written.is_some() { - break Ok(()); - } - match rx.recv_timeout(Duration::from_millis(10)) { - Ok(Pipe::Written(result)) => written = Some(result), - Ok(Pipe::Read(true, result)) => errors = Some(result), - Ok(Pipe::Read(false, result)) => output = Some(result), - Err(mpsc::RecvTimeoutError::Disconnected) if output.is_some() && errors.is_some() && written.is_some() => { - std::thread::sleep(Duration::from_millis(10)); - } - Err(mpsc::RecvTimeoutError::Disconnected) => break Err(IdeError::internal("formatter pipe reader failed")), - Err(mpsc::RecvTimeoutError::Timeout) => {} - } - }; - if result.is_err() { - kill(&mut child); - } - result?; - let output = output.unwrap()?; - let errors = errors.unwrap()?; - let status = status.unwrap(); - if !status.success() { - let diagnostics: String = String::from_utf8_lossy(&errors).into_owned(); - return Err(IdeError::invalid(if diagnostics.trim().is_empty() { - format!("Formatter exited with {status}") - } else { - diagnostics - })); - } - written.unwrap()?; - String::from_utf8(output).map_err(|e| IdeError::invalid(e.to_string())) -} - #[cfg(all(test, unix))] mod input_tests { use super::*; diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 4cfeb1ff0..aa413b6ff 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -8,6 +8,8 @@ mod commands; #[cfg(debug_assertions)] mod dev; mod events; +#[cfg(any(target_os = "linux", test))] +mod linux_window_state; mod logging; mod menu; mod remote_dev; @@ -68,16 +70,18 @@ pub fn run() { // Single instance first: a second launch focuses this one and opens its path argument. // Debug builds have a separate identifier per worktree. let app = app.plugin(tauri_plugin_single_instance::init(windows::on_second_instance)); + let app = app.plugin(tauri_plugin_opener::init()).plugin(tauri_plugin_dialog::init()); + #[cfg(target_os = "linux")] + let app = app.plugin(linux_window_state::plugin()); + #[cfg(not(target_os = "linux"))] + let app = app.plugin( + tauri_plugin_window_state::Builder::default() + .with_state_flags(window_state_flags()) + // One remembered geometry shared by every project's settings window. + .map_label(|l| if windows::is_settings_window(l) { "settings" } else { l }) + .build(), + ); let app = app - .plugin(tauri_plugin_opener::init()) - .plugin(tauri_plugin_dialog::init()) - .plugin( - tauri_plugin_window_state::Builder::default() - .with_state_flags(window_state_flags()) - // One remembered geometry shared by every project's settings window. - .map_label(|l| if windows::is_settings_window(l) { "settings" } else { l }) - .build(), - ) // Inert until an endpoint is configured (see `commands::window::app_check_update`). .plugin(tauri_plugin_updater::Builder::new().build()); // E2E automation bridge (Tauri MCP), debug builds only, localhost only. @@ -152,6 +156,8 @@ pub fn run() { Ok(()) }) .on_window_event(|window, event| match event { + #[cfg(target_os = "linux")] + tauri::WindowEvent::CloseRequested { .. } => linux_window_state::save_before_close(window.app_handle()), tauri::WindowEvent::Focused(true) => { application_focus::changed(window.app_handle(), true); if let Some(w) = window.try_state::() { @@ -204,6 +210,7 @@ pub fn run() { }); } +#[cfg(not(target_os = "linux"))] fn window_state_flags() -> tauri_plugin_window_state::StateFlags { use tauri_plugin_window_state::StateFlags; StateFlags::SIZE | StateFlags::POSITION | StateFlags::MAXIMIZED | StateFlags::FULLSCREEN diff --git a/src-tauri/src/linux_window_state.rs b/src-tauri/src/linux_window_state.rs new file mode 100644 index 000000000..3d9816dce --- /dev/null +++ b/src-tauri/src/linux_window_state.rs @@ -0,0 +1,483 @@ +//! Restore GTK geometry after mapping, using the scale of the window's actual output. + +mod geometry; +#[cfg(target_os = "linux")] +use geometry::*; +use tauri::utils::config::WindowConfig; + +fn build_hidden(config: &mut WindowConfig) -> bool { + std::mem::replace(&mut config.visible, false) +} + +#[cfg(target_os = "linux")] +mod runtime { + use super::*; + use parking_lot::Mutex; + use std::collections::HashMap; + use tauri::{AppHandle, Listener, Manager, Window, WindowEvent, plugin::TauriPlugin}; + + const FILE: &str = tauri_plugin_window_state::DEFAULT_FILENAME; + + #[derive(Default)] + enum Phase { + #[default] + Mapping, + Size { + target: Size, + scale: f64, + request: u64, + }, + Mode, + Tracking, + } + + #[derive(Default)] + struct Pending { + limits: Limits, + configured: bool, + presented: bool, + presentation_request: u64, + saved: Geometry, + normal: Size, + history: ModeHistory, + restore_position: bool, + phase: Phase, + mode_requested: bool, + mode_request: u64, + } + + #[derive(Default)] + struct Cache { + saved: HashMap, + windows: HashMap, + next_request: u64, + } + + struct Frame { + size: Size, + scale: f64, + mode: Mode, + position: Option>, + monitors: Vec, + work_area: Option, + visible: bool, + } + + fn frame(window: &Window) -> Option { + let size = window.inner_size().ok()?; + Some(Frame { + visible: window.is_visible().ok()?, + size: Size(size.width, size.height), + scale: window.scale_factor().ok()?, + mode: Mode { + maximized: window.is_maximized().ok()?, + fullscreen: window.is_fullscreen().ok()?, + minimized: window.is_minimized().ok()?, + }, + position: window.outer_position().ok(), + monitors: window.available_monitors().unwrap_or_default(), + work_area: window + .current_monitor() + .ok() + .flatten() + .map(|m| Size(m.work_area().size.width, m.work_area().size.height)), + }) + } + + fn key(label: &str) -> &str { + if crate::windows::is_settings_window(label) { "settings" } else { label } + } + + pub fn prepare(app: &AppHandle, config: &mut WindowConfig) -> bool { + let hint_monitor = app.primary_monitor().ok().flatten(); + let hint_scale = hint_monitor.as_ref().map_or(1.0, |m| m.scale_factor()); + let hint_area = hint_monitor.map(|m| Size(m.work_area().size.width, m.work_area().size.height)); + let cache = app.state::>(); + let mut cache = cache.lock(); + let saved = cache.saved.get(key(&config.label)).cloned().unwrap_or_else(|| Geometry { + maximized: config.maximized, + fullscreen: config.fullscreen, + ..Default::default() + }); + cache.next_request += 1; + let presentation_request = cache.next_request; + let pending = Pending { + presentation_request, + limits: Limits { + min: (config.min_width.unwrap_or(1.0), config.min_height.unwrap_or(1.0)), + max: (config.max_width, config.max_height), + }, + normal: Size(saved.width, saved.height), + saved, + restore_position: true, + ..Default::default() + }; + if pending.saved.width > 0 && pending.saved.height > 0 { + // This only reduces the first-frame jump; mapped geometry is authoritative. + let size = restore_size(pending.normal, hint_scale, pending.limits.in_work_area(hint_area, hint_scale)); + config.width = f64::from(size.0) / hint_scale; + config.height = f64::from(size.1) / hint_scale; + } + cache.windows.insert(config.label.clone(), pending); + // Establish a normal window before asking GTK to maximize or fullscreen it. + config.maximized = false; + config.fullscreen = false; + build_hidden(config) + } + + fn sample(window: &Window, allow_restore: bool, accept_constrained_size: bool) { + let Some(frame) = frame(window) else { return }; + if !frame.visible { + return; + } + let cache = window.state::>(); + let mut cache = cache.lock(); + cache.next_request += 1; + let request = cache.next_request; + let Some(pending) = cache.windows.get_mut(window.label()) else { return }; + if !pending.presented && pending.saved.width == 0 { + pending.normal = remember_size(pending.normal, frame.size, false, frame.mode); + } + if !pending.configured || !pending.presented { + return; + } + if !allow_restore && !matches!(pending.phase, Phase::Tracking) { + return; + } + let transition = if matches!(pending.phase, Phase::Tracking) { + pending.history.observe(frame.mode, allow_restore) + } else { + NormalTransition::Stable + }; + if transition == NormalTransition::WaitForResize { + let normal = pending.normal; + let saved = cache.saved.entry(key(window.label()).to_owned()).or_default(); + (saved.width, saved.height) = (normal.0, normal.1); + saved.maximized = frame.mode.maximized; + saved.fullscreen = frame.mode.fullscreen; + return; + } + if transition == NormalTransition::Restore { + // The compositor may have changed output/DPI while maximized or minimized. + pending.saved.set_normal_size(pending.normal); + pending.phase = Phase::Mapping; + pending.mode_requested = false; + } + let mut fit_settings = false; + let mut position = None; + if matches!(pending.phase, Phase::Mapping) { + if pending.restore_position { + pending.restore_position = false; + let saved = &pending.saved; + let (x, y) = if saved.maximized { (saved.prev_x, saved.prev_y) } else { (saved.x, saved.y) }; + if frame.monitors.iter().any(|m| { + let p = m.position(); + let s = m.size(); + i64::from(x) < i64::from(p.x) + i64::from(s.width) + && i64::from(x) + i64::from(saved.width) > i64::from(p.x) + && i64::from(y) < i64::from(p.y) + i64::from(s.height) + && i64::from(y) + i64::from(saved.height) > i64::from(p.y) + }) { + position = Some(tauri::PhysicalPosition::new(x, y)); + } + } + if pending.saved.width > 0 && pending.saved.height > 0 { + let target = restore_size( + Size(pending.saved.width, pending.saved.height), + frame.scale, + pending.limits.in_work_area(frame.work_area, frame.scale), + ); + pending.phase = Phase::Size { target, scale: frame.scale, request: 0 }; + } else { + pending.normal = remember_size(pending.normal, frame.size, false, frame.mode); + pending.phase = Phase::Mode; + } + } + if let Phase::Size { target, scale, request: sent } = pending.phase { + if frame.size != target && !accept_constrained_size { + if sent != 0 && scale == frame.scale { + return; + } + // Re-evaluate after output scale changes, rather than reusing monitor 0's scale. + let target = restore_size( + Size(pending.saved.width, pending.saved.height), + frame.scale, + pending.limits.in_work_area(frame.work_area, frame.scale), + ); + pending.phase = Phase::Size { target, scale: frame.scale, request }; + drop(cache); + if let Some(position) = position { + let _ = window.set_position(position); + } + if let Err(error) = window.set_size(tauri::PhysicalSize::new(target.0, target.1)) { + tracing::warn!("cannot restore Linux window size: {error}"); + } + settle_constrained_size(window, request); + return; + } + if scale != frame.scale && !accept_constrained_size { + pending.phase = Phase::Mapping; + drop(cache); + sample(window, true, false); + return; + } + pending.normal = remember_size(pending.normal, frame.size, false, frame.mode); + pending.phase = Phase::Mode; + } + if matches!(pending.phase, Phase::Mode) { + let (maximize, fullscreen) = (pending.saved.maximized, pending.saved.fullscreen); + let reached = if fullscreen { frame.mode.fullscreen } else { !maximize || frame.mode.maximized }; + let action = mode_action(reached, pending.mode_requested, accept_constrained_size); + if action == ModeAction::Wait { + return; + } + if action == ModeAction::Request { + pending.mode_requested = true; + pending.mode_request = request; + let mut snapshot = pending.saved.clone(); + (snapshot.width, snapshot.height) = (pending.normal.0, pending.normal.1); + cache.saved.insert(key(window.label()).to_owned(), snapshot); + drop(cache); + if let Some(position) = position { + let _ = window.set_position(position); + } + if maximize { + let _ = window.maximize(); + } + if fullscreen { + let _ = window.set_fullscreen(true); + } + settle_constrained_size(window, request); + return; + } + pending.phase = Phase::Tracking; + fit_settings = crate::windows::is_settings_window(window.label()); + } + pending.history = ModeHistory(frame.mode); + pending.normal = remember_size(pending.normal, frame.size, false, frame.mode); + let normal = pending.normal; + let saved = cache.saved.entry(key(window.label()).to_owned()).or_default(); + (saved.width, saved.height) = (normal.0, normal.1); + saved.maximized = frame.mode.maximized; + saved.fullscreen = frame.mode.fullscreen; + if frame.mode.normal() + && let Some(pos) = frame.position + { + (saved.x, saved.y, saved.prev_x, saved.prev_y) = (pos.x, pos.y, pos.x, pos.y); + } + drop(cache); + if let Some(position) = position { + let _ = window.set_position(position); + } + if fit_settings { + let app = window.app_handle().clone(); + let label = window.label().to_owned(); + let _ = window.run_on_main_thread(move || { + if let Some(parent) = crate::windows::settings_parent(&label).and_then(|p| app.get_webview_window(p)) + && let Some(settings) = app.get_webview_window(&label) + && let Err(error) = crate::windows::place_settings_window(&settings, &parent) + { + tracing::warn!("cannot fit restored settings window: {error}"); + } + }); + } + } + + fn settle_constrained_size(window: &Window, request: u64) { + let window = window.clone(); + tauri::async_runtime::spawn(async move { + tokio::time::sleep(std::time::Duration::from_secs(5)).await; + let cloned = window.clone(); + let _ = window.run_on_main_thread(move || { + let waiting = cloned.state::>().lock().windows.get(cloned.label()).is_some_and(|p| { + matches!(p.phase, Phase::Size { request: current, .. } if current == request) + || matches!(p.phase, Phase::Mode) && p.mode_request == request + }); + if waiting { + // Window-manager constraints may reject a size without producing an exact match. + sample(&cloned, true, true); + } + }); + }); + } + + pub fn normal_size_requested(app: &AppHandle, label: &str, size: tauri::PhysicalSize) { + let cache = app.state::>(); + let mut cache = cache.lock(); + cache.next_request += 1; + let request = cache.next_request; + if let Some(pending) = cache.windows.get_mut(label) { + pending.normal = Size(size.width, size.height); + pending.saved.set_normal_size(pending.normal); + pending.mode_requested = false; + pending.presented = false; + pending.presentation_request = request; + pending.phase = Phase::Mapping; + } + let saved = cache.saved.entry(key(label).to_owned()).or_default(); + saved.set_normal_size(Size(size.width, size.height)); + drop(cache); + if let Some(webview) = app.get_webview_window(label) { + request_presentation(webview.as_ref(), request); + } + } + + fn request_presentation(webview: &tauri::Webview, request: u64) { + // A configure precedes output-enter. A second animation frame follows the + // first paint, allowing GTK to learn the scale of the assigned output. + let script = r#" +(() => { + const state = window.__OPENIDEA_WINDOW_GEOMETRY__ ??= { request: 0, scheduled: false }; + state.request = __REQUEST_ID__; + state.ready ??= () => { + if (document.hidden || state.scheduled) return; + const request = state.request; + state.scheduled = true; + requestAnimationFrame(() => requestAnimationFrame(() => { + state.scheduled = false; + if (document.hidden) return; + const api = window.__TAURI_INTERNALS__; + const label = api?.metadata?.currentWindow?.label; + if (label) api.invoke('plugin:event|emit', { + event: 'openidea://window-state-presented', payload: { label, request } + }); + if (request !== state.request) state.ready(); + })); + }; + if (!state.listening) { + state.listening = true; + document.addEventListener('visibilitychange', state.ready); + } + state.ready(); +})(); +"# + .replace("__REQUEST_ID__", &request.to_string()); + let _ = webview.eval(script); + } + + pub fn save_before_close(app: &AppHandle) { + for window in app.webview_windows().values() { + sample(&window.as_ref().window(), false, false); + } + persist(app); + } + + fn persist(app: &AppHandle) { + let write = || -> Result<(), Box> { + let dir = app.path().app_config_dir()?; + std::fs::create_dir_all(&dir)?; + let cache = app.state::>(); + let data = serde_json::to_vec_pretty(&cache.lock().saved)?; + let temporary = dir.join(format!("{FILE}.tmp")); + std::fs::write(&temporary, data)?; + std::fs::rename(temporary, dir.join(FILE))?; + Ok(()) + }; + if let Err(error) = write() { + tracing::warn!("cannot save Linux window state: {error}"); + } + } + + pub fn plugin() -> TauriPlugin { + tauri::plugin::Builder::new("linux-window-state") + .setup(|app, _| { + let saved = app + .path() + .app_config_dir() + .ok() + .and_then(|dir| std::fs::read(dir.join(FILE)).ok()) + .and_then(|data| serde_json::from_slice(&data).ok()) + .unwrap_or_default(); + app.manage(Mutex::new(Cache { saved, ..Default::default() })); + let handle = app.clone(); + app.listen("openidea://window-state-presented", move |event| { + #[derive(serde::Deserialize)] + struct Presented { + label: String, + request: u64, + } + let Ok(presented) = serde_json::from_str::(event.payload()) else { return }; + let Some(webview) = handle.get_webview_window(&presented.label) else { return }; + let window = webview.as_ref().window(); + let cloned = window.clone(); + let _ = window.run_on_main_thread(move || { + if !cloned.is_visible().unwrap_or(false) { + return; + } + if let Some(pending) = cloned.state::>().lock().windows.get_mut(cloned.label()) { + if pending.presentation_request != presented.request { + return; + } + pending.presented = true; + } + sample(&cloned, true, false); + }); + }); + Ok(()) + }) + .on_page_load(|webview, payload| { + if payload.event() == tauri::webview::PageLoadEvent::Finished { + let request = webview + .state::>() + .lock() + .windows + .get(webview.window().label()) + .map(|p| p.presentation_request); + if let Some(request) = request { + request_presentation(webview, request); + } + } + }) + .on_window_ready(|window| { + let cloned = window.clone(); + window.on_window_event(move |event| match event { + WindowEvent::Resized(_) => { + if let Some(pending) = cloned.state::>().lock().windows.get_mut(cloned.label()) { + pending.configured = true; + } + let request = cloned + .state::>() + .lock() + .windows + .get(cloned.label()) + .filter(|p| !p.presented) + .map(|p| p.presentation_request); + if let Some(request) = request + && let Some(webview) = cloned.app_handle().get_webview_window(cloned.label()) + { + request_presentation(webview.as_ref(), request); + } + sample(&cloned, true, false); + } + WindowEvent::Moved(_) => sample(&cloned, false, false), + WindowEvent::Destroyed => { + cloned.state::>().lock().windows.remove(cloned.label()); + } + _ => {} + }); + }) + .on_event(|app, event| { + if let tauri::RunEvent::Exit = event { + persist(app); + } + }) + .build() + } +} + +#[cfg(target_os = "linux")] +pub use runtime::{normal_size_requested, plugin, prepare, save_before_close}; + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn hidden_creation_preserves_geometry() { + let mut config = WindowConfig { width: 1201.0, height: 801.0, ..Default::default() }; + assert!(build_hidden(&mut config)); + assert!(!config.visible); + assert_eq!((config.width, config.height), (1201.0, 801.0)); + assert!(!build_hidden(&mut config)); + } +} diff --git a/src-tauri/src/linux_window_state/geometry.rs b/src-tauri/src/linux_window_state/geometry.rs new file mode 100644 index 000000000..4bd747b4f --- /dev/null +++ b/src-tauri/src/linux_window_state/geometry.rs @@ -0,0 +1,237 @@ +use serde::{Deserialize, Serialize}; + +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub(super) struct Size(pub(super) u32, pub(super) u32); + +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub(super) struct Mode { + pub(super) maximized: bool, + pub(super) fullscreen: bool, + pub(super) minimized: bool, +} + +impl Mode { + pub(super) fn normal(self) -> bool { + !self.maximized && !self.fullscreen && !self.minimized + } +} + +#[derive(Clone, Copy, Debug, Default)] +pub(super) struct ModeHistory(pub(super) Mode); + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(super) enum NormalTransition { + Stable, + WaitForResize, + Restore, +} + +impl ModeHistory { + pub(super) fn observe(&mut self, current: Mode, can_restore: bool) -> NormalTransition { + let returned = !self.0.normal() && current.normal(); + if returned && !can_restore { + return NormalTransition::WaitForResize; + } + self.0 = current; + if returned { NormalTransition::Restore } else { NormalTransition::Stable } + } +} + +#[derive(Clone, Copy, Debug, Default)] +pub(super) struct Limits { + pub(super) min: (f64, f64), + pub(super) max: (Option, Option), +} + +impl Limits { + pub(super) fn in_work_area(self, area: Option, scale: f64) -> Self { + let Some(Size(width, height)) = area.filter(|s| s.0 > 0 && s.1 > 0) else { return self }; + Self { + max: ( + Some(self.max.0.unwrap_or(f64::INFINITY).min((f64::from(width) / scale).floor())), + Some(self.max.1.unwrap_or(f64::INFINITY).min((f64::from(height) / scale).floor())), + ), + ..self + } + } +} + +pub(super) fn restore_size(saved: Size, scale: f64, limits: Limits) -> Size { + let dimension = |pixels: u32, min: f64, max: Option| { + let logical = (f64::from(pixels) / scale).max(min).max(1.0); + // gtk_window_resize takes integer logical dimensions. + (logical.min(max.unwrap_or(f64::INFINITY).max(min)).round() * scale).round() as u32 + }; + Size(dimension(saved.0, limits.min.0, limits.max.0), dimension(saved.1, limits.min.1, limits.max.1)) +} + +pub(super) fn remember_size(previous: Size, current: Size, restoring: bool, mode: Mode) -> Size { + if restoring || !mode.normal() || current.0 == 0 || current.1 == 0 { previous } else { current } +} + +#[derive(Debug, PartialEq, Eq)] +pub(super) enum ModeAction { + Request, + Wait, + Track, +} + +pub(super) fn mode_action(reached: bool, requested: bool, timed_out: bool) -> ModeAction { + if reached { + ModeAction::Track + } else if !requested { + ModeAction::Request + } else if timed_out { + ModeAction::Track + } else { + ModeAction::Wait + } +} + +#[derive(Clone, Debug, Deserialize, Serialize)] +#[serde(default)] +pub(super) struct Geometry { + pub(super) width: u32, + pub(super) height: u32, + pub(super) x: i32, + pub(super) y: i32, + pub(super) prev_x: i32, + pub(super) prev_y: i32, + pub(super) maximized: bool, + pub(super) fullscreen: bool, + pub(super) visible: bool, + pub(super) decorated: bool, +} + +impl Geometry { + pub(super) fn set_normal_size(&mut self, size: Size) { + (self.width, self.height) = (size.0, size.1); + self.maximized = false; + self.fullscreen = false; + } +} + +impl Default for Geometry { + fn default() -> Self { + Self { + width: 0, + height: 0, + x: 0, + y: 0, + prev_x: 0, + prev_y: 0, + maximized: false, + fullscreen: false, + visible: true, + decorated: true, + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn constrained_size_does_not_skip_the_first_mode_request() { + assert_eq!(mode_action(false, false, true), ModeAction::Request); + assert_eq!(mode_action(false, false, false), ModeAction::Request); + assert_eq!(mode_action(false, true, false), ModeAction::Wait); + assert_eq!(mode_action(false, true, true), ModeAction::Track); + assert_eq!(mode_action(true, false, false), ModeAction::Track); + } + + #[test] + fn physical_sizes_use_the_actual_output_scale() { + for scale in [1.0, 2.0] { + assert_eq!(restore_size(Size(2468, 1356), scale, Limits::default()), Size(2468, 1356)); + } + // Saved at scale 1, mapped on scale 2: keep physical pixels, not twice that size. + assert_eq!(restore_size(Size(1234, 678), 2.0, Limits::default()), Size(1234, 678)); + } + + #[test] + fn gtk_limits_and_integer_logical_dimensions_are_applied_once() { + let limits = Limits { min: (800.0, 500.0), max: (Some(1600.0), Some(1000.0)) }; + assert_eq!(restore_size(Size(1201, 801), 1.0, limits), Size(1201, 801)); + assert_eq!(restore_size(Size(1201, 801), 2.0, limits), Size(1600, 1000)); + assert_eq!(restore_size(Size(2001, 1401), 2.0, limits), Size(2002, 1402)); + assert_eq!(restore_size(Size(9000, 6000), 2.0, limits), Size(3200, 2000)); + } + + #[test] + fn damaged_sizes_are_bounded_in_the_actual_outputs_units() { + let limits = Limits { min: (800.0, 500.0), ..Default::default() }; + for scale in [1.0, 2.0] { + let bounded = limits.in_work_area(Some(Size(3840, 2160)), scale); + assert_eq!(restore_size(Size(45068, 30000), scale, bounded), Size(3840, 2160)); + assert_eq!(restore_size(Size(2468, 1356), scale, bounded), Size(2468, 1356)); + } + } + + #[test] + fn restoration_and_non_normal_events_preserve_the_normal_size() { + let normal = Size(1234, 678); + for current in [Size(2468, 1356), Size(2648, 1602), Size(5476, 3204)] { + assert_eq!(remember_size(normal, current, true, Mode::default()), normal); + for mode in [ + Mode { maximized: true, ..Default::default() }, + Mode { fullscreen: true, ..Default::default() }, + Mode { minimized: true, ..Default::default() }, + ] { + assert_eq!(remember_size(normal, current, false, mode), normal); + } + } + assert_eq!(remember_size(normal, Size(1201, 801), false, Mode::default()), Size(1201, 801)); + assert_eq!(remember_size(normal, Size(0, 678), false, Mode::default()), normal); + } + + #[test] + fn a_mixed_dpi_maximized_launch_does_not_compound() { + let mut saved = Size(2468, 1356); + for _ in 0..3 { + saved = remember_size(saved, Size(saved.0 * 2, saved.1 * 2), true, Mode::default()); + let restored = restore_size(saved, 2.0, Limits::default()); + saved = remember_size(saved, restored, false, Mode::default()); + saved = remember_size(saved, Size(3840, 2160), false, Mode { maximized: true, ..Default::default() }); + assert_eq!(saved, Size(2468, 1356)); + } + } + + #[test] + fn moved_does_not_consume_the_unmaximize_resize() { + for mode in [ + Mode { maximized: true, ..Default::default() }, + Mode { fullscreen: true, ..Default::default() }, + Mode { minimized: true, ..Default::default() }, + ] { + let mut history = ModeHistory(mode); + assert_eq!(history.observe(Mode::default(), false), NormalTransition::WaitForResize); + assert_eq!(history.0, mode); + assert_eq!(history.observe(Mode::default(), true), NormalTransition::Restore); + assert_eq!(history.observe(Mode::default(), true), NormalTransition::Stable); + } + } + + #[test] + fn an_explicit_normal_frame_overrides_old_window_modes() { + let mut saved = Geometry { maximized: true, fullscreen: true, x: 123, y: 456, ..Default::default() }; + saved.set_normal_size(Size(1100, 700)); + assert_eq!((saved.width, saved.height), (1100, 700)); + assert!(!saved.maximized && !saved.fullscreen); + assert_eq!((saved.x, saved.y), (123, 456)); + } + + #[test] + fn state_files_remain_compatible_and_keep_closed_windows() { + let saved: std::collections::HashMap = serde_json::from_str( + r#"{"main":{"width":1234,"height":678,"maximized":true},"settings":{"width":1001,"height":701}}"#, + ) + .unwrap(); + let copy: std::collections::HashMap = + serde_json::from_value(serde_json::to_value(&saved).unwrap()).unwrap(); + assert_eq!(copy["settings"].height, 701); + assert!(copy["main"].maximized); + assert_eq!(copy["main"].width, 1234); + } +} diff --git a/src-tauri/src/window_groups.rs b/src-tauri/src/window_groups.rs index 4aaea46dc..66fe0c88f 100644 --- a/src-tauri/src/window_groups.rs +++ b/src-tauri/src/window_groups.rs @@ -60,7 +60,10 @@ fn copy_frame(from: &WebviewWindow, to: &WebviewWindow) -> IdeResult<()> { if !maximized { to.unmaximize().map_err(err)?; to.set_position(from.outer_position().map_err(err)?).map_err(err)?; - to.set_size(from.inner_size().map_err(err)?).map_err(err)?; + let size = from.inner_size().map_err(err)?; + #[cfg(target_os = "linux")] + crate::linux_window_state::normal_size_requested(to.app_handle(), to.label(), size); + to.set_size(size).map_err(err)?; } else { to.maximize().map_err(err)?; } diff --git a/src-tauri/src/windows.rs b/src-tauri/src/windows.rs index 54b6a35ce..f905ed51f 100644 --- a/src-tauri/src/windows.rs +++ b/src-tauri/src/windows.rs @@ -298,6 +298,8 @@ fn build_project_window(app: &AppHandle, mut config: WindowConfig) -> IdeResult< } else if let Some(a) = &appearance { config.background_color = Some(frame_color(a, false)); } + #[cfg(target_os = "linux")] + let show_after_build = crate::linux_window_state::prepare(app, &mut config); let window = WebviewWindowBuilder::from_config(app, &config) .and_then(|b| dev_font_preferences(b).build()) .map_err(|e| IdeError::internal(format!("cannot create window: {e}")))?; @@ -310,6 +312,10 @@ fn build_project_window(app: &AppHandle, mut config: WindowConfig) -> IdeResult< { let _ = window.set_background_color(Some(frame_color(&a, true))); } + #[cfg(target_os = "linux")] + if show_after_build { + window.show().map_err(|e| IdeError::internal(format!("cannot show window: {e}")))?; + } if visible { window.show().map_err(|e| IdeError::internal(format!("cannot show window: {e}")))?; if focused { @@ -765,6 +771,8 @@ pub fn open_settings_window( config.background_color = Some(color); } windows.set_launch(&label, Launch { settings_page: page, new_projects, ..Default::default() }); + #[cfg(target_os = "linux")] + crate::linux_window_state::prepare(app, &mut config); let builder = WebviewWindowBuilder::from_config(app, &config); // AppKit child windows can disappear when dragged to another display's Space. // The settings label already tracks ownership and parent-close cleanup. @@ -832,7 +840,7 @@ fn fit_settings_window(window: &WebviewWindow, parent: &WebviewWindow) -> tauri: /// may still be on another display with a different scale, so its size is converted with its own /// scale and its position is given in physical pixels. #[cfg(not(target_os = "macos"))] -fn place_settings_window(window: &WebviewWindow, parent: &WebviewWindow) -> tauri::Result<()> { +pub(super) fn place_settings_window(window: &WebviewWindow, parent: &WebviewWindow) -> tauri::Result<()> { let Some(monitor) = parent.current_monitor()?.or(parent.primary_monitor()?) else { return Ok(()); }; diff --git a/src/bindings.ts b/src/bindings.ts index c5e1504f8..645f8668a 100644 --- a/src/bindings.ts +++ b/src/bindings.ts @@ -3927,6 +3927,10 @@ export type LanguageCodeStyle = LanguageCodeStyle_Serialize | LanguageCodeStyle_ /** Editor › Code Style › › Tabs and Indents (unset fields inherit). */ export type LanguageCodeStyle_Deserialize = { + /** Java formatter: `default` | `intellij` | `custom` (other languages ignore it). */ + formatter?: string | null, + /** Eclipse formatter XML; relative paths start at the project root. */ + formatterPath?: string | null, useTab?: boolean | null, tabSize?: number | null, indent?: number | null, @@ -3939,6 +3943,10 @@ export type LanguageCodeStyle_Deserialize = { /** Editor › Code Style › › Tabs and Indents (unset fields inherit). */ export type LanguageCodeStyle_Serialize = { + /** Java formatter: `default` | `intellij` | `custom` (other languages ignore it). */ + formatter?: string | null, + /** Eclipse formatter XML; relative paths start at the project root. */ + formatterPath?: string | null, useTab?: boolean | null, tabSize?: number | null, indent?: number | null, diff --git a/src/features/agents/Agents.module.css b/src/features/agents/Agents.module.css index ff72619d1..a9b31d3f2 100644 --- a/src/features/agents/Agents.module.css +++ b/src/features/agents/Agents.module.css @@ -3,7 +3,6 @@ min-height: 0; display: flex; flex-direction: column; - container-type: inline-size; } .content { flex: 1; @@ -137,7 +136,7 @@ gap: 5px; } -/* A conversation docked in the tool window, above the task list. */ +/* A docked conversation owns the tool window; history opens on demand. */ .docked { flex: 1 1 0; min-height: 0; @@ -168,30 +167,31 @@ flex-direction: column; } .tasks { - flex: 2 1 0; + flex: 1; min-height: 0; min-width: 0; display: flex; flex-direction: column; - border-top: 1px solid var(--border); +} +.history { + display: flex; + flex-direction: column; + width: min(400px, calc(100vw - 24px)); + height: min(420px, 65vh); + overflow: hidden; +} +.historyTitle { + flex: none; + padding: 8px 12px; + border-bottom: 1px solid var(--border); + font-weight: 600; } -/* Keep the conversation usable in a short bottom tool window; the tree scrolls. */ -.docked + .tasks { - flex: 0 0 20%; - max-height: 160px; -} - -/* A wide bottom window has room beside the conversation, preserving message height. */ -@container (min-width: 600px) { - .content { - flex-direction: row; - } - .docked + .tasks { - flex: 0 0 220px; - max-width: 30%; - max-height: none; - border-top: 0; - border-left: 1px solid var(--border); - } +.location { + display: flex; + align-items: center; + gap: 2px; + padding: 1px; + border: 1px solid var(--border); + border-radius: var(--radius-small); } diff --git a/src/features/agents/AgentsView.test.tsx b/src/features/agents/AgentsView.test.tsx index a8ec9d370..a154ef438 100644 --- a/src/features/agents/AgentsView.test.tsx +++ b/src/features/agents/AgentsView.test.tsx @@ -6,9 +6,9 @@ import { registerActions } from "@/platform/actions"; import { EDITOR_TAB_MIME, type EditorInput } from "@/workbench/contributions"; import { confirmChecked, showError } from "@/workbench/messages"; import { ContextMenuHost, dragSession, hideContextMenu } from "@/ui"; -import { AgentsView } from "./AgentsView"; +import { AgentsHeaderActions, AgentsLocationActions, AgentsView } from "./AgentsView"; import { SESSION_KIND } from "./model"; -import { archiveTask, unarchiveTask, deleteTask, dockTask, openTask, undockTask, useAgents } from "./store"; +import { archiveTask, unarchiveTask, deleteTask, dockTask, openTask, setSessionLocation, useAgents } from "./store"; import { taskActions, taskMenu } from "./taskActions"; import styles from "./Agents.module.css"; @@ -39,7 +39,7 @@ vi.mock("./store", async (original) => ({ unarchiveTask: vi.fn(async () => {}), deleteTask: vi.fn(async () => {}), dockTask: vi.fn(), - undockTask: vi.fn(), + setSessionLocation: vi.fn(), })); vi.mock("./SessionEditor", () => ({ SessionEditor: () =>
})); @@ -103,6 +103,8 @@ beforeEach(() => { error: null, tasks: new Map([task("one"), task("two"), task("three"), task("four")].map((t) => [t.id, t])), docked: null, + homeOpen: false, + sessionLocation: "editor", }); dispose = registerActions(taskActions); }); @@ -115,6 +117,8 @@ afterEach(() => { function setup() { render( <> + + , @@ -158,24 +162,44 @@ describe("Agents selection", () => { }); describe("Agents docked view", () => { - it("shows the docked conversation above the list and switches it by selecting a task", () => { + it("opens session history on demand and closes it after switching tasks", () => { useAgents.setState({ docked: "one" }); setup(); expect(screen.getByTestId("docked-session")).toBeTruthy(); - expect(screen.getByRole("button", { name: "Open in Editor" })).toBeTruthy(); - expect(openTask).not.toHaveBeenCalled(); - select("two"); - expect(actions.executeAction).toHaveBeenCalledWith("agents.dockTask", "two"); - expect(dockTask).toHaveBeenCalledWith("two"); + expect(screen.getByRole("button", { name: "Chat in Editor" })).toBeTruthy(); expect(openTask).not.toHaveBeenCalled(); + expect(screen.queryByRole("tree", { name: "Agent tasks" })).toBeNull(); + fireEvent.click(screen.getByRole("button", { name: "Agent sessions" })); + fireEvent.click(within(screen.getByRole("tree", { name: "Agent tasks" })).getByText("two")); + expect(screen.queryByRole("dialog", { name: "Agent sessions" })).toBeNull(); + expect(actions.executeAction).toHaveBeenCalledWith("agents.openTask", "two"); + expect(openTask).toHaveBeenCalledWith("two"); }); - it("hides the docked task from the list below", () => { + it("lets keyboard users browse history before confirming a session", () => { useAgents.setState({ docked: "one" }); setup(); + fireEvent.click(screen.getByRole("button", { name: "Agent sessions" })); + const tree = screen.getByRole("tree", { name: "Agent tasks" }); + fireEvent.keyDown(tree, { key: "ArrowDown" }); + expect(screen.getByRole("dialog", { name: "Agent sessions" })).toBeTruthy(); + expect(openTask).not.toHaveBeenCalled(); + fireEvent.keyDown(tree, { key: "Enter" }); + expect(openTask).toHaveBeenCalledWith("two"); + expect(screen.queryByRole("dialog", { name: "Agent sessions" })).toBeNull(); + }); + + it("includes the current session in history and keeps the conversation mounted", () => { + useAgents.setState({ docked: "one" }); + setup(); + const session = screen.getByTestId("docked-session"); + fireEvent.click(screen.getByRole("button", { name: "Agent sessions" })); const tree = within(screen.getByRole("tree", { name: "Agent tasks" })); - expect(tree.queryByText("one")).toBeNull(); + expect(tree.getByText("one")).toBeTruthy(); expect(tree.getByText("two")).toBeTruthy(); + fireEvent.keyDown(document, { key: "Escape" }); + expect(screen.queryByRole("tree", { name: "Agent tasks" })).toBeNull(); + expect(screen.getByTestId("docked-session")).toBe(session); }); it("gives the whole tool window to the conversation when the docked task is the only one", () => { @@ -185,11 +209,15 @@ describe("Agents docked view", () => { expect(screen.getByTestId("docked-session")).toBeTruthy(); }); - it("undocks from the docked conversation's bar", () => { - useAgents.setState({ docked: "one" }); + it("switches placement from the global Agents header", () => { + useAgents.setState({ docked: "one", sessionLocation: "toolWindow" }); setup(); - fireEvent.click(screen.getByRole("button", { name: "Open in Editor" })); - expect(undockTask).toHaveBeenCalledTimes(1); + expect(screen.queryByRole("button", { name: "Open in Editor" })).toBeNull(); + expect(screen.getByRole("button", { name: "Chat in Tool Window" })).toHaveAttribute("aria-pressed", "true"); + fireEvent.click(screen.getByRole("button", { name: "Chat in Editor" })); + expect(setSessionLocation).toHaveBeenCalledWith("editor"); + fireEvent.click(screen.getByRole("button", { name: "Chat in Tool Window" })); + expect(setSessionLocation).toHaveBeenCalledWith("toolWindow"); }); it("opens tabs as usual without a docked conversation", () => { @@ -282,21 +310,12 @@ describe("Agents bulk actions", () => { }); describe("Agents task menu", () => { - it("offers to dock a task into the tool window", async () => { - setup(); - context("one"); - fireEvent.click(screen.getByRole("menuitem", { name: "Open in Tool Window" })); - await waitFor(() => expect(dockTask).toHaveBeenCalledWith("one")); - }); - - it("offers to move the docked task back to an editor", async () => { - useAgents.setState({ docked: "one" }); - setup(); - // The docked task is not listed; its menu lives on the docked conversation's header. + it.each([null, "one"])("keeps placement out of session menus (docked: %s)", (docked) => { + useAgents.setState({ docked }); const menu = taskMenu([useAgents.getState().tasks.get("one")!]); - expect(menu).toContainEqual(expect.objectContaining({ action: "agents.undockTask", label: "Open in Editor" })); - await taskActions.find((action) => action.id === "agents.undockTask")!.run({ ctx: {}, arg: "one" }); - await waitFor(() => expect(undockTask).toHaveBeenCalledTimes(1)); + expect(menu).not.toContainEqual(expect.objectContaining({ action: "agents.dockTask" })); + expect(menu).not.toContainEqual(expect.objectContaining({ action: "agents.undockTask" })); + expect(menu).toContainEqual(expect.objectContaining({ action: "agents.openTask" })); }); }); diff --git a/src/features/agents/AgentsView.tsx b/src/features/agents/AgentsView.tsx index 7dafe617a..30651b9d6 100644 --- a/src/features/agents/AgentsView.tsx +++ b/src/features/agents/AgentsView.tsx @@ -7,6 +7,7 @@ import { EmptyState, Icon, IconButton, + Popup, Spinner, T, Tree, @@ -18,16 +19,29 @@ import { SESSION_KIND, TASK_DRAG, age, branchLabel, groupTasks, sessionKey, type import { openHome, useAgents } from "./store"; import { taskMenu } from "./taskActions"; import { SessionEditor } from "./SessionEditor"; +import { HomeEditor } from "./HomeEditor"; import styles from "./Agents.module.css"; import { basename, useProject } from "@/platform/project"; import { useTx } from "@/platform/i18n"; import { executeAction } from "@/platform/actions"; export function AgentsHeaderActions() { + return ; +} + +export function AgentsLocationActions() { + const location = useAgents((s) => s.sessionLocation); + const tx = useTx(); return ( - <> - - +
+ + +
); } @@ -77,6 +91,10 @@ export function AgentsView() { const unread = useAgents((s) => s.unread); const providers = useAgents((s) => s.providers); const docked = useAgents((s) => s.docked); + const homeOpen = useAgents((s) => s.homeOpen); + const inline = docked !== null || homeOpen; + const historyRef = useRef(null); + const [historyOpen, setHistoryOpen] = useState(false); const [collapsed, setCollapsed] = useState>(() => new Set(["g:archived"])); const [selected, setSelected] = useState>(() => { const task = sessionTask(maybeEditorService()?.activeInput?.()); @@ -95,11 +113,7 @@ export function AgentsView() { }), [], ); - // The docked conversation is already shown above; listing it again is noise. - const roots = useMemo( - () => groupTasks([...tasks.values()].filter((t) => t.id !== docked)).map((group) => ({ kind: "group", group })), - [tasks, docked], - ); + const roots = useMemo(() => groupTasks(tasks.values()).map((group) => ({ kind: "group", group })), [tasks]); const expanded = useMemo(() => new Set(roots.map(nodeKey).filter((k) => !collapsed.has(k))), [roots, collapsed]); // Dropping a session editor tab anywhere on the tool window docks it. const viewRef = useRef(null); @@ -126,123 +140,156 @@ export function AgentsView() { ); if (error) return {error}; const providerName = (id: string) => providers.find((p) => p.id === id)?.name ?? id; - // While a conversation is docked, clicking a task switches it instead of opening a tab. - const showTask = (task: string) => void executeAction(docked ? "agents.dockTask" : "agents.openTask", task); + // Opening any session follows the global placement preference. + const showTask = (task: string) => { + setHistoryOpen(false); + void executeAction("agents.openTask", task); + }; const dockedTitle = docked ? (tasks.get(docked)?.title ?? docked) : null; const dragTask = (e: ReactPointerEvent, task: string) => startPointerDrag(e, e.currentTarget, { type: TASK_DRAG, payload: task }); + const taskList = ( +
+ + aria-label="Agent tasks" + roots={roots} + getKey={nodeKey} + getLabel={(n) => (n.kind === "group" ? n.group.label : n.task.title)} + getChildren={(n) => + n.kind === "group" ? n.group.tasks.map((task) => ({ kind: "task", task }) as Node) : undefined + } + isExpandable={(n) => n.kind === "group"} + expanded={expanded} + onExpandedChange={(_next, node, open) => + setCollapsed((c) => { + const s = new Set(c); + if (open) s.delete(nodeKey(node)); + else s.add(nodeKey(node)); + return s; + }) + } + selected={selected} + onSelectionChange={(sel, lead) => { + setSelected(sel); + if (!inline && sel.size === 1 && lead?.kind === "task" && sel.has(lead.task.id)) showTask(lead.task.id); + }} + onActivate={(n) => n.kind === "task" && showTask(n.task.id)} + onContextMenu={(n, sel, e) => { + if (n?.kind !== "task") return; + const selection = [...sel].flatMap((id) => { + const task = tasks.get(id); + return task ? [task] : []; + }); + showContextMenu(e, taskMenu(selection)); + }} + renderNode={(n) => + n.kind === "group" ? ( + + + {n.group.label} + + {n.group.tasks.length} + + ) : ( + dragTask(e, n.task.id)} + onClick={(e) => { + if (inline && !e.metaKey && !e.ctrlKey && !e.shiftKey) showTask(n.task.id); + }} + > + + + {n.task.title} + + {unread.has(n.task.id) && ( + + )} + {project?.workspace && ( + + {basename(n.task.projectRoot ?? project.root)} + + )} + + {providerName(n.task.provider)} + {n.task.isolation === "worktree" ? ` · ${branchLabel(n.task.branch)}` : ""} + + {age(n.task.updatedMs ?? 0)} + + ) + } + empty={ +
+ No agent tasks yet + +
+ } + /> +
+ ); + return (
- {docked && ( + {inline && (
+ { + setSelected(new Set(docked ? [docked] : [])); + setHistoryOpen((open) => !open); + }} + /> dragTask(e, docked)} + onPointerDown={(e) => docked && dragTask(e, docked)} > - {dockedTitle} + {homeOpen ? New Agent Task : dockedTitle} -
- + {homeOpen ? ( + + ) : ( + docked && ( + + ) + )}
)} - {/* Docked with nothing else to list: give the whole window to the conversation. */} - {(roots.length > 0 || !docked) && ( -
- - aria-label="Agent tasks" - roots={roots} - getKey={nodeKey} - getLabel={(n) => (n.kind === "group" ? n.group.label : n.task.title)} - getChildren={(n) => - n.kind === "group" ? n.group.tasks.map((task) => ({ kind: "task", task }) as Node) : undefined - } - isExpandable={(n) => n.kind === "group"} - expanded={expanded} - onExpandedChange={(_next, node, open) => - setCollapsed((c) => { - const s = new Set(c); - if (open) s.delete(nodeKey(node)); - else s.add(nodeKey(node)); - return s; - }) - } - selected={selected} - onSelectionChange={(sel, lead) => { - setSelected(sel); - if (sel.size === 1 && lead?.kind === "task" && sel.has(lead.task.id)) showTask(lead.task.id); - }} - onActivate={(n) => n.kind === "task" && showTask(n.task.id)} - onContextMenu={(n, sel, e) => { - if (n?.kind !== "task") return; - const selection = [...sel].flatMap((id) => { - const task = tasks.get(id); - return task ? [task] : []; - }); - showContextMenu(e, taskMenu(selection)); - }} - renderNode={(n) => - n.kind === "group" ? ( - - - {n.group.label} - - {n.group.tasks.length} - - ) : ( - dragTask(e, n.task.id)} - > - - - {n.task.title} - - {unread.has(n.task.id) && ( - - )} - {project?.workspace && ( - - {basename(n.task.projectRoot ?? project.root)} - - )} - - {providerName(n.task.provider)} - {n.task.isolation === "worktree" ? ` · ${branchLabel(n.task.branch)}` : ""} - - {age(n.task.updatedMs ?? 0)} - - ) - } - empty={ -
- No agent tasks yet - -
- } - /> -
+ {!inline && taskList} + {inline && ( + setHistoryOpen(false)} + aria-label={tx("Agent sessions")} + className={styles.history} + > +
+ Agent sessions +
+ {taskList} + )}
diff --git a/src/features/agents/Composer.module.css b/src/features/agents/Composer.module.css index f803502f0..e6f095e35 100644 --- a/src/features/agents/Composer.module.css +++ b/src/features/agents/Composer.module.css @@ -1,5 +1,7 @@ /* A framed text area (IDEA text field frame), with the pickers in its bottom row. */ .composer { + container: agent-composer / inline-size; + min-width: 0; position: relative; display: flex; flex-direction: column; @@ -44,9 +46,85 @@ padding: 4px 6px 6px; cursor: default; } +.toolbar { + display: flex; + flex: 1; + flex-wrap: wrap; + align-items: center; + min-width: 0; + gap: 2px; +} +.actions { + display: flex; + flex: none; + align-items: center; + gap: 2px; + margin-left: auto; +} .spacer { flex: 1; } +.configPicker { + display: flex; + min-width: 0; +} +.configPicker > button { + max-width: 100%; +} +.settingsMenu { + display: none; +} +.settingsPopup { + width: min(320px, calc(100vw - 24px)); + padding: 12px; +} +.settingsTitle { + margin-bottom: 12px; + font-weight: 600; +} +.settingsRow { + display: grid; + grid-template-columns: minmax(0, 80px) minmax(0, 1fr); + align-items: center; + gap: 12px; + margin-top: 8px; +} +.settingsLabel { + min-width: 0; + color: var(--fg-muted); + overflow-wrap: anywhere; +} +@container agent-composer (max-width: 600px) { + .bar { + flex-direction: column; + align-items: stretch; + gap: 6px; + } + .toolbar { + flex-wrap: nowrap; + } + .toolbar > button, + .toolbar > .configPicker { + flex: 0 1 auto; + min-width: 0; + max-width: calc((100% - 28px) / 2); + } + .configPicker > button { + width: 100%; + } + .inlineSetting { + display: none; + } + .settingsMenu { + display: flex; + flex: none; + margin-left: auto; + } + .actions { + width: 100%; + margin-left: 0; + } +} .pickerValue { display: inline-flex; align-items: baseline; diff --git a/src/features/agents/Composer.tsx b/src/features/agents/Composer.tsx index 21535d108..15ca6b5cb 100644 --- a/src/features/agents/Composer.tsx +++ b/src/features/agents/Composer.tsx @@ -403,30 +403,37 @@ export function Composer(props: ComposerProps) { }} />
e.stopPropagation()}> - {props.toolbar} - - { - const glue = text && !/\s$/.test(text) ? " " : ""; - setText(`${text}${glue}@`, text.length + glue.length + 1); - }} - /> - imageInput.current?.click()} /> - { - readImageFiles(e.target.files ?? []); - e.target.value = ""; - }} - /> - {props.actions} +
{props.toolbar}
+
+ { + const glue = text && !/\s$/.test(text) ? " " : ""; + setText(`${text}${glue}@`, text.length + glue.length + 1); + }} + /> + imageInput.current?.click()} + /> + { + readImageFiles(e.target.files ?? []); + e.target.value = ""; + }} + /> + + {props.actions} +
); diff --git a/src/features/agents/ConfigPickers.test.tsx b/src/features/agents/ConfigPickers.test.tsx index 2c7c737de..e0c0a828c 100644 --- a/src/features/agents/ConfigPickers.test.tsx +++ b/src/features/agents/ConfigPickers.test.tsx @@ -1,4 +1,4 @@ -import { fireEvent, render, screen } from "@testing-library/react"; +import { fireEvent, render, screen, within } from "@testing-library/react"; import { describe, expect, it, vi } from "vitest"; import type { AgentConfigOption } from "@/bindings"; import { ConfigPickers, configPickerOptions } from "./ConfigPickers"; @@ -85,3 +85,28 @@ it("sends the complete nested model id through ConfigPickers", () => { fireEvent.click(screen.getByRole("option", { name: "Beamcmc/Beam" })); expect(onChange).toHaveBeenCalledExactlyOnceWith("model", "9router/cmc/Beam"); }); + +it("edits secondary settings from the compact popup and restores focus when dismissed", () => { + const approval: AgentConfigOption = { + id: "approvals", + name: "Approvals", + category: "approval", + description: null, + current: "ask", + via: "option", + values: [value("ask", "Ask first"), value("bypass", "Bypass approvals")], + }; + const onChange = vi.fn(); + render(); + const trigger = screen.getByRole("button", { name: "Agent settings" }); + trigger.focus(); + fireEvent.click(trigger); + const dialog = screen.getByRole("dialog", { name: "Agent settings" }); + fireEvent.click(within(dialog).getByRole("button", { name: "Approvals" })); + fireEvent.click(screen.getByRole("option", { name: "Ask first" })); + expect(onChange).toHaveBeenCalledExactlyOnceWith("approvals", "ask"); + expect(dialog).toBeInTheDocument(); + fireEvent.keyDown(document, { key: "Escape" }); + expect(screen.queryByRole("dialog", { name: "Agent settings" })).not.toBeInTheDocument(); + expect(trigger).toHaveFocus(); +}); diff --git a/src/features/agents/ConfigPickers.tsx b/src/features/agents/ConfigPickers.tsx index 08a0e8af8..dbb6f2331 100644 --- a/src/features/agents/ConfigPickers.tsx +++ b/src/features/agents/ConfigPickers.tsx @@ -2,9 +2,11 @@ * Model, mode, approvals and thinking-effort dropdowns for a composer's bottom row. * Modes and approvals use icons; levels are plain text. */ +import { useRef, useState } from "react"; +import clsx from "clsx"; import type { AgentConfigOption } from "@/bindings"; import { useTx } from "@/platform/i18n"; -import { Select } from "@/ui"; +import { IconButton, Popup, Select, T } from "@/ui"; import type { SelectOption } from "@/ui/Select"; import { mainOptions, modeIcon } from "./composerModel"; import styles from "./Composer.module.css"; @@ -43,47 +45,100 @@ export function ConfigPickers({ onChange: (option: string, value: string) => void; }) { const tx = useTx(); + const [open, setOpen] = useState(false); + const anchor = useRef(null); + const options = mainOptions(config); + const secondary = options.filter((o) => o.category !== "model"); + const approval = secondary.find((o) => o.category === "approval" || o.category === "mode"); + const summary = secondary + .map((o) => { + const value = values?.[o.id] ?? o.current; + return `${tx(o.name)}: ${o.values.find((v) => v.value === value)?.name ?? tx("Default")}`; + }) + .join(" · "); return ( <> - {mainOptions(config).map((o) => { - const isMode = o.category === "mode"; - const value = values?.[o.id] ?? o.current ?? null; - const options = configPickerOptions(o); - - return ( - ({ + ...opt, + icon: + option.category === "approval" + ? opt.value === "bypass" + ? "unlock" + : "lock" + : option.category === "mode" + ? modeIcon(opt.value) + : undefined, + }))} + searchable={option.category === "model"} + onChange={(v) => onChange(option.id, v)} + renderValue={ + option.category === "thought_level" && !framed + ? (current) => ( + + {option.name} + {current?.label ?? tx("Default")} + + ) + : undefined + } + /> + ); +} diff --git a/src/features/agents/Home.module.css b/src/features/agents/Home.module.css index f86221152..67cad0ade 100644 --- a/src/features/agents/Home.module.css +++ b/src/features/agents/Home.module.css @@ -60,3 +60,47 @@ .warning { color: var(--danger); } + +.docked { + justify-content: flex-end; + padding: 16px 12px 12px; +} +.docked .inner { + gap: 8px; +} +.docked .where { + order: -1; + flex-wrap: nowrap; + min-width: 0; + gap: 6px; +} +.docked .project, +.docked .projectPicker { + flex: 0 1 auto; + min-width: 0; + max-width: 30%; + margin: 0; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} +.docked .branch, +.docked .basePicker { + order: 1; + flex: 1 1 0; + min-width: 0; + padding: 0; +} +.docked .isolation { + order: 2; + flex: 0 1 auto; + min-width: 0; + max-width: 45%; + margin-left: auto; +} +.branchName { + min-width: 0; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} diff --git a/src/features/agents/HomeEditor.tsx b/src/features/agents/HomeEditor.tsx index d7dfa29e9..3ff76b005 100644 --- a/src/features/agents/HomeEditor.tsx +++ b/src/features/agents/HomeEditor.tsx @@ -1,4 +1,5 @@ /** New Task page: one prompt box with the agent and its settings; where it works under it. */ +import clsx from "clsx"; import { useEffect, useState } from "react"; import type { AgentProbe, Branch } from "@/bindings"; import { executeAction } from "@/platform/actions"; @@ -7,7 +8,7 @@ import { useTx, useTxTemplate } from "@/platform/i18n"; import { basename, useProject } from "@/platform/project"; import { Icon, Select, T, type SelectOption } from "@/ui"; import { defaultTaskProject } from "./project"; -import { maybeEditorService } from "@/workbench/contributions"; +import { maybeEditorService, type EditorInput } from "@/workbench/contributions"; import { Composer } from "./Composer"; import { SubmitButton } from "./SubmitButton"; import { canSend, emptyDraft } from "./composerModel"; @@ -17,7 +18,7 @@ import { ADD_AGENT, providerOptions as agentOptions } from "./providerOptions"; import { HOME, NO_COMMANDS, refreshProviders, startTask, updateDraft, useAgentCommands, useAgents } from "./store"; import styles from "./Home.module.css"; -export function HomeEditor() { +export function HomeEditor({ docked = false }: { docked?: boolean; input?: EditorInput }) { const tx = useTx(); const txTemplate = useTxTemplate(); const draft = useAgents((s) => s.draft); @@ -86,15 +87,19 @@ export function HomeEditor() { const setOption = (id: string, value: string) => updateDraft({ config: { ...draft.config, [id]: value } }); return ( -
+
-

- What should the agent do? -

+ {!docked && ( +

+ What should the agent do? +

+ )} ({ value: path, label: basename(path), detail: path }))} onChange={(path) => { @@ -139,12 +145,15 @@ export function HomeEditor() { }} /> ) : ( - {members.length ? basename(root) : project?.name} + + {members.length ? basename(root) : project?.name} + )} {isGit && ( ({ value: b.name, label: b.name }))} onChange={(b) => updateDraft({ base: b === current ? null : b })} @@ -184,7 +198,7 @@ export function HomeEditor() { )}
- {probe && ( + {probe && (!docked || probe.error) && (

{probe.error ? txTemplate(`${provider?.name ?? probe.provider} didn't start: ${probe.error}`) diff --git a/src/features/agents/Session.module.css b/src/features/agents/Session.module.css index 5b104f185..a28570ef3 100644 --- a/src/features/agents/Session.module.css +++ b/src/features/agents/Session.module.css @@ -16,6 +16,20 @@ border-right: 1px solid var(--border); } +.conversation[hidden] { + display: none; +} +.docked { + grid-template-columns: minmax(0, 1fr); + overflow: hidden; +} +.docked .conversation { + border-right: 0; +} +.docked .changes { + width: auto; +} + /* The conversation hugs its content; spare space stays below the composer instead of inside the chat. */ .streamArea, @@ -568,6 +582,7 @@ width: 280px; } .changesHeader { + min-width: 0; display: flex; align-items: center; gap: 6px; @@ -577,11 +592,17 @@ border-bottom: 1px solid var(--border); } .changesTitle { + min-width: 0; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; font-weight: 600; color: var(--fg-strong); } .changesSummary { flex: none; + overflow: hidden; + text-overflow: ellipsis; padding: 4px 10px; color: var(--fg-subtle); white-space: pre; @@ -608,8 +629,11 @@ gap: 4px; } .changeName { - flex: none; + flex: 0 1 auto; + min-width: 0; white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; } .changeDir { flex: 1 1 0; diff --git a/src/features/agents/SessionEditor.tsx b/src/features/agents/SessionEditor.tsx index 00d2ae510..deddc4c4c 100644 --- a/src/features/agents/SessionEditor.tsx +++ b/src/features/agents/SessionEditor.tsx @@ -68,15 +68,13 @@ import { setFocusedSession, focusedSession, type FocusedSession } from "./sessio import { taskMenu } from "./taskActions"; import styles from "./Session.module.css"; -export function SessionEditor({ input }: { input: EditorInput }) { +export function SessionEditor({ input, docked = false }: { input: EditorInput; docked?: boolean }) { const id = (input.data as { task: string }).task; - return ; + return ; } -function TaskSession({ id }: { id: string }) { +function TaskSession({ id, docked }: { id: string; docked: boolean }) { const task = useAgents((s) => s.tasks.get(id)); - // The tool window bar above already shows the title of a docked conversation. - const docked = useAgents((s) => s.docked === id); const entries = useAgents((s) => s.entries.get(id)); const changes = useAgents((s) => s.changes.get(id)); const project = useProject((s) => s.project); @@ -84,7 +82,7 @@ function TaskSession({ id }: { id: string }) { task?.isolation === "local" ? task.status : null, project?.workspace ? (task?.projectRoot ?? project.root) : null, ); - const [changesOpen, setChangesOpen] = useChangesOpen(id, changes ?? null); + const [changesOpen, setChangesOpen] = useChangesOpen(id, changes ?? null, docked); // The subagent conversation shown, with the subagents that lead to it (empty: the task's own). const [path, setPath] = useState([]); const view = path.at(-1) ?? null; @@ -96,8 +94,10 @@ function TaskSession({ id }: { id: string }) { useEffect(() => { void loadEntries(id).catch(() => undefined); void refreshChanges(id); - return markVisible(id); }, [id]); + useEffect(() => { + if (!docked || !changesOpen) return markVisible(id); + }, [id, docked, changesOpen]); useEffect(() => { if (view) void loadEntries(id, view).catch(() => undefined); }, [id, view]); @@ -127,7 +127,7 @@ function TaskSession({ id }: { id: string }) { } }} > -

+ ); @@ -270,6 +276,7 @@ function Header({ className={styles.changesToggle} aria-pressed={changesOpen} disabled={count === 0 && !changesOpen} + aria-label={changesOpen ? tx("Hide changes") : tx("Show changes")} title={changesOpen ? tx("Hide changes") : tx("Show changes")} onClick={onToggleChanges} > @@ -1292,16 +1299,22 @@ function loadChangesOpen(): Record { /** * Whether the Changes pane is shown for `task`: the user's choice once made, else - * only while the task has changes (so it opens when the agent first edits a file). + * only while the task has changes. Docked sessions stay in the conversation until + * the user explicitly opens changes; navigation there does not change editor preferences. */ -function useChangesOpen(task: string, changes: AgentChange[] | null): [boolean, (open: boolean) => void] { +function useChangesOpen( + task: string, + changes: AgentChange[] | null, + docked: boolean, +): [boolean, (open: boolean) => void] { const [state, setState] = useState<{ task: string; open: boolean | undefined }>(() => ({ task, - open: loadChangesOpen()[task], + open: docked ? false : loadChangesOpen()[task], })); - const chosen = state.task === task ? state.open : loadChangesOpen()[task]; + const chosen = state.task === task ? state.open : docked ? false : loadChangesOpen()[task]; const set = (open: boolean) => { setState({ task, open }); + if (docked) return; try { globalThis.localStorage?.setItem(CHANGES_OPEN_KEY, JSON.stringify({ ...loadChangesOpen(), [task]: open })); } catch { @@ -1315,11 +1328,13 @@ function ChangesPane({ task, changes, head, + docked, onHide, }: { task: AgentTask; changes: AgentChange[] | null; head: string | null; + docked: boolean; onHide: () => void; }) { const [selected, setSelected] = useState>(() => new Set()); @@ -1328,6 +1343,7 @@ function ChangesPane({ return (