2 files changed, 40 insertions, 51 deletions

diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs
index 60359ef0..d45e7168 100644
--- a/helix-term/src/commands.rs
+++ b/helix-term/src/commands.rs
@@ -4,8 +4,8 @@ use helix_core::{
     object,
     regex::{self, Regex},
     register, selection,
-    state::{Direction, Granularity, State},
-    Change, ChangeSet, Range, Selection, Tendril, Transaction,
+    state::{coords_at_pos, pos_at_coords, Direction, Granularity, State},
+    Change, ChangeSet, Position, Range, Selection, Tendril, Transaction,
 };
 
 use once_cell::sync::Lazy;
@@ -222,62 +222,60 @@ pub fn extend_next_word_end(cx: &mut Context) {
     doc.set_selection(selection);
 }
 
-pub fn page_up(cx: &mut Context) {
-    let view = cx.view();
-    if view.first_line < PADDING {
+fn scroll(view: &mut View, offset: usize, direction: Direction) {
+    use Direction::*;
+    let text = view.doc.text().slice(..);
+
+    let last_line = view.last_line();
+    let cursor = coords_at_pos(text, view.doc.selection().cursor());
+    let doc_last_line = text.len_lines() - 1;
+
+    if direction == Backward && view.first_line == 0
+        || direction == Forward && last_line == doc_last_line
+    {
         return;
     }
 
-    view.first_line = view.first_line.saturating_sub(view.area.height as usize);
+    let scrolloff = PADDING; // min(user pref, half win width/height)
+
+    // cursor visual offset
+    let cursor_off = cursor.row - view.first_line;
 
-    if !view.check_cursor_in_view() {
-        let text = view.doc.text();
-        let pos = text.line_to_char(view.last_line().saturating_sub(PADDING));
-        view.doc.set_selection(Selection::point(pos));
+    view.first_line = match direction {
+        Forward => view.first_line + offset,
+        Backward => view.first_line.saturating_sub(offset),
     }
+    .min(doc_last_line);
+
+    // clamp into viewport
+    let line = (view.first_line + cursor_off).clamp(
+        view.first_line + scrolloff,
+        view.first_line + view.last_line().saturating_sub(scrolloff),
+    );
+
+    let pos = pos_at_coords(text, Position::new(line, cursor.col)); // this func will properly truncate to line end
+    view.doc.set_selection(Selection::point(pos));
 }
 
-pub fn page_down(cx: &mut Context) {
+pub fn page_up(cx: &mut Context) {
     let view = cx.view();
-    view.first_line += view.area.height as usize + PADDING;
+    scroll(view, view.area.height as usize, Direction::Backward);
+}
 
-    if view.first_line < view.doc.text().len_lines() {
-        let text = view.doc.text();
-        let pos = text.line_to_char(view.first_line as usize);
-        view.doc.set_selection(Selection::point(pos));
-    }
+pub fn page_down(cx: &mut Context) {
+    let view = cx.view();
+    scroll(view, view.area.height as usize, Direction::Forward);
 }
 
 pub fn half_page_up(cx: &mut Context) {
     let view = cx.view();
-    if view.first_line < PADDING {
-        return;
-    }
-
-    view.first_line = view
-        .first_line
-        .saturating_sub(view.area.height as usize / 2);
-
-    if !view.check_cursor_in_view() {
-        let text = &view.doc.text();
-        let pos = text.line_to_char(view.last_line() - PADDING);
-        view.doc.set_selection(Selection::point(pos));
-    }
+    scroll(view, view.area.height as usize / 2, Direction::Backward);
 }
 
 pub fn half_page_down(cx: &mut Context) {
     let view = cx.view();
-    let lines = view.doc.text().len_lines();
-    if view.first_line < lines.saturating_sub(view.area.height as usize) {
-        view.first_line += view.area.height as usize / 2;
-    }
-    if !view.check_cursor_in_view() {
-        let text = view.doc.text();
-        let pos = text.line_to_char(view.first_line as usize);
-        view.doc.set_selection(Selection::point(pos));
-    }
+    scroll(view, view.area.height as usize / 2, Direction::Forward);
 }
-// avoid select by default by having a visual mode switch that makes movements into selects
 
 pub fn extend_char_left(cx: &mut Context) {
     let count = cx.count;
@@ -439,8 +437,10 @@ pub fn search_selection(cx: &mut Context) {
 pub fn select_line(cx: &mut Context) {
     let count = cx.count;
     let doc = cx.doc();
+
     let pos = doc.selection().primary();
     let text = doc.text();
+
     let line = text.char_to_line(pos.head);
     let start = text.line_to_char(line);
     let end = text.line_to_char(line + count).saturating_sub(1);
diff --git a/helix-view/src/view.rs b/helix-view/src/view.rs
index 1d806da9..15f0171f 100644
--- a/helix-view/src/view.rs
+++ b/helix-view/src/view.rs
@@ -35,17 +35,6 @@ impl View {
         Ok(view)
     }
 
-    pub fn check_cursor_in_view(&self) -> bool {
-        let cursor = self.doc.selection().cursor();
-        let line = self.doc.text().char_to_line(cursor);
-        let document_end = self.first_line + self.area.height.saturating_sub(1) as usize;
-
-        if (line > document_end.saturating_sub(PADDING)) || (line < self.first_line + PADDING) {
-            return false;
-        }
-        true
-    }
-
     pub fn ensure_cursor_in_view(&mut self) {
         let cursor = self.doc.state.selection().cursor();
         let line = self.doc.text().char_to_line(cursor);