Pass a BufferRange to highlighters, fix subregions

Regions highlighter can now correctly be applied to only subrange
of the buffer, fixing some corner cases in recursive regions.

src/highlighter.hh

@@ -1,6 +1,7 @@
 #ifndef highlighter_hh_INCLUDED
 #define highlighter_hh_INCLUDED
 
+#include "coord.hh"
 #include "function_registry.hh"
 #include "memoryview.hh"
 #include "string.hh"
@@ -8,6 +9,22 @@
 
 #include <functional>
 
+namespace std
+{
+
+template<>
+struct hash<Kakoune::ByteCoord>
+{
+    size_t operator()(const Kakoune::ByteCoord& val) const
+    {
+        size_t seed = std::hash<int>()((int)val.line);
+        return seed ^ (std::hash<int>()((int)val.column) + 0x9e3779b9 +
+                       (seed << 6) + (seed >> 2));
+    }
+};
+
+}
+
 namespace Kakoune
 {
 
@@ -29,10 +46,12 @@ struct Highlighter;
 
 using HighlighterAndId = std::pair<String, std::unique_ptr<Highlighter>>;
 
+using BufferRange = std::pair<ByteCoord, ByteCoord>;
+
 struct Highlighter
 {
     virtual ~Highlighter() {}
-    virtual void highlight(const Context& context, HighlightFlags flags, DisplayBuffer& display_buffer) = 0;
+    virtual void highlight(const Context& context, HighlightFlags flags, DisplayBuffer& display_buffer, BufferRange range) = 0;
 
     virtual bool has_children() const { return false; }
     virtual Highlighter& get_child(StringView path) { throw runtime_error("this highlighter do not hold children"); }
@@ -45,9 +64,9 @@ template<typename Func>
 struct SimpleHighlighter : public Highlighter
 {
     SimpleHighlighter(Func func) : m_func(std::move(func)) {}
-    virtual void highlight(const Context& context, HighlightFlags flags, DisplayBuffer& display_buffer) override
+    virtual void highlight(const Context& context, HighlightFlags flags, DisplayBuffer& display_buffer, BufferRange range) override
     {
-        m_func(context, flags, display_buffer);
+        m_func(context, flags, display_buffer, range);
     }
 private:
     Func m_func;

src/highlighter_group.cc

@@ -4,10 +4,10 @@ namespace Kakoune
 {
 
 void HighlighterGroup::highlight(const Context& context, HighlightFlags flags,
-                                 DisplayBuffer& display_buffer)
+                                 DisplayBuffer& display_buffer, BufferRange range)
 {
     for (auto& hl : m_highlighters)
-       hl.second->highlight(context, flags, display_buffer);
+       hl.second->highlight(context, flags, display_buffer, range);
 }
 
 void HighlighterGroup::add_child(HighlighterAndId&& hl)

src/highlighter_group.hh

@@ -17,7 +17,7 @@ struct child_not_found : public runtime_error
 class HighlighterGroup : public Highlighter
 {
 public:
-    void highlight(const Context& context, HighlightFlags flags, DisplayBuffer& display_buffer) override;
+    void highlight(const Context& context, HighlightFlags flags, DisplayBuffer& display_buffer, BufferRange range) override;
 
     bool has_children() const { return true; }
     void add_child(HighlighterAndId&& hl) override;

src/highlighters.cc

@@ -134,7 +134,7 @@ void apply_highlighter(const Context& context,
     }
 
     region_display.compute_range();
-    highlighter.highlight(context, flags, region_display);
+    highlighter.highlight(context, flags, region_display, {begin, end});
 
     for (size_t i = 0; i < region_lines.size(); ++i)
     {
@@ -169,9 +169,8 @@ static HighlighterAndId create_fill_highlighter(HighlighterParameters params)
     get_face(facespec); // validate param
 
     auto func = [=](const Context& context, HighlightFlags flags,
-                    DisplayBuffer& display_buffer)
+                    DisplayBuffer& display_buffer, BufferRange range)
     {
-        auto range = display_buffer.range();
         highlight_range(display_buffer, range.first, range.second, true,
                         apply_face(get_face(facespec)));
     };
@@ -202,7 +201,7 @@ public:
     {
     }
 
-    void highlight(const Context& context, HighlightFlags flags, DisplayBuffer& display_buffer) override
+    void highlight(const Context& context, HighlightFlags flags, DisplayBuffer& display_buffer, BufferRange) override
     {
         if (flags != HighlightFlags::Highlight)
             return;
@@ -325,7 +324,7 @@ public:
           m_face_getter(std::move(face_getter)),
           m_highlighter(Regex(), FacesSpec{}) {}
 
-    void highlight(const Context& context, HighlightFlags flags, DisplayBuffer& display_buffer)
+    void highlight(const Context& context, HighlightFlags flags, DisplayBuffer& display_buffer, BufferRange range)
     {
         if (flags != HighlightFlags::Highlight)
             return;
@@ -340,7 +339,7 @@ public:
                 m_highlighter.reset(m_last_regex, m_last_face);
         }
         if (not m_last_regex.empty() and not m_last_face.empty())
-            m_highlighter.highlight(context, flags, display_buffer);
+            m_highlighter.highlight(context, flags, display_buffer, range);
     }
 
 private:
@@ -415,7 +414,7 @@ HighlighterAndId create_line_option_highlighter(HighlighterParameters params)
     GlobalScope::instance().options()[option_name].get<int>(); // verify option type now
 
     auto func = [=](const Context& context, HighlightFlags flags,
-                    DisplayBuffer& display_buffer)
+                    DisplayBuffer& display_buffer, BufferRange)
     {
         int line = context.options()[option_name].get<int>();
         highlight_range(display_buffer, {line-1, 0}, {line, 0}, false,
@@ -425,7 +424,7 @@ HighlighterAndId create_line_option_highlighter(HighlighterParameters params)
     return {"hlline_" + params[0], make_simple_highlighter(std::move(func))};
 }
 
-void expand_tabulations(const Context& context, HighlightFlags flags, DisplayBuffer& display_buffer)
+void expand_tabulations(const Context& context, HighlightFlags flags, DisplayBuffer& display_buffer, BufferRange)
 {
     const int tabstop = context.options()["tabstop"].get<int>();
     auto& buffer = context.buffer();
@@ -460,7 +459,7 @@ void expand_tabulations(const Context& context, HighlightFlags flags, DisplayBuf
     }
 }
 
-void show_whitespaces(const Context& context, HighlightFlags flags, DisplayBuffer& display_buffer)
+void show_whitespaces(const Context& context, HighlightFlags flags, DisplayBuffer& display_buffer, BufferRange)
 {
     const int tabstop = context.options()["tabstop"].get<int>();
     auto& buffer = context.buffer();
@@ -503,7 +502,7 @@ void show_whitespaces(const Context& context, HighlightFlags flags, DisplayBuffe
     }
 }
 
-void show_line_numbers(const Context& context, HighlightFlags flags, DisplayBuffer& display_buffer)
+void show_line_numbers(const Context& context, HighlightFlags flags, DisplayBuffer& display_buffer, BufferRange)
 {
     LineCount last_line = context.buffer().line_count();
     int digit_count = 0;
@@ -523,7 +522,7 @@ void show_line_numbers(const Context& context, HighlightFlags flags, DisplayBuff
     }
 }
 
-void show_matching_char(const Context& context, HighlightFlags flags, DisplayBuffer& display_buffer)
+void show_matching_char(const Context& context, HighlightFlags flags, DisplayBuffer& display_buffer, BufferRange)
 {
     const Face face = get_face("MatchingChar");
     using CodepointPair = std::pair<Codepoint, Codepoint>;
@@ -575,7 +574,7 @@ void show_matching_char(const Context& context, HighlightFlags flags, DisplayBuf
     }
 }
 
-void highlight_selections(const Context& context, HighlightFlags flags, DisplayBuffer& display_buffer)
+void highlight_selections(const Context& context, HighlightFlags flags, DisplayBuffer& display_buffer, BufferRange)
 {
     if (flags != HighlightFlags::Highlight)
         return;
@@ -602,7 +601,7 @@ void highlight_selections(const Context& context, HighlightFlags flags, DisplayB
     }
 }
 
-void expand_unprintable(const Context& context, HighlightFlags flags, DisplayBuffer& display_buffer)
+void expand_unprintable(const Context& context, HighlightFlags flags, DisplayBuffer& display_buffer, BufferRange)
 {
     auto& buffer = context.buffer();
     for (auto& line : display_buffer.lines())
@@ -648,7 +647,7 @@ HighlighterAndId create_flag_lines_highlighter(HighlighterParameters params)
     GlobalScope::instance().options()[option_name].get<std::vector<LineAndFlag>>();
 
     auto func = [=](const Context& context, HighlightFlags flags,
-                    DisplayBuffer& display_buffer)
+                    DisplayBuffer& display_buffer, BufferRange)
     {
         auto& lines_opt = context.options()[option_name];
         auto& lines = lines_opt.get<std::vector<LineAndFlag>>();
@@ -693,11 +692,11 @@ HighlighterAndId create_reference_highlighter(HighlighterParameters params)
     //DefinedHighlighters::instance().get_group(name, '/');
 
     auto func = [=](const Context& context, HighlightFlags flags,
-                    DisplayBuffer& display_buffer)
+                    DisplayBuffer& display_buffer, BufferRange range)
     {
         try
         {
-            DefinedHighlighters::instance().get_child(name).highlight(context, flags, display_buffer);
+            DefinedHighlighters::instance().get_child(name).highlight(context, flags, display_buffer, range);
         }
         catch (child_not_found&)
         {}
@@ -888,18 +887,18 @@ public:
             m_groups.append({m_default_group, HighlighterGroup{}});
     }
 
-    void highlight(const Context& context, HighlightFlags flags, DisplayBuffer& display_buffer)
+    void highlight(const Context& context, HighlightFlags flags, DisplayBuffer& display_buffer, BufferRange range)
     {
         if (flags != HighlightFlags::Highlight)
             return;
 
-        auto range = display_buffer.range();
+        auto display_range = display_buffer.range();
         const auto& buffer = context.buffer();
-        auto& regions = update_cache_ifn(buffer);
+        auto& regions = get_regions_for_range(buffer, range);
 
-        auto begin = std::lower_bound(regions.begin(), regions.end(), range.first,
+        auto begin = std::lower_bound(regions.begin(), regions.end(), display_range.first,
                                       [](const Region& r, ByteCoord c) { return r.end < c; });
-        auto end = std::lower_bound(begin, regions.end(), range.second,
+        auto end = std::lower_bound(begin, regions.end(), display_range.second,
                                     [](const Region& r, ByteCoord c) { return r.begin < c; });
         auto correct = [&](ByteCoord c) -> ByteCoord {
             if (buffer[c.line].length() == c.column)
@@ -910,7 +909,7 @@ public:
         auto default_group_it = m_groups.find(m_default_group);
         const bool apply_default = default_group_it != m_groups.end();
 
-        auto last_begin = range.first;
+        auto last_begin = display_range.first;
         for (; begin != end; ++begin)
         {
             if (apply_default and last_begin < begin->begin)
@@ -926,7 +925,7 @@ public:
                               it->second);
             last_begin = begin->end;
         }
-        if (apply_default and last_begin < range.second)
+        if (apply_default and last_begin < display_range.second)
             apply_highlighter(context, flags, display_buffer,
                               correct(last_begin), range.second,
                               default_group_it->second);
@@ -1018,7 +1017,7 @@ private:
     {
         size_t timestamp = 0;
         std::vector<RegionMatches> matches;
-        RegionList regions;
+        std::unordered_map<BufferRange, RegionList> regions;
     };
     BufferSideCache<Cache> m_cache;
 
@@ -1040,29 +1039,35 @@ private:
         return res;
     }
 
-    const RegionList& update_cache_ifn(const Buffer& buffer)
+    const RegionList& get_regions_for_range(const Buffer& buffer, BufferRange range)
     {
         Cache& cache = m_cache.get(buffer);
         const size_t buf_timestamp = buffer.timestamp();
-        if (cache.timestamp == buf_timestamp)
+        if (cache.timestamp != buf_timestamp)
-            return cache.regions;
+        {
+            if (cache.timestamp == 0)
+            {
+                cache.matches.resize(m_regions.size());
+                for (size_t i = 0; i < m_regions.size(); ++i)
+                    cache.matches[i] = m_regions[i].second.find_matches(buffer);
+            }
+            else
+            {
+                auto modifs = compute_line_modifications(buffer, cache.timestamp);
+                for (size_t i = 0; i < m_regions.size(); ++i)
+                    m_regions[i].second.update_matches(buffer, modifs, cache.matches[i]);
+            }
 
-        if (cache.timestamp == 0)
+            cache.regions.clear();
-        {
-            cache.matches.resize(m_regions.size());
-            for (size_t i = 0; i < m_regions.size(); ++i)
-                cache.matches[i] = m_regions[i].second.find_matches(buffer);
-        }
-        else
-        {
-            auto modifs = compute_line_modifications(buffer, cache.timestamp);
-            for (size_t i = 0; i < m_regions.size(); ++i)
-                m_regions[i].second.update_matches(buffer, modifs, cache.matches[i]);
         }
 
-        cache.regions.clear();
+        auto it = cache.regions.find(range);
+        if (it != cache.regions.end())
+            return it->second;
 
-        for (auto begin = find_next_begin(cache, ByteCoord{-1, 0}),
+        RegionList& regions = cache.regions[range];
+
+        for (auto begin = find_next_begin(cache, range.first),
                   end = RegionAndMatch{ 0, cache.matches[0].begin_matches.end() };
              begin != end; )
         {
@@ -1071,18 +1076,18 @@ private:
             auto beg_it = begin.second;
             auto end_it = matches.find_matching_end(beg_it->end_coord());
 
-            if (end_it == matches.end_matches.end())
+            if (end_it == matches.end_matches.end() or end_it->end_coord() >= range.second)
             {
-                cache.regions.push_back({ {beg_it->line, beg_it->begin},
+                regions.push_back({ {beg_it->line, beg_it->begin},
-                                           buffer.end_coord(),
+                                    range.second,
-                                           named_region.first });
+                                    named_region.first });
                 break;
             }
             else
             {
-                cache.regions.push_back({ beg_it->begin_coord(),
+                regions.push_back({ beg_it->begin_coord(),
-                                          end_it->end_coord(),
+                                   end_it->end_coord(),
-                                          named_region.first });
+                                   named_region.first });
                 auto end_coord = end_it->end_coord();
 
                 // With empty begin and end matches (for example if the regexes
@@ -1098,7 +1103,7 @@ private:
             }
         }
         cache.timestamp = buf_timestamp;
-        return cache.regions;
+        return regions;
     }
 };
 

src/window.cc

@@ -13,9 +13,9 @@ namespace Kakoune
 {
 
 // Implementation in highlighters.cc
-void highlight_selections(const Context& context, HighlightFlags flags, DisplayBuffer& display_buffer);
+void highlight_selections(const Context& context, HighlightFlags flags, DisplayBuffer& display_buffer, BufferRange range);
-void expand_tabulations(const Context& context, HighlightFlags flags, DisplayBuffer& display_buffer);
+void expand_tabulations(const Context& context, HighlightFlags flags, DisplayBuffer& display_buffer, BufferRange range);
-void expand_unprintable(const Context& context, HighlightFlags flags, DisplayBuffer& display_buffer);
+void expand_unprintable(const Context& context, HighlightFlags flags, DisplayBuffer& display_buffer, BufferRange range);
 
 Window::Window(Buffer& buffer)
     : Scope(buffer),
@@ -80,8 +80,9 @@ void Window::update_display_buffer(const Context& context)
     }
 
     m_display_buffer.compute_range();
-    m_highlighters.highlight(context, HighlightFlags::Highlight, m_display_buffer);
+    BufferRange range{{0,0}, buffer().end_coord()};
-    m_builtin_highlighters.highlight(context, HighlightFlags::Highlight, m_display_buffer);
+    m_highlighters.highlight(context, HighlightFlags::Highlight, m_display_buffer, range);
+    m_builtin_highlighters.highlight(context, HighlightFlags::Highlight, m_display_buffer, range);
 
     // cut the start of the line before m_position.column
     for (auto& line : lines)
@@ -180,8 +181,9 @@ void Window::scroll_to_keep_selection_visible_ifn(const Context& context)
     lines.emplace_back(AtomList{ {buffer(), cursor.line, cursor.line+1} });
 
     display_buffer.compute_range();
-    m_highlighters.highlight(context, HighlightFlags::MoveOnly, display_buffer);
+    BufferRange range{cursor.line, cursor.line + 1};
-    m_builtin_highlighters.highlight(context, HighlightFlags::MoveOnly, display_buffer);
+    m_highlighters.highlight(context, HighlightFlags::MoveOnly, display_buffer, range);
+    m_builtin_highlighters.highlight(context, HighlightFlags::MoveOnly, display_buffer, range);
 
     // now we can compute where the cursor is in display columns
     // (this is only valid if highlighting one line and multiple lines put
@@ -262,10 +264,12 @@ ByteCoordAndTarget Window::offset_coord(ByteCoordAndTarget coord, LineCount offs
     lines.emplace_back(AtomList{ {buffer(), line, line+1} });
     display_buffer.compute_range();
 
+    BufferRange range{ std::min(line, coord.line), std::max(line,coord.line)+1};
+
     InputHandler hook_handler{{ *m_buffer, Selection{} } };
     hook_handler.context().set_window(*this);
-    m_highlighters.highlight(hook_handler.context(), HighlightFlags::MoveOnly, display_buffer);
+    m_highlighters.highlight(hook_handler.context(), HighlightFlags::MoveOnly, display_buffer, range);
-    m_builtin_highlighters.highlight(hook_handler.context(), HighlightFlags::MoveOnly, display_buffer);
+    m_builtin_highlighters.highlight(hook_handler.context(), HighlightFlags::MoveOnly, display_buffer, range);
 
     CharCount column = coord.target == -1 ? find_display_column(lines[0], buffer(), coord) : coord.target;
     return { find_buffer_coord(lines[1], buffer(), column), column };