From 1fd28b6a5da1fad7fe77754b516097eba794f36f Mon Sep 17 00:00:00 2001 From: misson20000 Date: Sat, 27 Apr 2024 22:46:02 -0400 Subject: [PATCH] Fix cursor update behavior when a node is inserted or deleted over (but not AT) the cursor, as can happen now when doing so from a selection --- src/logic/tokenizer.rs | 53 +++++++++++------ src/model/listing/cursor.rs | 79 +++++++++++++++++++++---- src/model/listing/cursor/punctuation.rs | 2 +- src/model/listing/cursor/title.rs | 2 +- src/model/listing/layout.rs | 2 +- 5 files changed, 107 insertions(+), 31 deletions(-) diff --git a/src/logic/tokenizer.rs b/src/logic/tokenizer.rs index 56c4b11..a18bfc1 100644 --- a/src/logic/tokenizer.rs +++ b/src/logic/tokenizer.rs @@ -124,10 +124,11 @@ impl std::fmt::Debug for TokenizerStackEntry { } } -#[derive(Clone, Debug, Default)] +#[non_exhaustive] +#[derive(Clone, Debug, Default, PartialEq, Eq)] pub struct PortOptions { - additional_offset: addr::Size, - prefer_after_new_node: bool, + pub additional_offset: Option, + pub prefer_after_new_node: bool, } pub struct PortOptionsBuilder { @@ -203,8 +204,8 @@ impl PortOptionsBuilder { } } - pub fn additional_offset(mut self, offset: addr::Size) -> PortOptionsBuilder { - self.options.additional_offset = offset; + pub fn additional_offset>(mut self, offset: T) -> PortOptionsBuilder { + self.options.additional_offset = Some(offset.into()); self } @@ -297,7 +298,7 @@ impl Tokenizer { /// Applies a single change to the tokenizer state. #[instrument] - pub fn port_change(&mut self, new_root: &sync::Arc, change: &change::Change, options: &PortOptions) { + pub fn port_change(&mut self, new_root: &sync::Arc, change: &change::Change, options: &mut PortOptions) { /* Recreate our stack, processing descents and such, leaving off with some information about the node we're actually on now. */ let stack_state = match &self.stack { Some(parent) => Self::port_recurse(parent, new_root, change), @@ -413,8 +414,8 @@ impl Tokenizer { }; /* Respect the additional offset that the options requested. */ - if let Some(offset) = offset.as_mut() { - *offset = *offset + options.additional_offset; + if let (Some(offset), Some(additional_offset)) = (offset.as_mut(), options.additional_offset) { + *offset = *offset + additional_offset; } /* Adjust the offset and index. */ @@ -498,7 +499,12 @@ impl Tokenizer { /* This should've been handled earlier, but whatever. */ IntermediatePortState::Finished(finalized_state) => finalized_state, - IntermediatePortState::NormalContent(offset, index) => TokenizerState::MetaContent(self.estimate_line_begin(offset, index), index), + IntermediatePortState::NormalContent(offset, index) => { + let line_begin = self.estimate_line_begin(offset, index); + /* Output the difference between the requested "additional offset" and where the token actually will begin. */ + options.additional_offset = offset.map(|offset| offset - line_begin); + TokenizerState::MetaContent(line_begin, index) + }, /* Real TokenizerState doesn't support one-past-the-end for SummaryLabel and SummarySeparator, so need to fix if that would be the case. */ IntermediatePortState::SummaryLabel(index) if index < children.len() => TokenizerState::SummaryLabel(index), @@ -1711,19 +1717,28 @@ mod tests { }; } - fn assert_port_functionality(old_doc: &document::Document, new_doc: &document::Document, records: &[(token::Token, token::Token, PortOptions)]) { - let mut tokenizers: Vec<(Tokenizer, &token::Token, &token::Token, &PortOptions)> = records.iter().map(|(before_token, after_token, options)| (Tokenizer::at_beginning(old_doc.root.clone()), before_token, after_token, options)).collect(); + fn assert_port_functionality(old_doc: &document::Document, new_doc: &document::Document, records: &[(token::Token, token::Token, PortOptions, PortOptions)]) { + let mut tokenizers: Vec<(Tokenizer, &token::Token, &token::Token, &PortOptions, &PortOptions)> = records.iter().map( + |(before_token, after_token, before_options, after_options)| ( + Tokenizer::at_beginning(old_doc.root.clone()), + before_token, + after_token, + before_options, + after_options) + ).collect(); - for (tokenizer, before_token, _after_token, _) in tokenizers.iter_mut() { + for (tokenizer, before_token, _after_token, _, _) in tokenizers.iter_mut() { seek_to_token(tokenizer, before_token); } - for (tokenizer, _before_token, after_token, options) in tokenizers.iter_mut() { + for (tokenizer, _before_token, after_token, options_before, options_after) in tokenizers.iter_mut() { println!("tokenizer before port: {:#?}", tokenizer); - new_doc.changes_since_ref(old_doc, &mut |doc, change| tokenizer.port_change(&doc.root, change, options)); + let mut options = options_before.clone(); + new_doc.changes_since_ref(old_doc, &mut |doc, change| tokenizer.port_change(&doc.root, change, &mut options)); println!("tokenizer after port: {:#?}", tokenizer); assert_eq!(&peek(tokenizer), *after_token); + assert_eq!(&options, *options_after); /* Check that the ported tokenizer is the same as if we had created a new tokenizer and seeked it (if only we knew where to seek it to...), i.e. its internal state isn't corrupted in a way that doesn't happen during normal tokenizer movement. */ let mut clean_tokenizer = Tokenizer::at_beginning(new_doc.root.clone()); @@ -1749,7 +1764,7 @@ mod tests { .child(0x20, |b| b .name("child1.1") .size(0x18)) - .child(0x30, |b| b + .child(0x34, |b| b .name("child1.2") .size(0x18)) .child(0x48, |b| b @@ -1762,6 +1777,8 @@ mod tests { let old_doc = document::Builder::new(root).build(); let mut new_doc = old_doc.clone(); + + /* Delete child1.1 and child1.2. */ new_doc.change_for_debug(old_doc.delete_range(structure::SiblingRange::new(vec![1], 1, 2))).unwrap(); let (o_child_1_2, o_child_1_2_addr) = old_doc.lookup_node(&vec![1, 2]); @@ -1790,7 +1807,8 @@ mod tests { extent: addr::Extent::sized_u64(0x40, 0x8), line: addr::Extent::sized_u64(0x40, 0x10) }), - PortOptionsBuilder::new().build() + PortOptionsBuilder::new().additional_offset(0x4).build(), /* Asking about offset 0x14 into old child1.2 */ + PortOptionsBuilder::new().additional_offset(0x8).build(), /* Becomes offset 0x48 in new child1 */ ), ( token::Token::Hexdump(token::HexdumpToken { @@ -1814,7 +1832,8 @@ mod tests { extent: addr::Extent::sized_u64(0x10, 0xc), line: addr::Extent::sized_u64(0x10, 0x10), }), - PortOptionsBuilder::new().build() + PortOptionsBuilder::new().additional_offset(0x4).build(), + PortOptionsBuilder::new().additional_offset(0x4).build(), ), ]); } diff --git a/src/model/listing/cursor.rs b/src/model/listing/cursor.rs index 45ae215..73eeb14 100644 --- a/src/model/listing/cursor.rs +++ b/src/model/listing/cursor.rs @@ -154,22 +154,23 @@ impl Cursor { _ => {}, }; - let options = options.build(); - + let mut options = options.build(); + let mut tokenizer = self.tokenizer.clone(); + document.changes_since(&self.document, &mut |document, change| { - self.tokenizer.port_change( + tokenizer.port_change( &document.root, change, - &options); + &mut options); }); - - let mut tokenizer = self.tokenizer.clone(); - let class = match CursorClass::place_forward(&mut tokenizer, self.class.get_addr(), &self.class.get_placement_hint()) { + let offset = tokenizer.structure_position_offset() + options.additional_offset.unwrap_or(addr::unit::ZERO); + + let class = match CursorClass::place_forward(&mut tokenizer, offset, &self.class.get_placement_hint()) { Ok(cc) => cc, Err(PlacementFailure::HitBottomOfAddressSpace) => { tokenizer = self.tokenizer.clone(); - match CursorClass::place_backward(&mut tokenizer, self.class.get_addr(), &self.class.get_placement_hint()) { + match CursorClass::place_backward(&mut tokenizer, offset, &self.class.get_placement_hint()) { Ok(cc) => cc, Err(PlacementFailure::HitTopOfAddressSpace) => panic!("expected to be able to place cursor somewhere"), Err(_) => panic!("unexpected error from CursorClass::place_backward") @@ -283,10 +284,10 @@ impl CursorClass { /// one. fn new_placement(token: token::Token, offset: addr::Address, hint: &PlacementHint) -> Result { match token { - token::Token::Title(token) => title::Cursor::new_placement(token, offset, hint).map(CursorClass::Title).map_err(TokenKind::into_token), + token::Token::Title(token) => title::Cursor::new_placement(token, hint).map(CursorClass::Title).map_err(TokenKind::into_token), token::Token::Hexdump(token) => hexdump::Cursor::new_placement(token, offset, hint).map(CursorClass::Hexdump).map_err(TokenKind::into_token), - token::Token::SummaryPunctuation(token) if token.kind.accepts_cursor() => punctuation::Cursor::new_placement(token.into_token(), offset, hint).map(CursorClass::Punctuation), - token::Token::BlankLine(token) if token.accepts_cursor => punctuation::Cursor::new_placement(token.into_token(), offset, hint).map(CursorClass::Punctuation), + token::Token::SummaryPunctuation(token) if token.kind.accepts_cursor() => punctuation::Cursor::new_placement(token.into_token(), hint).map(CursorClass::Punctuation), + token::Token::BlankLine(token) if token.accepts_cursor => punctuation::Cursor::new_placement(token.into_token(), hint).map(CursorClass::Punctuation), _ => Err(token) } } @@ -593,4 +594,60 @@ mod tests { }).as_ref()); assert_matches!(&cursor.class, CursorClass::Hexdump(hxc) if hxc.offset == addr::unit::ZERO && hxc.low_nybble == false); } + + #[test] + fn node_insertion_around() { + let document_host = sync::Arc::new(document::Builder::default().host()); + let document = document_host.get(); + let mut cursor = Cursor::place(document.clone(), &vec![], 0x24.into(), PlacementHint::Unused).unwrap(); + + assert_eq!(cursor.class.get_token(), token::Token::Hexdump(token::HexdumpToken { + common: token::TokenCommon { + node: document.root.clone(), + node_path: structure::Path::default(), + node_addr: addr::unit::NULL, + depth: 1, + }, + extent: addr::Extent::sized(0x20.into(), 0x10.into()), + line: addr::Extent::sized(0x20.into(), 0x10.into()), + }).as_ref()); + assert_matches!(&cursor.class, CursorClass::Hexdump(hxc) if hxc.offset == 0x4.into() && hxc.low_nybble == false); + + let node = sync::Arc::new(structure::Node { + props: structure::Properties { + name: "child".to_string(), + title_display: structure::TitleDisplay::Minor, + children_display: structure::ChildrenDisplay::Full, + content_display: structure::ContentDisplay::Hexdump { + line_pitch: addr::Size::from(16), + gutter_pitch: addr::Size::from(8), + }, + locked: false, + }, + children: vec::Vec::new(), + size: addr::Size::from(0x30), + }); + + let document = document_host.change(document.insert_node( + vec![], + 0, + structure::Childhood::new(node.clone(), 0x12.into()) + )).unwrap(); + + /* port the cursor over */ + cursor.update(&document); + + /* make sure it winds up in the correct place */ + assert_eq!(cursor.class.get_token(), token::Token::Hexdump(token::HexdumpToken { + common: token::TokenCommon { + node: node.clone(), + node_path: vec![0], + node_addr: 0x12.into(), + depth: 2, + }, + extent: addr::Extent::sized(0x10.into(), 0x10.into()), + line: addr::Extent::sized(0x10.into(), 0x10.into()), + }).as_ref()); + assert_matches!(&cursor.class, CursorClass::Hexdump(hxc) if hxc.offset == 0x2.into() && hxc.low_nybble == false); + } } diff --git a/src/model/listing/cursor/punctuation.rs b/src/model/listing/cursor/punctuation.rs index a2b87e6..397f5a4 100644 --- a/src/model/listing/cursor/punctuation.rs +++ b/src/model/listing/cursor/punctuation.rs @@ -20,7 +20,7 @@ impl Cursor { } } - pub fn new_placement(token: token::Token, _offset: addr::Address, hint: &cursor::PlacementHint) -> Result { + pub fn new_placement(token: token::Token, hint: &cursor::PlacementHint) -> Result { match hint { /* we only place the cursor on punctuation if explicitly requested; * otherwise, we prefer to place it on a content token. */ diff --git a/src/model/listing/cursor/title.rs b/src/model/listing/cursor/title.rs index 595421b..abd265c 100644 --- a/src/model/listing/cursor/title.rs +++ b/src/model/listing/cursor/title.rs @@ -22,7 +22,7 @@ impl Cursor { } } - pub fn new_placement(token: token::TitleToken, _offset: addr::Address, hint: &cursor::PlacementHint) -> Result { + pub fn new_placement(token: token::TitleToken, hint: &cursor::PlacementHint) -> Result { match hint { /* we only place the cursor on a break header if explicitly requested; * otherwise, we prefer to place it on a content token. */ diff --git a/src/model/listing/layout.rs b/src/model/listing/layout.rs index 321b07a..b2f003a 100644 --- a/src/model/listing/layout.rs +++ b/src/model/listing/layout.rs @@ -667,7 +667,7 @@ impl WindowTokenizer for tokenizer::Tokenizer { } fn port_change(&mut self, new_doc: &sync::Arc, change: &document::change::Change) { - tokenizer::Tokenizer::port_change(self, &new_doc.root, change, &tokenizer::PortOptions::default()); + tokenizer::Tokenizer::port_change(self, &new_doc.root, change, &mut tokenizer::PortOptions::default()); } fn hit_top(&self) -> bool {