Bug [#2140]. Move rather than grow selection when insertion at start.

4 files changed, 114 insertions, 5 deletions

diff --git a/doc/ScintillaHistory.html b/doc/ScintillaHistory.html
index 8109018c1..05e03640f 100644
--- a/doc/ScintillaHistory.html
+++ b/doc/ScintillaHistory.html
@@ -567,6 +567,10 @@
 	Released 24 October 2019.
 	</li>
 	<li>
+	Move rather than grow selection when insertion at start.
+	<a href="https://sourceforge.net/p/scintilla/bugs/2140/">Bug #2140</a>.
+	</li>
+	<li>
 	Allow target to have virtual space.
 	Add methods for finding the virtual space at start and end of multiple selections.
 	<a href="https://sourceforge.net/p/scintilla/feature-requests/1316/">Feature #1316</a>.
diff --git a/src/Selection.cxx b/src/Selection.cxx
index 9b08ea001..5667bf234 100644
--- a/src/Selection.cxx
+++ b/src/Selection.cxx
@@ -23,12 +23,16 @@
 
 using namespace Scintilla;
 
-void SelectionPosition::MoveForInsertDelete(bool insertion, Sci::Position startChange, Sci::Position length) noexcept {
+void SelectionPosition::MoveForInsertDelete(bool insertion, Sci::Position startChange, Sci::Position length, bool moveForEqual) noexcept {
 	if (insertion) {
 		if (position == startChange) {
+			// Always consume virtual space
 			const Sci::Position virtualLengthRemove = std::min(length, virtualSpace);
 			virtualSpace -= virtualLengthRemove;
-			position += virtualLengthRemove;
+			if (moveForEqual) {
+				const Sci::Position lengthAfterVirtualRemove = length - virtualLengthRemove;
+				position += lengthAfterVirtualRemove;
+			}
 		} else if (position > startChange) {
 			position += length;
 		}
@@ -85,8 +89,17 @@ Sci::Position SelectionRange::Length() const noexcept {
 }
 
 void SelectionRange::MoveForInsertDelete(bool insertion, Sci::Position startChange, Sci::Position length) noexcept {
-	caret.MoveForInsertDelete(insertion, startChange, length);
-	anchor.MoveForInsertDelete(insertion, startChange, length);
+	// For insertions that occur at the start of the selection move both the start
+	// and end of the selection to preserve the selected length.
+	// The end will automatically move since it is after the insertion, so determine
+	// which position is the start and pass this into
+	// SelectionPosition::MoveForInsertDelete.
+	// There isn't any reason to move an empty selection so don't move it.
+	const bool caretStart = caret < anchor;
+	const bool anchorStart = anchor < caret;
+
+	caret.MoveForInsertDelete(insertion, startChange, length, caretStart);
+	anchor.MoveForInsertDelete(insertion, startChange, length, anchorStart);
 }
 
 bool SelectionRange::Contains(Sci::Position pos) const noexcept {
diff --git a/src/Selection.h b/src/Selection.h
index 11a6f097a..741b75277 100644
--- a/src/Selection.h
+++ b/src/Selection.h
@@ -23,7 +23,7 @@ public:
 		position = 0;
 		virtualSpace = 0;
 	}
-	void MoveForInsertDelete(bool insertion, Sci::Position startChange, Sci::Position length) noexcept;
+	void MoveForInsertDelete(bool insertion, Sci::Position startChange, Sci::Position length, bool moveForEqual) noexcept;
 	bool operator ==(const SelectionPosition &other) const noexcept {
 		return position == other.position && virtualSpace == other.virtualSpace;
 	}
diff --git a/test/simpleTests.py b/test/simpleTests.py
index f16d4268c..6cacf1045 100644
--- a/test/simpleTests.py
+++ b/test/simpleTests.py
@@ -1387,6 +1387,11 @@ class TestMultiSelection(unittest.TestCase):
 		# 3 lines of 3 characters
 		t = b"xxx\nxxx\nxxx"
 		self.ed.AddText(len(t), t)
+		
+	def textOfSelection(self, n):
+		self.ed.TargetStart = self.ed.GetSelectionNStart(n)
+		self.ed.TargetEnd = self.ed.GetSelectionNEnd(n)
+		return bytes(self.ed.GetTargetText())
 
 	def testSelectionCleared(self):
 		self.ed.ClearSelections()
@@ -1542,6 +1547,93 @@ class TestMultiSelection(unittest.TestCase):
 		self.ed.DropSelectionN(0)
 		self.assertEquals(self.ed.MainSelection, 2)
 
+	def partFromSelection(self, n):
+		# Return a tuple (order, text) from a selection part
+		# order is a boolean whether the caret is before the anchor
+		self.ed.TargetStart = self.ed.GetSelectionNStart(n)
+		self.ed.TargetEnd = self.ed.GetSelectionNEnd(n)
+		return (self.ed.GetSelectionNCaret(n) < self.ed.GetSelectionNAnchor(n), self.textOfSelection(n))
+
+	def replacePart(self, n, part):
+		startSelection = self.ed.GetSelectionNStart(n)
+		self.ed.TargetStart = startSelection
+		self.ed.TargetEnd = self.ed.GetSelectionNEnd(n)
+		direction, text = part
+		length = len(text)
+		self.ed.ReplaceTarget(len(text), text)
+		if direction:
+			self.ed.SetSelectionNCaret(n, startSelection)
+			self.ed.SetSelectionNAnchor(n, startSelection + length)
+		else:
+			self.ed.SetSelectionNAnchor(n, startSelection)
+			self.ed.SetSelectionNCaret(n, startSelection + length)
+
+	def swapSelections(self):
+		# Swap the selections
+		part0 = self.partFromSelection(0)
+		part1 = self.partFromSelection(1)
+
+		self.replacePart(1, part0)
+		self.replacePart(0, part1)
+
+	def checkAdjacentSelections(self, selections, invert):
+		# Only called from testAdjacentSelections to try one permutation
+		self.ed.ClearAll()
+		self.ed.EmptyUndoBuffer()
+		t = b"ab"
+		texts = (b'b', b'a') if invert else (b'a', b'b')
+		self.ed.AddText(len(t), t)
+		sel0, sel1 = selections
+		self.ed.SetSelection(sel0[0], sel0[1])
+		self.ed.AddSelection(sel1[0], sel1[1])
+		self.assertEquals(self.ed.Selections, 2)
+		self.assertEquals(self.textOfSelection(0), texts[0])
+		self.assertEquals(self.textOfSelection(1), texts[1])
+		self.swapSelections()
+		self.assertEquals(self.ed.Contents(), b'ba')
+		self.assertEquals(self.ed.Selections, 2)
+		self.assertEquals(self.textOfSelection(0), texts[1])
+		self.assertEquals(self.textOfSelection(1), texts[0])
+
+	def testAdjacentSelections(self):
+		# For various permutations of selections, try swapping the text and ensure that the
+		# selections remain distinct
+		self.checkAdjacentSelections(((1, 0),(1, 2)), False)
+		self.checkAdjacentSelections(((0, 1),(1, 2)), False)
+		self.checkAdjacentSelections(((1, 0),(2, 1)), False)
+		self.checkAdjacentSelections(((0, 1),(2, 1)), False)
+
+		# Reverse order, first selection is after second
+		self.checkAdjacentSelections(((1, 2),(1, 0)), True)
+		self.checkAdjacentSelections(((1, 2),(0, 1)), True)
+		self.checkAdjacentSelections(((2, 1),(1, 0)), True)
+		self.checkAdjacentSelections(((2, 1),(0, 1)), True)
+
+	def testInsertBefore(self):
+		self.ed.ClearAll()
+		t = b"a"
+		self.ed.AddText(len(t), t)
+		self.ed.SetSelection(0, 1)
+		self.assertEquals(self.textOfSelection(0), b'a')
+
+		self.ed.SetTargetRange(0, 0)
+		self.ed.ReplaceTarget(1, b'1')
+		self.assertEquals(self.ed.Contents(), b'1a')
+		self.assertEquals(self.textOfSelection(0), b'a')
+
+	def testInsertAfter(self):
+		self.ed.ClearAll()
+		t = b"a"
+		self.ed.AddText(len(t), t)
+		self.ed.SetSelection(0, 1)
+		self.assertEquals(self.textOfSelection(0), b'a')
+
+		self.ed.SetTargetRange(1, 1)
+		self.ed.ReplaceTarget(1, b'9')
+		self.assertEquals(self.ed.Contents(), b'a9')
+		self.assertEquals(self.textOfSelection(0), b'a')
+
+
 class TestModalSelection(unittest.TestCase):
 
 	def setUp(self):