From 082546a1e7631371d8828bd6162d4569bbafd2fb Mon Sep 17 00:00:00 2001
From: Kovid Goyal <kovid@kovidgoyal.net>
Date: Sun, 12 Apr 2020 13:28:21 +0530
Subject: [PATCH] Marks: Fix marks not handling wide characters and tab
 characters correctly

Fixes #2534
---
 docs/changelog.rst    |  3 +++
 kitty/line.c          | 46 ++++++++++++++++++++++++++++---------------
 kitty_tests/screen.py | 20 ++++++++++++++++++-
 3 files changed, 52 insertions(+), 17 deletions(-)

diff --git a/docs/changelog.rst b/docs/changelog.rst
index 3636076c3..0fd1b2ac6 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -34,6 +34,9 @@ To update |kitty|, :doc:`follow the instructions <binary>`.
 - Linux: Ignore keys is they are designated as layout/group/mode switch keys
   (:iss:`2519`)
 
+- Marks: Fix marks not handling wide characters and tab characters correctly
+  (:iss:`2534`)
+
 
 0.17.2 [2020-03-29]
 --------------------
diff --git a/kitty/line.c b/kitty/line.c
index b58caa4a6..a1ddcd9e9 100644
--- a/kitty/line.c
+++ b/kitty/line.c
@@ -684,6 +684,33 @@ report_marker_error(PyObject *marker) {
     } else PyErr_Clear();
 }
 
+static inline void
+apply_mark(Line *line, const attrs_type mark, index_type *cell_pos, unsigned int *match_pos) {
+#define MARK { line->gpu_cells[x].attrs &= ATTRS_MASK_WITHOUT_MARK; line->gpu_cells[x].attrs |= mark; }
+    index_type x = *cell_pos;
+    MARK;
+    if (line->cpu_cells[x].ch) {
+        (*match_pos)++;
+        if (line->cpu_cells[x].ch == '\t') {
+            unsigned num_cells_to_skip_for_tab = line->cpu_cells[x].cc_idx[0];
+            while (num_cells_to_skip_for_tab && x + 1 < line->xnum && line->cpu_cells[x+1].ch == ' ') {
+                x++;
+                num_cells_to_skip_for_tab--;
+                MARK;
+            }
+        } else if ((line->gpu_cells[x].attrs & WIDTH_MASK) > 1 && x + 1 < line->xnum && !line->cpu_cells[x+1].ch) {
+            x++;
+            MARK;
+        } else {
+            for (index_type i = 0; i < arraysz(line->cpu_cells[x].cc_idx); i++) {
+                if (line->cpu_cells[x].cc_idx[i]) (*match_pos)++;
+            }
+        }
+    }
+    *cell_pos = x + 1;
+#undef MARK
+}
+
 static inline void
 apply_marker(PyObject *marker, Line *line, const PyObject *text) {
     unsigned int l=0, r=0, col=0, match_pos=0;
@@ -695,33 +722,20 @@ apply_marker(PyObject *marker, Line *line, const PyObject *text) {
     if (iter == NULL) { report_marker_error(marker); return; }
     PyObject *match;
     index_type x = 0;
-#define INCREMENT_MATCH_POS { \
-    if (line->cpu_cells[x].ch) { \
-        match_pos++; \
-        for (index_type i = 0; i < arraysz(line->cpu_cells[x].cc_idx); i++) { \
-            if (line->cpu_cells[x].cc_idx[i]) match_pos++; \
-}}}
-
     while ((match = PyIter_Next(iter)) && x < line->xnum) {
         Py_DECREF(match);
         while (match_pos < l && x < line->xnum) {
-            line->gpu_cells[x].attrs &= ATTRS_MASK_WITHOUT_MARK;
-            INCREMENT_MATCH_POS;
-            x++;
+            apply_mark(line, 0, &x, &match_pos);
         }
         attrs_type am = (col & MARK_MASK) << MARK_SHIFT;
         while(x < line->xnum && match_pos <= r) {
-            line->gpu_cells[x].attrs &= ATTRS_MASK_WITHOUT_MARK;
-            line->gpu_cells[x].attrs |= am;
-            INCREMENT_MATCH_POS;
-            x++;
+            apply_mark(line, am, &x, &match_pos);
         }
 
     }
-    while(x < line->xnum) line->gpu_cells[x++].attrs &= ATTRS_MASK_WITHOUT_MARK;
     Py_DECREF(iter);
+    while(x < line->xnum) line->gpu_cells[x++].attrs &= ATTRS_MASK_WITHOUT_MARK;
     if (PyErr_Occurred()) report_marker_error(marker);
-#undef INCREMENT_MATCH_POS
 }
 
 void
diff --git a/kitty_tests/screen.py b/kitty_tests/screen.py
index 82bf73d56..0007fb42c 100644
--- a/kitty_tests/screen.py
+++ b/kitty_tests/screen.py
@@ -456,12 +456,16 @@ class TestScreen(BaseTest):
         self.ae(as_text(True), '\x1b[mababa\x1b[mbabab\n\x1b[mc\n\n')
 
     def test_user_marking(self):
+
+        def cells(*a, y=0, mark=3):
+            return [(x, y, mark) for x in a]
+
         s = self.create_screen()
         s.draw('abaa')
         s.carriage_return(), s.linefeed()
         s.draw('xyxyx')
         s.set_marker(marker_from_regex('a', 3))
-        self.ae(s.marked_cells(), [(0, 0, 3), (2, 0, 3), (3, 0, 3)])
+        self.ae(s.marked_cells(), cells(0, 2, 3))
         s.set_marker()
         self.ae(s.marked_cells(), [])
 
@@ -488,3 +492,17 @@ class TestScreen(BaseTest):
             self.assertTrue(s.scroll_to_next_mark(0, False))
             self.ae(s.scrolled_by, 10 - i - 1)
         self.ae(s.scrolled_by, 0)
+
+        s = self.create_screen()
+        s.draw('🐈ab')
+        s.set_marker(marker_from_regex('🐈', 3))
+        self.ae(s.marked_cells(), cells(0, 1))
+        s.set_marker(marker_from_regex('🐈a', 3))
+        self.ae(s.marked_cells(), cells(0, 1, 2))
+        s = self.create_screen(cols=20)
+        s.tab()
+        s.draw('ab')
+        s.set_marker(marker_from_regex('a', 3))
+        self.ae(s.marked_cells(), cells(8))
+        s.set_marker(marker_from_regex('\t', 3))
+        self.ae(s.marked_cells(), cells(0, 1, 2, 3, 4, 5, 6, 7))