From bb200053c9d136682ccbe7a05a0d1a6d75b916fe Mon Sep 17 00:00:00 2001 From: Ingrid Date: Fri, 22 Dec 2023 22:22:23 +0100 Subject: [PATCH 01/39] minimal implementation of shada currently only writes the header when closing --- Cargo.lock | 35 ++++++++++++++++ helix-loader/src/lib.rs | 24 +++++++++++ helix-term/Cargo.toml | 3 ++ helix-term/src/application.rs | 3 ++ helix-term/src/args.rs | 5 +++ helix-term/src/lib.rs | 1 + helix-term/src/main.rs | 2 + helix-term/src/shada.rs | 75 +++++++++++++++++++++++++++++++++++ helix-view/src/view.rs | 4 +- 9 files changed, 151 insertions(+), 1 deletion(-) create mode 100644 helix-term/src/shada.rs diff --git a/Cargo.lock b/Cargo.lock index 559e9eb8c444..3b145d23b1fa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -122,6 +122,12 @@ version = "3.16.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "79296716171880943b8470b5f8d03aa55eb2e645a4874bdbb28adb49162e012c" +[[package]] +name = "byteorder" +version = "1.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1fd0f2584146f6f2ef48085050886acf353beff7305ebd1ae69500e27c67f64b" + [[package]] name = "bytes" version = "1.7.1" @@ -1390,6 +1396,7 @@ dependencies = [ "open", "pulldown-cmark", "same-file", + "rmp-serde", "serde", "serde_json", "signal-hook", @@ -1948,6 +1955,12 @@ dependencies = [ "windows-targets 0.52.6", ] +[[package]] +name = "paste" +version = "1.0.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "de3145af08024dea9fa9914f381a17b8fc6034dfb00f3a84013f7ff43f29ed4c" + [[package]] name = "pathdiff" version = "0.2.1" @@ -2115,6 +2128,28 @@ version = "0.8.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2b15c43186be67a4fd63bee50d0303afffcef381492ebe2c5d87f324e1b8815c" +[[package]] +name = "rmp" +version = "0.8.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7f9860a6cc38ed1da53456442089b4dfa35e7cedaa326df63017af88385e6b20" +dependencies = [ + "byteorder", + "num-traits", + "paste", +] + +[[package]] +name = "rmp-serde" +version = "1.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bffea85eea980d8a74453e5d02a8d93028f3c34725de143085a844ebe953258a" +dependencies = [ + "byteorder", + "rmp", + "serde", +] + [[package]] name = "ropey" version = "1.6.1" diff --git a/helix-loader/src/lib.rs b/helix-loader/src/lib.rs index 0e7c134d013e..559b41533ba9 100644 --- a/helix-loader/src/lib.rs +++ b/helix-loader/src/lib.rs @@ -15,6 +15,8 @@ static CONFIG_FILE: once_cell::sync::OnceCell = once_cell::sync::OnceCe static LOG_FILE: once_cell::sync::OnceCell = once_cell::sync::OnceCell::new(); +static SHADA_FILE: once_cell::sync::OnceCell = once_cell::sync::OnceCell::new(); + pub fn initialize_config_file(specified_file: Option) { let config_file = specified_file.unwrap_or_else(default_config_file); ensure_parent_dir(&config_file); @@ -27,6 +29,12 @@ pub fn initialize_log_file(specified_file: Option) { LOG_FILE.set(log_file).ok(); } +pub fn initialize_shada_file(specified_file: Option) { + let shada_file = specified_file.unwrap_or_else(default_shada_file); + ensure_parent_dir(&shada_file); + SHADA_FILE.set(shada_file).ok(); +} + /// A list of runtime directories from highest to lowest priority /// /// The priority is: @@ -132,6 +140,14 @@ pub fn cache_dir() -> PathBuf { path } +pub fn state_dir() -> PathBuf { + // TODO: allow env var override + let strategy = choose_base_strategy().expect("Unable to find the state directory!"); + let mut path = strategy.state_dir().unwrap(); + path.push("helix"); + path +} + pub fn config_file() -> PathBuf { CONFIG_FILE.get().map(|path| path.to_path_buf()).unwrap() } @@ -140,6 +156,10 @@ pub fn log_file() -> PathBuf { LOG_FILE.get().map(|path| path.to_path_buf()).unwrap() } +pub fn shada_file() -> PathBuf { + SHADA_FILE.get().map(|path| path.to_path_buf()).unwrap() +} + pub fn workspace_config_file() -> PathBuf { find_workspace().0.join(".helix").join("config.toml") } @@ -152,6 +172,10 @@ pub fn default_log_file() -> PathBuf { cache_dir().join("helix.log") } +pub fn default_shada_file() -> PathBuf { + state_dir().join("helix.shada") +} + /// Merge two TOML documents, merging values from `right` onto `left` /// /// When an array exists in both `left` and `right`, `right`'s array is diff --git a/helix-term/Cargo.toml b/helix-term/Cargo.toml index 5b46a261c49b..9cfdcc8396e3 100644 --- a/helix-term/Cargo.toml +++ b/helix-term/Cargo.toml @@ -68,6 +68,9 @@ toml = "0.8" serde_json = "1.0" serde = { version = "1.0", features = ["derive"] } +# shada +rmp-serde = "1.1.2" + # ripgrep for global search grep-regex = "0.1.13" grep-searcher = "0.1.14" diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 36cb295cea4c..0a85119ea71a 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -27,6 +27,7 @@ use crate::{ handlers, job::Jobs, keymap::Keymaps, + shada, ui::{self, overlay::overlaid}, }; @@ -1268,6 +1269,8 @@ impl Application { )); } + shada::write_shada_file(); + errs } } diff --git a/helix-term/src/args.rs b/helix-term/src/args.rs index 853c1576881e..eb98c603c093 100644 --- a/helix-term/src/args.rs +++ b/helix-term/src/args.rs @@ -16,6 +16,7 @@ pub struct Args { pub verbosity: u64, pub log_file: Option, pub config_file: Option, + pub shada_file: Option, pub files: Vec<(PathBuf, Position)>, pub working_directory: Option, } @@ -61,6 +62,10 @@ impl Args { Some(path) => args.log_file = Some(path.into()), None => anyhow::bail!("--log must specify a path to write"), }, + "--shada" => match argv.next().as_deref() { + Some(path) => args.shada_file = Some(path.into()), + None => anyhow::bail!("--shada must specify a path to write"), + }, "-w" | "--working-dir" => match argv.next().as_deref() { Some(path) => { args.working_directory = if Path::new(path).is_dir() { diff --git a/helix-term/src/lib.rs b/helix-term/src/lib.rs index cf4fbd9fa7ae..fad07668f4ab 100644 --- a/helix-term/src/lib.rs +++ b/helix-term/src/lib.rs @@ -10,6 +10,7 @@ pub mod events; pub mod health; pub mod job; pub mod keymap; +pub mod shada; pub mod ui; use std::path::Path; diff --git a/helix-term/src/main.rs b/helix-term/src/main.rs index 385a04064d13..afe83b649edc 100644 --- a/helix-term/src/main.rs +++ b/helix-term/src/main.rs @@ -44,6 +44,7 @@ async fn main_impl() -> Result { helix_loader::initialize_config_file(args.config_file.clone()); helix_loader::initialize_log_file(args.log_file.clone()); + helix_loader::initialize_shada_file(args.shada_file.clone()); // Help has a higher priority and should be handled separately. if args.display_help { @@ -70,6 +71,7 @@ FLAGS: -v Increases logging verbosity each use for up to 3 times --log Specifies a file to use for logging (default file: {}) + --shada Specifies a file to use for shared data -V, --version Prints version information --vsplit Splits all given files vertically into different windows --hsplit Splits all given files horizontally into different windows diff --git a/helix-term/src/shada.rs b/helix-term/src/shada.rs new file mode 100644 index 000000000000..108ce036af07 --- /dev/null +++ b/helix-term/src/shada.rs @@ -0,0 +1,75 @@ +use helix_loader::{shada_file, VERSION_AND_GIT_HASH}; +use helix_view::view::ViewPosition; +use rmp_serde::Serializer; +use serde::{Deserialize, Serialize}; +use std::{ + fs::File, + time::{SystemTime, UNIX_EPOCH}, +}; + +// TODO: should this be non-exhaustive? +#[derive(Debug, Deserialize, Serialize)] +#[serde(rename = "H")] +struct Header { + generator: String, + version: String, + encoding: String, + max_kbyte: u32, + pid: u32, +} + +// TODO: should this be non-exhaustive? +#[derive(Debug, Deserialize, Serialize)] +struct FilePosition { + path: String, + position: ViewPosition, +} + +// TODO: should this be non-exhaustive? +#[derive(Debug, Deserialize, Serialize)] +enum EntryData { + Header(Header), + FilePosition(FilePosition), +} + +// TODO: should this be non-exhaustive? +#[derive(Debug, Deserialize, Serialize)] +struct Entry { + timestamp: u64, + data: EntryData, +} + +fn timestamp_now() -> u64 { + SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap_or_default() + .as_secs() +} + +fn generate_header() -> Entry { + Entry { + timestamp: timestamp_now(), + data: EntryData::Header(Header { + generator: "helix".to_string(), + version: VERSION_AND_GIT_HASH.to_string(), + // TODO: is this necessary? helix doesn't seem to expose an option + // for internal encoding like nvim does + encoding: "utf-8".to_string(), + max_kbyte: 100, + pid: std::process::id(), + }), + } +} + +pub fn write_shada_file() { + // TODO: merge existing file if exists + + // TODO: do something about this unwrap + let shada_file = File::create(shada_file()).unwrap(); + let mut serializer = Serializer::new(shada_file); + + let header = generate_header(); + + // TODO: do something about this unwrap + header.serialize(&mut serializer).unwrap(); +} diff --git a/helix-view/src/view.rs b/helix-view/src/view.rs index a229f01ea66a..9ebec0051256 100644 --- a/helix-view/src/view.rs +++ b/helix-view/src/view.rs @@ -18,6 +18,8 @@ use helix_core::{ VisualOffsetError::{PosAfterMaxRow, PosBeforeAnchorRow}, }; +use serde::{Deserialize, Serialize}; + use std::{ collections::{HashMap, VecDeque}, fmt, @@ -118,7 +120,7 @@ impl JumpList { } } -#[derive(Clone, Debug, PartialEq, Eq, Copy, Default)] +#[derive(Clone, Debug, PartialEq, Eq, Copy, Default, Serialize, Deserialize)] pub struct ViewPosition { pub anchor: usize, pub horizontal_offset: usize, From 4cad42cbc75425bc3ab9b53ec667774f308b6b2d Mon Sep 17 00:00:00 2001 From: Ingrid Date: Sat, 23 Dec 2023 16:56:38 +0100 Subject: [PATCH 02/39] disable writing shada in integration tests will probably need a sophisticated way of handling this eventually for testing shada behaviour, but for now this is fine --- helix-term/src/application.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 0a85119ea71a..5bd7ff0f67ec 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -1269,6 +1269,7 @@ impl Application { )); } + #[cfg(not(feature = "integration"))] shada::write_shada_file(); errs From 07706575c773aeec7d36d2dd74136640d18261bc Mon Sep 17 00:00:00 2001 From: Ingrid Date: Thu, 28 Dec 2023 15:59:00 +0100 Subject: [PATCH 03/39] switch from rmp-serde to bincode --- Cargo.lock | 61 ++++++++++++++++++----------------------- helix-term/Cargo.toml | 2 +- helix-term/src/shada.rs | 25 ++++++----------- helix-view/src/view.rs | 4 +-- 4 files changed, 37 insertions(+), 55 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3b145d23b1fa..12d080f61caa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -99,6 +99,25 @@ dependencies = [ "rustc-demangle", ] +[[package]] +name = "bincode" +version = "2.0.0-rc.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f11ea1a0346b94ef188834a65c068a03aec181c94896d481d7a0a40d85b0ce95" +dependencies = [ + "bincode_derive", + "serde", +] + +[[package]] +name = "bincode_derive" +version = "2.0.0-rc.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7e30759b3b99a1b802a7a3aa21c85c3ded5c28e1c83170d82d70f08bbf7f3e4c" +dependencies = [ + "virtue", +] + [[package]] name = "bitflags" version = "2.6.0" @@ -122,12 +141,6 @@ version = "3.16.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "79296716171880943b8470b5f8d03aa55eb2e645a4874bdbb28adb49162e012c" -[[package]] -name = "byteorder" -version = "1.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1fd0f2584146f6f2ef48085050886acf353beff7305ebd1ae69500e27c67f64b" - [[package]] name = "bytes" version = "1.7.1" @@ -1371,6 +1384,7 @@ version = "24.7.0" dependencies = [ "anyhow", "arc-swap", + "bincode", "chrono", "content_inspector", "crossterm", @@ -1396,7 +1410,6 @@ dependencies = [ "open", "pulldown-cmark", "same-file", - "rmp-serde", "serde", "serde_json", "signal-hook", @@ -1955,12 +1968,6 @@ dependencies = [ "windows-targets 0.52.6", ] -[[package]] -name = "paste" -version = "1.0.14" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "de3145af08024dea9fa9914f381a17b8fc6034dfb00f3a84013f7ff43f29ed4c" - [[package]] name = "pathdiff" version = "0.2.1" @@ -2128,28 +2135,6 @@ version = "0.8.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2b15c43186be67a4fd63bee50d0303afffcef381492ebe2c5d87f324e1b8815c" -[[package]] -name = "rmp" -version = "0.8.12" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7f9860a6cc38ed1da53456442089b4dfa35e7cedaa326df63017af88385e6b20" -dependencies = [ - "byteorder", - "num-traits", - "paste", -] - -[[package]] -name = "rmp-serde" -version = "1.1.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bffea85eea980d8a74453e5d02a8d93028f3c34725de143085a844ebe953258a" -dependencies = [ - "byteorder", - "rmp", - "serde", -] - [[package]] name = "ropey" version = "1.6.1" @@ -2678,6 +2663,12 @@ version = "0.9.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0b928f33d975fc6ad9f86c8f283853ad26bdd5b10b7f1542aa2fa15e2289105a" +[[package]] +name = "virtue" +version = "0.0.13" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9dcc60c0624df774c82a0ef104151231d37da4962957d691c011c852b2473314" + [[package]] name = "walkdir" version = "2.5.0" diff --git a/helix-term/Cargo.toml b/helix-term/Cargo.toml index 9cfdcc8396e3..c369bdeba254 100644 --- a/helix-term/Cargo.toml +++ b/helix-term/Cargo.toml @@ -69,7 +69,7 @@ serde_json = "1.0" serde = { version = "1.0", features = ["derive"] } # shada -rmp-serde = "1.1.2" +bincode = "2.0.0-rc.3" # ripgrep for global search grep-regex = "0.1.13" diff --git a/helix-term/src/shada.rs b/helix-term/src/shada.rs index 108ce036af07..9dc890e7effa 100644 --- a/helix-term/src/shada.rs +++ b/helix-term/src/shada.rs @@ -1,39 +1,36 @@ +use bincode::{encode_into_std_write, Decode, Encode}; use helix_loader::{shada_file, VERSION_AND_GIT_HASH}; -use helix_view::view::ViewPosition; -use rmp_serde::Serializer; -use serde::{Deserialize, Serialize}; +// use helix_view::view::ViewPosition; use std::{ fs::File, time::{SystemTime, UNIX_EPOCH}, }; // TODO: should this be non-exhaustive? -#[derive(Debug, Deserialize, Serialize)] -#[serde(rename = "H")] +#[derive(Debug, Encode, Decode)] struct Header { generator: String, version: String, - encoding: String, max_kbyte: u32, pid: u32, } // TODO: should this be non-exhaustive? -#[derive(Debug, Deserialize, Serialize)] +#[derive(Debug, Encode, Decode)] struct FilePosition { path: String, - position: ViewPosition, + // position: ViewPosition, } // TODO: should this be non-exhaustive? -#[derive(Debug, Deserialize, Serialize)] +#[derive(Debug, Encode, Decode)] enum EntryData { Header(Header), FilePosition(FilePosition), } // TODO: should this be non-exhaustive? -#[derive(Debug, Deserialize, Serialize)] +#[derive(Debug, Encode, Decode)] struct Entry { timestamp: u64, data: EntryData, @@ -52,9 +49,6 @@ fn generate_header() -> Entry { data: EntryData::Header(Header { generator: "helix".to_string(), version: VERSION_AND_GIT_HASH.to_string(), - // TODO: is this necessary? helix doesn't seem to expose an option - // for internal encoding like nvim does - encoding: "utf-8".to_string(), max_kbyte: 100, pid: std::process::id(), }), @@ -65,11 +59,10 @@ pub fn write_shada_file() { // TODO: merge existing file if exists // TODO: do something about this unwrap - let shada_file = File::create(shada_file()).unwrap(); - let mut serializer = Serializer::new(shada_file); + let mut shada_file = File::create(shada_file()).unwrap(); let header = generate_header(); // TODO: do something about this unwrap - header.serialize(&mut serializer).unwrap(); + encode_into_std_write(&header, &mut shada_file, bincode::config::standard()).unwrap(); } diff --git a/helix-view/src/view.rs b/helix-view/src/view.rs index 9ebec0051256..a229f01ea66a 100644 --- a/helix-view/src/view.rs +++ b/helix-view/src/view.rs @@ -18,8 +18,6 @@ use helix_core::{ VisualOffsetError::{PosAfterMaxRow, PosBeforeAnchorRow}, }; -use serde::{Deserialize, Serialize}; - use std::{ collections::{HashMap, VecDeque}, fmt, @@ -120,7 +118,7 @@ impl JumpList { } } -#[derive(Clone, Debug, PartialEq, Eq, Copy, Default, Serialize, Deserialize)] +#[derive(Clone, Debug, PartialEq, Eq, Copy, Default)] pub struct ViewPosition { pub anchor: usize, pub horizontal_offset: usize, From c3c13dcba1c43fbebf5fecb4c65d9a5c8a05826b Mon Sep 17 00:00:00 2001 From: Ingrid Date: Thu, 28 Dec 2023 17:33:08 +0100 Subject: [PATCH 04/39] rename shada to session --- helix-loader/src/lib.rs | 18 +++++++++--------- helix-term/Cargo.toml | 2 +- helix-term/src/application.rs | 4 ++-- helix-term/src/args.rs | 8 ++++---- helix-term/src/lib.rs | 2 +- helix-term/src/main.rs | 4 ++-- helix-term/src/{shada.rs => session.rs} | 8 ++++---- 7 files changed, 23 insertions(+), 23 deletions(-) rename helix-term/src/{shada.rs => session.rs} (84%) diff --git a/helix-loader/src/lib.rs b/helix-loader/src/lib.rs index 559b41533ba9..41d4a4ed53bc 100644 --- a/helix-loader/src/lib.rs +++ b/helix-loader/src/lib.rs @@ -15,7 +15,7 @@ static CONFIG_FILE: once_cell::sync::OnceCell = once_cell::sync::OnceCe static LOG_FILE: once_cell::sync::OnceCell = once_cell::sync::OnceCell::new(); -static SHADA_FILE: once_cell::sync::OnceCell = once_cell::sync::OnceCell::new(); +static SESSION_FILE: once_cell::sync::OnceCell = once_cell::sync::OnceCell::new(); pub fn initialize_config_file(specified_file: Option) { let config_file = specified_file.unwrap_or_else(default_config_file); @@ -29,10 +29,10 @@ pub fn initialize_log_file(specified_file: Option) { LOG_FILE.set(log_file).ok(); } -pub fn initialize_shada_file(specified_file: Option) { - let shada_file = specified_file.unwrap_or_else(default_shada_file); - ensure_parent_dir(&shada_file); - SHADA_FILE.set(shada_file).ok(); +pub fn initialize_session_file(specified_file: Option) { + let session_file = specified_file.unwrap_or_else(default_session_file); + ensure_parent_dir(&session_file); + SESSION_FILE.set(session_file).ok(); } /// A list of runtime directories from highest to lowest priority @@ -156,8 +156,8 @@ pub fn log_file() -> PathBuf { LOG_FILE.get().map(|path| path.to_path_buf()).unwrap() } -pub fn shada_file() -> PathBuf { - SHADA_FILE.get().map(|path| path.to_path_buf()).unwrap() +pub fn session_file() -> PathBuf { + SESSION_FILE.get().map(|path| path.to_path_buf()).unwrap() } pub fn workspace_config_file() -> PathBuf { @@ -172,8 +172,8 @@ pub fn default_log_file() -> PathBuf { cache_dir().join("helix.log") } -pub fn default_shada_file() -> PathBuf { - state_dir().join("helix.shada") +pub fn default_session_file() -> PathBuf { + state_dir().join("helix.session") } /// Merge two TOML documents, merging values from `right` onto `left` diff --git a/helix-term/Cargo.toml b/helix-term/Cargo.toml index c369bdeba254..7b167c1c0ee7 100644 --- a/helix-term/Cargo.toml +++ b/helix-term/Cargo.toml @@ -68,7 +68,7 @@ toml = "0.8" serde_json = "1.0" serde = { version = "1.0", features = ["derive"] } -# shada +# session persistence bincode = "2.0.0-rc.3" # ripgrep for global search diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 5bd7ff0f67ec..8b6564473449 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -27,7 +27,7 @@ use crate::{ handlers, job::Jobs, keymap::Keymaps, - shada, + session, ui::{self, overlay::overlaid}, }; @@ -1270,7 +1270,7 @@ impl Application { } #[cfg(not(feature = "integration"))] - shada::write_shada_file(); + session::write_session_file(); errs } diff --git a/helix-term/src/args.rs b/helix-term/src/args.rs index eb98c603c093..b3b97749e824 100644 --- a/helix-term/src/args.rs +++ b/helix-term/src/args.rs @@ -16,7 +16,7 @@ pub struct Args { pub verbosity: u64, pub log_file: Option, pub config_file: Option, - pub shada_file: Option, + pub session_file: Option, pub files: Vec<(PathBuf, Position)>, pub working_directory: Option, } @@ -62,9 +62,9 @@ impl Args { Some(path) => args.log_file = Some(path.into()), None => anyhow::bail!("--log must specify a path to write"), }, - "--shada" => match argv.next().as_deref() { - Some(path) => args.shada_file = Some(path.into()), - None => anyhow::bail!("--shada must specify a path to write"), + "--session-file" => match argv.next().as_deref() { + Some(path) => args.session_file = Some(path.into()), + None => anyhow::bail!("--session-file must specify a path to write"), }, "-w" | "--working-dir" => match argv.next().as_deref() { Some(path) => { diff --git a/helix-term/src/lib.rs b/helix-term/src/lib.rs index fad07668f4ab..f20544777a06 100644 --- a/helix-term/src/lib.rs +++ b/helix-term/src/lib.rs @@ -10,7 +10,7 @@ pub mod events; pub mod health; pub mod job; pub mod keymap; -pub mod shada; +pub mod session; pub mod ui; use std::path::Path; diff --git a/helix-term/src/main.rs b/helix-term/src/main.rs index afe83b649edc..7a26fb4ee9bc 100644 --- a/helix-term/src/main.rs +++ b/helix-term/src/main.rs @@ -44,7 +44,7 @@ async fn main_impl() -> Result { helix_loader::initialize_config_file(args.config_file.clone()); helix_loader::initialize_log_file(args.log_file.clone()); - helix_loader::initialize_shada_file(args.shada_file.clone()); + helix_loader::initialize_session_file(args.session_file.clone()); // Help has a higher priority and should be handled separately. if args.display_help { @@ -71,7 +71,7 @@ FLAGS: -v Increases logging verbosity each use for up to 3 times --log Specifies a file to use for logging (default file: {}) - --shada Specifies a file to use for shared data + --session-file Specifies a file to use for shared data -V, --version Prints version information --vsplit Splits all given files vertically into different windows --hsplit Splits all given files horizontally into different windows diff --git a/helix-term/src/shada.rs b/helix-term/src/session.rs similarity index 84% rename from helix-term/src/shada.rs rename to helix-term/src/session.rs index 9dc890e7effa..fc36a9a56f53 100644 --- a/helix-term/src/shada.rs +++ b/helix-term/src/session.rs @@ -1,5 +1,5 @@ use bincode::{encode_into_std_write, Decode, Encode}; -use helix_loader::{shada_file, VERSION_AND_GIT_HASH}; +use helix_loader::{session_file, VERSION_AND_GIT_HASH}; // use helix_view::view::ViewPosition; use std::{ fs::File, @@ -55,14 +55,14 @@ fn generate_header() -> Entry { } } -pub fn write_shada_file() { +pub fn write_session_file() { // TODO: merge existing file if exists // TODO: do something about this unwrap - let mut shada_file = File::create(shada_file()).unwrap(); + let mut session_file = File::create(session_file()).unwrap(); let header = generate_header(); // TODO: do something about this unwrap - encode_into_std_write(&header, &mut shada_file, bincode::config::standard()).unwrap(); + encode_into_std_write(&header, &mut session_file, bincode::config::standard()).unwrap(); } From 7aeb4e30bf8759d5c265c0f300a8498701cb89af Mon Sep 17 00:00:00 2001 From: Ingrid Date: Thu, 28 Dec 2023 21:13:04 +0100 Subject: [PATCH 05/39] switch to multi-file approach, implement persisting command history --- helix-loader/src/lib.rs | 21 +++++---- helix-term/src/application.rs | 4 -- helix-term/src/args.rs | 5 --- helix-term/src/main.rs | 3 +- helix-term/src/session.rs | 81 +++++++---------------------------- helix-term/src/ui/prompt.rs | 4 +- 6 files changed, 32 insertions(+), 86 deletions(-) diff --git a/helix-loader/src/lib.rs b/helix-loader/src/lib.rs index 41d4a4ed53bc..52263e7eccde 100644 --- a/helix-loader/src/lib.rs +++ b/helix-loader/src/lib.rs @@ -15,7 +15,7 @@ static CONFIG_FILE: once_cell::sync::OnceCell = once_cell::sync::OnceCe static LOG_FILE: once_cell::sync::OnceCell = once_cell::sync::OnceCell::new(); -static SESSION_FILE: once_cell::sync::OnceCell = once_cell::sync::OnceCell::new(); +static COMMAND_HISTFILE: once_cell::sync::OnceCell = once_cell::sync::OnceCell::new(); pub fn initialize_config_file(specified_file: Option) { let config_file = specified_file.unwrap_or_else(default_config_file); @@ -29,10 +29,10 @@ pub fn initialize_log_file(specified_file: Option) { LOG_FILE.set(log_file).ok(); } -pub fn initialize_session_file(specified_file: Option) { - let session_file = specified_file.unwrap_or_else(default_session_file); - ensure_parent_dir(&session_file); - SESSION_FILE.set(session_file).ok(); +pub fn initialize_command_histfile(specified_file: Option) { + let command_histfile = specified_file.unwrap_or_else(default_command_histfile); + ensure_parent_dir(&command_histfile); + COMMAND_HISTFILE.set(command_histfile).ok(); } /// A list of runtime directories from highest to lowest priority @@ -156,8 +156,11 @@ pub fn log_file() -> PathBuf { LOG_FILE.get().map(|path| path.to_path_buf()).unwrap() } -pub fn session_file() -> PathBuf { - SESSION_FILE.get().map(|path| path.to_path_buf()).unwrap() +pub fn command_histfile() -> PathBuf { + COMMAND_HISTFILE + .get() + .map(|path| path.to_path_buf()) + .unwrap() } pub fn workspace_config_file() -> PathBuf { @@ -172,8 +175,8 @@ pub fn default_log_file() -> PathBuf { cache_dir().join("helix.log") } -pub fn default_session_file() -> PathBuf { - state_dir().join("helix.session") +pub fn default_command_histfile() -> PathBuf { + state_dir().join("command_history") } /// Merge two TOML documents, merging values from `right` onto `left` diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 8b6564473449..36cb295cea4c 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -27,7 +27,6 @@ use crate::{ handlers, job::Jobs, keymap::Keymaps, - session, ui::{self, overlay::overlaid}, }; @@ -1269,9 +1268,6 @@ impl Application { )); } - #[cfg(not(feature = "integration"))] - session::write_session_file(); - errs } } diff --git a/helix-term/src/args.rs b/helix-term/src/args.rs index b3b97749e824..853c1576881e 100644 --- a/helix-term/src/args.rs +++ b/helix-term/src/args.rs @@ -16,7 +16,6 @@ pub struct Args { pub verbosity: u64, pub log_file: Option, pub config_file: Option, - pub session_file: Option, pub files: Vec<(PathBuf, Position)>, pub working_directory: Option, } @@ -62,10 +61,6 @@ impl Args { Some(path) => args.log_file = Some(path.into()), None => anyhow::bail!("--log must specify a path to write"), }, - "--session-file" => match argv.next().as_deref() { - Some(path) => args.session_file = Some(path.into()), - None => anyhow::bail!("--session-file must specify a path to write"), - }, "-w" | "--working-dir" => match argv.next().as_deref() { Some(path) => { args.working_directory = if Path::new(path).is_dir() { diff --git a/helix-term/src/main.rs b/helix-term/src/main.rs index 7a26fb4ee9bc..494dcb2faaaf 100644 --- a/helix-term/src/main.rs +++ b/helix-term/src/main.rs @@ -44,7 +44,7 @@ async fn main_impl() -> Result { helix_loader::initialize_config_file(args.config_file.clone()); helix_loader::initialize_log_file(args.log_file.clone()); - helix_loader::initialize_session_file(args.session_file.clone()); + helix_loader::initialize_command_histfile(None); // Help has a higher priority and should be handled separately. if args.display_help { @@ -71,7 +71,6 @@ FLAGS: -v Increases logging verbosity each use for up to 3 times --log Specifies a file to use for logging (default file: {}) - --session-file Specifies a file to use for shared data -V, --version Prints version information --vsplit Splits all given files vertically into different windows --hsplit Splits all given files horizontally into different windows diff --git a/helix-term/src/session.rs b/helix-term/src/session.rs index fc36a9a56f53..0b06c6266263 100644 --- a/helix-term/src/session.rs +++ b/helix-term/src/session.rs @@ -1,68 +1,19 @@ -use bincode::{encode_into_std_write, Decode, Encode}; -use helix_loader::{session_file, VERSION_AND_GIT_HASH}; -// use helix_view::view::ViewPosition; -use std::{ - fs::File, - time::{SystemTime, UNIX_EPOCH}, -}; - -// TODO: should this be non-exhaustive? -#[derive(Debug, Encode, Decode)] -struct Header { - generator: String, - version: String, - max_kbyte: u32, - pid: u32, -} - -// TODO: should this be non-exhaustive? -#[derive(Debug, Encode, Decode)] -struct FilePosition { - path: String, - // position: ViewPosition, -} - -// TODO: should this be non-exhaustive? -#[derive(Debug, Encode, Decode)] -enum EntryData { - Header(Header), - FilePosition(FilePosition), -} - -// TODO: should this be non-exhaustive? -#[derive(Debug, Encode, Decode)] -struct Entry { - timestamp: u64, - data: EntryData, -} - -fn timestamp_now() -> u64 { - SystemTime::now() - .duration_since(UNIX_EPOCH) - .unwrap_or_default() - .as_secs() -} - -fn generate_header() -> Entry { - Entry { - timestamp: timestamp_now(), - data: EntryData::Header(Header { - generator: "helix".to_string(), - version: VERSION_AND_GIT_HASH.to_string(), - max_kbyte: 100, - pid: std::process::id(), - }), - } -} - -pub fn write_session_file() { - // TODO: merge existing file if exists - - // TODO: do something about this unwrap - let mut session_file = File::create(session_file()).unwrap(); - - let header = generate_header(); +use helix_loader::command_histfile; +use std::{fs::OpenOptions, io::Write}; + +pub fn push_history(register: char, line: &str) { + let filepath = match register { + ':' => command_histfile(), + _ => return, + }; + + let mut file = OpenOptions::new() + .append(true) + .create(true) + .open(filepath) + // TODO: do something about this unwrap + .unwrap(); // TODO: do something about this unwrap - encode_into_std_write(&header, &mut session_file, bincode::config::standard()).unwrap(); + writeln!(file, "{}", line).unwrap(); } diff --git a/helix-term/src/ui/prompt.rs b/helix-term/src/ui/prompt.rs index f44020c584d9..9ffa10ccbb68 100644 --- a/helix-term/src/ui/prompt.rs +++ b/helix-term/src/ui/prompt.rs @@ -1,5 +1,5 @@ use crate::compositor::{Component, Compositor, Context, Event, EventResult}; -use crate::{alt, ctrl, key, shift, ui}; +use crate::{alt, ctrl, key, session, shift, ui}; use arc_swap::ArcSwap; use helix_core::syntax; use helix_view::document::Mode; @@ -613,6 +613,8 @@ impl Component for Prompt { { cx.editor.set_error(err.to_string()); } + #[cfg(not(feature = "integration"))] + session::push_history(register, &self.line); }; } From bdc7f141f25a4d4f3b72ec6bb75e1071a0539153 Mon Sep 17 00:00:00 2001 From: Ingrid Date: Fri, 29 Dec 2023 22:26:21 +0100 Subject: [PATCH 06/39] read command history from file --- helix-term/src/application.rs | 9 +++++++++ helix-term/src/session.rs | 18 +++++++++++++++++- helix-view/src/register.rs | 23 +++++++++++++++++++++++ 3 files changed, 49 insertions(+), 1 deletion(-) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 36cb295cea4c..63df614b3333 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -27,6 +27,7 @@ use crate::{ handlers, job::Jobs, keymap::Keymaps, + session, ui::{self, overlay::overlaid}, }; @@ -148,6 +149,14 @@ impl Application { handlers, ); + // TODO: do most of this in the background? + #[cfg(not(feature = "integration"))] + editor + .registers + .write_unreversed(':', session::read_command_history()) + // TODO: do something about this unwrap + .unwrap(); + let keys = Box::new(Map::new(Arc::clone(&config), |config: &Config| { &config.keys })); diff --git a/helix-term/src/session.rs b/helix-term/src/session.rs index 0b06c6266263..1019b6a9f39c 100644 --- a/helix-term/src/session.rs +++ b/helix-term/src/session.rs @@ -1,5 +1,9 @@ use helix_loader::command_histfile; -use std::{fs::OpenOptions, io::Write}; +use std::{ + fs::{File, OpenOptions}, + io::{self, BufRead, BufReader, Lines, Write}, + path::PathBuf, +}; pub fn push_history(register: char, line: &str) { let filepath = match register { @@ -17,3 +21,15 @@ pub fn push_history(register: char, line: &str) { // TODO: do something about this unwrap writeln!(file, "{}", line).unwrap(); } + +fn read_histfile(filepath: PathBuf) -> Lines> { + // TODO: do something about this unwrap + BufReader::new(File::open(filepath).unwrap()).lines() +} + +pub fn read_command_history() -> Vec { + read_histfile(command_histfile()) + .collect::>>() + // TODO: do something about this unwrap + .unwrap() +} diff --git a/helix-view/src/register.rs b/helix-view/src/register.rs index d286a85ccafe..6e9bec1a54f5 100644 --- a/helix-view/src/register.rs +++ b/helix-view/src/register.rs @@ -102,6 +102,29 @@ impl Registers { } } + pub fn write_unreversed(&mut self, name: char, values: Vec) -> Result<()> { + match name { + '_' => Ok(()), + '#' | '.' | '%' => Err(anyhow::anyhow!("Register {name} does not support writing")), + '*' | '+' => { + self.clipboard_provider.set_contents( + values.join(NATIVE_LINE_ENDING.as_str()), + match name { + '+' => ClipboardType::Clipboard, + '*' => ClipboardType::Selection, + _ => unreachable!(), + }, + )?; + self.inner.insert(name, values); + Ok(()) + } + _ => { + self.inner.insert(name, values); + Ok(()) + } + } + } + pub fn push(&mut self, name: char, mut value: String) -> Result<()> { match name { '_' => Ok(()), From 40997538012b112d9d7ac99e7503a6cd9226ab27 Mon Sep 17 00:00:00 2001 From: Ingrid Date: Sat, 30 Dec 2023 16:33:40 +0100 Subject: [PATCH 07/39] handle NotFound error when reading histfile before it has been created --- helix-term/src/session.rs | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/helix-term/src/session.rs b/helix-term/src/session.rs index 1019b6a9f39c..2a6ccce469f3 100644 --- a/helix-term/src/session.rs +++ b/helix-term/src/session.rs @@ -1,7 +1,7 @@ use helix_loader::command_histfile; use std::{ fs::{File, OpenOptions}, - io::{self, BufRead, BufReader, Lines, Write}, + io::{self, BufRead, BufReader, Write}, path::PathBuf, }; @@ -22,14 +22,23 @@ pub fn push_history(register: char, line: &str) { writeln!(file, "{}", line).unwrap(); } -fn read_histfile(filepath: PathBuf) -> Lines> { - // TODO: do something about this unwrap - BufReader::new(File::open(filepath).unwrap()).lines() +fn read_histfile(filepath: PathBuf) -> Vec { + match File::open(filepath) { + Ok(file) => { + BufReader::new(file) + .lines() + .collect::>>() + // TODO: do something about this unwrap + .unwrap() + } + Err(e) => match e.kind() { + io::ErrorKind::NotFound => Vec::new(), + // TODO: do something about this panic + _ => panic!(), + }, + } } pub fn read_command_history() -> Vec { read_histfile(command_histfile()) - .collect::>>() - // TODO: do something about this unwrap - .unwrap() } From 5993550b87f2a12ca049968e173e7fa80c92543a Mon Sep 17 00:00:00 2001 From: Ingrid Date: Sat, 30 Dec 2023 16:34:26 +0100 Subject: [PATCH 08/39] persist and load search history --- helix-loader/src/lib.rs | 19 +++++++++++++++++++ helix-term/src/application.rs | 6 ++++++ helix-term/src/main.rs | 1 + helix-term/src/session.rs | 7 ++++++- 4 files changed, 32 insertions(+), 1 deletion(-) diff --git a/helix-loader/src/lib.rs b/helix-loader/src/lib.rs index 52263e7eccde..fada9ffa70ef 100644 --- a/helix-loader/src/lib.rs +++ b/helix-loader/src/lib.rs @@ -17,6 +17,8 @@ static LOG_FILE: once_cell::sync::OnceCell = once_cell::sync::OnceCell: static COMMAND_HISTFILE: once_cell::sync::OnceCell = once_cell::sync::OnceCell::new(); +static SEARCH_HISTFILE: once_cell::sync::OnceCell = once_cell::sync::OnceCell::new(); + pub fn initialize_config_file(specified_file: Option) { let config_file = specified_file.unwrap_or_else(default_config_file); ensure_parent_dir(&config_file); @@ -35,6 +37,12 @@ pub fn initialize_command_histfile(specified_file: Option) { COMMAND_HISTFILE.set(command_histfile).ok(); } +pub fn initialize_search_histfile(specified_file: Option) { + let search_histfile = specified_file.unwrap_or_else(default_search_histfile); + ensure_parent_dir(&search_histfile); + SEARCH_HISTFILE.set(search_histfile).ok(); +} + /// A list of runtime directories from highest to lowest priority /// /// The priority is: @@ -163,6 +171,13 @@ pub fn command_histfile() -> PathBuf { .unwrap() } +pub fn search_histfile() -> PathBuf { + SEARCH_HISTFILE + .get() + .map(|path| path.to_path_buf()) + .unwrap() +} + pub fn workspace_config_file() -> PathBuf { find_workspace().0.join(".helix").join("config.toml") } @@ -179,6 +194,10 @@ pub fn default_command_histfile() -> PathBuf { state_dir().join("command_history") } +pub fn default_search_histfile() -> PathBuf { + state_dir().join("search_history") +} + /// Merge two TOML documents, merging values from `right` onto `left` /// /// When an array exists in both `left` and `right`, `right`'s array is diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 63df614b3333..f354b317ae20 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -156,6 +156,12 @@ impl Application { .write_unreversed(':', session::read_command_history()) // TODO: do something about this unwrap .unwrap(); + #[cfg(not(feature = "integration"))] + editor + .registers + .write_unreversed('/', session::read_search_history()) + // TODO: do something about this unwrap + .unwrap(); let keys = Box::new(Map::new(Arc::clone(&config), |config: &Config| { &config.keys diff --git a/helix-term/src/main.rs b/helix-term/src/main.rs index 494dcb2faaaf..123ec391a309 100644 --- a/helix-term/src/main.rs +++ b/helix-term/src/main.rs @@ -45,6 +45,7 @@ async fn main_impl() -> Result { helix_loader::initialize_config_file(args.config_file.clone()); helix_loader::initialize_log_file(args.log_file.clone()); helix_loader::initialize_command_histfile(None); + helix_loader::initialize_search_histfile(None); // Help has a higher priority and should be handled separately. if args.display_help { diff --git a/helix-term/src/session.rs b/helix-term/src/session.rs index 2a6ccce469f3..1c4174e789bb 100644 --- a/helix-term/src/session.rs +++ b/helix-term/src/session.rs @@ -1,4 +1,4 @@ -use helix_loader::command_histfile; +use helix_loader::{command_histfile, search_histfile}; use std::{ fs::{File, OpenOptions}, io::{self, BufRead, BufReader, Write}, @@ -8,6 +8,7 @@ use std::{ pub fn push_history(register: char, line: &str) { let filepath = match register { ':' => command_histfile(), + '/' => search_histfile(), _ => return, }; @@ -42,3 +43,7 @@ fn read_histfile(filepath: PathBuf) -> Vec { pub fn read_command_history() -> Vec { read_histfile(command_histfile()) } + +pub fn read_search_history() -> Vec { + read_histfile(search_histfile()) +} From 13ad38ad61a43c59ef01c465934a69f2e2c66c77 Mon Sep 17 00:00:00 2001 From: Ingrid Date: Sat, 30 Dec 2023 17:26:09 +0100 Subject: [PATCH 09/39] move session.rs from helix-term to helix-loader --- Cargo.lock | 26 --------------------- helix-loader/src/lib.rs | 1 + {helix-term => helix-loader}/src/session.rs | 2 +- helix-term/Cargo.toml | 3 --- helix-term/src/application.rs | 2 +- helix-term/src/lib.rs | 1 - helix-term/src/ui/prompt.rs | 3 ++- 7 files changed, 5 insertions(+), 33 deletions(-) rename {helix-term => helix-loader}/src/session.rs (95%) diff --git a/Cargo.lock b/Cargo.lock index 12d080f61caa..559e9eb8c444 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -99,25 +99,6 @@ dependencies = [ "rustc-demangle", ] -[[package]] -name = "bincode" -version = "2.0.0-rc.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f11ea1a0346b94ef188834a65c068a03aec181c94896d481d7a0a40d85b0ce95" -dependencies = [ - "bincode_derive", - "serde", -] - -[[package]] -name = "bincode_derive" -version = "2.0.0-rc.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7e30759b3b99a1b802a7a3aa21c85c3ded5c28e1c83170d82d70f08bbf7f3e4c" -dependencies = [ - "virtue", -] - [[package]] name = "bitflags" version = "2.6.0" @@ -1384,7 +1365,6 @@ version = "24.7.0" dependencies = [ "anyhow", "arc-swap", - "bincode", "chrono", "content_inspector", "crossterm", @@ -2663,12 +2643,6 @@ version = "0.9.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0b928f33d975fc6ad9f86c8f283853ad26bdd5b10b7f1542aa2fa15e2289105a" -[[package]] -name = "virtue" -version = "0.0.13" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9dcc60c0624df774c82a0ef104151231d37da4962957d691c011c852b2473314" - [[package]] name = "walkdir" version = "2.5.0" diff --git a/helix-loader/src/lib.rs b/helix-loader/src/lib.rs index fada9ffa70ef..233de8d4a405 100644 --- a/helix-loader/src/lib.rs +++ b/helix-loader/src/lib.rs @@ -1,5 +1,6 @@ pub mod config; pub mod grammar; +pub mod session; use helix_stdx::{env::current_working_dir, path}; diff --git a/helix-term/src/session.rs b/helix-loader/src/session.rs similarity index 95% rename from helix-term/src/session.rs rename to helix-loader/src/session.rs index 1c4174e789bb..63eb7ac8e40f 100644 --- a/helix-term/src/session.rs +++ b/helix-loader/src/session.rs @@ -1,4 +1,4 @@ -use helix_loader::{command_histfile, search_histfile}; +use crate::{command_histfile, search_histfile}; use std::{ fs::{File, OpenOptions}, io::{self, BufRead, BufReader, Write}, diff --git a/helix-term/Cargo.toml b/helix-term/Cargo.toml index 7b167c1c0ee7..5b46a261c49b 100644 --- a/helix-term/Cargo.toml +++ b/helix-term/Cargo.toml @@ -68,9 +68,6 @@ toml = "0.8" serde_json = "1.0" serde = { version = "1.0", features = ["derive"] } -# session persistence -bincode = "2.0.0-rc.3" - # ripgrep for global search grep-regex = "0.1.13" grep-searcher = "0.1.14" diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index f354b317ae20..3bddb728dd5c 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -1,6 +1,7 @@ use arc_swap::{access::Map, ArcSwap}; use futures_util::Stream; use helix_core::{diagnostic::Severity, pos_at_coords, syntax, Selection}; +use helix_loader::session; use helix_lsp::{ lsp::{self, notification::Notification}, util::lsp_range_to_range, @@ -27,7 +28,6 @@ use crate::{ handlers, job::Jobs, keymap::Keymaps, - session, ui::{self, overlay::overlaid}, }; diff --git a/helix-term/src/lib.rs b/helix-term/src/lib.rs index f20544777a06..cf4fbd9fa7ae 100644 --- a/helix-term/src/lib.rs +++ b/helix-term/src/lib.rs @@ -10,7 +10,6 @@ pub mod events; pub mod health; pub mod job; pub mod keymap; -pub mod session; pub mod ui; use std::path::Path; diff --git a/helix-term/src/ui/prompt.rs b/helix-term/src/ui/prompt.rs index 9ffa10ccbb68..ad840fe42f46 100644 --- a/helix-term/src/ui/prompt.rs +++ b/helix-term/src/ui/prompt.rs @@ -1,7 +1,8 @@ use crate::compositor::{Component, Compositor, Context, Event, EventResult}; -use crate::{alt, ctrl, key, session, shift, ui}; +use crate::{alt, ctrl, key, shift, ui}; use arc_swap::ArcSwap; use helix_core::syntax; +use helix_loader::session; use helix_view::document::Mode; use helix_view::input::KeyEvent; use helix_view::keyboard::KeyCode; From fe9c51439ce8d9d83a74e2043c03e88aa470be9b Mon Sep 17 00:00:00 2001 From: Ingrid Date: Tue, 2 Jan 2024 18:24:16 +0100 Subject: [PATCH 10/39] persist file history --- Cargo.lock | 10 +++++++++ helix-loader/Cargo.toml | 1 + helix-loader/src/lib.rs | 16 ++++++++++++++ helix-loader/src/session.rs | 43 ++++++++++++++++++++++++++++++++++++- helix-term/src/main.rs | 1 + helix-view/src/editor.rs | 13 +++++++++++ 6 files changed, 83 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 559e9eb8c444..3252681451ac 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -99,6 +99,15 @@ dependencies = [ "rustc-demangle", ] +[[package]] +name = "bincode" +version = "1.3.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b1f45e9417d87227c7a56d22e471c6206462cba514c7590c09aff4cf6d1ddcad" +dependencies = [ + "serde", +] + [[package]] name = "bitflags" version = "2.6.0" @@ -1289,6 +1298,7 @@ name = "helix-loader" version = "24.7.0" dependencies = [ "anyhow", + "bincode", "cc", "dunce", "etcetera", diff --git a/helix-loader/Cargo.toml b/helix-loader/Cargo.toml index b87a9184aa31..742e7ab57b27 100644 --- a/helix-loader/Cargo.toml +++ b/helix-loader/Cargo.toml @@ -24,6 +24,7 @@ etcetera = "0.8" tree-sitter.workspace = true once_cell = "1.20" log = "0.4" +bincode = "1.3.3" # TODO: these two should be on !wasm32 only diff --git a/helix-loader/src/lib.rs b/helix-loader/src/lib.rs index 233de8d4a405..902d18c78d73 100644 --- a/helix-loader/src/lib.rs +++ b/helix-loader/src/lib.rs @@ -20,6 +20,8 @@ static COMMAND_HISTFILE: once_cell::sync::OnceCell = once_cell::sync::O static SEARCH_HISTFILE: once_cell::sync::OnceCell = once_cell::sync::OnceCell::new(); +static FILE_HISTFILE: once_cell::sync::OnceCell = once_cell::sync::OnceCell::new(); + pub fn initialize_config_file(specified_file: Option) { let config_file = specified_file.unwrap_or_else(default_config_file); ensure_parent_dir(&config_file); @@ -44,6 +46,12 @@ pub fn initialize_search_histfile(specified_file: Option) { SEARCH_HISTFILE.set(search_histfile).ok(); } +pub fn initialize_file_histfile(specified_file: Option) { + let file_histfile = specified_file.unwrap_or_else(default_file_histfile); + ensure_parent_dir(&file_histfile); + FILE_HISTFILE.set(file_histfile).ok(); +} + /// A list of runtime directories from highest to lowest priority /// /// The priority is: @@ -179,6 +187,10 @@ pub fn search_histfile() -> PathBuf { .unwrap() } +pub fn file_histfile() -> PathBuf { + FILE_HISTFILE.get().map(|path| path.to_path_buf()).unwrap() +} + pub fn workspace_config_file() -> PathBuf { find_workspace().0.join(".helix").join("config.toml") } @@ -199,6 +211,10 @@ pub fn default_search_histfile() -> PathBuf { state_dir().join("search_history") } +pub fn default_file_histfile() -> PathBuf { + state_dir().join("file_history") +} + /// Merge two TOML documents, merging values from `right` onto `left` /// /// When an array exists in both `left` and `right`, `right`'s array is diff --git a/helix-loader/src/session.rs b/helix-loader/src/session.rs index 63eb7ac8e40f..33b3814b9cd7 100644 --- a/helix-loader/src/session.rs +++ b/helix-loader/src/session.rs @@ -1,10 +1,51 @@ -use crate::{command_histfile, search_histfile}; +use crate::{command_histfile, file_histfile, search_histfile}; +use bincode::serialize_into; +use serde::{Deserialize, Serialize}; use std::{ fs::{File, OpenOptions}, io::{self, BufRead, BufReader, Write}, path::PathBuf, }; +// TODO: should this contain a ViewPosition? +// it would require exposing that type in a new crate, re-exporting in helix-view, +// since this crate is a dependency of helix-view +#[derive(Debug, Serialize, Deserialize)] +pub struct FileHistoryEntry { + path: PathBuf, + anchor: usize, + vertical_offset: usize, + horizontal_offset: usize, +} + +impl FileHistoryEntry { + pub fn new( + path: PathBuf, + anchor: usize, + vertical_offset: usize, + horizontal_offset: usize, + ) -> Self { + Self { + path, + anchor, + vertical_offset, + horizontal_offset, + } + } +} + +pub fn push_file_history(entry: FileHistoryEntry) { + let file = OpenOptions::new() + .append(true) + .create(true) + .open(file_histfile()) + // TODO: do something about this unwrap + .unwrap(); + + // TODO: do something about this unwrap + serialize_into(file, &entry).unwrap(); +} + pub fn push_history(register: char, line: &str) { let filepath = match register { ':' => command_histfile(), diff --git a/helix-term/src/main.rs b/helix-term/src/main.rs index 123ec391a309..b72f6a687092 100644 --- a/helix-term/src/main.rs +++ b/helix-term/src/main.rs @@ -46,6 +46,7 @@ async fn main_impl() -> Result { helix_loader::initialize_log_file(args.log_file.clone()); helix_loader::initialize_command_histfile(None); helix_loader::initialize_search_histfile(None); + helix_loader::initialize_file_histfile(None); // Help has a higher priority and should be handled separately. if args.display_help { diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 6c585a8a7f2c..c852b81a4462 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -16,6 +16,7 @@ use crate::{ }; use dap::StackFrame; use helix_event::dispatch; +use helix_loader::session::{push_file_history, FileHistoryEntry}; use helix_vcs::DiffProviderRegistry; use futures_util::stream::select_all::SelectAll; @@ -1776,6 +1777,18 @@ impl Editor { } pub fn close(&mut self, id: ViewId) { + let view = self.tree.get(id); + // TODO: do something about this unwrap + let doc = self.document(view.doc).unwrap(); + if let Some(path) = doc.path() { + push_file_history(FileHistoryEntry::new( + path.to_owned(), + view.offset.anchor, + view.offset.vertical_offset, + view.offset.horizontal_offset, + )); + }; + // Remove selections for the closed view on all documents. for doc in self.documents_mut() { doc.remove_view(id); From 5ee4eaf191b644f72b121f8dac0b18ec315772f8 Mon Sep 17 00:00:00 2001 From: Ingrid Date: Fri, 9 Feb 2024 19:33:48 +0100 Subject: [PATCH 11/39] load file history It was necessary make pos in file args an option to prevent it from overwriting the file positions loaded from persistence. Alignment is not quite right... I think we need to persist selections instead of view positions, or disable center aligning --- helix-loader/src/session.rs | 29 ++++++++++++++++++++++++----- helix-term/src/application.rs | 29 ++++++++++++++++++++++++----- helix-term/src/args.rs | 19 +++++++++++-------- helix-term/src/commands/typed.rs | 6 ++++-- helix-view/src/editor.rs | 19 ++++++++++++++++++- 5 files changed, 81 insertions(+), 21 deletions(-) diff --git a/helix-loader/src/session.rs b/helix-loader/src/session.rs index 33b3814b9cd7..8faecd843b6b 100644 --- a/helix-loader/src/session.rs +++ b/helix-loader/src/session.rs @@ -1,5 +1,5 @@ use crate::{command_histfile, file_histfile, search_histfile}; -use bincode::serialize_into; +use bincode::{deserialize_from, serialize_into}; use serde::{Deserialize, Serialize}; use std::{ fs::{File, OpenOptions}, @@ -12,10 +12,10 @@ use std::{ // since this crate is a dependency of helix-view #[derive(Debug, Serialize, Deserialize)] pub struct FileHistoryEntry { - path: PathBuf, - anchor: usize, - vertical_offset: usize, - horizontal_offset: usize, + pub path: PathBuf, + pub anchor: usize, + pub vertical_offset: usize, + pub horizontal_offset: usize, } impl FileHistoryEntry { @@ -46,6 +46,25 @@ pub fn push_file_history(entry: FileHistoryEntry) { serialize_into(file, &entry).unwrap(); } +pub fn read_file_history() -> Vec { + match File::open(file_histfile()) { + Ok(file) => { + let mut read = BufReader::new(file); + let mut entries = Vec::new(); + // TODO: more sophisticated error handling + while let Ok(entry) = deserialize_from(&mut read) { + entries.push(entry); + } + entries + } + Err(e) => match e.kind() { + io::ErrorKind::NotFound => Vec::new(), + // TODO: do something about this panic + _ => panic!(), + }, + } +} + pub fn push_history(register: char, line: &str) { let filepath = match register { ':' => command_histfile(), diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 3bddb728dd5c..36264c915f45 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -16,6 +16,7 @@ use helix_view::{ graphics::Rect, theme, tree::Layout, + view::ViewPosition, Align, Editor, }; use serde_json::json; @@ -34,7 +35,12 @@ use crate::{ use log::{debug, error, info, warn}; #[cfg(not(feature = "integration"))] use std::io::stdout; -use std::{collections::btree_map::Entry, io::stdin, path::Path, sync::Arc}; +use std::{ + collections::{btree_map::Entry, HashMap}, + io::stdin, + path::Path, + sync::Arc, +}; #[cfg(not(windows))] use anyhow::Context; @@ -147,6 +153,16 @@ impl Application { &config.editor })), handlers, + HashMap::from_iter(session::read_file_history().iter().map(|entry| { + ( + entry.path.clone(), + ViewPosition { + anchor: entry.anchor, + horizontal_offset: entry.horizontal_offset, + vertical_offset: entry.vertical_offset, + }, + ) + })), ); // TODO: do most of this in the background? @@ -223,10 +239,13 @@ impl Application { // NOTE: this isn't necessarily true anymore. If // `--vsplit` or `--hsplit` are used, the file which is // opened last is focused on. - let view_id = editor.tree.focus; - let doc = doc_mut!(editor, &doc_id); - let pos = Selection::point(pos_at_coords(doc.text().slice(..), pos, true)); - doc.set_selection(view_id, pos); + if let Some(pos) = pos { + let view_id = editor.tree.focus; + let doc = doc_mut!(editor, &doc_id); + let pos = + Selection::point(pos_at_coords(doc.text().slice(..), pos, true)); + doc.set_selection(view_id, pos); + } } } diff --git a/helix-term/src/args.rs b/helix-term/src/args.rs index 853c1576881e..31002251ec86 100644 --- a/helix-term/src/args.rs +++ b/helix-term/src/args.rs @@ -16,7 +16,7 @@ pub struct Args { pub verbosity: u64, pub log_file: Option, pub config_file: Option, - pub files: Vec<(PathBuf, Position)>, + pub files: Vec<(PathBuf, Option)>, pub working_directory: Option, } @@ -106,7 +106,10 @@ impl Args { if let Some(file) = args.files.first_mut() { if line_number != 0 { - file.1.row = line_number; + file.1 = match file.1 { + Some(pos) => Some(Position::new(line_number, pos.col)), + None => Some(Position::new(line_number, 0)), + } } } @@ -115,8 +118,8 @@ impl Args { } /// Parse arg into [`PathBuf`] and position. -pub(crate) fn parse_file(s: &str) -> (PathBuf, Position) { - let def = || (PathBuf::from(s), Position::default()); +pub(crate) fn parse_file(s: &str) -> (PathBuf, Option) { + let def = || (PathBuf::from(s), None); if Path::new(s).exists() { return def(); } @@ -128,22 +131,22 @@ pub(crate) fn parse_file(s: &str) -> (PathBuf, Position) { /// Split file.rs:10:2 into [`PathBuf`], row and col. /// /// Does not validate if file.rs is a file or directory. -fn split_path_row_col(s: &str) -> Option<(PathBuf, Position)> { +fn split_path_row_col(s: &str) -> Option<(PathBuf, Option)> { let mut s = s.trim_end_matches(':').rsplitn(3, ':'); let col: usize = s.next()?.parse().ok()?; let row: usize = s.next()?.parse().ok()?; let path = s.next()?.into(); let pos = Position::new(row.saturating_sub(1), col.saturating_sub(1)); - Some((path, pos)) + Some((path, Some(pos))) } /// Split file.rs:10 into [`PathBuf`] and row. /// /// Does not validate if file.rs is a file or directory. -fn split_path_row(s: &str) -> Option<(PathBuf, Position)> { +fn split_path_row(s: &str) -> Option<(PathBuf, Option)> { let (path, row) = s.trim_end_matches(':').rsplit_once(':')?; let row: usize = row.parse().ok()?; let path = path.into(); let pos = Position::new(row.saturating_sub(1), 0); - Some((path, pos)) + Some((path, Some(pos))) } diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index c21743d0823a..d65b8ddae69f 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -130,8 +130,10 @@ fn open(cx: &mut compositor::Context, args: &[Cow], event: PromptEvent) -> // Otherwise, just open the file let _ = cx.editor.open(&path, Action::Replace)?; let (view, doc) = current!(cx.editor); - let pos = Selection::point(pos_at_coords(doc.text().slice(..), pos, true)); - doc.set_selection(view.id, pos); + if let Some(pos) = pos { + let pos = Selection::point(pos_at_coords(doc.text().slice(..), pos, true)); + doc.set_selection(view.id, pos); + } // does not affect opening a buffer without pos align_view(doc, view, Align::Center); } diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index c852b81a4462..ac44e3cf8b0d 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -1056,6 +1056,8 @@ pub struct Editor { pub debugger_events: SelectAll>, pub breakpoints: HashMap>, + pub old_file_locs: HashMap, + pub syn_loader: Arc>, pub theme_loader: Arc, /// last_theme is used for theme previews. We store the current theme here, @@ -1174,6 +1176,7 @@ impl Editor { syn_loader: Arc>, config: Arc>, handlers: Handlers, + old_file_locs: HashMap, ) -> Self { let language_servers = helix_lsp::Registry::new(syn_loader.clone()); let conf = config.load(); @@ -1201,6 +1204,7 @@ impl Editor { debugger: None, debugger_events: SelectAll::new(), breakpoints: HashMap::new(), + old_file_locs, syn_loader, theme_loader, last_theme: None, @@ -1676,6 +1680,18 @@ impl Editor { ); // initialize selection for view let doc = doc_mut!(self, &id); + + let view = self.tree.get_mut(view_id); + view.offset = self + .old_file_locs + .get(doc.path().unwrap()) + .map(|x| x.to_owned()) + .unwrap_or_default(); + doc.set_selection( + view_id, + Selection::single(view.offset.anchor, view.offset.anchor), + ); + doc.ensure_view_init(view_id); doc.mark_as_focused(); focus_lost @@ -1782,11 +1798,12 @@ impl Editor { let doc = self.document(view.doc).unwrap(); if let Some(path) = doc.path() { push_file_history(FileHistoryEntry::new( - path.to_owned(), + path.clone(), view.offset.anchor, view.offset.vertical_offset, view.offset.horizontal_offset, )); + self.old_file_locs.insert(path.to_owned(), view.offset); }; // Remove selections for the closed view on all documents. From b7fbbdeab6883a49779c61fc7655c2771ed3b0f4 Mon Sep 17 00:00:00 2001 From: Ingrid Date: Sat, 10 Feb 2024 21:47:01 +0100 Subject: [PATCH 12/39] encode register history with bincode, and merge logic with file history encoding was found to be necessary because registers can contain line endings, which breaks the previous lines-of-text format --- helix-loader/src/session.rs | 51 ++++++++++++++----------------------- helix-term/src/ui/prompt.rs | 2 +- 2 files changed, 20 insertions(+), 33 deletions(-) diff --git a/helix-loader/src/session.rs b/helix-loader/src/session.rs index 8faecd843b6b..691333d99fd9 100644 --- a/helix-loader/src/session.rs +++ b/helix-loader/src/session.rs @@ -3,7 +3,7 @@ use bincode::{deserialize_from, serialize_into}; use serde::{Deserialize, Serialize}; use std::{ fs::{File, OpenOptions}, - io::{self, BufRead, BufReader, Write}, + io::{self, BufReader}, path::PathBuf, }; @@ -34,11 +34,11 @@ impl FileHistoryEntry { } } -pub fn push_file_history(entry: FileHistoryEntry) { +fn push_history(filepath: PathBuf, entry: T) { let file = OpenOptions::new() .append(true) .create(true) - .open(file_histfile()) + .open(filepath) // TODO: do something about this unwrap .unwrap(); @@ -46,8 +46,8 @@ pub fn push_file_history(entry: FileHistoryEntry) { serialize_into(file, &entry).unwrap(); } -pub fn read_file_history() -> Vec { - match File::open(file_histfile()) { +fn read_history Deserialize<'a>>(filepath: PathBuf) -> Vec { + match File::open(filepath) { Ok(file) => { let mut read = BufReader::new(file); let mut entries = Vec::new(); @@ -65,45 +65,32 @@ pub fn read_file_history() -> Vec { } } -pub fn push_history(register: char, line: &str) { +pub fn push_file_history(entry: FileHistoryEntry) { + push_history(file_histfile(), entry) +} + +pub fn read_file_history() -> Vec { + read_history(file_histfile()) +} + +pub fn push_reg_history(register: char, line: &str) { let filepath = match register { ':' => command_histfile(), '/' => search_histfile(), _ => return, }; - let mut file = OpenOptions::new() - .append(true) - .create(true) - .open(filepath) - // TODO: do something about this unwrap - .unwrap(); - - // TODO: do something about this unwrap - writeln!(file, "{}", line).unwrap(); + push_history(filepath, line) } -fn read_histfile(filepath: PathBuf) -> Vec { - match File::open(filepath) { - Ok(file) => { - BufReader::new(file) - .lines() - .collect::>>() - // TODO: do something about this unwrap - .unwrap() - } - Err(e) => match e.kind() { - io::ErrorKind::NotFound => Vec::new(), - // TODO: do something about this panic - _ => panic!(), - }, - } +fn read_reg_history(filepath: PathBuf) -> Vec { + read_history(filepath) } pub fn read_command_history() -> Vec { - read_histfile(command_histfile()) + read_reg_history(command_histfile()) } pub fn read_search_history() -> Vec { - read_histfile(search_histfile()) + read_reg_history(search_histfile()) } diff --git a/helix-term/src/ui/prompt.rs b/helix-term/src/ui/prompt.rs index ad840fe42f46..4c8e766f4e61 100644 --- a/helix-term/src/ui/prompt.rs +++ b/helix-term/src/ui/prompt.rs @@ -615,7 +615,7 @@ impl Component for Prompt { cx.editor.set_error(err.to_string()); } #[cfg(not(feature = "integration"))] - session::push_history(register, &self.line); + session::push_reg_history(register, &self.line); }; } From f40125252dba41d5a29af2736d682c222223b396 Mon Sep 17 00:00:00 2001 From: Ingrid Date: Sat, 10 Feb 2024 22:03:08 +0100 Subject: [PATCH 13/39] avoid exposing internals of register.rs --- helix-loader/src/session.rs | 8 ++++++-- helix-term/src/application.rs | 4 ++-- helix-view/src/register.rs | 23 ----------------------- 3 files changed, 8 insertions(+), 27 deletions(-) diff --git a/helix-loader/src/session.rs b/helix-loader/src/session.rs index 691333d99fd9..7619ff36a2dd 100644 --- a/helix-loader/src/session.rs +++ b/helix-loader/src/session.rs @@ -88,9 +88,13 @@ fn read_reg_history(filepath: PathBuf) -> Vec { } pub fn read_command_history() -> Vec { - read_reg_history(command_histfile()) + let mut hist = read_reg_history(command_histfile()); + hist.reverse(); + hist } pub fn read_search_history() -> Vec { - read_reg_history(search_histfile()) + let mut hist = read_reg_history(search_histfile()); + hist.reverse(); + hist } diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 36264c915f45..a065a979aaae 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -169,13 +169,13 @@ impl Application { #[cfg(not(feature = "integration"))] editor .registers - .write_unreversed(':', session::read_command_history()) + .write(':', session::read_command_history()) // TODO: do something about this unwrap .unwrap(); #[cfg(not(feature = "integration"))] editor .registers - .write_unreversed('/', session::read_search_history()) + .write('/', session::read_search_history()) // TODO: do something about this unwrap .unwrap(); diff --git a/helix-view/src/register.rs b/helix-view/src/register.rs index 6e9bec1a54f5..d286a85ccafe 100644 --- a/helix-view/src/register.rs +++ b/helix-view/src/register.rs @@ -102,29 +102,6 @@ impl Registers { } } - pub fn write_unreversed(&mut self, name: char, values: Vec) -> Result<()> { - match name { - '_' => Ok(()), - '#' | '.' | '%' => Err(anyhow::anyhow!("Register {name} does not support writing")), - '*' | '+' => { - self.clipboard_provider.set_contents( - values.join(NATIVE_LINE_ENDING.as_str()), - match name { - '+' => ClipboardType::Clipboard, - '*' => ClipboardType::Selection, - _ => unreachable!(), - }, - )?; - self.inner.insert(name, values); - Ok(()) - } - _ => { - self.inner.insert(name, values); - Ok(()) - } - } - } - pub fn push(&mut self, name: char, mut value: String) -> Result<()> { match name { '_' => Ok(()), From a7ec4ae55a6f1a411ebf050e3b5f833a2706979a Mon Sep 17 00:00:00 2001 From: Ingrid Date: Sun, 11 Feb 2024 20:16:08 +0100 Subject: [PATCH 14/39] rename session to persistence --- helix-loader/src/lib.rs | 2 +- helix-loader/src/{session.rs => persistence.rs} | 0 helix-term/src/application.rs | 8 ++++---- helix-term/src/ui/prompt.rs | 4 ++-- helix-view/src/editor.rs | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) rename helix-loader/src/{session.rs => persistence.rs} (100%) diff --git a/helix-loader/src/lib.rs b/helix-loader/src/lib.rs index 902d18c78d73..325e140447ff 100644 --- a/helix-loader/src/lib.rs +++ b/helix-loader/src/lib.rs @@ -1,6 +1,6 @@ pub mod config; pub mod grammar; -pub mod session; +pub mod persistence; use helix_stdx::{env::current_working_dir, path}; diff --git a/helix-loader/src/session.rs b/helix-loader/src/persistence.rs similarity index 100% rename from helix-loader/src/session.rs rename to helix-loader/src/persistence.rs diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index a065a979aaae..ee2a29003647 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -1,7 +1,7 @@ use arc_swap::{access::Map, ArcSwap}; use futures_util::Stream; use helix_core::{diagnostic::Severity, pos_at_coords, syntax, Selection}; -use helix_loader::session; +use helix_loader::persistence; use helix_lsp::{ lsp::{self, notification::Notification}, util::lsp_range_to_range, @@ -153,7 +153,7 @@ impl Application { &config.editor })), handlers, - HashMap::from_iter(session::read_file_history().iter().map(|entry| { + HashMap::from_iter(persistence::read_file_history().iter().map(|entry| { ( entry.path.clone(), ViewPosition { @@ -169,13 +169,13 @@ impl Application { #[cfg(not(feature = "integration"))] editor .registers - .write(':', session::read_command_history()) + .write(':', persistence::read_command_history()) // TODO: do something about this unwrap .unwrap(); #[cfg(not(feature = "integration"))] editor .registers - .write('/', session::read_search_history()) + .write('/', persistence::read_search_history()) // TODO: do something about this unwrap .unwrap(); diff --git a/helix-term/src/ui/prompt.rs b/helix-term/src/ui/prompt.rs index 4c8e766f4e61..441482fbd2df 100644 --- a/helix-term/src/ui/prompt.rs +++ b/helix-term/src/ui/prompt.rs @@ -2,7 +2,7 @@ use crate::compositor::{Component, Compositor, Context, Event, EventResult}; use crate::{alt, ctrl, key, shift, ui}; use arc_swap::ArcSwap; use helix_core::syntax; -use helix_loader::session; +use helix_loader::persistence; use helix_view::document::Mode; use helix_view::input::KeyEvent; use helix_view::keyboard::KeyCode; @@ -615,7 +615,7 @@ impl Component for Prompt { cx.editor.set_error(err.to_string()); } #[cfg(not(feature = "integration"))] - session::push_reg_history(register, &self.line); + persistence::push_reg_history(register, &self.line); }; } diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index ac44e3cf8b0d..499582d4226c 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -16,7 +16,7 @@ use crate::{ }; use dap::StackFrame; use helix_event::dispatch; -use helix_loader::session::{push_file_history, FileHistoryEntry}; +use helix_loader::persistence::{push_file_history, FileHistoryEntry}; use helix_vcs::DiffProviderRegistry; use futures_util::stream::select_all::SelectAll; From 3c3b1afd4d1c2246686a303aa936a23decabfa34 Mon Sep 17 00:00:00 2001 From: Ingrid Date: Sun, 11 Feb 2024 20:49:17 +0100 Subject: [PATCH 15/39] promote type/implementation-specific persistence logic to helix-view --- helix-loader/src/persistence.rs | 66 +-------------------------------- helix-term/src/application.rs | 3 +- helix-term/src/ui/prompt.rs | 3 +- helix-view/src/editor.rs | 4 +- helix-view/src/lib.rs | 1 + helix-view/src/persistence.rs | 66 +++++++++++++++++++++++++++++++++ 6 files changed, 73 insertions(+), 70 deletions(-) create mode 100644 helix-view/src/persistence.rs diff --git a/helix-loader/src/persistence.rs b/helix-loader/src/persistence.rs index 7619ff36a2dd..926e0d4daeb2 100644 --- a/helix-loader/src/persistence.rs +++ b/helix-loader/src/persistence.rs @@ -1,4 +1,3 @@ -use crate::{command_histfile, file_histfile, search_histfile}; use bincode::{deserialize_from, serialize_into}; use serde::{Deserialize, Serialize}; use std::{ @@ -7,34 +6,7 @@ use std::{ path::PathBuf, }; -// TODO: should this contain a ViewPosition? -// it would require exposing that type in a new crate, re-exporting in helix-view, -// since this crate is a dependency of helix-view -#[derive(Debug, Serialize, Deserialize)] -pub struct FileHistoryEntry { - pub path: PathBuf, - pub anchor: usize, - pub vertical_offset: usize, - pub horizontal_offset: usize, -} - -impl FileHistoryEntry { - pub fn new( - path: PathBuf, - anchor: usize, - vertical_offset: usize, - horizontal_offset: usize, - ) -> Self { - Self { - path, - anchor, - vertical_offset, - horizontal_offset, - } - } -} - -fn push_history(filepath: PathBuf, entry: T) { +pub fn push_history(filepath: PathBuf, entry: T) { let file = OpenOptions::new() .append(true) .create(true) @@ -46,7 +18,7 @@ fn push_history(filepath: PathBuf, entry: T) { serialize_into(file, &entry).unwrap(); } -fn read_history Deserialize<'a>>(filepath: PathBuf) -> Vec { +pub fn read_history Deserialize<'a>>(filepath: PathBuf) -> Vec { match File::open(filepath) { Ok(file) => { let mut read = BufReader::new(file); @@ -64,37 +36,3 @@ fn read_history Deserialize<'a>>(filepath: PathBuf) -> Vec { }, } } - -pub fn push_file_history(entry: FileHistoryEntry) { - push_history(file_histfile(), entry) -} - -pub fn read_file_history() -> Vec { - read_history(file_histfile()) -} - -pub fn push_reg_history(register: char, line: &str) { - let filepath = match register { - ':' => command_histfile(), - '/' => search_histfile(), - _ => return, - }; - - push_history(filepath, line) -} - -fn read_reg_history(filepath: PathBuf) -> Vec { - read_history(filepath) -} - -pub fn read_command_history() -> Vec { - let mut hist = read_reg_history(command_histfile()); - hist.reverse(); - hist -} - -pub fn read_search_history() -> Vec { - let mut hist = read_reg_history(search_histfile()); - hist.reverse(); - hist -} diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index ee2a29003647..defc18210f2e 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -1,7 +1,6 @@ use arc_swap::{access::Map, ArcSwap}; use futures_util::Stream; use helix_core::{diagnostic::Severity, pos_at_coords, syntax, Selection}; -use helix_loader::persistence; use helix_lsp::{ lsp::{self, notification::Notification}, util::lsp_range_to_range, @@ -14,7 +13,7 @@ use helix_view::{ editor::{ConfigEvent, EditorEvent}, events::DiagnosticsDidChange, graphics::Rect, - theme, + persistence, theme, tree::Layout, view::ViewPosition, Align, Editor, diff --git a/helix-term/src/ui/prompt.rs b/helix-term/src/ui/prompt.rs index 441482fbd2df..d75fdd15091a 100644 --- a/helix-term/src/ui/prompt.rs +++ b/helix-term/src/ui/prompt.rs @@ -2,7 +2,6 @@ use crate::compositor::{Component, Compositor, Context, Event, EventResult}; use crate::{alt, ctrl, key, shift, ui}; use arc_swap::ArcSwap; use helix_core::syntax; -use helix_loader::persistence; use helix_view::document::Mode; use helix_view::input::KeyEvent; use helix_view::keyboard::KeyCode; @@ -17,7 +16,7 @@ use helix_core::{ }; use helix_view::{ graphics::{CursorKind, Margin, Rect}, - Editor, + persistence, Editor, }; type PromptCharHandler = Box; diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 499582d4226c..7bdb9a419b4d 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -9,6 +9,7 @@ use crate::{ handlers::Handlers, info::Info, input::KeyEvent, + persistence::{self, FileHistoryEntry}, register::Registers, theme::{self, Theme}, tree::{self, Tree}, @@ -16,7 +17,6 @@ use crate::{ }; use dap::StackFrame; use helix_event::dispatch; -use helix_loader::persistence::{push_file_history, FileHistoryEntry}; use helix_vcs::DiffProviderRegistry; use futures_util::stream::select_all::SelectAll; @@ -1797,7 +1797,7 @@ impl Editor { // TODO: do something about this unwrap let doc = self.document(view.doc).unwrap(); if let Some(path) = doc.path() { - push_file_history(FileHistoryEntry::new( + persistence::push_file_history(FileHistoryEntry::new( path.clone(), view.offset.anchor, view.offset.vertical_offset, diff --git a/helix-view/src/lib.rs b/helix-view/src/lib.rs index d54b49ef5400..0044ac618008 100644 --- a/helix-view/src/lib.rs +++ b/helix-view/src/lib.rs @@ -13,6 +13,7 @@ pub mod handlers; pub mod info; pub mod input; pub mod keyboard; +pub mod persistence; pub mod register; pub mod theme; pub mod tree; diff --git a/helix-view/src/persistence.rs b/helix-view/src/persistence.rs new file mode 100644 index 000000000000..7d9b24c0c5fd --- /dev/null +++ b/helix-view/src/persistence.rs @@ -0,0 +1,66 @@ +use helix_loader::{ + command_histfile, file_histfile, + persistence::{push_history, read_history}, + search_histfile, +}; +use serde::{Deserialize, Serialize}; +use std::path::PathBuf; + +// TODO: should this contain a ViewPosition? +#[derive(Debug, Serialize, Deserialize)] +pub struct FileHistoryEntry { + pub path: PathBuf, + pub anchor: usize, + pub vertical_offset: usize, + pub horizontal_offset: usize, +} + +impl FileHistoryEntry { + pub fn new( + path: PathBuf, + anchor: usize, + vertical_offset: usize, + horizontal_offset: usize, + ) -> Self { + Self { + path, + anchor, + vertical_offset, + horizontal_offset, + } + } +} + +pub fn push_file_history(entry: FileHistoryEntry) { + push_history(file_histfile(), entry) +} + +pub fn read_file_history() -> Vec { + read_history(file_histfile()) +} + +pub fn push_reg_history(register: char, line: &str) { + let filepath = match register { + ':' => command_histfile(), + '/' => search_histfile(), + _ => return, + }; + + push_history(filepath, line) +} + +fn read_reg_history(filepath: PathBuf) -> Vec { + read_history(filepath) +} + +pub fn read_command_history() -> Vec { + let mut hist = read_reg_history(command_histfile()); + hist.reverse(); + hist +} + +pub fn read_search_history() -> Vec { + let mut hist = read_reg_history(search_histfile()); + hist.reverse(); + hist +} From 41a52326491e4485ae9d2d70e551be35cc9e7cd3 Mon Sep 17 00:00:00 2001 From: Ingrid Date: Sun, 11 Feb 2024 22:44:24 +0100 Subject: [PATCH 16/39] store ViewPosition and Selection directly in FileHistoryEntry --- Cargo.lock | 3 +++ helix-core/Cargo.toml | 2 +- helix-core/src/selection.rs | 5 +++-- helix-term/src/application.rs | 16 +++++----------- helix-view/src/editor.rs | 26 +++++++++++++------------- helix-view/src/persistence.rs | 21 ++++++++------------- helix-view/src/view.rs | 3 ++- 7 files changed, 35 insertions(+), 41 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3252681451ac..a5e8494f4b1f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2310,6 +2310,9 @@ name = "smallvec" version = "1.13.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3c5e1a9a646d36c3599cd173a41282daf47c44583ad367b8e6837255952e5c67" +dependencies = [ + "serde", +] [[package]] name = "smartstring" diff --git a/helix-core/Cargo.toml b/helix-core/Cargo.toml index d245ec13a23f..22c91ca174f8 100644 --- a/helix-core/Cargo.toml +++ b/helix-core/Cargo.toml @@ -21,7 +21,7 @@ helix-loader = { path = "../helix-loader" } helix-parsec = { path = "../helix-parsec" } ropey = { version = "1.6.1", default-features = false, features = ["simd"] } -smallvec = "1.13" +smallvec = { version = "1.13", features = ["serde"] } smartstring = "1.0.1" unicode-segmentation = "1.12" # unicode-width is changing width definitions diff --git a/helix-core/src/selection.rs b/helix-core/src/selection.rs index 76de63628d1e..9958266eac91 100644 --- a/helix-core/src/selection.rs +++ b/helix-core/src/selection.rs @@ -13,6 +13,7 @@ use crate::{ }; use helix_stdx::range::is_subset; use helix_stdx::rope::{self, RopeSliceExt}; +use serde::{Deserialize, Serialize}; use smallvec::{smallvec, SmallVec}; use std::{borrow::Cow, iter, slice}; use tree_sitter::Node; @@ -51,7 +52,7 @@ use tree_sitter::Node; /// single grapheme inward from the range's edge. There are a /// variety of helper methods on `Range` for working in terms of /// that block cursor, all of which have `cursor` in their name. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] pub struct Range { /// The anchor of the range: the side that doesn't move when extending. pub anchor: usize, @@ -413,7 +414,7 @@ impl From for helix_stdx::Range { /// A selection consists of one or more selection ranges. /// invariant: A selection can never be empty (always contains at least primary range). -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct Selection { ranges: SmallVec<[Range; 1]>, primary_index: usize, diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index defc18210f2e..4dde6f98921b 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -15,7 +15,6 @@ use helix_view::{ graphics::Rect, persistence, theme, tree::Layout, - view::ViewPosition, Align, Editor, }; use serde_json::json; @@ -152,16 +151,11 @@ impl Application { &config.editor })), handlers, - HashMap::from_iter(persistence::read_file_history().iter().map(|entry| { - ( - entry.path.clone(), - ViewPosition { - anchor: entry.anchor, - horizontal_offset: entry.horizontal_offset, - vertical_offset: entry.vertical_offset, - }, - ) - })), + HashMap::from_iter( + persistence::read_file_history() + .into_iter() + .map(|entry| (entry.path.clone(), (entry.view_position, entry.selection))), + ), ); // TODO: do most of this in the background? diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 7bdb9a419b4d..579b1c8058c0 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -1056,7 +1056,7 @@ pub struct Editor { pub debugger_events: SelectAll>, pub breakpoints: HashMap>, - pub old_file_locs: HashMap, + pub old_file_locs: HashMap, pub syn_loader: Arc>, pub theme_loader: Arc, @@ -1176,7 +1176,7 @@ impl Editor { syn_loader: Arc>, config: Arc>, handlers: Handlers, - old_file_locs: HashMap, + old_file_locs: HashMap, ) -> Self { let language_servers = helix_lsp::Registry::new(syn_loader.clone()); let conf = config.load(); @@ -1681,16 +1681,15 @@ impl Editor { // initialize selection for view let doc = doc_mut!(self, &id); - let view = self.tree.get_mut(view_id); - view.offset = self + if let Some((view_position, selection)) = self .old_file_locs .get(doc.path().unwrap()) .map(|x| x.to_owned()) - .unwrap_or_default(); - doc.set_selection( - view_id, - Selection::single(view.offset.anchor, view.offset.anchor), - ); + { + let view = self.tree.get_mut(view_id); + view.offset = view_position; + doc.set_selection(view_id, selection); + } doc.ensure_view_init(view_id); doc.mark_as_focused(); @@ -1797,13 +1796,14 @@ impl Editor { // TODO: do something about this unwrap let doc = self.document(view.doc).unwrap(); if let Some(path) = doc.path() { + // TODO: can the arg here be a reference? would save cloning persistence::push_file_history(FileHistoryEntry::new( path.clone(), - view.offset.anchor, - view.offset.vertical_offset, - view.offset.horizontal_offset, + view.offset, + doc.selection(id).clone(), )); - self.old_file_locs.insert(path.to_owned(), view.offset); + self.old_file_locs + .insert(path.to_owned(), (view.offset, doc.selection(id).clone())); }; // Remove selections for the closed view on all documents. diff --git a/helix-view/src/persistence.rs b/helix-view/src/persistence.rs index 7d9b24c0c5fd..d4a00cf04747 100644 --- a/helix-view/src/persistence.rs +++ b/helix-view/src/persistence.rs @@ -1,3 +1,4 @@ +use helix_core::Selection; use helix_loader::{ command_histfile, file_histfile, persistence::{push_history, read_history}, @@ -6,27 +7,21 @@ use helix_loader::{ use serde::{Deserialize, Serialize}; use std::path::PathBuf; -// TODO: should this contain a ViewPosition? +use crate::view::ViewPosition; + #[derive(Debug, Serialize, Deserialize)] pub struct FileHistoryEntry { pub path: PathBuf, - pub anchor: usize, - pub vertical_offset: usize, - pub horizontal_offset: usize, + pub view_position: ViewPosition, + pub selection: Selection, } impl FileHistoryEntry { - pub fn new( - path: PathBuf, - anchor: usize, - vertical_offset: usize, - horizontal_offset: usize, - ) -> Self { + pub fn new(path: PathBuf, view_position: ViewPosition, selection: Selection) -> Self { Self { path, - anchor, - vertical_offset, - horizontal_offset, + view_position, + selection, } } } diff --git a/helix-view/src/view.rs b/helix-view/src/view.rs index a229f01ea66a..4f8387a8ec2c 100644 --- a/helix-view/src/view.rs +++ b/helix-view/src/view.rs @@ -17,6 +17,7 @@ use helix_core::{ Transaction, VisualOffsetError::{PosAfterMaxRow, PosBeforeAnchorRow}, }; +use serde::{Deserialize, Serialize}; use std::{ collections::{HashMap, VecDeque}, @@ -118,7 +119,7 @@ impl JumpList { } } -#[derive(Clone, Debug, PartialEq, Eq, Copy, Default)] +#[derive(Clone, Debug, PartialEq, Eq, Copy, Default, Serialize, Deserialize)] pub struct ViewPosition { pub anchor: usize, pub horizontal_offset: usize, From db766ed5296d94b5e16a1589fb75beca34a5ea77 Mon Sep 17 00:00:00 2001 From: Ingrid Date: Fri, 16 Feb 2024 21:08:06 +0100 Subject: [PATCH 17/39] fix quirky file persistence behaviour --- helix-term/src/application.rs | 4 +- helix-term/src/commands/typed.rs | 3 +- helix-view/src/editor.rs | 76 ++++++++++++++++++++++---------- 3 files changed, 57 insertions(+), 26 deletions(-) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 4dde6f98921b..4a995f1082d5 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -253,8 +253,8 @@ impl Application { )); // align the view to center after all files are loaded, // does not affect views without pos since it is at the top - let (view, doc) = current!(editor); - align_view(doc, view, Align::Center); + // let (view, doc) = current!(editor); + // align_view(doc, view, Align::Center); } } else { editor.new_file(Action::VerticalSplit); diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index d65b8ddae69f..57f2685d884c 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -135,7 +135,8 @@ fn open(cx: &mut compositor::Context, args: &[Cow], event: PromptEvent) -> doc.set_selection(view.id, pos); } // does not affect opening a buffer without pos - align_view(doc, view, Align::Center); + // TODO: ensure removing this will not cause problems + // align_view(doc, view, Align::Center); } } Ok(()) diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 579b1c8058c0..733e14473fe8 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -13,6 +13,7 @@ use crate::{ register::Registers, theme::{self, Theme}, tree::{self, Tree}, + view::ViewPosition, Document, DocumentId, View, ViewId, }; use dap::StackFrame; @@ -1575,6 +1576,7 @@ impl Editor { fn replace_document_in_view(&mut self, current_view: ViewId, doc_id: DocumentId) { let scrolloff = self.config().scrolloff; let view = self.tree.get_mut(current_view); + view.doc = doc_id; view.doc = doc_id; let doc = doc_mut!(self, &doc_id); @@ -1681,16 +1683,6 @@ impl Editor { // initialize selection for view let doc = doc_mut!(self, &id); - if let Some((view_position, selection)) = self - .old_file_locs - .get(doc.path().unwrap()) - .map(|x| x.to_owned()) - { - let view = self.tree.get_mut(view_id); - view.offset = view_position; - doc.set_selection(view_id, selection); - } - doc.ensure_view_init(view_id); doc.mark_as_focused(); focus_lost @@ -1762,9 +1754,14 @@ impl Editor { let path = helix_stdx::path::canonicalize(path); let id = self.document_id_by_path(&path); + // TODO: surely there's a neater way to do this? + let mut new_doc = false; + let id = if let Some(id) = id { id } else { + new_doc = true; + let mut doc = Document::open( &path, None, @@ -1788,28 +1785,44 @@ impl Editor { }; self.switch(id, action); + + if new_doc { + if let Some((view_position, selection)) = + self.old_file_locs.get(&path).map(|x| x.to_owned()) + { + let (view, doc) = current!(self); + view.offset = view_position; + doc.set_selection(view.id, selection); + } + } + Ok(id) } pub fn close(&mut self, id: ViewId) { - let view = self.tree.get(id); - // TODO: do something about this unwrap - let doc = self.document(view.doc).unwrap(); - if let Some(path) = doc.path() { - // TODO: can the arg here be a reference? would save cloning - persistence::push_file_history(FileHistoryEntry::new( - path.clone(), - view.offset, - doc.selection(id).clone(), - )); - self.old_file_locs - .insert(path.to_owned(), (view.offset, doc.selection(id).clone())); - }; + let offset = self.tree.get(id).offset.clone(); + + let mut file_locs = Vec::new(); // Remove selections for the closed view on all documents. for doc in self.documents_mut() { + if let Some(path) = doc.path() { + file_locs.push((path.clone(), offset, doc.selection(id).clone())); + }; + doc.remove_view(id); } + + for loc in file_locs { + // TODO: can the arg here be a reference? would save cloning + persistence::push_file_history(FileHistoryEntry::new( + loc.0.clone(), + loc.1, + loc.2.clone(), + )); + self.old_file_locs.insert(loc.0, (loc.1, loc.2)); + } + self.tree.remove(id); self._refresh(); } @@ -1831,11 +1844,14 @@ impl Editor { tokio::spawn(language_server.text_document_did_close(doc.identifier())); } + #[derive(Debug)] enum Action { Close(ViewId), ReplaceDoc(ViewId, DocumentId), } + let mut file_locs = Vec::new(); + let actions: Vec = self .tree .views_mut() @@ -1843,6 +1859,10 @@ impl Editor { view.remove_document(&doc_id); if view.doc == doc_id { + if let Some(path) = doc.path() { + file_locs.push((path.clone(), view.offset, doc.selection(view.id).clone())); + }; + // something was previously open in the view, switch to previous doc if let Some(prev_doc) = view.docs_access_history.pop() { Some(Action::ReplaceDoc(view.id, prev_doc)) @@ -1856,6 +1876,16 @@ impl Editor { }) .collect(); + for loc in file_locs { + // TODO: can the arg here be a reference? would save cloning + persistence::push_file_history(FileHistoryEntry::new( + loc.0.clone(), + loc.1, + loc.2.clone(), + )); + self.old_file_locs.insert(loc.0, (loc.1, loc.2)); + } + for action in actions { match action { Action::Close(view_id) => { From 25778b1f5fc2eb681eaf0348291773bb1d4d10b6 Mon Sep 17 00:00:00 2001 From: Ingrid Date: Mon, 19 Feb 2024 19:51:39 +0100 Subject: [PATCH 18/39] fix integration tests --- helix-term/src/application.rs | 2 -- helix-term/src/ui/prompt.rs | 1 - helix-term/tests/test/helpers.rs | 22 ++++++++++++++++++++-- helix-view/src/editor.rs | 25 ++++++++++++++++++------- 4 files changed, 38 insertions(+), 12 deletions(-) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 4a995f1082d5..087eece63503 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -159,13 +159,11 @@ impl Application { ); // TODO: do most of this in the background? - #[cfg(not(feature = "integration"))] editor .registers .write(':', persistence::read_command_history()) // TODO: do something about this unwrap .unwrap(); - #[cfg(not(feature = "integration"))] editor .registers .write('/', persistence::read_search_history()) diff --git a/helix-term/src/ui/prompt.rs b/helix-term/src/ui/prompt.rs index d75fdd15091a..144e5c6309db 100644 --- a/helix-term/src/ui/prompt.rs +++ b/helix-term/src/ui/prompt.rs @@ -613,7 +613,6 @@ impl Component for Prompt { { cx.editor.set_error(err.to_string()); } - #[cfg(not(feature = "integration"))] persistence::push_reg_history(register, &self.line); }; } diff --git a/helix-term/tests/test/helpers.rs b/helix-term/tests/test/helpers.rs index 70b3f4022c00..3e4c952c2cf6 100644 --- a/helix-term/tests/test/helpers.rs +++ b/helix-term/tests/test/helpers.rs @@ -8,9 +8,10 @@ use std::{ use anyhow::bail; use crossterm::event::{Event, KeyEvent}; use helix_core::{diagnostic::Severity, test, Selection, Transaction}; +use helix_loader; use helix_term::{application::Application, args::Args, config::Config, keymap::merge_keys}; use helix_view::{current_ref, doc, editor::LspConfig, input::parse_macro, Editor}; -use tempfile::NamedTempFile; +use tempfile::{NamedTempFile, TempPath}; use tokio_stream::wrappers::UnboundedReceiverStream; /// Specify how to set up the input text with line feeds @@ -229,6 +230,22 @@ pub fn test_syntax_loader(overrides: Option) -> helix_core::syntax::Load helix_core::syntax::Loader::new(lang.try_into().unwrap()).unwrap() } +fn init_persistence_files() -> anyhow::Result<(TempPath, TempPath, TempPath)> { + let command_file = NamedTempFile::new()?; + let command_path = command_file.into_temp_path(); + helix_loader::initialize_command_histfile(Some(command_path.to_path_buf())); + + let search_file = NamedTempFile::new()?; + let search_path = search_file.into_temp_path(); + helix_loader::initialize_search_histfile(Some(search_path.to_path_buf())); + + let file_file = NamedTempFile::new()?; + let file_path = file_file.into_temp_path(); + helix_loader::initialize_file_histfile(Some(file_path.to_path_buf())); + + Ok((command_path, search_path, file_path)) +} + /// Use this for very simple test cases where there is one input /// document, selection, and sequence of key presses, and you just /// want to verify the resulting document and selection. @@ -236,6 +253,7 @@ pub async fn test_with_config>( app_builder: AppBuilder, test_case: T, ) -> anyhow::Result<()> { + let (_, _, _) = init_persistence_files()?; let test_case = test_case.into(); let app = app_builder.build()?; @@ -345,7 +363,7 @@ impl AppBuilder { path: P, pos: Option, ) -> Self { - self.args.files.push((path.into(), pos.unwrap_or_default())); + self.args.files.push((path.into(), pos)); self } diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 733e14473fe8..2c41d19780c0 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -1791,8 +1791,13 @@ impl Editor { self.old_file_locs.get(&path).map(|x| x.to_owned()) { let (view, doc) = current!(self); - view.offset = view_position; - doc.set_selection(view.id, selection); + + let doc_len = doc.text().len_chars(); + // don't restore the view and selection if the selection goes beyond the file's end + if !selection.ranges().iter().any(|range| range.to() >= doc_len) { + view.offset = view_position; + doc.set_selection(view.id, selection); + } } } @@ -1800,16 +1805,22 @@ impl Editor { } pub fn close(&mut self, id: ViewId) { - let offset = self.tree.get(id).offset.clone(); + let offset = self.tree.get(id).offset; let mut file_locs = Vec::new(); - // Remove selections for the closed view on all documents. for doc in self.documents_mut() { - if let Some(path) = doc.path() { - file_locs.push((path.clone(), offset, doc.selection(id).clone())); - }; + // Persist file location history for this view + // FIXME: The view offset here is currently wrong when a doc is not current for that view. + // Right now it uses the current offset of the view, which is on a another document. + // We need to persist ViewPositions on documents a la PR #7568, then fetch that here. + if doc.selections().contains_key(&id) { + if let Some(path) = doc.path() { + file_locs.push((path.clone(), offset, doc.selection(id).clone())); + } + } + // Remove selections for the closed view on all documents. doc.remove_view(id); } From 393390fc67a18180889d6f9306a6795accca13aa Mon Sep 17 00:00:00 2001 From: Ingrid Date: Mon, 19 Feb 2024 21:07:36 +0100 Subject: [PATCH 19/39] save cloning by passing by ref to persistence functions --- helix-loader/src/persistence.rs | 4 ++-- helix-view/src/editor.rs | 32 ++++++++++++++++---------------- helix-view/src/persistence.rs | 4 ++-- 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/helix-loader/src/persistence.rs b/helix-loader/src/persistence.rs index 926e0d4daeb2..73e9d05bb693 100644 --- a/helix-loader/src/persistence.rs +++ b/helix-loader/src/persistence.rs @@ -6,7 +6,7 @@ use std::{ path::PathBuf, }; -pub fn push_history(filepath: PathBuf, entry: T) { +pub fn push_history(filepath: PathBuf, entry: &T) { let file = OpenOptions::new() .append(true) .create(true) @@ -15,7 +15,7 @@ pub fn push_history(filepath: PathBuf, entry: T) { .unwrap(); // TODO: do something about this unwrap - serialize_into(file, &entry).unwrap(); + serialize_into(file, entry).unwrap(); } pub fn read_history Deserialize<'a>>(filepath: PathBuf) -> Vec { diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 2c41d19780c0..5742952c2245 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -1816,7 +1816,11 @@ impl Editor { // We need to persist ViewPositions on documents a la PR #7568, then fetch that here. if doc.selections().contains_key(&id) { if let Some(path) = doc.path() { - file_locs.push((path.clone(), offset, doc.selection(id).clone())); + file_locs.push(FileHistoryEntry::new( + path.clone(), + offset, + doc.selection(id).clone(), + )); } } @@ -1825,13 +1829,9 @@ impl Editor { } for loc in file_locs { - // TODO: can the arg here be a reference? would save cloning - persistence::push_file_history(FileHistoryEntry::new( - loc.0.clone(), - loc.1, - loc.2.clone(), - )); - self.old_file_locs.insert(loc.0, (loc.1, loc.2)); + persistence::push_file_history(&loc); + self.old_file_locs + .insert(loc.path, (loc.view_position, loc.selection)); } self.tree.remove(id); @@ -1871,7 +1871,11 @@ impl Editor { if view.doc == doc_id { if let Some(path) = doc.path() { - file_locs.push((path.clone(), view.offset, doc.selection(view.id).clone())); + file_locs.push(FileHistoryEntry::new( + path.clone(), + view.offset, + doc.selection(view.id).clone(), + )); }; // something was previously open in the view, switch to previous doc @@ -1888,13 +1892,9 @@ impl Editor { .collect(); for loc in file_locs { - // TODO: can the arg here be a reference? would save cloning - persistence::push_file_history(FileHistoryEntry::new( - loc.0.clone(), - loc.1, - loc.2.clone(), - )); - self.old_file_locs.insert(loc.0, (loc.1, loc.2)); + persistence::push_file_history(&loc); + self.old_file_locs + .insert(loc.path, (loc.view_position, loc.selection)); } for action in actions { diff --git a/helix-view/src/persistence.rs b/helix-view/src/persistence.rs index d4a00cf04747..bb02d771452e 100644 --- a/helix-view/src/persistence.rs +++ b/helix-view/src/persistence.rs @@ -26,7 +26,7 @@ impl FileHistoryEntry { } } -pub fn push_file_history(entry: FileHistoryEntry) { +pub fn push_file_history(entry: &FileHistoryEntry) { push_history(file_histfile(), entry) } @@ -34,7 +34,7 @@ pub fn read_file_history() -> Vec { read_history(file_histfile()) } -pub fn push_reg_history(register: char, line: &str) { +pub fn push_reg_history(register: char, line: &String) { let filepath = match register { ':' => command_histfile(), '/' => search_histfile(), From 4efee37769a2e852375769aa3ea9e2dfc501b262 Mon Sep 17 00:00:00 2001 From: Ingrid Date: Mon, 19 Feb 2024 21:11:18 +0100 Subject: [PATCH 20/39] persist clipboard --- helix-loader/src/lib.rs | 16 ++++++++++++++++ helix-loader/src/persistence.rs | 14 ++++++++++++++ helix-term/src/application.rs | 5 +++++ helix-term/src/commands.rs | 3 +++ helix-term/src/main.rs | 1 + helix-term/tests/test/helpers.rs | 10 +++++++--- helix-view/src/persistence.rs | 12 ++++++++++-- 7 files changed, 56 insertions(+), 5 deletions(-) diff --git a/helix-loader/src/lib.rs b/helix-loader/src/lib.rs index 325e140447ff..5d6e34c5b516 100644 --- a/helix-loader/src/lib.rs +++ b/helix-loader/src/lib.rs @@ -22,6 +22,8 @@ static SEARCH_HISTFILE: once_cell::sync::OnceCell = once_cell::sync::On static FILE_HISTFILE: once_cell::sync::OnceCell = once_cell::sync::OnceCell::new(); +static CLIPBOARD_FILE: once_cell::sync::OnceCell = once_cell::sync::OnceCell::new(); + pub fn initialize_config_file(specified_file: Option) { let config_file = specified_file.unwrap_or_else(default_config_file); ensure_parent_dir(&config_file); @@ -52,6 +54,12 @@ pub fn initialize_file_histfile(specified_file: Option) { FILE_HISTFILE.set(file_histfile).ok(); } +pub fn initialize_clipboard_file(specified_file: Option) { + let clipboard_file = specified_file.unwrap_or_else(default_clipboard_file); + ensure_parent_dir(&clipboard_file); + CLIPBOARD_FILE.set(clipboard_file).ok(); +} + /// A list of runtime directories from highest to lowest priority /// /// The priority is: @@ -191,6 +199,10 @@ pub fn file_histfile() -> PathBuf { FILE_HISTFILE.get().map(|path| path.to_path_buf()).unwrap() } +pub fn clipboard_file() -> PathBuf { + CLIPBOARD_FILE.get().map(|path| path.to_path_buf()).unwrap() +} + pub fn workspace_config_file() -> PathBuf { find_workspace().0.join(".helix").join("config.toml") } @@ -215,6 +227,10 @@ pub fn default_file_histfile() -> PathBuf { state_dir().join("file_history") } +pub fn default_clipboard_file() -> PathBuf { + state_dir().join("clipboard") +} + /// Merge two TOML documents, merging values from `right` onto `left` /// /// When an array exists in both `left` and `right`, `right`'s array is diff --git a/helix-loader/src/persistence.rs b/helix-loader/src/persistence.rs index 73e9d05bb693..9cc5a516b460 100644 --- a/helix-loader/src/persistence.rs +++ b/helix-loader/src/persistence.rs @@ -6,6 +6,20 @@ use std::{ path::PathBuf, }; +pub fn write_history(filepath: PathBuf, entries: &Vec) { + let file = OpenOptions::new() + .write(true) + .create(true) + .open(filepath) + // TODO: do something about this unwrap + .unwrap(); + + for entry in entries { + // TODO: do something about this unwrap + serialize_into(&file, &entry).unwrap(); + } +} + pub fn push_history(filepath: PathBuf, entry: &T) { let file = OpenOptions::new() .append(true) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 087eece63503..28b9eb4fec3b 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -169,6 +169,11 @@ impl Application { .write('/', persistence::read_search_history()) // TODO: do something about this unwrap .unwrap(); + editor + .registers + .write('"', persistence::read_clipboard_file()) + // TODO: do something about this unwrap + .unwrap(); let keys = Box::new(Map::new(Arc::clone(&config), |config: &Config| { &config.keys diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 04e39e5ee9b4..07b0f9cc28cf 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -44,6 +44,7 @@ use helix_view::{ info::Info, input::KeyEvent, keyboard::KeyCode, + persistence, theme::Style, tree, view::View, @@ -4315,6 +4316,8 @@ fn yank_impl(editor: &mut Editor, register: char) { .collect(); let selections = values.len(); + persistence::write_clipboard_file(&values); + match editor.registers.write(register, values) { Ok(_) => editor.set_status(format!( "yanked {selections} selection{} to register {register}", diff --git a/helix-term/src/main.rs b/helix-term/src/main.rs index b72f6a687092..dcc49119ca3b 100644 --- a/helix-term/src/main.rs +++ b/helix-term/src/main.rs @@ -47,6 +47,7 @@ async fn main_impl() -> Result { helix_loader::initialize_command_histfile(None); helix_loader::initialize_search_histfile(None); helix_loader::initialize_file_histfile(None); + helix_loader::initialize_clipboard_file(None); // Help has a higher priority and should be handled separately. if args.display_help { diff --git a/helix-term/tests/test/helpers.rs b/helix-term/tests/test/helpers.rs index 3e4c952c2cf6..623f5df3e77a 100644 --- a/helix-term/tests/test/helpers.rs +++ b/helix-term/tests/test/helpers.rs @@ -230,7 +230,7 @@ pub fn test_syntax_loader(overrides: Option) -> helix_core::syntax::Load helix_core::syntax::Loader::new(lang.try_into().unwrap()).unwrap() } -fn init_persistence_files() -> anyhow::Result<(TempPath, TempPath, TempPath)> { +fn init_persistence_files() -> anyhow::Result<(TempPath, TempPath, TempPath, TempPath)> { let command_file = NamedTempFile::new()?; let command_path = command_file.into_temp_path(); helix_loader::initialize_command_histfile(Some(command_path.to_path_buf())); @@ -243,7 +243,11 @@ fn init_persistence_files() -> anyhow::Result<(TempPath, TempPath, TempPath)> { let file_path = file_file.into_temp_path(); helix_loader::initialize_file_histfile(Some(file_path.to_path_buf())); - Ok((command_path, search_path, file_path)) + let clipboard_file = NamedTempFile::new()?; + let clipboard_path = clipboard_file.into_temp_path(); + helix_loader::initialize_clipboard_file(Some(clipboard_path.to_path_buf())); + + Ok((command_path, search_path, file_path, clipboard_path)) } /// Use this for very simple test cases where there is one input @@ -253,7 +257,7 @@ pub async fn test_with_config>( app_builder: AppBuilder, test_case: T, ) -> anyhow::Result<()> { - let (_, _, _) = init_persistence_files()?; + let (_, _, _, _) = init_persistence_files()?; let test_case = test_case.into(); let app = app_builder.build()?; diff --git a/helix-view/src/persistence.rs b/helix-view/src/persistence.rs index bb02d771452e..9f0235769b6e 100644 --- a/helix-view/src/persistence.rs +++ b/helix-view/src/persistence.rs @@ -1,7 +1,7 @@ use helix_core::Selection; use helix_loader::{ - command_histfile, file_histfile, - persistence::{push_history, read_history}, + clipboard_file, command_histfile, file_histfile, + persistence::{push_history, read_history, write_history}, search_histfile, }; use serde::{Deserialize, Serialize}; @@ -59,3 +59,11 @@ pub fn read_search_history() -> Vec { hist.reverse(); hist } + +pub fn write_clipboard_file(values: &Vec) { + write_history(clipboard_file(), values) +} + +pub fn read_clipboard_file() -> Vec { + read_history(clipboard_file()) +} From 599010d1b9ad3a6fc92de7b26448fd8fb7efe860 Mon Sep 17 00:00:00 2001 From: Ingrid Date: Wed, 1 May 2024 17:05:17 +0200 Subject: [PATCH 21/39] add on/off config options for persistence --- helix-term/src/application.rs | 51 +++++++++++++++++++++-------------- helix-term/src/commands.rs | 4 ++- helix-term/src/ui/prompt.rs | 6 ++++- helix-view/src/editor.rs | 28 +++++++++++++------ 4 files changed, 59 insertions(+), 30 deletions(-) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 28b9eb4fec3b..60ada699284c 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -143,6 +143,15 @@ impl Application { let mut compositor = Compositor::new(area); let config = Arc::new(ArcSwap::from_pointee(config)); let handlers = handlers::setup(config.clone()); + let old_file_locs = if config.load().editor.persist_old_files { + HashMap::from_iter( + persistence::read_file_history() + .into_iter() + .map(|entry| (entry.path.clone(), (entry.view_position, entry.selection))), + ) + } else { + HashMap::new() + }; let mut editor = Editor::new( area, theme_loader.clone(), @@ -151,29 +160,31 @@ impl Application { &config.editor })), handlers, - HashMap::from_iter( - persistence::read_file_history() - .into_iter() - .map(|entry| (entry.path.clone(), (entry.view_position, entry.selection))), - ), + old_file_locs, ); // TODO: do most of this in the background? - editor - .registers - .write(':', persistence::read_command_history()) - // TODO: do something about this unwrap - .unwrap(); - editor - .registers - .write('/', persistence::read_search_history()) - // TODO: do something about this unwrap - .unwrap(); - editor - .registers - .write('"', persistence::read_clipboard_file()) - // TODO: do something about this unwrap - .unwrap(); + if config.load().editor.persist_commands { + editor + .registers + .write(':', persistence::read_command_history()) + // TODO: do something about this unwrap + .unwrap(); + } + if config.load().editor.persist_search { + editor + .registers + .write('/', persistence::read_search_history()) + // TODO: do something about this unwrap + .unwrap(); + } + if config.load().editor.persist_clipboard { + editor + .registers + .write('"', persistence::read_clipboard_file()) + // TODO: do something about this unwrap + .unwrap(); + } let keys = Box::new(Map::new(Arc::clone(&config), |config: &Config| { &config.keys diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 07b0f9cc28cf..fe914341b2dd 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -4316,7 +4316,9 @@ fn yank_impl(editor: &mut Editor, register: char) { .collect(); let selections = values.len(); - persistence::write_clipboard_file(&values); + if editor.config().persist_clipboard { + persistence::write_clipboard_file(&values); + } match editor.registers.write(register, values) { Ok(_) => editor.set_status(format!( diff --git a/helix-term/src/ui/prompt.rs b/helix-term/src/ui/prompt.rs index 144e5c6309db..7d09926ef243 100644 --- a/helix-term/src/ui/prompt.rs +++ b/helix-term/src/ui/prompt.rs @@ -613,7 +613,11 @@ impl Component for Prompt { { cx.editor.set_error(err.to_string()); } - persistence::push_reg_history(register, &self.line); + if (cx.editor.config().persist_commands && register == ':') + || (cx.editor.config().persist_search && register == '/') + { + persistence::push_reg_history(register, &self.line); + } }; } diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 5742952c2245..ec1a40c586ac 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -362,6 +362,10 @@ pub struct Config { pub end_of_line_diagnostics: DiagnosticFilter, // Set to override the default clipboard provider pub clipboard_provider: ClipboardProvider, + pub persist_old_files: bool, + pub persist_commands: bool, + pub persist_search: bool, + pub persist_clipboard: bool, } #[derive(Debug, Clone, PartialEq, Deserialize, Serialize, Eq, PartialOrd, Ord)] @@ -1003,6 +1007,10 @@ impl Default for Config { inline_diagnostics: InlineDiagnosticsConfig::default(), end_of_line_diagnostics: DiagnosticFilter::Disable, clipboard_provider: ClipboardProvider::default(), + persist_old_files: false, + persist_commands: false, + persist_search: false, + persist_clipboard: false, } } } @@ -1828,10 +1836,12 @@ impl Editor { doc.remove_view(id); } - for loc in file_locs { - persistence::push_file_history(&loc); - self.old_file_locs - .insert(loc.path, (loc.view_position, loc.selection)); + if self.config().persist_old_files { + for loc in file_locs { + persistence::push_file_history(&loc); + self.old_file_locs + .insert(loc.path, (loc.view_position, loc.selection)); + } } self.tree.remove(id); @@ -1891,10 +1901,12 @@ impl Editor { }) .collect(); - for loc in file_locs { - persistence::push_file_history(&loc); - self.old_file_locs - .insert(loc.path, (loc.view_position, loc.selection)); + if self.config().persist_old_files { + for loc in file_locs { + persistence::push_file_history(&loc); + self.old_file_locs + .insert(loc.path, (loc.view_position, loc.selection)); + } } for action in actions { From bb20c0e244cec7c5b180b94338bb4718c465a1b5 Mon Sep 17 00:00:00 2001 From: Ingrid Date: Wed, 1 May 2024 19:04:27 +0200 Subject: [PATCH 22/39] fix bug: writes on untruncated histfiles --- helix-loader/src/persistence.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/helix-loader/src/persistence.rs b/helix-loader/src/persistence.rs index 9cc5a516b460..f2f9395018f1 100644 --- a/helix-loader/src/persistence.rs +++ b/helix-loader/src/persistence.rs @@ -10,6 +10,7 @@ pub fn write_history(filepath: PathBuf, entries: &Vec) { let file = OpenOptions::new() .write(true) .create(true) + .truncate(true) .open(filepath) // TODO: do something about this unwrap .unwrap(); From 7bf76243dbe7f0c0426c50f58f84ee975f975fba Mon Sep 17 00:00:00 2001 From: Ingrid Date: Wed, 1 May 2024 19:06:32 +0200 Subject: [PATCH 23/39] trim persistence files --- helix-loader/src/persistence.rs | 13 +++++++++++++ helix-term/src/application.rs | 23 ++++++++++++++++++++++- helix-view/src/persistence.rs | 16 ++++++++++++++-- 3 files changed, 49 insertions(+), 3 deletions(-) diff --git a/helix-loader/src/persistence.rs b/helix-loader/src/persistence.rs index f2f9395018f1..c9ec9687199d 100644 --- a/helix-loader/src/persistence.rs +++ b/helix-loader/src/persistence.rs @@ -51,3 +51,16 @@ pub fn read_history Deserialize<'a>>(filepath: PathBuf) -> Vec { }, } } + +pub fn trim_history Deserialize<'a>>( + filepath: PathBuf, + limit: usize, +) { + // TODO: can we remove this clone? + let history: Vec = read_history(filepath.clone()); + if history.len() > limit { + let trim_start = history.len() - limit; + let trimmed_history = history[trim_start..].to_vec(); + write_history(filepath, &trimmed_history); + } +} diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 60ada699284c..3bc62bd96750 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -25,7 +25,7 @@ use crate::{ compositor::{Compositor, Event}, config::Config, handlers, - job::Jobs, + job::{Job, Jobs}, keymap::Keymaps, ui::{self, overlay::overlaid}, }; @@ -309,6 +309,27 @@ impl Application { jobs: Jobs::new(), lsp_progress: LspProgressMap::new(), }; + app.jobs.add( + Job::new(async { + persistence::trim_file_history(5); + Ok(()) + }) + .wait_before_exiting(), + ); + app.jobs.add( + Job::new(async { + persistence::trim_command_history(5); + Ok(()) + }) + .wait_before_exiting(), + ); + app.jobs.add( + Job::new(async { + persistence::trim_search_history(5); + Ok(()) + }) + .wait_before_exiting(), + ); Ok(app) } diff --git a/helix-view/src/persistence.rs b/helix-view/src/persistence.rs index 9f0235769b6e..332fc03929e1 100644 --- a/helix-view/src/persistence.rs +++ b/helix-view/src/persistence.rs @@ -1,7 +1,7 @@ use helix_core::Selection; use helix_loader::{ clipboard_file, command_histfile, file_histfile, - persistence::{push_history, read_history, write_history}, + persistence::{push_history, read_history, trim_history, write_history}, search_histfile, }; use serde::{Deserialize, Serialize}; @@ -9,7 +9,7 @@ use std::path::PathBuf; use crate::view::ViewPosition; -#[derive(Debug, Serialize, Deserialize)] +#[derive(Clone, Debug, Serialize, Deserialize)] pub struct FileHistoryEntry { pub path: PathBuf, pub view_position: ViewPosition, @@ -34,6 +34,10 @@ pub fn read_file_history() -> Vec { read_history(file_histfile()) } +pub fn trim_file_history(limit: usize) { + trim_history::(file_histfile(), limit) +} + pub fn push_reg_history(register: char, line: &String) { let filepath = match register { ':' => command_histfile(), @@ -54,12 +58,20 @@ pub fn read_command_history() -> Vec { hist } +pub fn trim_command_history(limit: usize) { + trim_history::(command_histfile(), limit) +} + pub fn read_search_history() -> Vec { let mut hist = read_reg_history(search_histfile()); hist.reverse(); hist } +pub fn trim_search_history(limit: usize) { + trim_history::(search_histfile(), limit) +} + pub fn write_clipboard_file(values: &Vec) { write_history(clipboard_file(), values) } From 0009e153249e9801670881b0860d6e172f9ced90 Mon Sep 17 00:00:00 2001 From: Ingrid Date: Wed, 1 May 2024 20:53:10 +0200 Subject: [PATCH 24/39] add config option to exclude files form old_file_locs --- Cargo.lock | 12 +++++++++++ Cargo.toml | 1 + helix-core/Cargo.toml | 2 +- helix-view/Cargo.toml | 3 +++ helix-view/src/editor.rs | 43 +++++++++++++++++++++++++++++++++------- helix-view/src/lib.rs | 1 + helix-view/src/regex.rs | 37 ++++++++++++++++++++++++++++++++++ 7 files changed, 91 insertions(+), 8 deletions(-) create mode 100644 helix-view/src/regex.rs diff --git a/Cargo.lock b/Cargo.lock index a5e8494f4b1f..25adff9ffcca 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1469,9 +1469,11 @@ dependencies = [ "log", "once_cell", "parking_lot", + "regex", "rustix", "serde", "serde_json", + "serde_regex", "slotmap", "tempfile", "thiserror 2.0.7", @@ -2207,6 +2209,16 @@ dependencies = [ "serde", ] +[[package]] +name = "serde_regex" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a8136f1a4ea815d7eac4101cfd0b16dc0cb5e1fe1b8609dfd728058656b7badf" +dependencies = [ + "regex", + "serde", +] + [[package]] name = "serde_repr" version = "0.1.19" diff --git a/Cargo.toml b/Cargo.toml index 753be4b462c4..c7999a056859 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -42,6 +42,7 @@ tree-sitter = { version = "0.22" } nucleo = "0.5.0" slotmap = "1.0.7" thiserror = "2.0" +regex = "1" [workspace.package] version = "24.7.0" diff --git a/helix-core/Cargo.toml b/helix-core/Cargo.toml index 22c91ca174f8..797b6e701297 100644 --- a/helix-core/Cargo.toml +++ b/helix-core/Cargo.toml @@ -35,7 +35,7 @@ slotmap.workspace = true tree-sitter.workspace = true once_cell = "1.20" arc-swap = "1" -regex = "1" +regex.workspace = true bitflags = "2.6" ahash = "0.8.11" hashbrown = { version = "0.14.5", features = ["raw"] } diff --git a/helix-view/Cargo.toml b/helix-view/Cargo.toml index 6f71fa05204f..266db07ef10c 100644 --- a/helix-view/Cargo.toml +++ b/helix-view/Cargo.toml @@ -46,9 +46,12 @@ chardetng = "0.1" serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" +serde_regex = "1.1.0" toml = "0.8" log = "~0.4" +regex.workspace = true + parking_lot = "0.12.3" thiserror.workspace = true diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index ec1a40c586ac..63d3919a7b24 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -10,6 +10,7 @@ use crate::{ info::Info, input::KeyEvent, persistence::{self, FileHistoryEntry}, + regex::EqRegex, register::Registers, theme::{self, Theme}, tree::{self, Tree}, @@ -61,6 +62,8 @@ use arc_swap::{ ArcSwap, }; +use regex::Regex; + pub const DEFAULT_AUTO_SAVE_DELAY: u64 = 3000; fn deserialize_duration_millis<'de, D>(deserializer: D) -> Result @@ -366,6 +369,7 @@ pub struct Config { pub persist_commands: bool, pub persist_search: bool, pub persist_clipboard: bool, + pub persistence_file_exclusions: Vec, } #[derive(Debug, Clone, PartialEq, Deserialize, Serialize, Eq, PartialOrd, Ord)] @@ -1011,6 +1015,11 @@ impl Default for Config { persist_commands: false, persist_search: false, persist_clipboard: false, + // TODO: any more defaults we should add here? + persistence_file_exclusions: [r".*/\.git/.*"] + .iter() + .map(|s| Regex::new(s).unwrap().into()) + .collect(), } } } @@ -1794,7 +1803,13 @@ impl Editor { self.switch(id, action); - if new_doc { + if new_doc + && !self + .config() + .persistence_file_exclusions + .iter() + .any(|r| r.is_match(&path.to_string_lossy())) + { if let Some((view_position, selection)) = self.old_file_locs.get(&path).map(|x| x.to_owned()) { @@ -1838,9 +1853,16 @@ impl Editor { if self.config().persist_old_files { for loc in file_locs { - persistence::push_file_history(&loc); - self.old_file_locs - .insert(loc.path, (loc.view_position, loc.selection)); + if !self + .config() + .persistence_file_exclusions + .iter() + .any(|r| r.is_match(&loc.path.to_string_lossy())) + { + persistence::push_file_history(&loc); + self.old_file_locs + .insert(loc.path, (loc.view_position, loc.selection)); + } } } @@ -1903,9 +1925,16 @@ impl Editor { if self.config().persist_old_files { for loc in file_locs { - persistence::push_file_history(&loc); - self.old_file_locs - .insert(loc.path, (loc.view_position, loc.selection)); + if !self + .config() + .persistence_file_exclusions + .iter() + .any(|r| r.is_match(&loc.path.to_string_lossy())) + { + persistence::push_file_history(&loc); + self.old_file_locs + .insert(loc.path, (loc.view_position, loc.selection)); + } } } diff --git a/helix-view/src/lib.rs b/helix-view/src/lib.rs index 0044ac618008..61955dcf12f5 100644 --- a/helix-view/src/lib.rs +++ b/helix-view/src/lib.rs @@ -14,6 +14,7 @@ pub mod info; pub mod input; pub mod keyboard; pub mod persistence; +pub mod regex; pub mod register; pub mod theme; pub mod tree; diff --git a/helix-view/src/regex.rs b/helix-view/src/regex.rs new file mode 100644 index 000000000000..8a491a3d8965 --- /dev/null +++ b/helix-view/src/regex.rs @@ -0,0 +1,37 @@ +use regex::Regex; +use serde::{Deserialize, Serialize}; +use std::{ + cmp::{Eq, PartialEq}, + ops::Deref, +}; + +/// Wrapper type for regex::Regex that only exists so we can implement Eq on it, as that's needed +/// to put it in editor::Config +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(transparent)] +pub struct EqRegex { + #[serde(with = "serde_regex")] + inner: Regex, +} + +impl From for EqRegex { + fn from(value: Regex) -> Self { + EqRegex { inner: value } + } +} + +impl Deref for EqRegex { + type Target = Regex; + + fn deref(&self) -> &Self::Target { + &self.inner + } +} + +impl PartialEq for EqRegex { + fn eq(&self, other: &Self) -> bool { + self.as_str() == other.as_str() + } +} + +impl Eq for EqRegex {} From 286bc85314a36e74638ec402524ed83271090fbb Mon Sep 17 00:00:00 2001 From: Ingrid Date: Wed, 1 May 2024 21:12:38 +0200 Subject: [PATCH 25/39] add trim config options for persistence --- helix-term/src/application.rs | 49 +++++++++++++++++++---------------- helix-view/src/editor.rs | 6 +++++ 2 files changed, 33 insertions(+), 22 deletions(-) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 3bc62bd96750..e27998e2f1ea 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -295,6 +295,32 @@ impl Application { ]) .context("build signal handler")?; + let jobs = Jobs::new(); + let file_trim = config.load().editor.persistence_old_files_trim; + jobs.add( + Job::new(async move { + persistence::trim_file_history(file_trim); + Ok(()) + }) + .wait_before_exiting(), + ); + let commands_trim = config.load().editor.persistence_commands_trim; + jobs.add( + Job::new(async move { + persistence::trim_command_history(commands_trim); + Ok(()) + }) + .wait_before_exiting(), + ); + let search_trim = config.load().editor.persistence_search_trim; + jobs.add( + Job::new(async move { + persistence::trim_search_history(search_trim); + Ok(()) + }) + .wait_before_exiting(), + ); + let app = Self { compositor, terminal, @@ -306,30 +332,9 @@ impl Application { syn_loader, signals, - jobs: Jobs::new(), + jobs, lsp_progress: LspProgressMap::new(), }; - app.jobs.add( - Job::new(async { - persistence::trim_file_history(5); - Ok(()) - }) - .wait_before_exiting(), - ); - app.jobs.add( - Job::new(async { - persistence::trim_command_history(5); - Ok(()) - }) - .wait_before_exiting(), - ); - app.jobs.add( - Job::new(async { - persistence::trim_search_history(5); - Ok(()) - }) - .wait_before_exiting(), - ); Ok(app) } diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 63d3919a7b24..dc95bfb3e04c 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -370,6 +370,9 @@ pub struct Config { pub persist_search: bool, pub persist_clipboard: bool, pub persistence_file_exclusions: Vec, + pub persistence_old_files_trim: usize, + pub persistence_commands_trim: usize, + pub persistence_search_trim: usize, } #[derive(Debug, Clone, PartialEq, Deserialize, Serialize, Eq, PartialOrd, Ord)] @@ -1020,6 +1023,9 @@ impl Default for Config { .iter() .map(|s| Regex::new(s).unwrap().into()) .collect(), + persistence_old_files_trim: 100, + persistence_commands_trim: 100, + persistence_search_trim: 100, } } } From 091c210811f690b7aab25073e6da38cc5fc044c0 Mon Sep 17 00:00:00 2001 From: Ingrid Date: Sun, 5 May 2024 13:39:55 +0200 Subject: [PATCH 26/39] add command to reload history --- helix-term/src/commands/typed.rs | 66 ++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index 57f2685d884c..70ea2bed7b76 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -2532,6 +2532,65 @@ fn read(cx: &mut compositor::Context, args: &[Cow], event: PromptEvent) -> Ok(()) } +fn reload_history( + cx: &mut compositor::Context, + _args: &[Cow], + event: PromptEvent, +) -> anyhow::Result<()> { + if event != PromptEvent::Validate { + return Ok(()); + } + + if cx.editor.config().persist_old_files { + cx.editor.old_file_locs = HashMap::from_iter( + persistence::read_file_history() + .into_iter() + .map(|entry| (entry.path.clone(), (entry.view_position, entry.selection))), + ); + let file_trim = cx.editor.config().persistence_old_files_trim; + cx.jobs.add( + Job::new(async move { + persistence::trim_file_history(file_trim); + Ok(()) + }) + .wait_before_exiting(), + ); + } + if cx.editor.config().persist_commands { + cx.editor + .registers + .write(':', persistence::read_command_history())?; + let commands_trim = cx.editor.config().persistence_commands_trim; + cx.jobs.add( + Job::new(async move { + persistence::trim_command_history(commands_trim); + Ok(()) + }) + .wait_before_exiting(), + ); + } + if cx.editor.config().persist_search { + cx.editor + .registers + .write('/', persistence::read_search_history())?; + let search_trim = cx.editor.config().persistence_search_trim; + cx.jobs.add( + Job::new(async move { + persistence::trim_search_history(search_trim); + Ok(()) + }) + .wait_before_exiting(), + ); + } + if cx.editor.config().persist_clipboard { + cx.editor + .registers + .write('"', persistence::read_clipboard_file())?; + } + + Ok(()) +} + pub const TYPABLE_COMMAND_LIST: &[TypableCommand] = &[ TypableCommand { name: "quit", @@ -3153,6 +3212,13 @@ pub const TYPABLE_COMMAND_LIST: &[TypableCommand] = &[ fun: read, signature: CommandSignature::positional(&[completers::filename]), }, + TypableCommand { + name: "reload-history", + aliases: &[], + doc: "Reload history files for persistent state", + fun: reload_history, + signature: CommandSignature::none(), + }, ]; pub static TYPABLE_COMMAND_MAP: Lazy> = From 41f61a13cf992c2fc8eb07b59512e93229ebc7a8 Mon Sep 17 00:00:00 2001 From: Ingrid Date: Mon, 2 Sep 2024 18:21:58 +0200 Subject: [PATCH 27/39] fix rebase breakage --- helix-view/src/editor.rs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index dc95bfb3e04c..dc4302a1aa53 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -1824,7 +1824,7 @@ impl Editor { let doc_len = doc.text().len_chars(); // don't restore the view and selection if the selection goes beyond the file's end if !selection.ranges().iter().any(|range| range.to() >= doc_len) { - view.offset = view_position; + doc.set_view_offset(view.id, view_position); doc.set_selection(view.id, selection); } } @@ -1834,20 +1834,15 @@ impl Editor { } pub fn close(&mut self, id: ViewId) { - let offset = self.tree.get(id).offset; - let mut file_locs = Vec::new(); for doc in self.documents_mut() { // Persist file location history for this view - // FIXME: The view offset here is currently wrong when a doc is not current for that view. - // Right now it uses the current offset of the view, which is on a another document. - // We need to persist ViewPositions on documents a la PR #7568, then fetch that here. if doc.selections().contains_key(&id) { if let Some(path) = doc.path() { file_locs.push(FileHistoryEntry::new( path.clone(), - offset, + doc.view_offset(id), doc.selection(id).clone(), )); } @@ -1911,7 +1906,7 @@ impl Editor { if let Some(path) = doc.path() { file_locs.push(FileHistoryEntry::new( path.clone(), - view.offset, + doc.view_offset(view.id), doc.selection(view.id).clone(), )); }; From 1cc2299a0ca7554a09b3d0c5d9f4fb116242467e Mon Sep 17 00:00:00 2001 From: Ingrid Date: Mon, 2 Sep 2024 19:05:26 +0200 Subject: [PATCH 28/39] add .*/COMMIT_EDITMSG to persistent file exclusions useful in the case of bare git repos, where the git dir is not always named .git, and so the previous exclusion wouldn't catch it. --- helix-view/src/editor.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index dc4302a1aa53..7e102921f420 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -1019,7 +1019,7 @@ impl Default for Config { persist_search: false, persist_clipboard: false, // TODO: any more defaults we should add here? - persistence_file_exclusions: [r".*/\.git/.*"] + persistence_file_exclusions: [r".*/\.git/.*", r".*/COMMIT_EDITMSG"] .iter() .map(|s| Regex::new(s).unwrap().into()) .collect(), From f1af50988f2b868f8f5cf6f2d2ba3176f720491c Mon Sep 17 00:00:00 2001 From: Ingrid Date: Tue, 3 Sep 2024 14:58:20 +0200 Subject: [PATCH 29/39] default to /helix/state if state dir is None --- helix-loader/src/lib.rs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/helix-loader/src/lib.rs b/helix-loader/src/lib.rs index 5d6e34c5b516..264c6ff03f3e 100644 --- a/helix-loader/src/lib.rs +++ b/helix-loader/src/lib.rs @@ -168,9 +168,17 @@ pub fn cache_dir() -> PathBuf { pub fn state_dir() -> PathBuf { // TODO: allow env var override let strategy = choose_base_strategy().expect("Unable to find the state directory!"); - let mut path = strategy.state_dir().unwrap(); - path.push("helix"); - path + match strategy.state_dir() { + Some(mut path) => { + path.push("helix"); + path + } + None => { + let mut path = strategy.cache_dir(); + path.push("helix/state"); + path + } + } } pub fn config_file() -> PathBuf { From fa6314797c05649f8c1f0d7a9895d6584a26b342 Mon Sep 17 00:00:00 2001 From: Ingrid Date: Mon, 9 Sep 2024 17:18:45 +0200 Subject: [PATCH 30/39] run docgen --- book/src/generated/typable-cmd.md | 1 + 1 file changed, 1 insertion(+) diff --git a/book/src/generated/typable-cmd.md b/book/src/generated/typable-cmd.md index f0d9a0f492a5..4fb6d41919ce 100644 --- a/book/src/generated/typable-cmd.md +++ b/book/src/generated/typable-cmd.md @@ -88,3 +88,4 @@ | `:move`, `:mv` | Move the current buffer and its corresponding file to a different path | | `:yank-diagnostic` | Yank diagnostic(s) under primary cursor to register, or clipboard by default | | `:read`, `:r` | Load a file into buffer | +| `:reload-history` | Reload history files for persistent state | From 45cfed37331eb12488de37b087f306171e5596d0 Mon Sep 17 00:00:00 2001 From: Ingrid Date: Mon, 9 Sep 2024 18:01:01 +0200 Subject: [PATCH 31/39] split persistence config options into own struct --- helix-term/src/application.rs | 14 +++---- helix-term/src/commands.rs | 2 +- helix-term/src/commands/typed.rs | 14 +++---- helix-term/src/ui/prompt.rs | 4 +- helix-view/src/editor.rs | 67 ++++++++++++++++++++------------ 5 files changed, 59 insertions(+), 42 deletions(-) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index e27998e2f1ea..869fceed536f 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -143,7 +143,7 @@ impl Application { let mut compositor = Compositor::new(area); let config = Arc::new(ArcSwap::from_pointee(config)); let handlers = handlers::setup(config.clone()); - let old_file_locs = if config.load().editor.persist_old_files { + let old_file_locs = if config.load().editor.persistence.old_files { HashMap::from_iter( persistence::read_file_history() .into_iter() @@ -164,21 +164,21 @@ impl Application { ); // TODO: do most of this in the background? - if config.load().editor.persist_commands { + if config.load().editor.persistence.commands { editor .registers .write(':', persistence::read_command_history()) // TODO: do something about this unwrap .unwrap(); } - if config.load().editor.persist_search { + if config.load().editor.persistence.search { editor .registers .write('/', persistence::read_search_history()) // TODO: do something about this unwrap .unwrap(); } - if config.load().editor.persist_clipboard { + if config.load().editor.persistence.clipboard { editor .registers .write('"', persistence::read_clipboard_file()) @@ -296,7 +296,7 @@ impl Application { .context("build signal handler")?; let jobs = Jobs::new(); - let file_trim = config.load().editor.persistence_old_files_trim; + let file_trim = config.load().editor.persistence.old_files_trim; jobs.add( Job::new(async move { persistence::trim_file_history(file_trim); @@ -304,7 +304,7 @@ impl Application { }) .wait_before_exiting(), ); - let commands_trim = config.load().editor.persistence_commands_trim; + let commands_trim = config.load().editor.persistence.commands_trim; jobs.add( Job::new(async move { persistence::trim_command_history(commands_trim); @@ -312,7 +312,7 @@ impl Application { }) .wait_before_exiting(), ); - let search_trim = config.load().editor.persistence_search_trim; + let search_trim = config.load().editor.persistence.search_trim; jobs.add( Job::new(async move { persistence::trim_search_history(search_trim); diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index fe914341b2dd..e09ebecb42e8 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -4316,7 +4316,7 @@ fn yank_impl(editor: &mut Editor, register: char) { .collect(); let selections = values.len(); - if editor.config().persist_clipboard { + if editor.config().persistence.clipboard { persistence::write_clipboard_file(&values); } diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index 70ea2bed7b76..5590ede50f6e 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -2541,13 +2541,13 @@ fn reload_history( return Ok(()); } - if cx.editor.config().persist_old_files { + if cx.editor.config().persistence.old_files { cx.editor.old_file_locs = HashMap::from_iter( persistence::read_file_history() .into_iter() .map(|entry| (entry.path.clone(), (entry.view_position, entry.selection))), ); - let file_trim = cx.editor.config().persistence_old_files_trim; + let file_trim = cx.editor.config().persistence.old_files_trim; cx.jobs.add( Job::new(async move { persistence::trim_file_history(file_trim); @@ -2556,11 +2556,11 @@ fn reload_history( .wait_before_exiting(), ); } - if cx.editor.config().persist_commands { + if cx.editor.config().persistence.commands { cx.editor .registers .write(':', persistence::read_command_history())?; - let commands_trim = cx.editor.config().persistence_commands_trim; + let commands_trim = cx.editor.config().persistence.commands_trim; cx.jobs.add( Job::new(async move { persistence::trim_command_history(commands_trim); @@ -2569,11 +2569,11 @@ fn reload_history( .wait_before_exiting(), ); } - if cx.editor.config().persist_search { + if cx.editor.config().persistence.search { cx.editor .registers .write('/', persistence::read_search_history())?; - let search_trim = cx.editor.config().persistence_search_trim; + let search_trim = cx.editor.config().persistence.search_trim; cx.jobs.add( Job::new(async move { persistence::trim_search_history(search_trim); @@ -2582,7 +2582,7 @@ fn reload_history( .wait_before_exiting(), ); } - if cx.editor.config().persist_clipboard { + if cx.editor.config().persistence.clipboard { cx.editor .registers .write('"', persistence::read_clipboard_file())?; diff --git a/helix-term/src/ui/prompt.rs b/helix-term/src/ui/prompt.rs index 7d09926ef243..5473575f92ab 100644 --- a/helix-term/src/ui/prompt.rs +++ b/helix-term/src/ui/prompt.rs @@ -613,8 +613,8 @@ impl Component for Prompt { { cx.editor.set_error(err.to_string()); } - if (cx.editor.config().persist_commands && register == ':') - || (cx.editor.config().persist_search && register == '/') + if (cx.editor.config().persistence.commands && register == ':') + || (cx.editor.config().persistence.search && register == '/') { persistence::push_reg_history(register, &self.line); } diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 7e102921f420..83903a64313d 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -365,14 +365,7 @@ pub struct Config { pub end_of_line_diagnostics: DiagnosticFilter, // Set to override the default clipboard provider pub clipboard_provider: ClipboardProvider, - pub persist_old_files: bool, - pub persist_commands: bool, - pub persist_search: bool, - pub persist_clipboard: bool, - pub persistence_file_exclusions: Vec, - pub persistence_old_files_trim: usize, - pub persistence_commands_trim: usize, - pub persistence_search_trim: usize, + pub persistence: PersistenceConfig, } #[derive(Debug, Clone, PartialEq, Deserialize, Serialize, Eq, PartialOrd, Ord)] @@ -957,6 +950,38 @@ pub enum PopupBorderConfig { Menu, } +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "kebab-case", default, deny_unknown_fields)] +pub struct PersistenceConfig { + pub old_files: bool, + pub commands: bool, + pub search: bool, + pub clipboard: bool, + pub old_files_exclusions: Vec, + pub old_files_trim: usize, + pub commands_trim: usize, + pub search_trim: usize, +} + +impl Default for PersistenceConfig { + fn default() -> Self { + Self { + old_files: false, + commands: false, + search: false, + clipboard: false, + // TODO: any more defaults we should add here? + old_files_exclusions: [r".*/\.git/.*", r".*/COMMIT_EDITMSG"] + .iter() + .map(|s| Regex::new(s).unwrap().into()) + .collect(), + old_files_trim: 100, + commands_trim: 100, + search_trim: 100, + } + } +} + impl Default for Config { fn default() -> Self { Self { @@ -1014,18 +1039,7 @@ impl Default for Config { inline_diagnostics: InlineDiagnosticsConfig::default(), end_of_line_diagnostics: DiagnosticFilter::Disable, clipboard_provider: ClipboardProvider::default(), - persist_old_files: false, - persist_commands: false, - persist_search: false, - persist_clipboard: false, - // TODO: any more defaults we should add here? - persistence_file_exclusions: [r".*/\.git/.*", r".*/COMMIT_EDITMSG"] - .iter() - .map(|s| Regex::new(s).unwrap().into()) - .collect(), - persistence_old_files_trim: 100, - persistence_commands_trim: 100, - persistence_search_trim: 100, + persistence: PersistenceConfig::default(), } } } @@ -1812,7 +1826,8 @@ impl Editor { if new_doc && !self .config() - .persistence_file_exclusions + .persistence + .old_files_exclusions .iter() .any(|r| r.is_match(&path.to_string_lossy())) { @@ -1852,11 +1867,12 @@ impl Editor { doc.remove_view(id); } - if self.config().persist_old_files { + if self.config().persistence.old_files { for loc in file_locs { if !self .config() - .persistence_file_exclusions + .persistence + .old_files_exclusions .iter() .any(|r| r.is_match(&loc.path.to_string_lossy())) { @@ -1924,11 +1940,12 @@ impl Editor { }) .collect(); - if self.config().persist_old_files { + if self.config().persistence.old_files { for loc in file_locs { if !self .config() - .persistence_file_exclusions + .persistence + .old_files_exclusions .iter() .any(|r| r.is_match(&loc.path.to_string_lossy())) { From 7eb4183589d15897b5a8f01000939839ca646a71 Mon Sep 17 00:00:00 2001 From: Ingrid Date: Wed, 11 Sep 2024 15:49:27 +0200 Subject: [PATCH 32/39] add documentation for persistent state --- book/src/editor.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/book/src/editor.md b/book/src/editor.md index feec09fd0e34..3adb49fd62ac 100644 --- a/book/src/editor.md +++ b/book/src/editor.md @@ -463,3 +463,20 @@ end-of-line-diagnostics = "hint" [editor.inline-diagnostics] cursor-line = "warning" # show warnings and errors on the cursorline inline ``` + +### `[editor.persistence]` Section + +Options for persisting editor state between sessions. + +The state is formatted with bincode, and stored in files in the state directory (`~/.local/state/helix` on Unix, `~\Local Settings\Application Data\helix\state` on Windows). You can reset your persisted state (and recover from any corruption) by deleting these files. + +| Key | Description | Default | +| --- | ----------- | ------- | +| `old-files` | whether to persist file locations between sessions ( when you reopen the a file, it will open at the place you last closed it) | `false` | +| `commands` | whether to persist command history between sessions | `false` | +| `search` | whether to persist search history between sessions | `false` | +| `clipboard` | whether to persist helix's internal clipboard between sessions | `false` | +| `old-files-exclusions` | a list of regexes defining file paths to exclude from persistence | `[".*/\.git/.*", ".*/COMMIT_EDITMSG"]` | +| `old-files-trim` | number of old-files entries to keep when helix trims the state files at startup | `100` | +| `commands-trim` | number of command history entries to keep when helix trims the state files at startup | `100` | +| `search-trim` | number of search history entries to keep when helix trims the state files at startup | `100` | From 24b802935b68cef5af24eb8fc96897272f280d13 Mon Sep 17 00:00:00 2001 From: Ingrid Date: Sun, 22 Sep 2024 13:15:44 +0200 Subject: [PATCH 33/39] only trim persistent state files if persistent state is enabled --- helix-term/src/application.rs | 54 +++++++++++++++++++---------------- 1 file changed, 30 insertions(+), 24 deletions(-) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 869fceed536f..1eb98ab8c7cd 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -296,30 +296,36 @@ impl Application { .context("build signal handler")?; let jobs = Jobs::new(); - let file_trim = config.load().editor.persistence.old_files_trim; - jobs.add( - Job::new(async move { - persistence::trim_file_history(file_trim); - Ok(()) - }) - .wait_before_exiting(), - ); - let commands_trim = config.load().editor.persistence.commands_trim; - jobs.add( - Job::new(async move { - persistence::trim_command_history(commands_trim); - Ok(()) - }) - .wait_before_exiting(), - ); - let search_trim = config.load().editor.persistence.search_trim; - jobs.add( - Job::new(async move { - persistence::trim_search_history(search_trim); - Ok(()) - }) - .wait_before_exiting(), - ); + if config.load().editor.persistence.old_files { + let file_trim = config.load().editor.persistence.old_files_trim; + jobs.add( + Job::new(async move { + persistence::trim_file_history(file_trim); + Ok(()) + }) + .wait_before_exiting(), + ); + } + if config.load().editor.persistence.commands { + let commands_trim = config.load().editor.persistence.commands_trim; + jobs.add( + Job::new(async move { + persistence::trim_command_history(commands_trim); + Ok(()) + }) + .wait_before_exiting(), + ); + } + if config.load().editor.persistence.search { + let search_trim = config.load().editor.persistence.search_trim; + jobs.add( + Job::new(async move { + persistence::trim_search_history(search_trim); + Ok(()) + }) + .wait_before_exiting(), + ); + } let app = Self { compositor, From c60ad8949d38af525318d9079a05594c3e03b474 Mon Sep 17 00:00:00 2001 From: Ingrid Date: Sun, 22 Sep 2024 13:17:05 +0200 Subject: [PATCH 34/39] add integration test for persistent state --- helix-term/tests/integration.rs | 1 + helix-term/tests/test/helpers.rs | 23 +---- helix-term/tests/test/persistence.rs | 142 +++++++++++++++++++++++++++ 3 files changed, 144 insertions(+), 22 deletions(-) create mode 100644 helix-term/tests/test/persistence.rs diff --git a/helix-term/tests/integration.rs b/helix-term/tests/integration.rs index 35214bcb8011..6d5b3caa7e44 100644 --- a/helix-term/tests/integration.rs +++ b/helix-term/tests/integration.rs @@ -20,6 +20,7 @@ mod test { mod commands; mod languages; mod movement; + mod persistence; mod prompt; mod splits; } diff --git a/helix-term/tests/test/helpers.rs b/helix-term/tests/test/helpers.rs index 623f5df3e77a..be39caedd03a 100644 --- a/helix-term/tests/test/helpers.rs +++ b/helix-term/tests/test/helpers.rs @@ -11,7 +11,7 @@ use helix_core::{diagnostic::Severity, test, Selection, Transaction}; use helix_loader; use helix_term::{application::Application, args::Args, config::Config, keymap::merge_keys}; use helix_view::{current_ref, doc, editor::LspConfig, input::parse_macro, Editor}; -use tempfile::{NamedTempFile, TempPath}; +use tempfile::NamedTempFile; use tokio_stream::wrappers::UnboundedReceiverStream; /// Specify how to set up the input text with line feeds @@ -230,26 +230,6 @@ pub fn test_syntax_loader(overrides: Option) -> helix_core::syntax::Load helix_core::syntax::Loader::new(lang.try_into().unwrap()).unwrap() } -fn init_persistence_files() -> anyhow::Result<(TempPath, TempPath, TempPath, TempPath)> { - let command_file = NamedTempFile::new()?; - let command_path = command_file.into_temp_path(); - helix_loader::initialize_command_histfile(Some(command_path.to_path_buf())); - - let search_file = NamedTempFile::new()?; - let search_path = search_file.into_temp_path(); - helix_loader::initialize_search_histfile(Some(search_path.to_path_buf())); - - let file_file = NamedTempFile::new()?; - let file_path = file_file.into_temp_path(); - helix_loader::initialize_file_histfile(Some(file_path.to_path_buf())); - - let clipboard_file = NamedTempFile::new()?; - let clipboard_path = clipboard_file.into_temp_path(); - helix_loader::initialize_clipboard_file(Some(clipboard_path.to_path_buf())); - - Ok((command_path, search_path, file_path, clipboard_path)) -} - /// Use this for very simple test cases where there is one input /// document, selection, and sequence of key presses, and you just /// want to verify the resulting document and selection. @@ -257,7 +237,6 @@ pub async fn test_with_config>( app_builder: AppBuilder, test_case: T, ) -> anyhow::Result<()> { - let (_, _, _, _) = init_persistence_files()?; let test_case = test_case.into(); let app = app_builder.build()?; diff --git a/helix-term/tests/test/persistence.rs b/helix-term/tests/test/persistence.rs new file mode 100644 index 000000000000..34bacb4264f9 --- /dev/null +++ b/helix-term/tests/test/persistence.rs @@ -0,0 +1,142 @@ +use super::*; +use helix_term::{config::Config, keymap}; +use helix_view::editor; +use std::{fs::File, io::Read}; +use tempfile::{NamedTempFile, TempPath}; + +fn init_persistence_files() -> anyhow::Result<(TempPath, TempPath, TempPath, TempPath)> { + let command_file = NamedTempFile::new()?; + let command_path = command_file.into_temp_path(); + helix_loader::initialize_command_histfile(Some(command_path.to_path_buf())); + + let search_file = NamedTempFile::new()?; + let search_path = search_file.into_temp_path(); + helix_loader::initialize_search_histfile(Some(search_path.to_path_buf())); + + let file_file = NamedTempFile::new()?; + let file_path = file_file.into_temp_path(); + helix_loader::initialize_file_histfile(Some(file_path.to_path_buf())); + + let clipboard_file = NamedTempFile::new()?; + let clipboard_path = clipboard_file.into_temp_path(); + helix_loader::initialize_clipboard_file(Some(clipboard_path.to_path_buf())); + + Ok((command_path, search_path, file_path, clipboard_path)) +} + +fn config_with_persistence() -> Config { + let mut editor_config = editor::Config::default(); + editor_config.persistence.old_files = true; + editor_config.persistence.commands = true; + editor_config.persistence.search = true; + editor_config.persistence.clipboard = true; + editor_config.persistence.search_trim = 3; + + Config { + theme: None, + keys: keymap::default(), + editor: editor_config, + } +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_persistence() -> anyhow::Result<()> { + let (_, search_histfile_path, _, _) = init_persistence_files()?; + let mut file = tempfile::NamedTempFile::new()?; + + // Session 1: + // open a new file, + // add a newline, then a, + // write-quit + test_key_sequence( + &mut helpers::AppBuilder::new() + .with_config(config_with_persistence()) + .with_file(file.path(), None) + .build()?, + // TODO: remove the h with a bugfix? + Some("oah:wq"), + // Some(&|app| { + // assert!(!app.editor.is_err(), "error: {:?}", app.editor.get_status()); + // }), + None, + true, + ) + .await?; + + // Sanity check contents of file after first session + helpers::assert_file_has_content(&mut file, "\na\n")?; + + // Session 2: + // open same file, + // add newline, then b, + // copy the line + // search for "a" + // go back down to b + // use last command + test_key_sequence( + &mut helpers::AppBuilder::new() + .with_config(config_with_persistence()) + .with_file(file.path(), None) + .build()?, + Some("obxy/aj:"), + None, + true, + ) + .await?; + + // This verifies both that the file position was persisted (since the b is inserted after the + // a), and the command history (":" resolves to the ":wq" from session 1) + helpers::assert_file_has_content(&mut file, "\na\nb\n")?; + + // Session 3: + // open same file, + // paste + // use last search + // append a + // search for "1", "2", and "3" in sequence. + // use last command + test_key_sequence( + &mut helpers::AppBuilder::new() + .with_config(config_with_persistence()) + .with_file(file.path(), None) + .build()?, + Some("p/aa/1/2/3:"), + None, + true, + ) + .await?; + + // This verifies search history was persisted ("/" resolves to "/a" from session 2), and + // the clipboard was persisted (paste pastes the "b\n" copied in session 2) + helpers::assert_file_has_content(&mut file, "\naa\nb\nb\n")?; + + // Session 4: + // open same file + // use last command + test_key_sequence( + &mut helpers::AppBuilder::new() + .with_config(config_with_persistence()) + .with_file(file.path(), None) + .build()?, + Some(":"), + None, + true, + ) + .await?; + + // NOTE: This time we check the search history file, instead of the edited file + let mut search_histfile = File::open(search_histfile_path)?; + let mut search_histfile_contents = String::new(); + search_histfile.read_to_string(&mut search_histfile_contents)?; + // This verifies that trimming the persistent state files is working correctly, because + // session 3 sent more searches (4: "/a", "/1", "/2", "/3") than the trim limit (3), so when + // session 4 starts, it should perform a trim, removing the oldest entry ("/a") while leaving + // the other 3 intact. + // The weird looking format of the string is because persistence data is encoded using bincode. + assert_eq!( + search_histfile_contents, + "\u{1}\0\0\0\0\0\0\01\u{1}\0\0\0\0\0\0\02\u{1}\0\0\0\0\0\0\03" + ); + + Ok(()) +} From 3933bf5cf515feb10f038c1d216ed8ad62a66706 Mon Sep 17 00:00:00 2001 From: Ingrid Date: Sun, 22 Sep 2024 14:14:37 +0200 Subject: [PATCH 35/39] avoid repeated loading of config to check persistence config in startup --- helix-term/src/application.rs | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 1eb98ab8c7cd..594ac7ec98e4 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -143,7 +143,8 @@ impl Application { let mut compositor = Compositor::new(area); let config = Arc::new(ArcSwap::from_pointee(config)); let handlers = handlers::setup(config.clone()); - let old_file_locs = if config.load().editor.persistence.old_files { + let persistence_config = config.load().editor.persistence.clone(); + let old_file_locs = if persistence_config.old_files { HashMap::from_iter( persistence::read_file_history() .into_iter() @@ -164,21 +165,21 @@ impl Application { ); // TODO: do most of this in the background? - if config.load().editor.persistence.commands { + if persistence_config.commands { editor .registers .write(':', persistence::read_command_history()) // TODO: do something about this unwrap .unwrap(); } - if config.load().editor.persistence.search { + if persistence_config.search { editor .registers .write('/', persistence::read_search_history()) // TODO: do something about this unwrap .unwrap(); } - if config.load().editor.persistence.clipboard { + if persistence_config.clipboard { editor .registers .write('"', persistence::read_clipboard_file()) @@ -296,8 +297,8 @@ impl Application { .context("build signal handler")?; let jobs = Jobs::new(); - if config.load().editor.persistence.old_files { - let file_trim = config.load().editor.persistence.old_files_trim; + if persistence_config.old_files { + let file_trim = persistence_config.old_files_trim; jobs.add( Job::new(async move { persistence::trim_file_history(file_trim); @@ -306,8 +307,8 @@ impl Application { .wait_before_exiting(), ); } - if config.load().editor.persistence.commands { - let commands_trim = config.load().editor.persistence.commands_trim; + if persistence_config.commands { + let commands_trim = persistence_config.commands_trim; jobs.add( Job::new(async move { persistence::trim_command_history(commands_trim); @@ -316,8 +317,8 @@ impl Application { .wait_before_exiting(), ); } - if config.load().editor.persistence.search { - let search_trim = config.load().editor.persistence.search_trim; + if persistence_config.search { + let search_trim = persistence_config.search_trim; jobs.add( Job::new(async move { persistence::trim_search_history(search_trim); From d407d3dcfdf8faa3e53167115740e6ab92ebad7c Mon Sep 17 00:00:00 2001 From: Ingrid Date: Sun, 22 Sep 2024 16:41:46 +0200 Subject: [PATCH 36/39] address hanging TODOs --- helix-loader/src/persistence.rs | 26 ++++++++++++++++---------- helix-term/src/application.rs | 20 ++++++-------------- helix-term/src/commands/typed.rs | 4 +--- helix-term/tests/test/persistence.rs | 16 ++++++---------- helix-view/src/editor.rs | 17 +++++++---------- helix-view/src/persistence.rs | 6 +++--- 6 files changed, 39 insertions(+), 50 deletions(-) diff --git a/helix-loader/src/persistence.rs b/helix-loader/src/persistence.rs index c9ec9687199d..01997779424a 100644 --- a/helix-loader/src/persistence.rs +++ b/helix-loader/src/persistence.rs @@ -12,11 +12,9 @@ pub fn write_history(filepath: PathBuf, entries: &Vec) { .create(true) .truncate(true) .open(filepath) - // TODO: do something about this unwrap .unwrap(); for entry in entries { - // TODO: do something about this unwrap serialize_into(&file, &entry).unwrap(); } } @@ -26,19 +24,23 @@ pub fn push_history(filepath: PathBuf, entry: &T) { .append(true) .create(true) .open(filepath) - // TODO: do something about this unwrap .unwrap(); - // TODO: do something about this unwrap serialize_into(file, entry).unwrap(); } -pub fn read_history Deserialize<'a>>(filepath: PathBuf) -> Vec { +pub fn read_history Deserialize<'a>>(filepath: &PathBuf) -> Vec { match File::open(filepath) { Ok(file) => { let mut read = BufReader::new(file); let mut entries = Vec::new(); - // TODO: more sophisticated error handling + // FIXME: Can we do better error handling here? It's unfortunate that bincode doesn't + // distinguish an empty reader from an actual error. + // + // Perhaps we could use the underlying bufreader to check for emptiness in the while + // condition, then we could know any errors from bincode should be surfaced or logged. + // BufRead has a method `has_data_left` that would work for this, but at the time of + // writing it is nightly-only and experimental :( while let Ok(entry) = deserialize_from(&mut read) { entries.push(entry); } @@ -46,8 +48,13 @@ pub fn read_history Deserialize<'a>>(filepath: PathBuf) -> Vec { } Err(e) => match e.kind() { io::ErrorKind::NotFound => Vec::new(), - // TODO: do something about this panic - _ => panic!(), + // Going through the potential errors listed from the docs: + // - `InvalidInput` can't happen since we aren't setting options + // - `AlreadyExists` can't happen since we aren't setting `create_new` + // - `PermissionDenied` could happen if someone really borked their file permissions + // in `~/.local`, but helix already panics in that case, and I think a panic is + // acceptable. + _ => unreachable!(), }, } } @@ -56,8 +63,7 @@ pub fn trim_history Deserialize<'a>>( filepath: PathBuf, limit: usize, ) { - // TODO: can we remove this clone? - let history: Vec = read_history(filepath.clone()); + let history: Vec = read_history(&filepath); if history.len() > limit { let trim_start = history.len() - limit; let trimmed_history = history[trim_start..].to_vec(); diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 594ac7ec98e4..c233403df0bc 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -164,26 +164,23 @@ impl Application { old_file_locs, ); - // TODO: do most of this in the background? + // Should we be doing these in background tasks? if persistence_config.commands { editor .registers .write(':', persistence::read_command_history()) - // TODO: do something about this unwrap .unwrap(); } if persistence_config.search { editor .registers .write('/', persistence::read_search_history()) - // TODO: do something about this unwrap .unwrap(); } if persistence_config.clipboard { editor .registers .write('"', persistence::read_clipboard_file()) - // TODO: do something about this unwrap .unwrap(); } @@ -229,7 +226,7 @@ impl Application { None => Action::Load, }; let old_id = editor.document_id_by_path(&file); - let doc_id = match editor.open(&file, action) { + match editor.open(&file, action) { // Ignore irregular files during application init. Err(DocumentOpenError::IrregularFile) => { nr_of_files -= 1; @@ -239,20 +236,19 @@ impl Application { // We can't open more than 1 buffer for 1 file, in this case we already have opened this file previously Ok(doc_id) if old_id == Some(doc_id) => { nr_of_files -= 1; - doc_id } - Ok(doc_id) => doc_id, + Ok(_) => (), }; // with Action::Load all documents have the same view // NOTE: this isn't necessarily true anymore. If // `--vsplit` or `--hsplit` are used, the file which is // opened last is focused on. if let Some(pos) = pos { - let view_id = editor.tree.focus; - let doc = doc_mut!(editor, &doc_id); + let (view, doc) = current!(editor); let pos = Selection::point(pos_at_coords(doc.text().slice(..), pos, true)); - doc.set_selection(view_id, pos); + doc.set_selection(view.id, pos); + align_view(doc, view, Align::Center); } } } @@ -266,10 +262,6 @@ impl Application { nr_of_files, if nr_of_files == 1 { "" } else { "s" } // avoid "Loaded 1 files." grammo )); - // align the view to center after all files are loaded, - // does not affect views without pos since it is at the top - // let (view, doc) = current!(editor); - // align_view(doc, view, Align::Center); } } else { editor.new_file(Action::VerticalSplit); diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index 5590ede50f6e..c37f0b3f9b60 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -133,10 +133,8 @@ fn open(cx: &mut compositor::Context, args: &[Cow], event: PromptEvent) -> if let Some(pos) = pos { let pos = Selection::point(pos_at_coords(doc.text().slice(..), pos, true)); doc.set_selection(view.id, pos); + align_view(doc, view, Align::Center); } - // does not affect opening a buffer without pos - // TODO: ensure removing this will not cause problems - // align_view(doc, view, Align::Center); } } Ok(()) diff --git a/helix-term/tests/test/persistence.rs b/helix-term/tests/test/persistence.rs index 34bacb4264f9..ac244ecda427 100644 --- a/helix-term/tests/test/persistence.rs +++ b/helix-term/tests/test/persistence.rs @@ -53,11 +53,7 @@ async fn test_persistence() -> anyhow::Result<()> { .with_config(config_with_persistence()) .with_file(file.path(), None) .build()?, - // TODO: remove the h with a bugfix? - Some("oah:wq"), - // Some(&|app| { - // assert!(!app.editor.is_err(), "error: {:?}", app.editor.get_status()); - // }), + Some("oa:wq"), None, true, ) @@ -69,10 +65,10 @@ async fn test_persistence() -> anyhow::Result<()> { // Session 2: // open same file, // add newline, then b, - // copy the line + // copy the line ("b\n") // search for "a" // go back down to b - // use last command + // use last command (write-quit) test_key_sequence( &mut helpers::AppBuilder::new() .with_config(config_with_persistence()) @@ -91,10 +87,10 @@ async fn test_persistence() -> anyhow::Result<()> { // Session 3: // open same file, // paste - // use last search + // use last search ("/a") // append a // search for "1", "2", and "3" in sequence. - // use last command + // use last command (write-quit) test_key_sequence( &mut helpers::AppBuilder::new() .with_config(config_with_persistence()) @@ -112,7 +108,7 @@ async fn test_persistence() -> anyhow::Result<()> { // Session 4: // open same file - // use last command + // use last command (write-quit) test_key_sequence( &mut helpers::AppBuilder::new() .with_config(config_with_persistence()) diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 83903a64313d..a7e43e02edce 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -1791,14 +1791,9 @@ impl Editor { let path = helix_stdx::path::canonicalize(path); let id = self.document_id_by_path(&path); - // TODO: surely there's a neater way to do this? - let mut new_doc = false; - - let id = if let Some(id) = id { - id + let (id, new_doc) = if let Some(id) = id { + (id, false) } else { - new_doc = true; - let mut doc = Document::open( &path, None, @@ -1818,11 +1813,13 @@ impl Editor { let id = self.new_document(doc); self.launch_language_servers(id); - id + (id, true) }; self.switch(id, action); + // Restore file position + // This needs to happen after the call to switch, since switch messes with view offsets if new_doc && !self .config() @@ -1837,8 +1834,8 @@ impl Editor { let (view, doc) = current!(self); let doc_len = doc.text().len_chars(); - // don't restore the view and selection if the selection goes beyond the file's end - if !selection.ranges().iter().any(|range| range.to() >= doc_len) { + // Don't restore the view and selection if the selection goes beyond the file's end + if !selection.ranges().iter().any(|range| range.to() > doc_len) { doc.set_view_offset(view.id, view_position); doc.set_selection(view.id, selection); } diff --git a/helix-view/src/persistence.rs b/helix-view/src/persistence.rs index 332fc03929e1..754b08aa4b8a 100644 --- a/helix-view/src/persistence.rs +++ b/helix-view/src/persistence.rs @@ -31,7 +31,7 @@ pub fn push_file_history(entry: &FileHistoryEntry) { } pub fn read_file_history() -> Vec { - read_history(file_histfile()) + read_history(&file_histfile()) } pub fn trim_file_history(limit: usize) { @@ -49,7 +49,7 @@ pub fn push_reg_history(register: char, line: &String) { } fn read_reg_history(filepath: PathBuf) -> Vec { - read_history(filepath) + read_history(&filepath) } pub fn read_command_history() -> Vec { @@ -77,5 +77,5 @@ pub fn write_clipboard_file(values: &Vec) { } pub fn read_clipboard_file() -> Vec { - read_history(clipboard_file()) + read_history(&clipboard_file()) } From ea5d2df1dc86d550149d49510f833b02da262da2 Mon Sep 17 00:00:00 2001 From: Ingrid Date: Sun, 22 Sep 2024 17:00:18 +0200 Subject: [PATCH 37/39] fix line feed handling in integration test for windows --- helix-term/tests/test/persistence.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/helix-term/tests/test/persistence.rs b/helix-term/tests/test/persistence.rs index ac244ecda427..94cdad3cdfd6 100644 --- a/helix-term/tests/test/persistence.rs +++ b/helix-term/tests/test/persistence.rs @@ -60,7 +60,7 @@ async fn test_persistence() -> anyhow::Result<()> { .await?; // Sanity check contents of file after first session - helpers::assert_file_has_content(&mut file, "\na\n")?; + helpers::assert_file_has_content(&mut file, &LineFeedHandling::Native.apply("\na\n"))?; // Session 2: // open same file, @@ -82,7 +82,7 @@ async fn test_persistence() -> anyhow::Result<()> { // This verifies both that the file position was persisted (since the b is inserted after the // a), and the command history (":" resolves to the ":wq" from session 1) - helpers::assert_file_has_content(&mut file, "\na\nb\n")?; + helpers::assert_file_has_content(&mut file, &LineFeedHandling::Native.apply("\na\nb\n"))?; // Session 3: // open same file, @@ -104,7 +104,7 @@ async fn test_persistence() -> anyhow::Result<()> { // This verifies search history was persisted ("/" resolves to "/a" from session 2), and // the clipboard was persisted (paste pastes the "b\n" copied in session 2) - helpers::assert_file_has_content(&mut file, "\naa\nb\nb\n")?; + helpers::assert_file_has_content(&mut file, &LineFeedHandling::Native.apply("\naa\nb\nb\n"))?; // Session 4: // open same file From 6d3fda8c18a303470b991e2532e8c7f30ab553d1 Mon Sep 17 00:00:00 2001 From: "Daniel J. Bell" Date: Tue, 4 Aug 2026 07:14:41 -0400 Subject: [PATCH 38/39] docs: persistence merge notes for future rebases --- docs/persistence-merge-notes.md | 290 ++++++++++++++++++++++++++++++++ 1 file changed, 290 insertions(+) create mode 100644 docs/persistence-merge-notes.md diff --git a/docs/persistence-merge-notes.md b/docs/persistence-merge-notes.md new file mode 100644 index 000000000000..0e8e63c044d3 --- /dev/null +++ b/docs/persistence-merge-notes.md @@ -0,0 +1,290 @@ +# Persistence merge notes + +How cross-session history was ported from upstream PR +[helix-editor/helix#9143](https://github.com/helix-editor/helix/pull/9143) +(`intarga/helix` branch `persistent_state`) onto this gj1118-based tree. + +Published branch on this fork: **`persistent_state`** +([bellhyve/helix](https://github.com/bellhyve/helix/tree/persistent_state)). +Frozen upstream tip: `vendor/intarga-persistent_state` (`ea5d2df1`). + +**Purpose of this doc:** simplify rebase after a gj1118/`master` bump. + +**First land commit:** `0f409a7a` (merge of `intarga/persistent_state` + conflict +resolution + one behavioral fix). + +--- + +## What this feature is + +Opt-in persistence under `[editor.persistence]`: + +| Key | Effect | Default | +|-----|--------|---------| +| `commands` | `:` history survives quit | `false` | +| `search` | `/` history survives quit | `false` | +| `old-files` | reopen file at last cursor/view | `false` | +| `clipboard` | Helix internal `"` register | `false` | +| `*-trim` | max entries kept on startup trim | `100` | +| `old-files-exclusions` | regex paths skipped for old-files | git + `COMMIT_EDITMSG` | + +On-disk format: **bincode**, append/rewrite under XDG state: + +- Unix: `~/.local/state/helix/` +- files: `command_history`, `search_history`, `file_history`, `clipboard` + +Command: `:reload-history` — re-read histfiles into the live session (and +re-queue trim jobs). + +**Not** session restore of splits/buffers. **Not** a plugin — it touches +registers, `Editor::open`/`close`/`close_document`, startup, and config. + +Upstream status as of 2026-08: PR closed unmerged (author fatigue / no +maintainer review). Branch still existed at `intarga/helix:persistent_state` +(`ea5d2df1`). + +--- + +## Source map from #9143 + +New files from #9143 (probably keep whole): + +- `helix-loader/src/persistence.rs` — bincode read/write/trim +- `helix-view/src/persistence.rs` — register + file-history helpers +- `helix-view/src/regex.rs` — `EqRegex` wrapper for config exclusions +- `helix-term/tests/test/persistence.rs` — multi-session integration test +- book section: `book/src/editor.md` → `[editor.persistence]` + +Conflicts: + +| Area | Why it conflicts | +|------|------------------| +| `helix-view/src/editor.rs` | Config struct, `Editor::new` signature, `open`/`close`/`close_document` | +| `helix-term/src/application.rs` | startup load of hist + trim jobs; file-open CLI positions | +| `helix-term/src/commands/typed.rs` | `:reload-history`; command table grows often in gj1118 | +| `helix-term/src/ui/prompt.rs` | push `:`/`/` lines on validate | +| `helix-term/src/commands.rs` | yank → optional clipboard file write | +| `helix-term/src/main.rs` | `initialize_*_histfile` / clipboard path | +| `helix-loader/src/lib.rs` | `state_dir`, histfile OnceCells + getters | +| Cargo.toml(s) | `bincode`, `serde_regex`, `regex` workspace, `smallvec` serde feature | +| `helix-core/src/selection.rs` | `Serialize`/`Deserialize` on `Range`/`Selection` | +| `helix-view/src/view.rs` | same on `ViewPosition` | + +--- + +## Merge strategy that worked + +1. **Remote:** `git remote add intarga https://github.com/intarga/helix.git` (if missing). +2. **Fetch:** `git fetch intarga persistent_state`. +3. **Merge** (prefer over raw patch — 3-way helps): + ```bash + git merge intarga/persistent_state --no-ff + ``` +4. **Do not** blindly take “theirs” on big gj1118 files (`editor.rs`, + `application.rs`, `typed.rs`). Those carry noice cmdline, plugins, + workspace-trust, notifications, local_search, etc. +5. **Do** take new persistence modules and loader histfile helpers intact. +6. **Cargo.lock:** restore ours if mangled, then let `cargo check -p helix-term` + add only `bincode` / `serde_regex`. Avoid `cargo generate-lockfile`. + +If `intarga/persistent_state` disappears later, recover from this branch’s +merge commit `0f409a7a`, from `vendor/intarga-persistent_state`, or from the +upstream PR patch on #9143. + +--- + +## Conflict playbook (by file) + +### Always keep both modules (loader) + +```rust +pub mod workspace_trust; +pub mod persistence; +``` + +Keep **both** `data_dir()` (gj1118/upstream HEAD) and `state_dir()` (persistence). +Histfiles live under `state_dir()`, not cache. + +### Cargo / deps + +Workspace `Cargo.toml`: keep current workspace deps; add: + +```toml +regex = "1" +``` + +`helix-core`: keep workspace `ropey` / `bitflags` / `foldhash`; use + +```toml +smallvec = { version = "1.15", features = ["serde"] } # version: match tree +regex.workspace = true +``` + +`helix-view`: keep workspace `toml` / `parking_lot`; add `serde_regex`, +`regex.workspace = true`. + +`helix-loader`: add `bincode = "1.3.3"` (version flexible if API stable). + +### `Editor::new` signature + +Persistence adds a final argument: + +```rust +old_file_locs: HashMap, +``` + +**Keep** gj1118’s `workspace_trust: WorkspaceTrust` argument. Order used on +first land: + +```text +area, theme_loader, syn_loader, config, handlers, workspace_trust, old_file_locs +``` + +Every `Editor::new(` call site must pass both (today: `application.rs` only). + +### `application.rs` startup sequence + +After `ArcSwap` config exists: + +1. `persistence_config = config.load().editor.persistence.clone()` +2. If `old_files`: build `old_file_locs` from `persistence::read_file_history()` +3. `Editor::new(..., workspace_trust, old_file_locs)` +4. If enabled: `registers.write(':', command_history)`, same for `/` and `"` +5. Queue trim jobs with `Job::new(...).wait_before_exiting()` (Jobs::add is + `&self` — `mut jobs` not required) + +Imports needed: `persistence`, `Job`, `HashMap`. + +### Opinionated fix (not in original PR) — CLI position clobber + +**Problem:** After `editor.open()`, gj1118/HEAD always applied CLI file +positions. `parse_file` yields `Position::default()` (0,0) when the user did +not pass `:line`. That **wipes** `old-files` restore from inside `open()`. + +**Fix** (keep this on every rebase): + +```rust +let apply_cli_pos = match pos.as_slice() { + [] => false, + [p] if *p == helix_core::Position::default() => false, + _ => true, +}; +if apply_cli_pos { + // existing set_selection from CLI coords +} +``` + +**Tradeoff:** `hx file:1:1` is also (0,0) after saturating_sub and will not +force-apply. Do **not** drop this fix or the integration test fails on +session-2 insert order. + +Original PR used `Vec<(PathBuf, Option)>` so “no line” was `None`. +This tree uses `IndexMap>` — keep that API; only skip +implicit default. + +### `typed.rs` + +- Add `reload_history` with current signature: + `fn(..., args: Args, event: PromptEvent)` (not old `&[Cow]`). +- Register at end of `TYPABLE_COMMAND_LIST` with + `CommandCompleter::none()` + `Signature { positionals: (0, Some(0)), .. }`. +- gj1118 often appends commands (notifications, workspace-trust, plugins). + Re-apply the table entry after those blocks; don’t delete theirs. + +### `close` / `close_document` + +When saving `FileHistoryEntry`, `doc.path()` is `&Path` — use +`path.to_path_buf()`, not `path.clone()` (type error). + +`:wq` → `write_*` then `quit` → `editor.close(view_id)`. File locs must be +recorded in **`close`**, not only `close_document`. + +### Tests + +- `helix-term/tests/integration.rs`: `mod persistence;` +- `helpers.rs` `with_file`: prefer empty position list when `pos` is `None`: + ```rust + self.args.files.insert(path.into(), pos.map(|p| vec![p]).unwrap_or_default()); + ``` + so tests don’t inject a fake (0,0). +- Histfiles use `OnceCell` — `initialize_*_histfile(Some(temp))` only works + once per process. The persistence test sets temps at start; fine for that + test binary run. + +### Args / parse_file + +If a merge tries to change `files` to `Vec<(PathBuf, Option)>`, +**reject** and keep IndexMap + multi-position behavior unless you’re ready to +update every call site (application open loop, helpers, `:open`). + +--- + +## Problems hit on first land + +1. **Huge conflict hunks in `editor.rs` / `typed.rs` / `application.rs`** + Auto-merge interleaved multi-hundred-line HEAD-only UI with small PR + inserts. Resolution: `git checkout --ours` those three, then **surgically** + re-apply persistence (Config field, PersistenceConfig type, open/close + hooks, startup load, reload command). Faster than hand-merging noice UI. + +2. **`cargo generate-lockfile`** + Unlocked the world; pulled `kstring` needing rustc 1.96 while Homebrew was + 1.94. Fix: restore `Cargo.lock` from HEAD, `cargo check` to add only new + crates. + +3. **Integration test failed first run (`\nb\na\n` vs `\na\nb\n`)** + Root cause: CLI default position clobber (see above). Not a broken histfile. + +4. **`cp target/release/hx ~/.cargo/bin/hx` → exit 137** + Prefer `cargo install --path helix-term --locked --force`. After install, + `hx -V` should print. + +5. **Upstream PR is not “almost merged”** + Closed by author after ping spam; no maintainer sign-off. Treat as + vendored feature on this fork, not something that will land upstream soon. + +--- + +## Config snippet + +```toml +[editor.persistence] +commands = true +search = true +old-files = true +# clipboard = true +# commands-trim = 100 +# search-trim = 100 +# old-files-trim = 100 +``` + +Nuke corrupted state: `rm -rf ~/.local/state/helix` + +Verify after a rebase: + +```bash +cargo test -p helix-term --features integration test_persistence +``` + +--- + +## Rebase checklist + +- [ ] `git merge master` (or `upstream/master`) into `persistent_state` +- [ ] `helix-loader`: both `workspace_trust` + `persistence`; both `data_dir` + `state_dir` +- [ ] `Editor::new` still has `workspace_trust` **and** `old_file_locs` +- [ ] `application.rs`: load hist before new; register writes; trim jobs; **CLI pos skip** +- [ ] `prompt.rs`: push_reg_history on `:` / `/` +- [ ] `typed.rs`: `:reload-history` still in table +- [ ] `close` / `close_document`: `to_path_buf()` on paths +- [ ] `cargo check -p helix-term` +- [ ] `cargo test -p helix-term --features integration test_persistence` +- [ ] Manual: quit/reopen, history recall in `:` and `/` + +--- + +## Credits + +- Original implementation: **intarga** — [helix-editor/helix#9143](https://github.com/helix-editor/helix/pull/9143) +- Base editor fork: **gj1118/helix** +- Port + CLI position fix + these notes: **bellhyve/helix**, 2026-08 From dec8ae04592fdf11b050027eb10c0967e91034ba Mon Sep 17 00:00:00 2001 From: "Daniel J. Bell" Date: Thu, 6 Aug 2026 22:23:06 -0400 Subject: [PATCH 39/39] fmt --- helix-loader/src/lib.rs | 2 +- helix-term/src/application.rs | 3 +-- helix-term/src/commands.rs | 7 +------ helix-term/tests/test/helpers.rs | 7 +++---- helix-view/src/editor.rs | 5 ++--- 5 files changed, 8 insertions(+), 16 deletions(-) diff --git a/helix-loader/src/lib.rs b/helix-loader/src/lib.rs index 5552dbddd41d..0587a1dce7e2 100644 --- a/helix-loader/src/lib.rs +++ b/helix-loader/src/lib.rs @@ -1,7 +1,7 @@ pub mod config; pub mod grammar; -pub mod workspace_trust; pub mod persistence; +pub mod workspace_trust; use helix_stdx::{env::current_working_dir, path}; diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 45d9e8d92152..9de223bab478 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -13,8 +13,7 @@ use helix_view::{ editor::{ConfigEvent, EditorEvent}, events::EditorConfigDidChange, graphics::Rect, - persistence, - theme, + persistence, theme, tree::Layout, Align, Editor, }; diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index cb2ec725444b..eb0afee1ff18 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -80,12 +80,7 @@ use crate::{ use crate::job::{self, Jobs}; use std::{ - cmp::Ordering, - collections::{HashMap, HashSet}, - error::Error, - fmt, - future::Future, - io::Read, + cmp::Ordering, collections::HashSet, error::Error, fmt, future::Future, io::Read, num::NonZeroUsize, }; diff --git a/helix-term/tests/test/helpers.rs b/helix-term/tests/test/helpers.rs index 7b83694a2fd7..6bc7acfd78b5 100644 --- a/helix-term/tests/test/helpers.rs +++ b/helix-term/tests/test/helpers.rs @@ -374,10 +374,9 @@ impl AppBuilder { path: P, pos: Option, ) -> Self { - self.args.files.insert( - path.into(), - pos.map(|p| vec![p]).unwrap_or_default(), - ); + self.args + .files + .insert(path.into(), pos.map(|p| vec![p]).unwrap_or_default()); self } diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 6c8ed851716c..6c50c89391f4 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -10,8 +10,8 @@ use crate::{ info::Info, input::KeyEvent, persistence::{self, FileHistoryEntry}, - register::Registers, regex::EqRegex, + register::Registers, theme::{self, Theme}, tree::{self, Dimension, Resize, Tree}, view::ViewPosition, @@ -19,8 +19,8 @@ use crate::{ }; use helix_event::dispatch; use helix_loader::workspace_trust::{ImplicitTrustLevel, TrustQuery, WorkspaceTrust}; -use regex::Regex; use helix_vcs::DiffProviderRegistry; +use regex::Regex; use futures_util::stream::select_all::SelectAll; use futures_util::StreamExt; @@ -1712,7 +1712,6 @@ impl Default for CompletionHighlight { } } - #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "kebab-case", default, deny_unknown_fields)] pub struct PersistenceConfig {