Refactor column highlighter to make it more robust

Support arbitrary orders for column highlighters (it was previously
failing when column highlighters were not applied in column order).

Fix show_matching tab handling at the same time (horizontal scrolling,
tab characters and show_matching were behaving badly).

Window highlighting now runs user highlighters, then built-ins for each
phases, instead of running all phases for user highlighters, then all
phases for built-ins.

We now consider unprintable character to be 1-column width as we know
we will display them as "�".

Fixes #1615
Fixes #1023

src/display_buffer.cc

@@ -92,16 +92,23 @@ DisplayLine::iterator DisplayLine::split(iterator it, BufferCoord pos)
     return m_atoms.insert(it, std::move(atom));
 }
 
-DisplayLine::iterator DisplayLine::split(iterator it, ColumnCount pos)
+DisplayLine::iterator DisplayLine::split(iterator it, ColumnCount count)
 {
-    kak_assert(it->type() == DisplayAtom::Text);
+    kak_assert(count > 0);
-    kak_assert(pos > 0);
+    kak_assert(count < it->length());
-    kak_assert(pos < it->length());
 
-    DisplayAtom atom(it->m_text.substr(0, pos).str());
+    if (it->type() == DisplayAtom::Text or it->type() == DisplayAtom::ReplacedRange)
-    it->m_text = it->m_text.substr(pos).str();
+    {
+        DisplayAtom atom = *it;
+        atom.m_text = atom.m_text.substr(0, count).str();
+        it->m_text = it->m_text.substr(count).str();
         return m_atoms.insert(it, std::move(atom));
     }
+    auto pos = utf8::advance(get_iterator(it->buffer(), it->begin()),
+                             get_iterator(it->buffer(), it->end()), 
+                             count).coord();
+    return split(it, pos);
+}
 
 DisplayLine::iterator DisplayLine::insert(iterator it, DisplayAtom atom)
 {

src/highlighters.cc

@@ -615,59 +615,37 @@ HighlighterAndId create_column_highlighter(HighlighterParameters params)
         if (column < 0)
             return;
 
-        const Buffer& buffer = context.buffer();
-        const int tabstop = context.options()["tabstop"].get<int>();
         auto face = get_face(facespec);
+        auto win_column = context.window().position().column;
         for (auto& line : display_buffer.lines())
         {
-            const LineCount buf_line = line.range().begin.line;
+            auto target_col = column - win_column; 
-            const ByteCount byte_col = get_byte_to_column(buffer, tabstop, {buf_line, column});
+            if (target_col < 0)
-            const BufferCoord coord{buf_line, byte_col};
+                return;
+
             bool found = false;
-            if (buffer.is_valid(coord) and not buffer.is_end(coord))
+            auto first_buf = find_if(line, [](auto& atom) { return atom.has_buffer_range(); });
+            for (auto atom_it = first_buf; atom_it != line.end(); ++atom_it)
             {
-                for (auto atom_it = line.begin(); atom_it != line.end(); ++atom_it)
+                const auto atom_len = atom_it->length();
+                if (target_col < atom_len)
                 {
-                    if (atom_it->type() != DisplayAtom::Range)
+                    if (target_col > 0)
-                        continue;
+                        atom_it = ++line.split(atom_it, target_col);
-
+                    if (atom_it->length() > 1)
-                    kak_assert(atom_it->begin().line == buf_line);
+                        atom_it = line.split(atom_it, 1_col);
-                    if (coord >= atom_it->begin() and coord < atom_it->end())
+                    atom_it->face = merge_faces(atom_it->face, face);
-                    {
-                        if (coord > atom_it->begin())
-                            atom_it = ++line.split(atom_it, coord);
-                        if (buffer.next(coord) < atom_it->end())
-                            atom_it = line.split(atom_it, buffer.next(coord));
-
-                        apply_face(face)(*atom_it);
                     found = true;
                     break;
                 }
+                target_col -= atom_len;
             }
-            }
+            if (found)
-            if (not found)
+                continue;
-            {
-                ColumnCount last_buffer_col = context.window().position().column;
-                for (auto& atom : line)
-                {
-                    if (atom.has_buffer_range())
-                    {
-                        auto pos = atom.end();
-                        if (pos.column == 0)
-                            pos = {pos.line-1, buffer[pos.line-1].length()};
-                        if (pos != atom.begin())
-                            last_buffer_col = get_column(buffer, tabstop, pos);
-                    }
-                }
 
-                ColumnCount count = column - last_buffer_col;
+            if (target_col > 0)
-                if (count >= 0)
+                line.push_back({String{' ', target_col}});
-                {
+            line.push_back({" ", face});
-                    if (count > 0)
-                        line.push_back({String{' ',  count}});
-                    line.push_back({String{" "}, face});
-                }
-            }
         }
     };
 
@@ -930,6 +908,7 @@ void show_whitespaces(const Context& context, HighlightPass, DisplayBuffer& disp
     const int tabstop = context.options()["tabstop"].get<int>();
     auto whitespaceface = get_face("Whitespace");
     auto& buffer = context.buffer();
+    auto win_column = context.window().position().column;
     for (auto& line : display_buffer.lines())
     {
         for (auto atom_it = line.begin(); atom_it != line.end(); ++atom_it)
@@ -952,9 +931,10 @@ void show_whitespaces(const Context& context, HighlightPass, DisplayBuffer& disp
 
                     if (cp == '\t')
                     {
-                        int column = (int)get_column(buffer, tabstop, coord);
+                        const ColumnCount column = get_column(buffer, tabstop, coord);
-                        int count = tabstop - (column % tabstop);
+                        const ColumnCount count = tabstop - (column % tabstop) -
-                        atom_it->replace(tab + String(tabpad[(CharCount)0], CharCount{count-1}));
+                                                  std::max(win_column - column, 0_col);
+                        atom_it->replace(tab + String(tabpad[(CharCount)0], count - tab.column_length()));
                     }
                     else if (cp == ' ')
                         atom_it->replace(spc.str());
@@ -996,7 +976,7 @@ HighlighterAndId show_whitespaces_factory(HighlighterParameters params)
                           get_param("lf", "¬"),
                           get_param("nbsp", "⍽"));
 
-    return {"show_whitespaces", make_highlighter(std::move(func))};
+    return {"show_whitespaces", make_highlighter(std::move(func), HighlightPass::Move)};
 }
 
 struct LineNumbersHighlighter : Highlighter

src/unicode.hh

@@ -58,7 +58,7 @@ inline ColumnCount codepoint_width(Codepoint c) noexcept
     if (c == '\n')
         return 1;
     const auto width = wcwidth((wchar_t)c);
-    return width > 0 ? width : 0;
+    return width >= 0 ? width : 1;
 }
 
 enum class CharCategories

src/window.cc

@@ -136,15 +136,18 @@ const DisplayBuffer& Window::update_display_buffer(const Context& context)
             buffer()[buffer_line].length()
           : get_byte_to_column(buffer(), tabstop, {buffer_line, m_position.column + m_range.column});
 
+        // The display buffer always has at least one buffer atom, which might be empty if
+        // beg_byte == end_byte
         lines.emplace_back(AtomList{ {buffer(), {buffer_line, beg_byte}, {buffer_line, end_byte}} });
     }
 
     m_display_buffer.compute_range();
     BufferRange range{{0,0}, buffer().end_coord()};
     for (auto pass : { HighlightPass::Wrap, HighlightPass::Move, HighlightPass::Colorize })
+    {
         m_highlighters.highlight(context, pass, m_display_buffer, range);
-    for (auto pass : { HighlightPass::Wrap, HighlightPass::Move, HighlightPass::Colorize })
         m_builtin_highlighters.highlight(context, pass, m_display_buffer, range);
+    }
 
     m_display_buffer.optimize();
 

test/highlight/column/multi-columns/display

@@ -0,0 +1,6 @@
+{ "jsonrpc": "2.0", "method": "draw", "params": [[[{ "face": { "fg": "default", "bg": "default", "attributes": [] }, "contents": "1│" }, { "face": { "fg": "black", "bg": "white", "attributes": [] }, "contents": "a" }, { "face": { "fg": "default", "bg": "default", "attributes": [] }, "contents": "\u000a" }, { "face": { "fg": "default", "bg": "red", "attributes": [] }, "contents": " " }, { "face": { "fg": "default", "bg": "default", "attributes": [] }, "contents": " " }, { "face": { "fg": "default", "bg": "blue", "attributes": [] }, "contents": " " }, { "face": { "fg": "default", "bg": "default", "attributes": [] }, "contents": " " }, { "face": { "fg": "default", "bg": "green", "attributes": [] }, "contents": " " }], [{ "face": { "fg": "default", "bg": "default", "attributes": [] }, "contents": "2│" }, { "face": { "fg": "default", "bg": "default", "attributes": [] }, "contents": "  " }, { "face": { "fg": "default", "bg": "red", "attributes": [] }, "contents": " " }, { "face": { "fg": "default", "bg": "default", "attributes": [] }, "contents": " " }, { "face": { "fg": "default", "bg": "blue", "attributes": [] }, "contents": "a" }, { "face": { "fg": "default", "bg": "default", "attributes": [] }, "contents": "b" }, { "face": { "fg": "default", "bg": "green", "attributes": [] }, "contents": "\u000a" }], [{ "face": { "fg": "default", "bg": "default", "attributes": [] }, "contents": "3│" }, { "face": { "fg": "default", "bg": "default", "attributes": [] }, "contents": "ab" }, { "face": { "fg": "default", "bg": "red", "attributes": [] }, "contents": "c" }, { "face": { "fg": "default", "bg": "default", "attributes": [] }, "contents": "\u000a" }, { "face": { "fg": "default", "bg": "blue", "attributes": [] }, "contents": " " }, { "face": { "fg": "default", "bg": "default", "attributes": [] }, "contents": " " }, { "face": { "fg": "default", "bg": "green", "attributes": [] }, "contents": " " }], [{ "face": { "fg": "default", "bg": "default", "attributes": [] }, "contents": "4│" }, { "face": { "fg": "default", "bg": "default", "attributes": [] }, "contents": "ab" }, { "face": { "fg": "default", "bg": "red", "attributes": [] }, "contents": "c" }, { "face": { "fg": "default", "bg": "default", "attributes": [] }, "contents": "d" }, { "face": { "fg": "default", "bg": "blue", "attributes": [] }, "contents": "\u000a" }, { "face": { "fg": "default", "bg": "default", "attributes": [] }, "contents": " " }, { "face": { "fg": "default", "bg": "green", "attributes": [] }, "contents": " " }]], { "fg": "default", "bg": "default", "attributes": [] }, { "fg": "blue", "bg": "default", "attributes": [] }] }
+{ "jsonrpc": "2.0", "method": "menu_hide", "params": [] }
+{ "jsonrpc": "2.0", "method": "info_hide", "params": [] }
+{ "jsonrpc": "2.0", "method": "draw_status", "params": [[], [{ "face": { "fg": "default", "bg": "default", "attributes": [] }, "contents": "out 1:1 " }, { "face": { "fg": "black", "bg": "yellow", "attributes": [] }, "contents": "" }, { "face": { "fg": "default", "bg": "default", "attributes": [] }, "contents": " " }, { "face": { "fg": "blue", "bg": "default", "attributes": [] }, "contents": "1 sel" }, { "face": { "fg": "default", "bg": "default", "attributes": [] }, "contents": " - unnamed0@[kak-tests]" }], { "fg": "cyan", "bg": "default", "attributes": [] }] }
+{ "jsonrpc": "2.0", "method": "set_cursor", "params": ["buffer", { "line": 0, "column": 2 }] }
+{ "jsonrpc": "2.0", "method": "refresh", "params": [true] }

test/highlight/column/multi-columns/in

@@ -0,0 +1,4 @@
+a
+	ab
+abc
+abcd

test/highlight/column/multi-columns/rc

@@ -0,0 +1,5 @@
+add-highlighter number_lines
+set window tabstop 4
+add-highlighter column 3 default,red
+add-highlighter column 7 default,green
+add-highlighter column 5 default,blue