From 773a2a56ffb5a63821ab3e69d45d29771d03d354 Mon Sep 17 00:00:00 2001 From: misson20000 Date: Sat, 20 Jan 2024 22:39:04 -0500 Subject: [PATCH] Update properties editor to be able to edit multiple nodes at once using new Change::AlterNodesBulk --- src/view/error.rs | 2 + src/view/props_editor.rs | 278 ++++++++++++++++++++++++++++----------- src/view/window.rs | 2 + 3 files changed, 205 insertions(+), 77 deletions(-) diff --git a/src/view/error.rs b/src/view/error.rs index 5c3829a..6456950 100644 --- a/src/view/error.rs +++ b/src/view/error.rs @@ -22,6 +22,7 @@ pub enum Action { InsertNodeParseSize, InsertNode, Nest, + EditProperties, ModifyTreeSelection, RubberBandSelection, @@ -72,6 +73,7 @@ impl Error { Action::InsertNodeParseSize => "Failed to parse size.", Action::InsertNode => "Failed to insert node.", Action::Nest => "Failed to nest nodes.", + Action::EditProperties => "Failed to edit node properties.", Action::ModifyTreeSelection => "Failed to modify tree selection.", Action::RubberBandSelection => "Failed to rubber-band select.", diff --git a/src/view/props_editor.rs b/src/view/props_editor.rs index 83d34f6..ede6e1c 100644 --- a/src/view/props_editor.rs +++ b/src/view/props_editor.rs @@ -1,4 +1,5 @@ use std::cell; +use std::rc; use std::rc::Rc; use std::sync; @@ -11,15 +12,28 @@ use crate::model::document; use crate::model::document::structure; use crate::model::selection; use crate::model::versioned::Versioned; +use crate::view::error; use crate::view::helpers; use crate::view::window; +enum PropsEditorMode { + Deactivated, + Single { + path: structure::Path, + props: structure::Properties, + }, + Many { + selection: sync::Arc, + props: structure::MaybeProperties, + }, +} + struct PropsInterior { document_host: sync::Arc, selection_host: sync::Arc, selection: sync::Arc, - current: Option<(structure::Path, structure::Properties)>, + mode: PropsEditorMode, subscriber: helpers::AsyncSubscriber, } @@ -43,6 +57,7 @@ pub struct PropsEditor { in_update: cell::Cell, interior: cell::RefCell>, + window: cell::RefCell>, } impl PropsEditor { @@ -80,6 +95,7 @@ impl PropsEditor { path_display, in_update: cell::Cell::new(false), interior: cell::RefCell::new(None), + window: Default::default(), }; pe.unbind(); @@ -87,49 +103,116 @@ impl PropsEditor { let pe = Rc::new(pe); pe.name_entry.buffer().connect_text_notify(clone!(@weak pe => move |buffer| { - pe.change_prop(|props| props.name = buffer.text().to_string()); + pe.apply_props(structure::MaybeProperties::new_name(buffer.text().to_string())); })); pe.title_display.connect_selected_notify(clone!(@weak pe => move |dd| { - pe.change_prop(|props| props.title_display = match dd.selected() { + pe.apply_props(structure::MaybeProperties::new_title_display(match dd.selected() { 0 => structure::TitleDisplay::Inline, 1 => structure::TitleDisplay::Major, 2 => structure::TitleDisplay::Minor, + gtk::INVALID_LIST_POSITION => return, x => panic!("unexpected selected index: {}", x) - }); + })); })); pe.children_display.connect_selected_notify(clone!(@weak pe => move |dd| { - pe.change_prop(|props| props.children_display = match dd.selected() { + pe.apply_props(structure::MaybeProperties::new_children_display(match dd.selected() { 0 => structure::ChildrenDisplay::None, 1 => structure::ChildrenDisplay::Summary, 2 => structure::ChildrenDisplay::Full, + gtk::INVALID_LIST_POSITION => return, x => panic!("unexpected selected index: {}", x) - }); + })); })); pe.content_display.connect_selected_notify(clone!(@weak pe => move |dd| { - pe.change_prop(|props| props.content_display = match dd.selected() { + pe.apply_props(structure::MaybeProperties::new_content_display(match dd.selected() { 0 => structure::ContentDisplay::None, 1 => structure::ContentDisplay::default_hexdump(), 2 => structure::ContentDisplay::Hexstring, + gtk::INVALID_LIST_POSITION => return, x => panic!("unexpected selected index: {}", x) - }); + })); })); pe } - fn change_prop(&self, cb: F) { + pub fn bind_window(&self, window: &Rc) { + *self.window.borrow_mut() = Rc::downgrade(window); + } + + fn apply_props(&self, prop_changes: structure::MaybeProperties) { if !self.in_update.get() { let mut interior_guard = self.interior.borrow_mut(); if let Some(interior) = interior_guard.as_mut() { - if let Some((path, props)) = interior.current.as_mut() { - cb(props); + let window = match self.window.borrow().upgrade() { + Some(window) => window, + None => return, + }; + + match match &mut interior.mode { + PropsEditorMode::Deactivated => { /* silently discard this error */ return; }, + + PropsEditorMode::Single { path, props } => { + props.apply_changes(prop_changes); + + interior.document_host.change(interior.selection.document.alter_node(path.clone(), props.clone())) + }, + + PropsEditorMode::Many { selection, props } => { + props.apply_changes(prop_changes.clone()); + + interior.document_host.change(interior.selection.document.alter_nodes_bulk(selection.clone(), prop_changes)) + }, + } { + Ok(new_document) => { + /* Sometimes gtk will cause us to issue two property updates in + * quick succession without a chance for the selection to get + * updated and for us to pick that up, which would cause the second + * update to get issued against an old version of the selection + * referencing an old version of the document. We can't have that, + * so update the selection synchronously right here right now. */ + + let new_selection = match interior.selection_host.change(selection::tree::Change::DocumentUpdated(new_document)) { + Ok(new) => new, + + Err((selection::tree::ApplyError::WasUpToDate, _)) => return, + + Err((error, attempted_version)) => { + window.report_error(error::Error { + while_attempting: error::Action::EditProperties, + trouble: error::Trouble::TreeSelectionUpdateFailure { + error, + attempted_version + }, + level: error::Level::Warning, + is_bug: true, + }); + + return + }, + }; + + self.in_update.set(true); + new_selection.changes_since(&interior.selection.clone(), &mut |selection, record| self.update_selection_internal(interior, selection.clone(), Some(record))); + self.in_update.set(false); + }, - if let Err(e) = interior.document_host.change(interior.selection.document.alter_node(path.clone(), props.clone())) { - println!("failed to alter node: {:?}", e); - } + Err((error, attempted_version)) => { + window.report_error(error::Error { + while_attempting: error::Action::EditProperties, + trouble: error::Trouble::DocumentUpdateFailure { + error, + attempted_version + }, + level: error::Level::Error, + is_bug: false, + }); + + return; + }, } } } @@ -150,12 +233,12 @@ impl PropsEditor { selection_host: selection_host.clone(), selection: selection.clone(), - current: None, + mode: PropsEditorMode::Deactivated, subscriber: helpers::subscribe_to_updates(Rc::downgrade(self), selection_host, selection.clone(), |pe, new_sel| pe.update_selection(new_sel)), }; - self.update_selection_internal(&mut interior, selection, true); + self.update_selection_internal(&mut interior, selection, None); *self.interior.borrow_mut() = Some(interior); } @@ -163,81 +246,122 @@ impl PropsEditor { self.in_update.set(true); let mut interior_guard = self.interior.borrow_mut(); if let Some(interior) = interior_guard.as_mut() { - selection.changes_since(&interior.selection.clone(), &mut |selection, record| self.update_selection_internal(interior, selection.clone(), record.selection_changed)); + selection.changes_since(&interior.selection.clone(), &mut |selection, record| self.update_selection_internal(interior, selection.clone(), Some(record))); } drop(interior_guard); self.in_update.set(false); } - fn update_selection_internal(&self, interior: &mut PropsInterior, selection: sync::Arc, changed: bool) { - let path = selection.single_selected(); + fn selection_to_mode(selection: &sync::Arc) -> PropsEditorMode { + if selection.any_selected() { + if let Some(path) = selection.single_selected() { + let (node, _addr) = selection.document.lookup_node(&path); + PropsEditorMode::Single { path, props: node.props.clone() } + } else if let Some(props) = structure::MaybeProperties::common_between(selection.node_iter()) { + PropsEditorMode::Many { + selection: selection.clone(), + props + } + } else { + PropsEditorMode::Deactivated + } + } else { + PropsEditorMode::Deactivated + } + } + + fn update_selection_internal(&self, interior: &mut PropsInterior, selection: sync::Arc, record: Option<&selection::tree::ChangeRecord>) { + let new_mode = Self::selection_to_mode(&selection); + let changed = record.map_or(true, |record| record.selection_changed); interior.selection = selection.clone(); - if let Some(path) = &path { - let (node, _addr) = selection.document.lookup_node(&path); - - if changed || interior.current.as_ref().map(|(_path, props)| props) != Some(&node.props) { - interior.current = Some(((*path).clone(), node.props.clone())); - self.update_controls(Some(&node.props)); - } else { + match (&interior.mode, &new_mode) { + (_, PropsEditorMode::Deactivated) => { + self.deactivate_controls(); + }, + (PropsEditorMode::Single { props: props_old, .. }, PropsEditorMode::Single { props: props_new, .. }) if props_old == props_new && !changed => { /* If the selection didn't change and the properties don't disagree with what we think they are, DON'T update the interactive controls. This resets text box cursor positions. */ + }, + (PropsEditorMode::Many { props: props_old, .. }, PropsEditorMode::Many { props: props_new, .. }) if props_old == props_new && !changed => { + /* If the selection didn't change and the properties don't disagree with what we think they are, DON'T update the interactive controls. This resets text box cursor positions. */ + }, + (_, PropsEditorMode::Single { props, .. }) => self.update_controls(&structure::MaybeProperties::new(props.clone())), + (_, PropsEditorMode::Many { props, .. }) => self.update_controls(&props), + } + + match &new_mode { + PropsEditorMode::Single { path, .. } => { + self.update_path_control(&selection.document, Some(path)); + }, + _ => { + self.update_path_control(&selection.document, None); } - self.update_path_control(&selection.document, Some(path)); - } else { - interior.current = None; - self.update_controls(None); - self.update_path_control(&selection.document, None); } + + interior.mode = new_mode; } + + fn update_controls(&self, props: &structure::MaybeProperties) { + match &props.name { + Some(name) => self.name_entry.set_text(name), + None => self.name_entry.set_text(""), + } - fn update_controls(&self, props: Option<&structure::Properties>) { - if let Some(props) = props { - self.name_entry.set_text(&props.name); - - self.title_display.set_model(Some(&self.title_model)); - self.title_display.set_selected(match props.title_display { - structure::TitleDisplay::Inline => 0, - structure::TitleDisplay::Major => 1, - structure::TitleDisplay::Minor => 2, - }); - - self.children_display.set_model(Some(&self.children_model)); - self.children_display.set_selected(match props.children_display { - structure::ChildrenDisplay::None => 0, - structure::ChildrenDisplay::Summary => 1, - structure::ChildrenDisplay::Full => 2, - }); - - self.content_display.set_model(Some(&self.content_model)); - self.content_display.set_selected(match props.content_display { - structure::ContentDisplay::None => 0, - structure::ContentDisplay::Hexdump { .. } => 1, - structure::ContentDisplay::Hexstring => 2, - }); - - self.locked.set_active(props.locked); - - self.name_entry.set_sensitive(true); - self.size_entry.set_sensitive(true); - self.title_display.set_sensitive(true); - self.children_display.set_sensitive(true); - self.content_display.set_sensitive(true); - self.locked.set_sensitive(true); - } else { - self.name_entry.set_text(""); - self.title_display.set_model(gio::ListModel::NONE); - self.children_display.set_model(gio::ListModel::NONE); - self.content_display.set_model(gio::ListModel::NONE); - self.locked.set_active(false); - - self.name_entry.set_sensitive(false); - self.size_entry.set_sensitive(false); - self.title_display.set_sensitive(false); - self.children_display.set_sensitive(false); - self.content_display.set_sensitive(false); - self.locked.set_sensitive(false); + self.title_display.set_model(Some(&self.title_model)); + self.title_display.set_selected(match props.title_display { + Some(structure::TitleDisplay::Inline) => 0, + Some(structure::TitleDisplay::Major) => 1, + Some(structure::TitleDisplay::Minor) => 2, + None => gtk::INVALID_LIST_POSITION, + }); + + self.children_display.set_model(Some(&self.children_model)); + self.children_display.set_selected(match &props.children_display { + Some(structure::ChildrenDisplay::None) => 0, + Some(structure::ChildrenDisplay::Summary) => 1, + Some(structure::ChildrenDisplay::Full) => 2, + None => gtk::INVALID_LIST_POSITION, + }); + + self.content_display.set_model(Some(&self.content_model)); + self.content_display.set_selected(match &props.content_display { + Some(structure::ContentDisplay::None) => 0, + Some(structure::ContentDisplay::Hexdump { .. }) => 1, + Some(structure::ContentDisplay::Hexstring) => 2, + None => gtk::INVALID_LIST_POSITION, + }); + + match &props.locked { + Some(x) => { + self.locked.set_inconsistent(false); + self.locked.set_active(*x); + }, + None => self.locked.set_inconsistent(true), } + + self.name_entry.set_sensitive(true); + self.size_entry.set_sensitive(true); + self.title_display.set_sensitive(true); + self.children_display.set_sensitive(true); + self.content_display.set_sensitive(true); + self.locked.set_sensitive(true); + } + + fn deactivate_controls(&self) { + self.name_entry.set_text(""); + self.title_display.set_model(gio::ListModel::NONE); + self.children_display.set_model(gio::ListModel::NONE); + self.content_display.set_model(gio::ListModel::NONE); + self.locked.set_inconsistent(false); + self.locked.set_active(false); + + self.name_entry.set_sensitive(false); + self.size_entry.set_sensitive(false); + self.title_display.set_sensitive(false); + self.children_display.set_sensitive(false); + self.content_display.set_sensitive(false); + self.locked.set_sensitive(false); } fn update_path_control(&self, document: &document::Document, path: Option<&structure::Path>) { diff --git a/src/view/window.rs b/src/view/window.rs index 5724f92..c83204f 100644 --- a/src/view/window.rs +++ b/src/view/window.rs @@ -225,6 +225,8 @@ impl CharmWindow { context: cell::RefCell::new(None), }); + w.props_editor.bind_window(&w); + w.window.connect_close_request(clone!(@strong w => move |_| { /* This is especially important because it destroys actions which might have their own toplevel windows that * would otherwise keep the process alive. */