From a91053ade06cddb0b288d480a060e2b0936d915b Mon Sep 17 00:00:00 2001
From: David Brochart <david.brochart@gmail.com>
Date: Wed, 18 Oct 2023 13:22:25 +0200
Subject: [PATCH] Review

---
 src/textual/document/_document.py | 50 ++++++++++---------------------
 tests/document/test_document.py   | 34 +++++++++++++++++++++
 2 files changed, 49 insertions(+), 35 deletions(-)

diff --git a/src/textual/document/_document.py b/src/textual/document/_document.py
index 69a7b35d60..6fa2341911 100644
--- a/src/textual/document/_document.py
+++ b/src/textual/document/_document.py
@@ -90,28 +90,6 @@ def text(self) -> str:
     def newline(self) -> Newline:
         """Return the line separator used in the document."""
 
-    @abstractmethod
-    def get_index_from_location(self, location: Location) -> int:
-        """Given a location, returns the index from the document's text.
-
-        Args:
-            location: The location in the document.
-
-        Returns:
-            The index in the document's text.
-        """
-
-    @abstractmethod
-    def get_location_from_index(self, index: int) -> Location:
-        """Given an index in the document's text, returns the corresponding location.
-
-        Args:
-            index: The index in the document's text.
-
-        Returns:
-            The corresponding location.
-        """
-
     @abstractmethod
     def get_line(self, index: int) -> str:
         """Returns the line with the given index from the document.
@@ -351,10 +329,10 @@ def get_index_from_location(self, location: Location) -> int:
         Returns:
             The index in the document's text.
         """
-        row, col = location
-        index = row * len(self.newline) + col
-        for i in range(row):
-            index += len(self.get_line(i))
+        row, column = location
+        index = row * len(self.newline) + column
+        for line_index in range(row):
+            index += len(self.get_line(line_index))
         return index
 
     def get_location_from_index(self, index: int) -> Location:
@@ -366,15 +344,17 @@ def get_location_from_index(self, index: int) -> Location:
         Returns:
             The corresponding location.
         """
-        idx = 0
-        newline_len = len(self.newline)
-        for i in range(self.line_count):
-            next_idx = idx + len(self.get_line(i)) + newline_len
-            if index < next_idx:
-                return (i, index - idx)
-            elif index == next_idx:
-                return (i + 1, 0)
-            idx = next_idx
+        column_index = 0
+        newline_length = len(self.newline)
+        for line_index in range(self.line_count):
+            next_column_index = (
+                column_index + len(self.get_line(line_index)) + newline_length
+            )
+            if index < next_column_index:
+                return (line_index, index - column_index)
+            elif index == next_column_index:
+                return (line_index + 1, 0)
+            column_index = next_column_index
 
     def get_line(self, index: int) -> str:
         """Returns the line with the given index from the document.
diff --git a/tests/document/test_document.py b/tests/document/test_document.py
index b6e9952782..a7dbc86cdc 100644
--- a/tests/document/test_document.py
+++ b/tests/document/test_document.py
@@ -98,3 +98,37 @@ def test_get_selected_text_no_newline_at_end_of_file_windows():
     document = Document(TEXT_WINDOWS)
     selection = document.get_text_range((0, 0), (2, 0))
     assert selection == TEXT_WINDOWS
+
+
+@pytest.mark.parametrize(
+    "text", [TEXT, TEXT_NEWLINE, TEXT_WINDOWS, TEXT_WINDOWS_NEWLINE]
+)
+def test_index_from_location(text):
+    document = Document(text)
+    lines = text.split(document.newline)
+    assert document.get_index_from_location((0, 0)) == 0
+    assert document.get_index_from_location((0, len(lines[0]))) == len(lines[0])
+    assert document.get_index_from_location((1, 0)) == len(lines[0]) + len(
+        document.newline
+    )
+    assert document.get_index_from_location((len(lines) - 1, len(lines[-1]))) == len(
+        text
+    )
+
+
+@pytest.mark.parametrize(
+    "text", [TEXT, TEXT_NEWLINE, TEXT_WINDOWS, TEXT_WINDOWS_NEWLINE]
+)
+def test_location_from_index(text):
+    document = Document(text)
+    lines = text.split(document.newline)
+    assert document.get_location_from_index(0) == (0, 0)
+    assert document.get_location_from_index(len(lines[0])) == (0, len(lines[0]))
+    assert document.get_location_from_index(len(lines[0]) + len(document.newline)) == (
+        1,
+        0,
+    )
+    assert document.get_location_from_index(len(text)) == (
+        len(lines) - 1,
+        len(lines[-1]),
+    )