Return a FillResult struct from RunStyles::FillRange instead of modifying

arguments as that is clumsy when converting types.

7 files changed, 72 insertions, 58 deletions

diff --git a/src/Decoration.cxx b/src/Decoration.cxx
index 5a653a589..0470f26f9 100644
--- a/src/Decoration.cxx
+++ b/src/Decoration.cxx
@@ -90,18 +90,18 @@ void DecorationList::SetCurrentValue(int value) {
 	currentValue = value ? value : 1;
 }
 
-bool DecorationList::FillRange(Sci::Position &position, int value, Sci::Position &fillLength) {
+FillResult<Sci::Position> DecorationList::FillRange(Sci::Position position, int value, Sci::Position fillLength) {
 	if (!current) {
 		current = DecorationFromIndicator(currentIndicator);
 		if (!current) {
 			current = Create(currentIndicator, lengthDocument);
 		}
 	}
-	const bool changed = current->rs.FillRange(position, value, fillLength);
+	const FillResult<Sci::Position> fr = current->rs.FillRange(position, value, fillLength);
 	if (current->Empty()) {
 		Delete(currentIndicator);
 	}
-	return changed;
+	return fr;
 }
 
 void DecorationList::InsertSpace(Sci::Position position, Sci::Position insertLength) {
diff --git a/src/Decoration.h b/src/Decoration.h
index 0d799920e..86397fde1 100644
--- a/src/Decoration.h
+++ b/src/Decoration.h
@@ -51,8 +51,8 @@ public:
 	void SetCurrentValue(int value);
 	int GetCurrentValue() const { return currentValue; }
 
-	// Returns true if some values may have changed
-	bool FillRange(Sci::Position &position, int value, Sci::Position &fillLength);
+	// Returns with changed=true if some values may have changed
+	FillResult<Sci::Position> FillRange(Sci::Position position, int value, Sci::Position fillLength);
 
 	void InsertSpace(Sci::Position position, Sci::Position insertLength);
 	void DeleteRange(Sci::Position position, Sci::Position deleteLength);
diff --git a/src/Document.cxx b/src/Document.cxx
index cb2892c96..e0373a752 100644
--- a/src/Document.cxx
+++ b/src/Document.cxx
@@ -2243,11 +2243,11 @@ void SCI_METHOD Document::DecorationSetCurrentIndicator(int indicator) {
 }
 
 void SCI_METHOD Document::DecorationFillRange(Sci_Position position, int value, Sci_Position fillLength) {
-	Sci::Position sciPosition = static_cast<Sci::Position>(position);
-	Sci::Position sciFillLength = static_cast<Sci::Position>(fillLength);
-	if (decorations.FillRange(sciPosition, value, sciFillLength)) {
+	const FillResult<Sci::Position> fr = decorations.FillRange(
+		static_cast<Sci::Position>(position), value, static_cast<Sci::Position>(fillLength));
+	if (fr.changed) {
 		const DocModification mh(SC_MOD_CHANGEINDICATOR | SC_PERFORMED_USER,
-							sciPosition, sciFillLength);
+							fr.position, fr.fillLength);
 		NotifyModified(mh);
 	}
 }
diff --git a/src/RunStyles.cxx b/src/RunStyles.cxx
index fa8844bfe..03692f673 100644
--- a/src/RunStyles.cxx
+++ b/src/RunStyles.cxx
@@ -126,13 +126,14 @@ DISTANCE RunStyles<DISTANCE, STYLE>::EndRun(DISTANCE position) const {
 }
 
 template <typename DISTANCE, typename STYLE>
-bool RunStyles<DISTANCE, STYLE>::FillRange(DISTANCE &position, STYLE value, DISTANCE &fillLength) {
+FillResult<DISTANCE> RunStyles<DISTANCE, STYLE>::FillRange(DISTANCE position, STYLE value, DISTANCE fillLength) {
+	const FillResult<DISTANCE> resultNoChange{false, position, fillLength};
 	if (fillLength <= 0) {
-		return false;
+		return resultNoChange;
 	}
 	DISTANCE end = position + fillLength;
 	if (end > Length()) {
-		return false;
+		return resultNoChange;
 	}
 	DISTANCE runEnd = RunFromPosition(end);
 	if (styles->ValueAt(runEnd) == value) {
@@ -140,7 +141,7 @@ bool RunStyles<DISTANCE, STYLE>::FillRange(DISTANCE &position, STYLE value, DIST
 		end = starts->PositionFromPartition(runEnd);
 		if (position >= end) {
 			// Whole range is already same as value so no action
-			return false;
+			return resultNoChange;
 		}
 		fillLength = end - position;
 	} else {
@@ -159,6 +160,7 @@ bool RunStyles<DISTANCE, STYLE>::FillRange(DISTANCE &position, STYLE value, DIST
 		}
 	}
 	if (runStart < runEnd) {
+		const FillResult<DISTANCE> result{ true, position, fillLength };
 		styles->SetValueAt(runStart, value);
 		// Remove each old run over the range
 		for (DISTANCE run=runStart+1; run<runEnd; run++) {
@@ -169,16 +171,15 @@ bool RunStyles<DISTANCE, STYLE>::FillRange(DISTANCE &position, STYLE value, DIST
 		RemoveRunIfSameAsPrevious(runStart);
 		runEnd = RunFromPosition(end);
 		RemoveRunIfEmpty(runEnd);
-		return true;
+		return result;
 	} else {
-		return false;
+		return resultNoChange;
 	}
 }
 
 template <typename DISTANCE, typename STYLE>
 void RunStyles<DISTANCE, STYLE>::SetValueAt(DISTANCE position, STYLE value) {
-	DISTANCE len = 1;
-	FillRange(position, value, len);
+	FillRange(position, value, 1);
 }
 
 template <typename DISTANCE, typename STYLE>
diff --git a/src/RunStyles.h b/src/RunStyles.h
index c28621334..af2eb3ec6 100644
--- a/src/RunStyles.h
+++ b/src/RunStyles.h
@@ -12,6 +12,16 @@
 
 namespace Scintilla {
 
+// Return for RunStyles::FillRange reports if anything was changed and the
+// range that was changed. This may be trimmed from the requested range
+// when some of the requested range already had the requested value.
+template <typename DISTANCE>
+struct FillResult {
+	bool changed;
+	DISTANCE position;
+	DISTANCE fillLength;
+};
+
 template <typename DISTANCE, typename STYLE>
 class RunStyles {
 private:
@@ -33,8 +43,8 @@ public:
 	DISTANCE FindNextChange(DISTANCE position, DISTANCE end) const;
 	DISTANCE StartRun(DISTANCE position) const;
 	DISTANCE EndRun(DISTANCE position) const;
-	// Returns true if some values may have changed
-	bool FillRange(DISTANCE &position, STYLE value, DISTANCE &fillLength);
+	// Returns changed=true if some values may have changed
+	FillResult<DISTANCE> FillRange(DISTANCE position, STYLE value, DISTANCE fillLength);
 	void SetValueAt(DISTANCE position, STYLE value);
 	void InsertSpace(DISTANCE position, DISTANCE insertLength);
 	void DeleteAll();
diff --git a/test/unit/testDecoration.cxx b/test/unit/testDecoration.cxx
index 3e18b1665..d2fbc7df2 100644
--- a/test/unit/testDecoration.cxx
+++ b/test/unit/testDecoration.cxx
@@ -73,19 +73,18 @@ TEST_CASE("DecorationList") {
 		const int value = 59;
 		Sci::Position position = 4;
 		Sci::Position fillLength = 3;
-		bool changed = decol.FillRange(position, value, fillLength);
-		REQUIRE(changed);
-		REQUIRE(position == 4);
-		REQUIRE(fillLength == 3);
-		REQUIRE(fillLength == 3);
+		auto fr = decol.FillRange(position, value, fillLength);
+		REQUIRE(fr.changed);
+		REQUIRE(fr.position == 4);
+		REQUIRE(fr.fillLength == 3);
 		REQUIRE(decol.ValueAt(indicator, 5) == value);
 		REQUIRE(decol.AllOnFor(5) == (1 << indicator));
 		REQUIRE(decol.Start(indicator, 5) == 4);
 		REQUIRE(decol.End(indicator, 5) == 7);
 		const int indicatorB=6;
 		decol.SetCurrentIndicator(indicatorB);
-		changed = decol.FillRange(position, value, fillLength);
-		REQUIRE(changed);
+		fr = decol.FillRange(position, value, fillLength);
+		REQUIRE(fr.changed);
 		REQUIRE(decol.AllOnFor(5) == ((1 << indicator) | (1 << indicatorB)));
 		decol.DeleteRange(5, 1);
 		REQUIRE(decol.Start(indicatorB, 5) == 4);
diff --git a/test/unit/testRunStyles.cxx b/test/unit/testRunStyles.cxx
index 75daac26e..3edb61657 100644
--- a/test/unit/testRunStyles.cxx
+++ b/test/unit/testRunStyles.cxx
@@ -21,6 +21,14 @@ using namespace Scintilla;
 
 // Test RunStyles.
 
+namespace Scintilla {	// Xcode clang 9.0 doesn't like this when in the unnamed namespace
+	bool operator==(const FillResult<int> &fra, const FillResult<int> &frb) {
+		return fra.changed == frb.changed &&
+			fra.position == frb.position &&
+			fra.fillLength == frb.fillLength;
+	}
+}
+
 TEST_CASE("RunStyles") {
 
 	RunStyles<int, int> rs;
@@ -93,9 +101,8 @@ TEST_CASE("RunStyles") {
 		rs.InsertSpace(0, 5);
 		int startFill = 1;
 		int lengthFill = 3;
-		REQUIRE(true == rs.FillRange(startFill, 99, lengthFill));
-		REQUIRE(1 == startFill);
-		REQUIRE(3 == lengthFill);
+		const auto fr = rs.FillRange(startFill, 99, lengthFill);
+		REQUIRE(FillResult<int>{true, 1, 3} == fr);
 
 		REQUIRE(0 == rs.ValueAt(0));
 		REQUIRE(99 == rs.ValueAt(1));
@@ -114,16 +121,14 @@ TEST_CASE("RunStyles") {
 		rs.InsertSpace(0, 5);
 		int startFill = 1;
 		int lengthFill = 3;
-		REQUIRE(true == rs.FillRange(startFill, 99, lengthFill));
-		REQUIRE(1 == startFill);
-		REQUIRE(3 == lengthFill);
+		const auto fr = rs.FillRange(startFill, 99, lengthFill);
+		REQUIRE(FillResult<int>{true, 1, 3} == fr);
 
 		int startFill2 = 2;
 		int lengthFill2 = 1;
 		// Compiler warnings if 'false' used instead of '0' as expected value:
-		REQUIRE(false == rs.FillRange(startFill2, 99, lengthFill2));
-		REQUIRE(2 == startFill2);
-		REQUIRE(1 == lengthFill2);
+		const auto fr2 = rs.FillRange(startFill2, 99, lengthFill2);
+		REQUIRE(FillResult<int>{false, 2, 1} == fr2);
 		REQUIRE(0 == rs.ValueAt(0));
 		REQUIRE(99 == rs.ValueAt(1));
 		REQUIRE(99 == rs.ValueAt(2));
@@ -136,15 +141,13 @@ TEST_CASE("RunStyles") {
 		rs.InsertSpace(0, 5);
 		int startFill = 1;
 		int lengthFill = 2;
-		REQUIRE(true == rs.FillRange(startFill, 99, lengthFill));
-		REQUIRE(1 == startFill);
-		REQUIRE(2 == lengthFill);
+		const auto fr = rs.FillRange(startFill, 99, lengthFill);
+		REQUIRE(FillResult<int>{true, 1, 2} == fr);
 
 		int startFill2 = 2;
 		int lengthFill2 = 2;
-		REQUIRE(true == rs.FillRange(startFill2, 99, lengthFill2));
-		REQUIRE(3 == startFill2);
-		REQUIRE(1 == lengthFill2);
+		const auto fr2 = rs.FillRange(startFill2, 99, lengthFill2);
+		REQUIRE(FillResult<int>{true, 3, 1} == fr2);
 		REQUIRE(3 == rs.Runs());
 	}
 
@@ -174,9 +177,8 @@ TEST_CASE("RunStyles") {
 		rs.InsertSpace(0, 5);
 		int startFill = 1;
 		int lengthFill = 3;
-		REQUIRE(true == rs.FillRange(startFill, 99, lengthFill));
-		REQUIRE(1 == startFill);
-		REQUIRE(3 == lengthFill);
+		const auto fr = rs.FillRange(startFill, 99, lengthFill);
+		REQUIRE(FillResult<int>{true, 1, 3} == fr);
 
 		REQUIRE(0 == rs.Find(0,0));
 		REQUIRE(1 == rs.Find(99,0));
@@ -211,11 +213,13 @@ TEST_CASE("RunStyles") {
 		REQUIRE(true == rs.AllSameAs(0));
 		int startFill = 1;
 		int lengthFill = 3;
-		REQUIRE(true == rs.FillRange(startFill, 99, lengthFill));
+		const auto fr = rs.FillRange(startFill, 99, lengthFill);
+		REQUIRE(true == fr.changed);
 		REQUIRE(false == rs.AllSame());
 		REQUIRE(false == rs.AllSameAs(88));
 		REQUIRE(false == rs.AllSameAs(0));
-		REQUIRE(true == rs.FillRange(startFill, 0, lengthFill));
+		const auto fr2 = rs.FillRange(startFill, 0, lengthFill);
+		REQUIRE(true == fr2.changed);
 		REQUIRE(true == rs.AllSame());
 		REQUIRE(false == rs.AllSameAs(88));
 		REQUIRE(true == rs.AllSameAs(0));
@@ -227,29 +231,27 @@ TEST_CASE("RunStyles") {
 
 		int startFill = 1;
 		int lengthFill = 1;
-		REQUIRE(true == rs.FillRange(startFill, 99, lengthFill));
-		REQUIRE(1 == startFill);
-		REQUIRE(1 == lengthFill);
+		const auto fr = rs.FillRange(startFill, 99, lengthFill);
+		REQUIRE(FillResult<int>{true, 1, 1} == fr);
 		REQUIRE(3 == rs.Runs());
 
 		startFill = 2;
 		lengthFill = 1;
-		REQUIRE(true == rs.FillRange(startFill, 99, lengthFill));
-		REQUIRE(2 == startFill);
-		REQUIRE(1 == lengthFill);
+		const auto fr2 = rs.FillRange(startFill, 99, lengthFill);
+		REQUIRE(FillResult<int>{true, 2, 1} == fr2);
 		REQUIRE(3 == rs.Runs());
 
 		startFill = 1;
 		lengthFill = 1;
-		REQUIRE(true == rs.FillRange(startFill, 0, lengthFill));
+		const auto fr3 = rs.FillRange(startFill, 0, lengthFill);
+		REQUIRE(FillResult<int>{true, 1, 1} == fr3);
 		REQUIRE(3 == rs.Runs());
-		REQUIRE(1 == lengthFill);
 
 		startFill = 2;
 		lengthFill = 1;
-		REQUIRE(true == rs.FillRange(startFill, 0, lengthFill));
+		const auto fr4 = rs.FillRange(startFill, 0, lengthFill);
+		REQUIRE(FillResult<int>{true, 2, 1} == fr4);
 		REQUIRE(1 == rs.Runs());
-		REQUIRE(1 == lengthFill);
 
 		REQUIRE(-1 == rs.Find(0,6));
 	}
@@ -284,7 +286,8 @@ TEST_CASE("RunStyles") {
 		rs.InsertSpace(0, 3);
 		int startFill = 1;
 		int lengthFill = 1;
-		REQUIRE(true == rs.FillRange(startFill, 99, lengthFill));
+		const auto fr = rs.FillRange(startFill, 99, lengthFill);
+		REQUIRE(true == fr.changed);
 		REQUIRE(3 == rs.Length());
 		REQUIRE(3 == rs.Runs());
 		rs.DeleteRange(1, 1);
@@ -296,7 +299,8 @@ TEST_CASE("RunStyles") {
 		rs.InsertSpace(0, 2);
 		int startFill = 1;
 		int lengthFill = 1;
-		REQUIRE(true == rs.FillRange(startFill, 99, lengthFill));
+		const auto fr = rs.FillRange(startFill, 99, lengthFill);
+		REQUIRE(true == fr.changed);
 		REQUIRE(2 == rs.Length());
 		REQUIRE(2 == rs.Runs());
 		REQUIRE(0 == rs.StartRun(0));