about summary refs log tree commit diff stats
diff options
context:
space:
mode:
authorKartik K. Agaram <vc@akkartik.com>2023-09-20 13:39:29 -0700
committerKartik K. Agaram <vc@akkartik.com>2023-09-20 13:39:29 -0700
commitbd6f7d48e76182218877564e8ca672e657f4ef56 (patch)
treee2ed5bd8592ad604612fb2d8f1b49079b0103ae0
parentc43d884b6ffb94803bee9f9e788e4b3a2f74f23b (diff)
downloadlines.love-bd6f7d48e76182218877564e8ca672e657f4ef56.tar.gz
bugfix: clear selection when clicking above or below lines
Matt Wynne pointed out that snap.love would crash when a node went off
screen. While debugging it I noticed that selection1 was being set when
it shouldn't be.

Turns out I introduced a bug when I fixed the inscript bug back in June
(commit 9656e137742). One invariant I want to preserve is: selection1
should be unset after a mouse click (press and release without
intervening drag). This invariant was violated in my bugfix back in
June. I was concerned only with selection back then, and I didn't
realize I was breaking the mouse click case (in a fairly subtle way; you
can have selection set, and when it's set identically to the cursor
everything looks the same).

I think there might still be an issue in snap.love after this fix. I
noticed screen_bottom1.pos was nil, and as far as I recall that should
never happen.
-rw-r--r--edit.lua40
-rw-r--r--source_edit.lua40
-rw-r--r--source_text_tests.lua24
-rw-r--r--text_tests1
-rw-r--r--text_tests.lua24
5 files changed, 103 insertions, 26 deletions
diff --git a/edit.lua b/edit.lua
index 45588df..7d3c26f 100644
--- a/edit.lua
+++ b/edit.lua
@@ -291,7 +291,7 @@ function edit.mouse_press(State, x,y, mouse_button)
     end
   end
 
-  -- still here? click is below all screen lines
+  -- still here? mouse press is below all screen lines
   State.old_cursor1 = State.cursor1
   State.old_selection1 = State.selection1
   State.mousepress_shift = App.shift_down()
@@ -313,6 +313,12 @@ function edit.mouse_release(State, x,y, mouse_button)
     end
   else
 --?     print_and_log('edit.mouse_release: no current drawing')
+    if y < State.top then
+      State.cursor1 = {line=State.screen_top1.line, pos=State.screen_top1.pos}
+      edit.clean_up_mouse_press(State)
+      return
+    end
+
     for line_index,line in ipairs(State.lines) do
       if line.mode == 'text' then
         if Text.in_line(State, line_index, x,y) then
@@ -322,25 +328,33 @@ function edit.mouse_release(State, x,y, mouse_button)
               pos=Text.to_pos_on_line(State, line_index, x, y),
           }
 --?           print_and_log(('edit.mouse_release: cursor now %d,%d'):format(State.cursor1.line, State.cursor1.pos))
-          if State.mousepress_shift then
-            if State.old_selection1.line == nil then
-              State.selection1 = State.old_cursor1
-            else
-              State.selection1 = State.old_selection1
-            end
-          end
-          State.old_cursor1, State.old_selection1, State.mousepress_shift = nil
-          if eq(State.cursor1, State.selection1) then
-            State.selection1 = {}
-          end
-          break
+          edit.clean_up_mouse_press(State)
+          return
         end
       end
     end
+
+    -- still here? mouse release is below all screen lines
+    State.cursor1.line, State.cursor1.pos = State.screen_bottom1.line, Text.pos_at_end_of_screen_line(State, State.screen_bottom1)
+    edit.clean_up_mouse_press(State)
 --?     print_and_log(('edit.mouse_release: finally selection %s,%s cursor %d,%d'):format(tostring(State.selection1.line), tostring(State.selection1.pos), State.cursor1.line, State.cursor1.pos))
   end
 end
 
+function edit.clean_up_mouse_press(State)
+  if State.mousepress_shift then
+    if State.old_selection1.line == nil then
+      State.selection1 = State.old_cursor1
+    else
+      State.selection1 = State.old_selection1
+    end
+  end
+  State.old_cursor1, State.old_selection1, State.mousepress_shift = nil
+  if eq(State.cursor1, State.selection1) then
+    State.selection1 = {}
+  end
+end
+
 function edit.mouse_wheel_move(State, dx,dy)
   if dy > 0 then
     State.cursor1 = {line=State.screen_top1.line, pos=State.screen_top1.pos}
diff --git a/source_edit.lua b/source_edit.lua
index 310853a..2bf0697 100644
--- a/source_edit.lua
+++ b/source_edit.lua
@@ -295,7 +295,7 @@ function edit.mouse_press(State, x,y, mouse_button)
     end
   end
 
-  -- still here? click is below all screen lines
+  -- still here? mouse press is below all screen lines
   State.old_cursor1 = State.cursor1
   State.old_selection1 = State.selection1
   State.mousepress_shift = App.shift_down()
@@ -317,6 +317,12 @@ function edit.mouse_release(State, x,y, mouse_button)
     end
   else
 --?     print_and_log('edit.mouse_release: no current drawing')
+    if y < State.top then
+      State.cursor1 = {line=State.screen_top1.line, pos=State.screen_top1.pos}
+      edit.clean_up_mouse_press(State)
+      return
+    end
+
     for line_index,line in ipairs(State.lines) do
       if line.mode == 'text' then
         if Text.in_line(State, line_index, x,y) then
@@ -326,25 +332,33 @@ function edit.mouse_release(State, x,y, mouse_button)
               pos=Text.to_pos_on_line(State, line_index, x, y),
           }
 --?           print_and_log(('edit.mouse_release: cursor now %d,%d'):format(State.cursor1.line, State.cursor1.pos))
-          if State.mousepress_shift then
-            if State.old_selection1.line == nil then
-              State.selection1 = State.old_cursor1
-            else
-              State.selection1 = State.old_selection1
-            end
-          end
-          State.old_cursor1, State.old_selection1, State.mousepress_shift = nil
-          if eq(State.cursor1, State.selection1) then
-            State.selection1 = {}
-          end
-          break
+          edit.clean_up_mouse_press(State)
+          return
         end
       end
     end
+
+    -- still here? mouse release is below all screen lines
+    State.cursor1.line, State.cursor1.pos = State.screen_bottom1.line, Text.pos_at_end_of_screen_line(State, State.screen_bottom1)
+    edit.clean_up_mouse_press(State)
 --?     print_and_log(('edit.mouse_release: finally selection %s,%s cursor %d,%d'):format(tostring(State.selection1.line), tostring(State.selection1.pos), State.cursor1.line, State.cursor1.pos))
   end
 end
 
+function edit.clean_up_mouse_press(State)
+  if State.mousepress_shift then
+    if State.old_selection1.line == nil then
+      State.selection1 = State.old_cursor1
+    else
+      State.selection1 = State.old_selection1
+    end
+  end
+  State.old_cursor1, State.old_selection1, State.mousepress_shift = nil
+  if eq(State.cursor1, State.selection1) then
+    State.selection1 = {}
+  end
+end
+
 function edit.mouse_wheel_move(State, dx,dy)
   if dy > 0 then
     State.cursor1 = {line=State.screen_top1.line, pos=State.screen_top1.pos}
diff --git a/source_text_tests.lua b/source_text_tests.lua
index c2e054a..2dc3adb 100644
--- a/source_text_tests.lua
+++ b/source_text_tests.lua
@@ -275,6 +275,7 @@ function test_click_to_left_of_line()
   Editor_state.cursor1 = {line=1, pos=3}
   Editor_state.screen_top1 = {line=1, pos=1}
   Editor_state.screen_bottom1 = {}
+  Editor_state.selection1 = {}
   -- click to the left of the line
   edit.draw(Editor_state)
   edit.run_after_mouse_click(Editor_state, Editor_state.left-4,Editor_state.top+5, 1)
@@ -294,6 +295,7 @@ function test_click_takes_margins_into_account()
   Editor_state.cursor1 = {line=2, pos=1}
   Editor_state.screen_top1 = {line=1, pos=1}
   Editor_state.screen_bottom1 = {}
+  Editor_state.selection1 = {}
   -- click on the other line
   edit.draw(Editor_state)
   edit.run_after_mouse_click(Editor_state, Editor_state.left+8,Editor_state.top+5, 1)
@@ -312,11 +314,33 @@ function test_click_on_empty_line()
   Editor_state.cursor1 = {line=2, pos=1}
   Editor_state.screen_top1 = {line=1, pos=1}
   Editor_state.screen_bottom1 = {}
+  Editor_state.selection1 = {}
   -- click on the empty line
   edit.draw(Editor_state)
   edit.run_after_mouse_click(Editor_state, Editor_state.left+8,Editor_state.top+5, 1)
   -- cursor moves
   check_eq(Editor_state.cursor1.line, 1, 'cursor')
+  -- selection remains empty
+  check_nil(Editor_state.selection1.line, 'selection is empty to avoid perturbing future edits')
+end
+
+function test_click_below_all_lines()
+  -- display one line
+  App.screen.init{width=50, height=80}
+  Editor_state = edit.initialize_test_state()
+  Editor_state.lines = load_array{'abc'}
+  Text.redraw_all(Editor_state)
+  Editor_state.cursor1 = {line=1, pos=1}
+  Editor_state.screen_top1 = {line=1, pos=1}
+  Editor_state.screen_bottom1 = {}
+  Editor_state.selection1 = {}
+  -- click below first line
+  edit.draw(Editor_state)
+  edit.run_after_mouse_click(Editor_state, Editor_state.left+8,Editor_state.top+50, 1)
+  -- cursor doesn't move
+  check_eq(Editor_state.cursor1.line, 1, 'cursor')
+  -- selection remains empty
+  check_nil(Editor_state.selection1.line, 'selection is empty to avoid perturbing future edits')
 end
 
 function test_draw_text()
diff --git a/text_tests b/text_tests
index f39d47e..2a31131 100644
--- a/text_tests
+++ b/text_tests
@@ -23,6 +23,7 @@ click on wrapping line rendered from partway at top of screen
 click past end of wrapping line
 click past end of wrapping line containing non ascii
 click past end of word wrapping line
+click below final line does nothing
 
 # cursor movement
 move left
diff --git a/text_tests.lua b/text_tests.lua
index 21a085a..bee6c31 100644
--- a/text_tests.lua
+++ b/text_tests.lua
@@ -275,6 +275,7 @@ function test_click_to_left_of_line()
   Editor_state.cursor1 = {line=1, pos=3}
   Editor_state.screen_top1 = {line=1, pos=1}
   Editor_state.screen_bottom1 = {}
+  Editor_state.selection1 = {}
   -- click to the left of the line
   edit.draw(Editor_state)
   edit.run_after_mouse_click(Editor_state, Editor_state.left-4,Editor_state.top+5, 1)
@@ -294,6 +295,7 @@ function test_click_takes_margins_into_account()
   Editor_state.cursor1 = {line=2, pos=1}
   Editor_state.screen_top1 = {line=1, pos=1}
   Editor_state.screen_bottom1 = {}
+  Editor_state.selection1 = {}
   -- click on the other line
   edit.draw(Editor_state)
   edit.run_after_mouse_click(Editor_state, Editor_state.left+8,Editor_state.top+5, 1)
@@ -312,11 +314,33 @@ function test_click_on_empty_line()
   Editor_state.cursor1 = {line=2, pos=1}
   Editor_state.screen_top1 = {line=1, pos=1}
   Editor_state.screen_bottom1 = {}
+  Editor_state.selection1 = {}
   -- click on the empty line
   edit.draw(Editor_state)
   edit.run_after_mouse_click(Editor_state, Editor_state.left+8,Editor_state.top+5, 1)
   -- cursor moves
   check_eq(Editor_state.cursor1.line, 1, 'cursor')
+  -- selection remains empty
+  check_nil(Editor_state.selection1.line, 'selection is empty to avoid perturbing future edits')
+end
+
+function test_click_below_all_lines()
+  -- display one line
+  App.screen.init{width=50, height=80}
+  Editor_state = edit.initialize_test_state()
+  Editor_state.lines = load_array{'abc'}
+  Text.redraw_all(Editor_state)
+  Editor_state.cursor1 = {line=1, pos=1}
+  Editor_state.screen_top1 = {line=1, pos=1}
+  Editor_state.screen_bottom1 = {}
+  Editor_state.selection1 = {}
+  -- click below first line
+  edit.draw(Editor_state)
+  edit.run_after_mouse_click(Editor_state, Editor_state.left+8,Editor_state.top+50, 1)
+  -- cursor doesn't move
+  check_eq(Editor_state.cursor1.line, 1, 'cursor')
+  -- selection remains empty
+  check_nil(Editor_state.selection1.line, 'selection is empty to avoid perturbing future edits')
 end
 
 function test_draw_text()