feat: Add better highlighting and new CI abilities (#355)

* Revert "Revert "fix: Better highlights (#344)" (#350)"

This reverts commit 7950fc8ba0.

* better highlights take 2

* fixup

* install fd find for sameness

* add some debug output

* more deterministic

* better ci

.github/workflows/ci.yml

@@ -18,9 +18,11 @@ jobs:
 
       - name: Prepare
         run: |
+          sudo apt install fd-find
           mkdir -p ~/.local/share/nvim/site/pack/vendor/start
           git clone --depth 1 https://github.com/nvim-lua/plenary.nvim ~/.local/share/nvim/site/pack/vendor/start/plenary.nvim
           git clone --depth 1 https://github.com/nvim-lua/popup.nvim ~/.local/share/nvim/site/pack/vendor/start/popup.nvim
+          git clone --depth 1 https://github.com/kyazdani42/nvim-web-devicons ~/.local/share/nvim/site/pack/vendor/start/nvim-web-devicons
           ln -s $(pwd) ~/.local/share/nvim/site/pack/vendor/start
 
       - name: Run tests
@@ -44,6 +46,7 @@ jobs:
 
       - name: Prepare
         run: |
+          sudo apt install fd-find
           test -d build || {
             mkdir -p build
             wget https://github.com/neovim/neovim/releases/download/nightly/nvim.appimage
@@ -53,6 +56,7 @@ jobs:
           mkdir -p ~/.local/share/nvim/site/pack/vendor/start
           git clone --depth 1 https://github.com/nvim-lua/plenary.nvim ~/.local/share/nvim/site/pack/vendor/start/plenary.nvim
           git clone --depth 1 https://github.com/nvim-lua/popup.nvim ~/.local/share/nvim/site/pack/vendor/start/popup.nvim
+          git clone --depth 1 https://github.com/kyazdani42/nvim-web-devicons ~/.local/share/nvim/site/pack/vendor/start/nvim-web-devicons
           ln -s $(pwd) ~/.local/share/nvim/site/pack/vendor/start
 
       - name: Run tests

.gitignore

@@ -0,0 +1 @@
+build/

Makefile

@@ -1,5 +1,5 @@
 test:
-	nvim --headless -c 'PlenaryBustedDirectory lua/tests/automated/'
+	nvim --headless --noplugin -u scripts/minimal_init.vim -c "PlenaryBustedDirectory lua/tests/automated/ { minimal_init = './scripts/minimal_init.vim' }"
 
 lint:
 	luacheck lua/telescope

lua/telescope/pickers.lua

@@ -14,6 +14,7 @@ local utils = require('telescope.utils')
 
 local layout_strategies = require('telescope.pickers.layout_strategies')
 local entry_display = require('telescope.pickers.entry_display')
+local p_highlights = require('telescope.pickers.highlights')
 local p_scroller = require('telescope.pickers.scroller')
 
 local EntryManager = require('telescope.entry_manager')
@@ -39,8 +40,6 @@ local extend = function(opts, defaults)
   return result
 end
 
-local ns_telescope_selection = a.nvim_create_namespace('telescope_selection')
-local ns_telescope_entry = a.nvim_create_namespace('telescope_entry')
 local ns_telescope_matching = a.nvim_create_namespace('telescope_matching')
 local ns_telescope_prompt = a.nvim_create_namespace('telescope_prompt')
 local ns_telescope_prompt_prefix = a.nvim_create_namespace('telescope_prompt_prefix')
@@ -130,6 +129,14 @@ function Picker:new(opts)
     obj.sorting_strategy
   )
 
+  obj.highlighter = p_highlights.new(obj)
+
+  if opts.on_complete then
+    for _, on_complete_item in ipairs(opts.on_complete) do
+      obj:register_completion_callback(on_complete_item)
+    end
+  end
+
   return obj
 end
 
@@ -178,7 +185,7 @@ function Picker:get_window_options(max_columns, max_lines)
 end
 
 --- Take a row and get an index.
----@note: Rows are 0-indexed, and `index` is 1 indexed (table index)
+--- @note: Rows are 0-indexed, and `index` is 1 indexed (table index)
 ---@param index number: The row being displayed
 ---@return number The row for the picker to display in
 function Picker:get_row(index)
@@ -252,7 +259,7 @@ function Picker:highlight_displayed_rows(results_bufnr, prompt)
   vim.api.nvim_buf_clear_namespace(results_bufnr, ns_telescope_matching, 0, -1)
 
   local displayed_rows = vim.api.nvim_buf_get_lines(results_bufnr, 0, -1, false)
-  for row_index = 1, #displayed_rows do
+  for row_index = 1, math.min(#displayed_rows, self.max_results) do
     local display = displayed_rows[row_index]
 
     self:highlight_one_row(results_bufnr, prompt, display, row_index - 1)
@@ -374,6 +381,7 @@ function Picker:find()
 
     a.nvim_buf_add_highlight(prompt_bufnr, ns_telescope_prompt_prefix, 'TelescopePromptPrefix', 0, 0, #prompt_prefix)
   end
+  self.prompt_prefix = prompt_prefix
 
   -- Temporarily disabled: Draw the screen ASAP. This makes things feel speedier.
   -- vim.cmd [[redraw]]
@@ -421,10 +429,7 @@ function Picker:find()
       return
     end
 
-    local prompt = vim.trim(
-      vim.api.nvim_buf_get_lines(prompt_bufnr, first_line, last_line, false)[1]:sub(#prompt_prefix)
-    )
-
+    local prompt = self:_get_prompt()
     if self.sorter then
       self.sorter:_start(prompt)
     end
@@ -555,6 +560,8 @@ function Picker:find()
   vim.api.nvim_buf_attach(prompt_bufnr, false, {
     on_lines = on_lines,
     on_detach = vim.schedule_wrap(function()
+      self:_reset_highlights()
+
       on_lines = nil
 
       -- TODO: Can we add a "cleanup" / "teardown" function that completely removes these.
@@ -683,19 +690,21 @@ function Picker:add_selection(row)
 end
 
 function Picker:display_multi_select(results_bufnr)
-  for entry, _ in pairs(self.multi_select) do
-    local index = self.manager:find_entry(entry)
-    if index then
-      vim.api.nvim_buf_add_highlight(
-        results_bufnr,
-        ns_telescope_selection,
-        "TelescopeMultiSelection",
-        self:get_row(index),
-        0,
-        -1
-      )
-    end
-  end
+  if true then return end
+
+  -- for entry, _ in pairs(self.multi_select) do
+  --   local index = self.manager:find_entry(entry)
+  --   if index then
+  --     vim.api.nvim_buf_add_highlight(
+  --       results_bufnr,
+  --       ns_telescope_selection,
+  --       "TelescopeMultiSelection",
+  --       self:get_row(index),
+  --       0,
+  --       -1
+  --     )
+  --   end
+  -- end
 end
 
 function Picker:reset_selection()
@@ -740,26 +749,27 @@ function Picker:set_selection(row)
       return
     end
 
-    local prompt = vim.api.nvim_buf_get_lines(self.prompt_bufnr, 0, 1, false)[1]
+    local prompt = self:_get_prompt()
 
     -- Handle adding '> ' to beginning of selections
     if self._selection_row then
-      local old_selection = a.nvim_buf_get_lines(results_bufnr, self._selection_row, self._selection_row + 1, false)[1]
+      local display, display_highlights = entry_display.resolve(self, self._selection_entry)
 
-      if old_selection then
-        local old_display = '  ' .. old_selection:sub(3)
-        a.nvim_buf_set_lines(results_bufnr, self._selection_row, self._selection_row + 1, false, {old_display})
+      if display then
+        display = '  ' .. display
+        a.nvim_buf_set_lines(results_bufnr, self._selection_row, self._selection_row + 1, false, {display})
 
-        if prompt and self.sorter and self.sorter.highlighter then
-          self:highlight_one_row(results_bufnr, prompt, old_display, self._selection_row)
-        end
+        self.highlighter:hi_display(self._selection_row, '  ', display_highlights)
+        self.highlighter:hi_sorter(self._selection_row, prompt, display)
       end
     end
 
     local caret = '>'
-    local display = string.format('%s %s', caret,
-      (a.nvim_buf_get_lines(results_bufnr, row, row + 1, false)[1] or ''):sub(3)
-    )
+    -- local display = string.format('%s %s', caret,
+    --   (a.nvim_buf_get_lines(results_bufnr, row, row + 1, false)[1] or ''):sub(3)
+    -- )
+    local display, display_highlights = entry_display.resolve(self, entry)
+    display = caret .. ' ' .. display
 
     -- TODO: You should go back and redraw the highlights for this line from the sorter.
     -- That's the only smart thing to do.
@@ -769,29 +779,12 @@ function Picker:set_selection(row)
     end
     a.nvim_buf_set_lines(results_bufnr, row, row + 1, false, {display})
 
-    a.nvim_buf_clear_namespace(results_bufnr, ns_telescope_selection, 0, -1)
-    a.nvim_buf_add_highlight(
-      results_bufnr,
-      ns_telescope_selection,
-      'TelescopeSelectionCaret',
-      row,
-      0,
-      #caret
-    )
-    a.nvim_buf_add_highlight(
-      results_bufnr,
-      ns_telescope_selection,
-      'TelescopeSelection',
-      row,
-      #caret,
-      -1
-    )
+    self.highlighter:hi_selection(row, caret)
+    self.highlighter:hi_display(row, '  ', display_highlights)
+    self.highlighter:hi_sorter(row, prompt, display)
 
-    self:display_multi_select(results_bufnr)
-
-    if prompt and self.sorter and self.sorter.highlighter then
-      self:highlight_one_row(results_bufnr, prompt, display, row)
-    end
+    -- TODO: Actually implement this for real TJ, don't leave around half implemented code plz :)
+    -- self:display_multi_select(results_bufnr)
   end)
 
   if not set_ok then
@@ -850,15 +843,7 @@ function Picker:entry_adder(index, entry, score)
 
     local set_ok = pcall(vim.api.nvim_buf_set_lines, self.results_bufnr, row, row + 1, false, {display})
     if set_ok and display_highlights then
-      -- TODO: This should actually be done during the cursor moving stuff annoyingly.... didn't see this bug yesterday.
-      for _, hl_block in ipairs(display_highlights) do
-        a.nvim_buf_add_highlight(self.results_bufnr,
-                                 ns_telescope_entry,
-                                 hl_block[2],
-                                 row,
-                                 #prefix + hl_block[1][1],
-                                 #prefix + hl_block[1][2])
-      end
+      self.highlighter:hi_display(row, prefix, display_highlights)
     end
 
     -- This pretty much only fails when people leave newlines in their results.
@@ -917,7 +902,7 @@ end
 
 function Picker:_on_complete()
   for _, v in ipairs(self._completion_callbacks) do
-    v(self)
+    pcall(v, self)
   end
 end
 
@@ -946,6 +931,16 @@ function pickers.on_close_prompt(prompt_bufnr)
   picker.close_windows(status)
 end
 
+function Picker:_get_prompt()
+  return vim.trim(
+    vim.api.nvim_buf_get_lines(self.prompt_bufnr, 0, 1, false)[1]:sub(#self.prompt_prefix)
+  )
+end
+
+function Picker:_reset_highlights()
+  self.highlighter:clear_display()
+end
+
 pickers._Picker = Picker
 
 

lua/telescope/pickers/_tests.lua

@@ -0,0 +1,167 @@
+local assert = require('luassert')
+local builtin = require('telescope.builtin')
+
+local Job = require("plenary.job")
+
+local tester = {}
+
+local replace_terms = function(input)
+  return vim.api.nvim_replace_termcodes(input, true, false, true)
+end
+
+local nvim_feed = function(text, feed_opts)
+  feed_opts = feed_opts or "m"
+
+  vim.api.nvim_feedkeys(text, feed_opts, true)
+end
+
+tester.picker_feed = function(input, test_cases, debug)
+  input = replace_terms(input)
+
+  return coroutine.wrap(function()
+    for i = 1, #input do
+      local char = input:sub(i, i)
+      nvim_feed(char, "")
+
+      -- TODO: I'm not 100% sure this is a hack or not...
+      -- it's possible these characters  could still have an on_complete... but i'm not sure.
+      if string.match(char, "%g") then
+        coroutine.yield()
+      end
+    end
+
+    vim.wait(100, function() end)
+
+    local timer = vim.loop.new_timer()
+    timer:start(200, 0, vim.schedule_wrap(function()
+      if test_cases.post_close then
+        for k, v in ipairs(test_cases.post_close) do
+          io.stderr:write(vim.fn.json_encode({ case = k, expected = v[1], actual = v[2]() }))
+          io.stderr:write("\n")
+        end
+      end
+      vim.wait(10)
+
+      if debug then
+        return
+      end
+
+      vim.defer_fn(function()
+        vim.cmd [[qa!]]
+      end, 15)
+    end))
+
+    if not debug then
+      vim.schedule(function()
+        if test_cases.post_typed then
+          for k, v in ipairs(test_cases.post_typed) do
+            io.stderr:write(vim.fn.json_encode({ case = k, expected = v[1], actual = v[2]() }))
+            io.stderr:write("\n")
+          end
+        end
+
+        nvim_feed(replace_terms("<CR>"), "")
+      end)
+    end
+    coroutine.yield()
+  end)
+end
+
+-- local test_cases = {
+--   post_typed = {
+--   },
+--   post_close = {
+--     { "README.md", function() return "README.md" end },
+--   },
+-- }
+tester.builtin_picker = function(key, input, test_cases, opts)
+  opts = opts or {}
+  local debug = opts.debug or false
+
+  opts.on_complete = {
+    tester.picker_feed(input, test_cases, debug)
+  }
+
+  builtin[key](opts)
+end
+
+local get_results_from_file = function(file)
+  local j = Job:new {
+    command = 'nvim',
+    args = {
+      '--noplugin',
+      '-u',
+      'scripts/minimal_init.vim',
+      '-c',
+      'luafile ' .. file
+    },
+  }
+
+  j:sync()
+
+  local results = j:stderr_result()
+  local result_table = {}
+  for _, v in ipairs(results) do
+    table.insert(result_table, vim.fn.json_decode(v))
+  end
+
+  return result_table
+end
+
+local check_results = function(results)
+  -- TODO: We should get all the test cases here that fail, not just the first one.
+  for _, v in ipairs(results) do
+    assert.are.same(v.expected, v.actual)
+  end
+end
+
+tester.run_string = function(contents)
+  local tempname = vim.fn.tempname()
+
+  contents = [[
+  -- TODO: Add globals!
+  -- local tester = require('telescope.pickers._tests')
+  local tester = require('telescope.pickers._tests')
+
+  local get_picker = function()
+    local state = require('telescope.state')
+    return state.get_status(vim.api.nvim_get_current_buf()).picker
+  end
+
+  local get_results_bufnr = function()
+    local state = require('telescope.state')
+    return state.get_status(vim.api.nvim_get_current_buf()).results_bufnr
+  end
+
+  local GetFile = function() return vim.fn.fnamemodify(vim.api.nvim_buf_get_name(0), ":t") end
+
+  local GetPrompt = function() return vim.api.nvim_buf_get_lines(0, 0, -1, false)[1] end
+
+  local GetResults = function()
+    return vim.api.nvim_buf_get_lines(get_results_bufnr(), 0, -1, false)
+  end
+
+  local GetLastResult = function()
+    local results = GetResults()
+    return results[#results]
+  end
+  ]] .. contents
+
+  vim.fn.writefile(vim.split(contents, "\n"), tempname)
+  local result_table = get_results_from_file(tempname)
+  vim.fn.delete(tempname)
+
+  check_results(result_table)
+  -- assert.are.same(result_table.expected, result_table.actual)
+end
+
+tester.run_file = function(filename)
+  local file = './lua/tests/pickers/' .. filename .. '.lua'
+
+  local result_table = get_results_from_file(file)
+  assert.are.same(result_table.expected, result_table.actual)
+end
+
+
+
+return tester

lua/telescope/pickers/highlights.lua

@@ -0,0 +1,82 @@
+local a = vim.api
+
+local highlights = {}
+
+local ns_telescope_selection = a.nvim_create_namespace('telescope_selection')
+local ns_telescope_entry = a.nvim_create_namespace('telescope_entry')
+
+local Highlighter = {}
+Highlighter.__index = Highlighter
+
+function Highlighter:new(picker)
+  return setmetatable({
+    picker = picker,
+  }, self)
+end
+
+function Highlighter:hi_display(row, prefix, display_highlights)
+  -- This is the bug that made my highlight fixes not work.
+  -- We will leave the solutino commented, so the test fails.
+  if not display_highlights or vim.tbl_isempty(display_highlights) then
+    return
+  end
+
+  local results_bufnr = assert(self.picker.results_bufnr, "Must have a results bufnr")
+
+  a.nvim_buf_clear_namespace(results_bufnr, ns_telescope_entry, row, row + 1)
+  local len_prefix = #prefix
+
+  for _, hl_block in ipairs(display_highlights) do
+    a.nvim_buf_add_highlight(
+      results_bufnr,
+      ns_telescope_entry,
+      hl_block[2],
+      row,
+      len_prefix + hl_block[1][1],
+      len_prefix + hl_block[1][2]
+    )
+  end
+end
+
+function Highlighter:clear_display()
+  a.nvim_buf_clear_namespace(self.picker.results_bufnr, ns_telescope_entry, 0, -1)
+end
+
+function Highlighter:hi_sorter(row, prompt, display)
+  local picker = self.picker
+  if not picker.sorter or not picker.sorter.highlighter then
+    return
+  end
+
+  local results_bufnr = assert(self.picker.results_bufnr, "Must have a results bufnr")
+  picker:highlight_one_row(results_bufnr, prompt, display, row)
+end
+
+function Highlighter:hi_selection(row, caret)
+  local results_bufnr = assert(self.picker.results_bufnr, "Must have a results bufnr")
+
+  a.nvim_buf_clear_namespace(results_bufnr, ns_telescope_selection, 0, -1)
+  a.nvim_buf_add_highlight(
+    results_bufnr,
+    ns_telescope_selection,
+    'TelescopeSelectionCaret',
+    row,
+    0,
+    #caret
+  )
+
+  a.nvim_buf_add_highlight(
+    results_bufnr,
+    ns_telescope_selection,
+    'TelescopeSelection',
+    row,
+    #caret,
+    -1
+  )
+end
+
+highlights.new = function(...)
+  return Highlighter:new(...)
+end
+
+return highlights

lua/tests/automated/pickers/find_files_spec.lua

@@ -0,0 +1,108 @@
+require('plenary.reload').reload_module('telescope')
+
+local tester = require('telescope.pickers._tests')
+
+describe('builtin.find_files', function()
+  it('should find the readme', function()
+    tester.run_file('find_files__readme')
+  end)
+
+  it('should not display devicons when disabled', function()
+    tester.run_string [[
+      tester.builtin_picker('find_files', 'README.md', {
+        post_typed = {
+          { "> README.md", GetPrompt },
+          { "> README.md", GetLastResult },
+        },
+        post_close = {
+          { 'README.md', GetFile },
+          { 'README.md', GetFile },
+        }
+      }, {
+        disable_devicons = true,
+        sorter = require('telescope.sorters').get_fzy_sorter(),
+      })
+    ]]
+  end)
+
+  it('use devicons, if it has it when enabled', function()
+    if not pcall(require, 'nvim-web-devicons') then
+      return
+    end
+
+    tester.run_string [[
+      tester.builtin_picker('find_files', 'README.md', {
+        post_typed = {
+          { "> README.md", GetPrompt },
+          { ">  README.md", GetLastResult }
+        },
+        post_close = {
+          { 'README.md', GetFile },
+          { 'README.md', GetFile },
+        }
+      }, {
+        disable_devicons = false,
+        sorter = require('telescope.sorters').get_fzy_sorter(),
+      })
+    ]]
+  end)
+
+  it('should find the readme, using lowercase', function()
+    tester.run_string [[
+      tester.builtin_picker('find_files', 'readme.md', {
+        post_close = {
+          { 'README.md', GetFile },
+        }
+      })
+    ]]
+  end)
+
+  it('should find the pickers.lua, using lowercase', function()
+    tester.run_string [[
+      tester.builtin_picker('find_files', 'pickers.lua', {
+        post_close = {
+          { 'pickers.lua', GetFile },
+        }
+      })
+    ]]
+  end)
+
+  it('should find the pickers.lua', function()
+    tester.run_string [[
+      tester.builtin_picker('find_files', 'pickers.lua', {
+        post_close = {
+          { 'pickers.lua', GetFile },
+          { 'pickers.lua', GetFile },
+        }
+      })
+    ]]
+  end)
+
+  it('should be able to c-n the items', function()
+    tester.run_string [[
+      tester.builtin_picker('find_files', 'fixtures/file<c-p>', {
+        post_typed = {
+          {
+            {
+              "  lua/tests/fixtures/file_abc.txt",
+              "> lua/tests/fixtures/file_a.txt",
+            }, function()
+            local res = GetResults()
+
+              return {
+                res[#res - 1],
+                res[#res],
+              }
+            end
+          },
+        },
+        post_close = {
+          { 'file_abc.txt', GetFile },
+        },
+      }, {
+        sorter = require('telescope.sorters').get_fzy_sorter(),
+        disable_devicons = true,
+      })
+    ]]
+  end)
+end)

lua/tests/manual/auto_picker.lua

@@ -71,6 +71,7 @@ local find_files = function(opts)
     feed("<CR>", '')
 
     coroutine.yield()
+    print("STILL CALLED?")
   end))
 
   p:find()

lua/tests/pickers/find_files__readme.lua

@@ -0,0 +1,7 @@
+local tester = require('telescope.pickers._tests')
+
+tester.builtin_picker('find_files', 'README.md', {
+  post_close = {
+    {'README.md', function() return vim.fn.fnamemodify(vim.api.nvim_buf_get_name(0), ":.") end },
+  }
+})

lua/tests/pickers/find_files__with_ctrl_n.lua

@@ -0,0 +1,24 @@
+pcall(function() RELOAD('telescope') end)
+
+local builtin = require('telescope.builtin')
+local tester = require('telescope.pickers._tests')
+
+local key = 'find_files'
+local input =  'fixtures/file<c-p>'
+local expected = 'lua/tests/fixtures/file_2.txt'
+local get_actual = function()
+  return vim.fn.fnamemodify(vim.api.nvim_buf_get_name(0), ":.")
+end
+
+-- local on_complete_item = tester.picker_feed(input, expected, get_actual, true)
+-- 
+-- builtin[key] {
+--   on_complete = { on_complete_item }
+-- }
+
+tester.builtin_picker('find_files', 'fixtures/file<c-p>', 'lua/tests/fixtures/file_2.txt', function()
+  return vim.fn.fnamemodify(vim.api.nvim_buf_get_name(0), ":.")
+end, {
+  debug = false,
+})
+

scratch/digia_init.vim

@@ -0,0 +1,16 @@
+set rtp+=.
+set rtp+=../plenary.nvim/
+set rtp+=../popup.nvim/
+
+
+set statusline=""
+set statusline+=%<%f:%l:%v " filename:col:line/total lines
+set statusline+=\ "
+set statusline+=%h%m%r " help/modified/readonly
+set statusline+=\ "
+set statusline+=[%{&ft}] " filetype
+set statusline+=%= " alignment group
+set statusline+=\ "
+
+" nnoremap <silent> <c-p> :lua require('telescope.builtin').git_files()<CR>
+nnoremap <silent> <c-p> :lua require("telescope.builtin").find_files{ find_command = { "rg", "--smart-case", "--files", "--hidden", "--follow", "-g", "!.git/*" } }<CR>

scratch/minimal_init.vim

@@ -1,7 +0,0 @@
-
-
-set rtp+=/home/tj/plugins/popup.nvim/
-set rtp+=/home/tj/plugins/plenary.nvim/
-set rtp+=/home/tj/plugins/telescope.nvim/
-
-nnoremap ,,x :luafile %<CR>

scripts/minimal_init.vim

@@ -0,0 +1,6 @@
+set rtp+=.
+set rtp+=../plenary.nvim/
+set rtp+=../popup.nvim/
+
+runtime! plugin/plenary.vim
+runtime! plugin/telescope.vim