Vertical cursor movement based on logical (not visual) position in line

src/model/listing/cursor/hexdump.rs

@@ -1,5 +1,6 @@
 use crate::model::addr;
 use crate::model::listing::cursor;
+use crate::model::listing::line;
 use crate::model::listing::token;
 use crate::model::listing::token::TokenKind;
 
@@ -15,38 +16,49 @@ impl Cursor {
     pub fn new_transition(token: token::HexdumpToken, hint: &cursor::TransitionHint) -> Result<Cursor, token::Token> {
         let extent = token.extent;
         let limit = (extent.length() - addr::unit::BIT).floor();
+
+        let (offset, low_nybble) = match hint {
+            cursor::TransitionHint::MoveLeftLarge => (addr::Size::from(limit.bytes & !7), false),
+
+            cursor::TransitionHint::MoveVertical {
+                horizontal_position: cursor::HorizontalPosition::Hexdump(offset_in_line, low_nybble),
+                line,
+                ..
+            } => {
+                let line_extent = match line.ty {
+                    line::LineType::Hexdump { line_extent, .. } => line_extent,
+                    _ => return Err(token::Token::Hexdump(token)),
+                };
+                let offset = line_extent.begin + *offset_in_line;
+                if offset >= extent.end {
+                    return Err(token::Token::Hexdump(token));
+                } else if offset < extent.begin {
+                    /* the first line after a child can have a first token that
+                     * starts in the middle of the line, and the horizontal
+                     * position can point before that token starts. */
+                    (addr::unit::ZERO, false)
+                } else {
+                    (offset - extent.begin, *low_nybble)
+                }
+            },
+
+            op if op.is_left() => (limit, true),
+            op if op.is_right() => (addr::unit::ZERO, false),
+            _ => (addr::unit::ZERO, false)
+        };
 
         Ok(Cursor {
             token,
             extent,
-            offset: match hint.op {
-                cursor::TransitionOp::MoveLeftLarge => addr::Size::from(limit.bytes & !7),
-                op if op.is_left() => limit,
-                op if op.is_right() => addr::unit::ZERO,
-                _ => match &hint.class {
-                    cursor::TransitionHintClass::Hexdump(hth) => std::cmp::min(hth.offset, limit),
-                    _ => addr::unit::ZERO,
-                },
-            },
-            low_nybble: match (&hint.class, hint.op) {
-                /* decide from op */
-                (_, cursor::TransitionOp::MoveLeftLarge) => false,
-                (_, op) if op.is_left() => true,
-                (_, op) if op.is_right() => false,
-                /* if we have an intended offset and had to truncate it, we should place at the end of the line */
-                (cursor::TransitionHintClass::Hexdump(hth), _) if hth.offset > limit => true,
-                /* if we have an intended offset and didn't have to truncate it, try to carry the low_nybble flag over from a previous HexCursor */
-                (cursor::TransitionHintClass::Hexdump(hth), _) => hth.low_nybble,
-                /* last resort, if the op is seriously misbehaving and is neither left nor right */
-                _ => false,
-            },
+            offset,
+            low_nybble,
         })
     }
 
     pub fn new_placement(token: token::HexdumpToken, offset: addr::Address, hint: &cursor::PlacementHint) -> Result<Cursor, token::Token> {
         let extent = token.extent;
         let limit = (extent.length() - addr::unit::BIT).floor();
-        
+
         Ok(Cursor {
             token,
             extent,
@@ -86,13 +98,14 @@ impl cursor::CursorClassExt for Cursor {
         })
     }
 
-    fn get_transition_hint(&self) -> cursor::TransitionHintClass {
-        cursor::TransitionHintClass::Hexdump(HexdumpTransitionHint {
-            offset: self.offset,
-            low_nybble: self.low_nybble
-        })
+    fn get_horizontal_position_in_line(&self, line: &line::Line) -> cursor::HorizontalPosition {
+        if let line::LineType::Hexdump { line_extent, .. } = &line.ty {
+            cursor::HorizontalPosition::Hexdump(self.extent.begin.to_size() + self.offset - line_extent.begin.to_size(), self.low_nybble)
+        } else {
+            panic!("attempted to get horizontal position of HexdumpCursor on a non-Hexdump line");
+        }
     }
-
+    
     fn move_left(&mut self) -> cursor::MovementResult {
         if self.low_nybble {
             self.low_nybble = false;

src/model/listing/cursor/punctuation.rs

@@ -1,5 +1,6 @@
 use crate::model::addr;
 use crate::model::listing::cursor;
+use crate::model::listing::line;
 use crate::model::listing::token;
 use crate::model::listing::token::TokenKind;
 
@@ -10,13 +11,15 @@ pub struct Cursor {
 
 impl Cursor {
     pub fn new_transition(token: token::Token, hint: &cursor::TransitionHint) -> Result<Cursor, token::Token> {
-        if hint.op.is_entry() {
-            /* skip over punctuation tokens for entry */
-            Err(token)
+        if match hint {
+            hint if hint.is_entry() => false,
+            cursor::TransitionHint::MoveVertical { horizontal_position: cursor::HorizontalPosition::Unspecified, .. } => true,
+            cursor::TransitionHint::MoveVertical { .. } => false,
+            _ => true,
+        } {
+            Ok(Cursor { token })
         } else {
-            Ok(Cursor {
-                token
-            })
+            Err(token)
         }
     }
 
@@ -53,10 +56,10 @@ impl cursor::CursorClassExt for Cursor {
         cursor::PlacementHint::Punctuation
     }
 
-    fn get_transition_hint(&self) -> cursor::TransitionHintClass {
-        cursor::TransitionHintClass::Unused
+    fn get_horizontal_position_in_line(&self, _line: &line::Line) -> cursor::HorizontalPosition {
+        cursor::HorizontalPosition::Unspecified
     }
-
+    
     fn move_left(&mut self) -> cursor::MovementResult {
         cursor::MovementResult::HitStart
     }

src/model/listing/cursor/title.rs

@@ -1,5 +1,6 @@
 use crate::model::addr;
 use crate::model::listing::cursor;
+use crate::model::listing::line;
 use crate::model::listing::token;
 use crate::model::listing::token::TokenKind;
 
@@ -12,13 +13,16 @@ pub struct Cursor {
 
 impl Cursor {
     pub fn new_transition(token: token::TitleToken, hint: &cursor::TransitionHint) -> Result<Cursor, token::TitleToken> {
-        if hint.op.is_entry() {
-            /* skip over title tokens for entry */
-            Err(token)
+        if match hint {
+            hint if hint.is_entry() => false,
+            cursor::TransitionHint::MoveVertical { horizontal_position: cursor::HorizontalPosition::Title, .. } => true,
+            cursor::TransitionHint::MoveVertical { horizontal_position: cursor::HorizontalPosition::Unspecified, .. } => true,
+            cursor::TransitionHint::MoveVertical { .. } => false,
+            _ => true,
+        } {
+            Ok(Cursor { token })
         } else {
-            Ok(Cursor {
-                token
-            })
+            Err(token)
         }
     }
 
@@ -55,8 +59,8 @@ impl cursor::CursorClassExt for Cursor {
         cursor::PlacementHint::Title
     }
 
-    fn get_transition_hint(&self) -> cursor::TransitionHintClass {
-        cursor::TransitionHintClass::Unused
+    fn get_horizontal_position_in_line(&self, _line: &line::Line) -> cursor::HorizontalPosition {
+        cursor::HorizontalPosition::Title
     }
 
     #[instrument]

src/view/crashreport.rs

@@ -42,7 +42,12 @@ pub enum Circumstance {
     InWindow(u64),
     TreeSelectionUpdate(sync::Arc<model::selection::tree::Host>, sync::Arc<document::Document>),
     ListingSelectionUpdate(sync::Arc<model::selection::listing::Host>, sync::Arc<document::Document>),
-    Goto(sync::Arc<document::Document>, document::structure::Path, model::addr::Address, model::listing::cursor::PlacementHint),
+    Goto {
+        document: sync::Arc<document::Document>,
+        path: document::structure::Path,
+        offset: model::addr::Address,
+        hint: String,
+    },
     LWDocumentUpdate(sync::Arc<document::Document>, sync::Arc<document::Document>),
     LWSelectionUpdate(sync::Arc<model::selection::ListingSelection>, sync::Arc<model::selection::ListingSelection>),
 }
@@ -603,8 +608,8 @@ impl Circumstance {
                     write!(d, "    - {:?}\n", change).unwrap();
                 });
             },
-            Circumstance::Goto(_doc, path, addr, hint) => {
-                write!(d, "While performing goto({:?}, {}, {:?}).\n", path, addr, hint)?;
+            Circumstance::Goto { document: _, path, offset, hint } => {
+                write!(d, "While performing goto({:?}, {}, {}).\n", path, offset, hint)?;
             },
             Circumstance::LWDocumentUpdate(old, new) => {
                 write!(d, "While updating listing widget for new document.\n")?;

src/view/listing.rs

@@ -477,7 +477,12 @@ impl ListingWidget {
 
         let _circumstances = crashreport::circumstances([
             crashreport::Circumstance::InWindow(interior.charm_window_id),
-            crashreport::Circumstance::Goto(document.clone(), path.clone(), offset, hint.clone()),
+            crashreport::Circumstance::Goto {
+                document: document.clone(),
+                path: path.clone(),
+                offset,
+                hint: format!("{:?}", hint),
+            },
         ]);
 
         if interior.document.generation() != document.generation() {
@@ -740,8 +745,8 @@ impl Interior {
             /* basic cursor   key    shift  ctrl  */
             (gdk::Key::Left,  false, false) => self.cursor_transaction(|c| c.move_left(),  facet::scroll::EnsureCursorInViewDirection::Up),
             (gdk::Key::Right, false, false) => self.cursor_transaction(|c| c.move_right(), facet::scroll::EnsureCursorInViewDirection::Down),
-            //(gdk::keys::constants::Up,    false, false) => self.cursor_transaction(|c| c.move_up(),    facet::scroll::EnsureCursorInViewDirection::Up),
-            //(gdk::keys::constants::Down,  false, false) => self.cursor_transaction(|c| c.move_down(),  facet::scroll::EnsureCursorInViewDirection::Down),
+            (gdk::Key::Up,    false, false) => self.cursor_transaction(|c| c.move_up(),    facet::scroll::EnsureCursorInViewDirection::Up),
+            (gdk::Key::Down,  false, false) => self.cursor_transaction(|c| c.move_down(),  facet::scroll::EnsureCursorInViewDirection::Down),
 
             /* fast cursor    key    shift  ctrl  */
             //(gdk::keys::constants::Left,  false, true ) => self.cursor_transaction(|c| c.move_left_large(),    facet::scroll::EnsureCursorInViewDirection::Up),

src/view/listing/facet/cursor.rs

@@ -133,8 +133,8 @@ impl CursorView {
 
     pub fn move_left(&mut self) { self.movement(|c| c.move_left()); }
     pub fn move_right(&mut self) { self.movement(|c| c.move_right()); }
-    //pub fn move_up(&mut self) { self.movement(|c| c.move_up()); }
-    //pub fn move_down(&mut self) { self.movement(|c| c.move_down()); }
+    pub fn move_up(&mut self) { self.movement(|c| c.move_up()); }
+    pub fn move_down(&mut self) { self.movement(|c| c.move_down()); }
     //pub fn move_to_start_of_line(&mut self) { self.movement(|c| c.move_to_start_of_line()); }
     //pub fn move_to_end_of_line(&mut self) { self.movement(|c| c.move_to_end_of_line()); }
     pub fn move_left_large(&mut self) { self.movement(|c| c.move_left_large()); }

src/view/listing/line.rs

@@ -42,6 +42,12 @@ enum LineViewType {
     },
 }
 
+impl std::fmt::Debug for LineViewType {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.debug_struct("LineViewType").finish()
+    }
+}
+
 pub struct Line {
     ev_draw: facet::Event,
     ev_work: facet::Event,