From 6bdf609caaf4eb1c137f503f147d1e4e4f3e8676 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Mon, 14 Jun 2021 23:26:05 -0400 Subject: [PATCH] Remove RwLock for registers Registers are stored inside `Editor` and accessed without `RwLock`. To work around ownership, I added a sister method to `Editor::current`: `Editor::current_with_context`. I tried to modify `Editor::current` directly but it's used at a lot of places so I reverted into this for now at least. --- helix-core/src/register.rs | 77 ++++++++++++++----- helix-term/src/commands.rs | 145 +++++++++++++++++++++--------------- helix-term/src/ui/editor.rs | 4 +- helix-term/src/ui/mod.rs | 7 +- helix-view/src/editor.rs | 14 +++- 5 files changed, 161 insertions(+), 86 deletions(-) diff --git a/helix-core/src/register.rs b/helix-core/src/register.rs index 0176d23e..cc881a17 100644 --- a/helix-core/src/register.rs +++ b/helix-core/src/register.rs @@ -1,20 +1,63 @@ -use crate::Tendril; -use once_cell::sync::Lazy; -use std::{collections::HashMap, sync::RwLock}; - -// TODO: could be an instance on Editor -static REGISTRY: Lazy>>> = - Lazy::new(|| RwLock::new(HashMap::new())); - -/// Read register values. -pub fn get(register_name: char) -> Option> { - let registry = REGISTRY.read().unwrap(); - registry.get(®ister_name).cloned() // TODO: no cloning +use std::collections::HashMap; + +#[derive(Debug)] +pub struct Register { + name: char, + values: Vec, +} + +impl Register { + pub fn new(name: char) -> Self { + Self { + name, + values: Vec::new(), + } + } + + pub fn new_with_values(name: char, values: Vec) -> Self { + Self { name, values } + } + + pub fn name(&self) -> char { + self.name + } + + pub fn read(&self) -> &Vec { + &self.values + } + + pub fn write(&mut self, values: Vec) { + self.values = values; + } } -/// Read register values. -// restoring: bool -pub fn set(register_name: char, values: Vec) { - let mut registry = REGISTRY.write().unwrap(); - registry.insert(register_name, values); +/// Currently just wraps a `HashMap` of `Register`s +#[derive(Debug, Default)] +pub struct Registers { + inner: HashMap, +} + +impl Registers { + pub fn get(&self, name: char) -> Option<&Register> { + self.inner.get(&name) + } + + pub fn get_mut(&mut self, name: char) -> Option<&mut Register> { + self.inner.get_mut(&name) + } + + pub fn get_or_insert(&mut self, name: char) -> &mut Register { + self.inner + .entry(name) + .or_insert_with(|| Register::new(name)) + } + + pub fn write(&mut self, name: char, values: Vec) { + self.inner + .insert(name, Register::new_with_values(name, values)); + } + + pub fn read(&self, name: char) -> Option<&Vec> { + self.get(name).map(|reg| reg.read()) + } } diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 382137d1..c80716d4 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -4,8 +4,9 @@ use helix_core::{ movement::{self, Direction}, object, pos_at_coords, regex::{self, Regex}, - register, search, selection, Change, ChangeSet, Position, Range, Rope, RopeSlice, Selection, - SmallVec, Tendril, Transaction, + register::{self, Register, Registers}, + search, selection, Change, ChangeSet, Position, Range, Rope, RopeSlice, Selection, SmallVec, + Tendril, Transaction, }; use helix_view::{ @@ -39,7 +40,7 @@ use crossterm::event::{KeyCode, KeyEvent}; use once_cell::sync::Lazy; pub struct Context<'a> { - pub register: helix_view::RegisterSelection, + pub selected_register: helix_view::RegisterSelection, pub count: Option, pub editor: &'a mut Editor, @@ -65,6 +66,11 @@ impl<'a> Context<'a> { self.editor.current() } + #[inline] + pub fn current_with_registers(&mut self) -> (&mut View, &mut Document, &mut Registers) { + self.editor.current_with_registers() + } + /// Push a new component onto the compositor. pub fn push_layer(&mut self, mut component: Box) { self.callback = Some(Box::new(|compositor: &mut Compositor| { @@ -631,7 +637,7 @@ pub fn select_all(cx: &mut Context) { } pub fn select_regex(cx: &mut Context) { - let prompt = ui::regex_prompt(cx, "select:".to_string(), move |view, doc, regex| { + let prompt = ui::regex_prompt(cx, "select:".to_string(), move |view, doc, _, regex| { let text = doc.text().slice(..); if let Some(selection) = selection::select_on_matches(text, doc.selection(view.id), ®ex) { @@ -643,7 +649,7 @@ pub fn select_regex(cx: &mut Context) { } pub fn split_selection(cx: &mut Context) { - let prompt = ui::regex_prompt(cx, "split:".to_string(), move |view, doc, regex| { + let prompt = ui::regex_prompt(cx, "split:".to_string(), move |view, doc, _, regex| { let text = doc.text().slice(..); let selection = selection::split_on_matches(text, doc.selection(view.id), ®ex); doc.set_selection(view.id, selection); @@ -714,20 +720,24 @@ pub fn search(cx: &mut Context) { let contents = doc.text().slice(..).to_string(); let view_id = view.id; - let prompt = ui::regex_prompt(cx, "search:".to_string(), move |view, doc, regex| { - search_impl(doc, view, &contents, ®ex, false); - // TODO: only store on enter (accept), not update - register::set('\\', vec![regex.as_str().to_string()]); - }); + let prompt = ui::regex_prompt( + cx, + "search:".to_string(), + move |view, doc, registers, regex| { + search_impl(doc, view, &contents, ®ex, false); + // TODO: only store on enter (accept), not update + registers.write('\\', vec![regex.as_str().to_string()]); + }, + ); cx.push_layer(Box::new(prompt)); } // can't search next for ""compose"" for some reason pub fn search_next_impl(cx: &mut Context, extend: bool) { - if let Some(query) = register::get('\\') { + let (view, doc, registers) = cx.current_with_registers(); + if let Some(query) = registers.read('\\') { let query = query.first().unwrap(); - let (view, doc) = cx.current(); let contents = doc.text().slice(..).to_string(); let regex = Regex::new(query).unwrap(); search_impl(doc, view, &contents, ®ex, extend); @@ -747,7 +757,7 @@ pub fn search_selection(cx: &mut Context) { let contents = doc.text().slice(..); let query = doc.selection(view.id).primary().fragment(contents); let regex = regex::escape(&query); - register::set('\\', vec![regex]); + cx.editor.registers.write('\\', vec![regex]); search_next(cx); } @@ -794,7 +804,7 @@ pub fn extend_line(cx: &mut Context) { // heuristic: append changes to history after each command, unless we're in insert mode -fn delete_selection_impl(reg: char, doc: &mut Document, view_id: ViewId) { +fn delete_selection_impl(reg: &mut Register, doc: &mut Document, view_id: ViewId) { // first yank the selection let values: Vec = doc .selection(view_id) @@ -802,7 +812,7 @@ fn delete_selection_impl(reg: char, doc: &mut Document, view_id: ViewId) { .map(Cow::into_owned) .collect(); - register::set(reg, values); + reg.write(values); // then delete let transaction = @@ -815,8 +825,9 @@ fn delete_selection_impl(reg: char, doc: &mut Document, view_id: ViewId) { } pub fn delete_selection(cx: &mut Context) { - let reg = cx.register.name(); - let (view, doc) = cx.current(); + let reg_name = cx.selected_register.name(); + let (view, doc, registers) = cx.current_with_registers(); + let reg = registers.get_or_insert(reg_name); delete_selection_impl(reg, doc, view.id); doc.append_changes_to_history(view.id); @@ -826,8 +837,9 @@ pub fn delete_selection(cx: &mut Context) { } pub fn change_selection(cx: &mut Context) { - let reg = cx.register.name(); - let (view, doc) = cx.current(); + let reg_name = cx.selected_register.name(); + let (view, doc, registers) = cx.current_with_registers(); + let reg = registers.get_or_insert(reg_name); delete_selection_impl(reg, doc, view.id); enter_insert_mode(doc); } @@ -2227,10 +2239,12 @@ pub fn yank(cx: &mut Context) { let msg = format!( "yanked {} selection(s) to register {}", values.len(), - cx.register.name() + cx.selected_register.name() ); - register::set(cx.register.name(), values); + cx.editor + .registers + .write(cx.selected_register.name(), values); cx.editor.set_status(msg) } @@ -2241,47 +2255,48 @@ enum Paste { After, } -fn paste_impl(reg: char, doc: &mut Document, view: &View, action: Paste) -> Option { - if let Some(values) = register::get(reg) { - let repeat = std::iter::repeat( - values - .last() - .map(|value| Tendril::from_slice(value)) - .unwrap(), - ); +fn paste_impl( + values: &[String], + doc: &mut Document, + view: &View, + action: Paste, +) -> Option { + let repeat = std::iter::repeat( + values + .last() + .map(|value| Tendril::from_slice(value)) + .unwrap(), + ); - // if any of values ends \n it's linewise paste - let linewise = values.iter().any(|value| value.ends_with('\n')); + // if any of values ends \n it's linewise paste + let linewise = values.iter().any(|value| value.ends_with('\n')); - let mut values = values.into_iter().map(Tendril::from).chain(repeat); + let mut values = values.iter().cloned().map(Tendril::from).chain(repeat); - let text = doc.text(); + let text = doc.text(); - let transaction = - Transaction::change_by_selection(doc.text(), doc.selection(view.id), |range| { - let pos = match (action, linewise) { - // paste linewise before - (Paste::Before, true) => text.line_to_char(text.char_to_line(range.from())), - // paste linewise after - (Paste::After, true) => text.line_to_char(text.char_to_line(range.to()) + 1), - // paste insert - (Paste::Before, false) => range.from(), - // paste append - (Paste::After, false) => range.to() + 1, - }; - (pos, pos, Some(values.next().unwrap())) - }); - return Some(transaction); - } - None + let transaction = Transaction::change_by_selection(text, doc.selection(view.id), |range| { + let pos = match (action, linewise) { + // paste linewise before + (Paste::Before, true) => text.line_to_char(text.char_to_line(range.from())), + // paste linewise after + (Paste::After, true) => text.line_to_char(text.char_to_line(range.to()) + 1), + // paste insert + (Paste::Before, false) => range.from(), + // paste append + (Paste::After, false) => range.to() + 1, + }; + (pos, pos, Some(values.next().unwrap())) + }); + + Some(transaction) } pub fn replace_with_yanked(cx: &mut Context) { - let reg = cx.register.name(); - - if let Some(values) = register::get(reg) { - let (view, doc) = cx.current(); + let reg_name = cx.selected_register.name(); + let (view, doc, registers) = cx.current_with_registers(); + if let Some(values) = registers.read(reg_name) { if let Some(yank) = values.first() { let transaction = Transaction::change_by_selection(doc.text(), doc.selection(view.id), |range| { @@ -2307,20 +2322,26 @@ pub fn replace_with_yanked(cx: &mut Context) { // default insert pub fn paste_after(cx: &mut Context) { - let reg = cx.register.name(); - let (view, doc) = cx.current(); + let reg_name = cx.selected_register.name(); + let (view, doc, registers) = cx.current_with_registers(); - if let Some(transaction) = paste_impl(reg, doc, view, Paste::After) { + if let Some(transaction) = registers + .read(reg_name) + .and_then(|values| paste_impl(values, doc, view, Paste::After)) + { doc.apply(&transaction, view.id); doc.append_changes_to_history(view.id); } } pub fn paste_before(cx: &mut Context) { - let reg = cx.register.name(); - let (view, doc) = cx.current(); + let reg_name = cx.selected_register.name(); + let (view, doc, registers) = cx.current_with_registers(); - if let Some(transaction) = paste_impl(reg, doc, view, Paste::Before) { + if let Some(transaction) = registers + .read(reg_name) + .and_then(|values| paste_impl(values, doc, view, Paste::Before)) + { doc.apply(&transaction, view.id); doc.append_changes_to_history(view.id); } @@ -2494,7 +2515,7 @@ pub fn join_selections(cx: &mut Context) { pub fn keep_selections(cx: &mut Context) { // keep selections matching regex - let prompt = ui::regex_prompt(cx, "keep:".to_string(), move |view, doc, regex| { + let prompt = ui::regex_prompt(cx, "keep:".to_string(), move |view, doc, _, regex| { let text = doc.text().slice(..); if let Some(selection) = selection::keep_matches(text, doc.selection(view.id), ®ex) { @@ -2788,7 +2809,7 @@ pub fn select_register(cx: &mut Context) { .. } = event { - cx.editor.register.select(ch); + cx.editor.selected_register.select(ch); } }) } diff --git a/helix-term/src/ui/editor.rs b/helix-term/src/ui/editor.rs index 4b4b9fb2..5913df29 100644 --- a/helix-term/src/ui/editor.rs +++ b/helix-term/src/ui/editor.rs @@ -570,7 +570,7 @@ impl EditorView { // debug_assert!(cxt.count != 0); // set the register - cxt.register = cxt.editor.register.take(); + cxt.selected_register = cxt.editor.selected_register.take(); if let Some(command) = self.keymap[&mode].get(&event) { command(cxt); @@ -610,7 +610,7 @@ impl Component for EditorView { let mode = doc.mode(); let mut cxt = commands::Context { - register: helix_view::RegisterSelection::default(), + selected_register: helix_view::RegisterSelection::default(), editor: &mut cx.editor, count: None, callback: None, diff --git a/helix-term/src/ui/mod.rs b/helix-term/src/ui/mod.rs index 7e4464bc..77e53690 100644 --- a/helix-term/src/ui/mod.rs +++ b/helix-term/src/ui/mod.rs @@ -20,6 +20,7 @@ pub use tui::layout::Rect; pub use tui::style::{Color, Modifier, Style}; use helix_core::regex::Regex; +use helix_core::register::Registers; use helix_view::{Document, Editor, View}; use std::path::{Path, PathBuf}; @@ -27,7 +28,7 @@ use std::path::{Path, PathBuf}; pub fn regex_prompt( cx: &mut crate::commands::Context, prompt: String, - fun: impl Fn(&mut View, &mut Document, Regex) + 'static, + fun: impl Fn(&mut View, &mut Document, &mut Registers, Regex) + 'static, ) -> Prompt { let view_id = cx.view().id; let snapshot = cx.doc().selection(view_id).clone(); @@ -53,13 +54,13 @@ pub fn regex_prompt( match Regex::new(input) { Ok(regex) => { - let (view, doc) = editor.current(); + let (view, doc, registers) = editor.current_with_registers(); // revert state to what it was before the last update // TODO: also revert text doc.set_selection(view.id, snapshot.clone()); - fun(view, doc, regex); + fun(view, doc, registers, regex); view.ensure_cursor_in_view(doc); } diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index ef0d8213..2fd01817 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -8,13 +8,15 @@ use slotmap::SlotMap; use anyhow::Error; pub use helix_core::diagnostic::Severity; +pub use helix_core::register::Registers; #[derive(Debug)] pub struct Editor { pub tree: Tree, pub documents: SlotMap, pub count: Option, - pub register: RegisterSelection, + pub selected_register: RegisterSelection, + pub registers: Registers, pub theme: Theme, pub language_servers: helix_lsp::Registry, @@ -59,9 +61,10 @@ impl Editor { tree: Tree::new(area), documents: SlotMap::with_key(), count: None, - register: RegisterSelection::default(), + selected_register: RegisterSelection::default(), theme, language_servers, + registers: Registers::default(), status_msg: None, } } @@ -239,6 +242,13 @@ impl Editor { (view, doc) } + pub fn current_with_registers(&mut self) -> (&mut View, &mut Document, &mut Registers) { + let view = self.tree.get_mut(self.tree.focus); + let doc = &mut self.documents[view.doc]; + let registers = &mut self.registers; + (view, doc, registers) + } + pub fn view(&self) -> &View { self.tree.get(self.tree.focus) }