Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

live_grep highlighting shouldn't be fuzzy or include the filenames #2272

Open
ehaynes99 opened this issue Dec 12, 2022 · 7 comments · May be fixed by #2536
Open

live_grep highlighting shouldn't be fuzzy or include the filenames #2272

ehaynes99 opened this issue Dec 12, 2022 · 7 comments · May be fixed by #2536
Labels
bug Something isn't working

Comments

@ehaynes99
Copy link

ehaynes99 commented Dec 12, 2022

Description

The default behavior of live_grep is to pass it to rg as a regex. Using rg from the cli, its highlighting highlights matches of that regex in the results.

Using the default options for live_grep, the sorter uses the fuzzy highlighter on the result: https://github.com/nvim-telescope/telescope.nvim/blob/master/lua/telescope/sorters.lua#L491-L493.

This results in highlighting that is rather nonsensical. As a visual cue, it actually makes it look like the search is broken (to be clear, it's behaving correctly, it just looks like it's not).

  • There's a fundamental mismatch between what the highlighter is trying to do and what the search actually does
  • The highlighter is processing the combined string with filename and content, which gives the impression that the filename had some impact on the search

I'm happy to take a stab at a PR, but it'll be towards the end of the year. I'm curious if it's possible to capture the output of rg with color and treat ansii escapes as "quotes". That would be ideal IMO, as then telescope wouldn't need to attempt any logic around it, simply delegating it to ripgrep. However, if that's not possible, at least a default that splits the string after the filename/index part and does a regex match on that would be an improvement.

image

image

image

image

Neovim version

NVIM v0.8.1
Build type: Release
LuaJIT 2.1.0-beta3

Operating system and version

Arch Linux - 5.15.81-1-lts

Telescope version / branch / rev

76ea9a8

checkhealth telescope

telescope: require("telescope.health").check()
========================================================================
## Checking for required plugins
  - OK: plenary installed.
  - WARNING: nvim-treesitter not found. 

## Checking external dependencies
  - OK: rg: found ripgrep 13.0.0
  - OK: fd: found fd 8.5.3

## ===== Installed extensions =====

## Telescope Extension: `fzf`
  - OK: lib working as expected
  - OK: file_sorter correctly configured
  - OK: generic_sorter correctly configured

Steps to reproduce

Any search with regex characters

Expected behavior

Highlighting would highlight the terms within the text that matched the search

Actual behavior

Highlighting highlights... some stuff

Minimal config

-- not configuration specific

vim.cmd [[set runtimepath=$VIMRUNTIME]]
vim.cmd [[set packpath=/tmp/nvim/site]]
local package_root = '/tmp/nvim/site/pack'
local install_path = package_root .. '/packer/start/packer.nvim'
local function load_plugins()
  require('packer').startup {
    {
      'wbthomason/packer.nvim',
      {
        'nvim-telescope/telescope.nvim',
        requires = {
          'nvim-lua/plenary.nvim',
          { 'nvim-telescope/telescope-fzf-native.nvim', run = 'make' },
        },
      },
      -- ADD PLUGINS THAT ARE _NECESSARY_ FOR REPRODUCING THE ISSUE
    },
    config = {
      package_root = package_root,
      compile_path = install_path .. '/plugin/packer_compiled.lua',
      display = { non_interactive = true },
    },
  }
end
_G.load_config = function()
  require('telescope').setup()
  require('telescope').load_extension('fzf')
  -- ADD INIT.LUA SETTINGS THAT ARE _NECESSARY_ FOR REPRODUCING THE ISSUE
end
if vim.fn.isdirectory(install_path) == 0 then
  print("Installing Telescope and dependencies.")
  vim.fn.system { 'git', 'clone', '--depth=1', 'https://github.com/wbthomason/packer.nvim', install_path }
end
load_plugins()
require('packer').sync()
vim.cmd [[autocmd User PackerComplete ++once echo "Ready!" | lua load_config()]]

-- for convenience
vim.keymap.set('n', '<space>ft', '<cmd>Telescope live_grep<CR>')
 -- to avoid burning your retinas on search ;-)
vim.cmd('colorscheme habamax')
@ehaynes99 ehaynes99 added the bug Something isn't working label Dec 12, 2022
@dasupradyumna
Copy link

+1 for this issue.
As shown the screenshots posted by @ehaynes99, especially when there is no regex in the search string, the file paths in the resulting entries are highlighted over the actual detected string in the files, when the search string is a substring of the file paths.

@jamestrew
Copy link
Contributor

Rather than taking the colored rg output and parsing ansi color codes, we could use either the --json to get the output as a json or --replace to use a custom delimiter for matches.

--json would probably be over kill. There's a lot to parse.

$ rg --json --smart-case foo
{"type":"begin","data":{"path":{"text":"foo.md"}}}
{"type":"match","data":{"path":{"text":"foo.md"},"lines":{"text":"foo\n"},"line_number":1,"absolute_offset":0,"submatches":[{"match":{"text":"foo"},"start":0,"end":3}]}}
{"type":"end","data":{"path":{"text":"foo.md"},"binary_offset":null,"stats":{"elapsed":{"secs":0,"nanos":14727,"human":"0.000015s"},"searches":1,"searches_with_match":1,"bytes_searched":4,"bytes_printed":221,"matched_lines":1,"matches":1}}}
{"type":"begin","data":{"path":{"text":"foobar.md"}}}
{"type":"match","data":{"path":{"text":"foobar.md"},"lines":{"text":"foobar\n"},"line_number":1,"absolute_offset":0,"submatches":[{"match":{"text":"foo"},"start":0,"end":3}]}}
{"type":"end","data":{"path":{"text":"foobar.md"},"binary_offset":null,"stats":{"elapsed":{"secs":0,"nanos":2815,"human":"0.000003s"},"searches":1,"searches_with_match":1,"bytes_searched":7,"bytes_printed":230,"matched_lines":1,"matches":1}}}
{"type":"begin","data":{"path":{"text":"test.lua"}}}
{"type":"match","data":{"path":{"text":"test.lua"},"lines":{"text":"  args = {\"foo\"},\n"},"line_number":8,"absolute_offset":168,"submatches":[{"match":{"text":"foo"},"start":11,"end":14}]}}
{"type":"end","data":{"path":{"text":"test.lua"},"binary_offset":null,"stats":{"elapsed":{"secs":0,"nanos":5463,"human":"0.000005s"},"searches":1,"searches_with_match":1,"bytes_searched":317,"bytes_printed":245,"matched_lines":1,"matches":1}}}
{"data":{"elapsed_total":{"human":"0.002163s","nanos":2163299,"secs":0},"stats":{"bytes_printed":696,"bytes_searched":328,"elapsed":{"human":"0.000023s","nanos":23005,"secs":0},"matched_lines":3,"matches":3,"searches":3,"searches_with_match":3}},"type":"summary"}

--replace or -r for short would be my guess for how we'd achieve this.

$ rg --vimgrep --smart-case -r '<<<$0>>>' foo                  
foo.md:1:1:<<<foo>>>
foobar.md:1:1:<<<foo>>>bar
test.lua:8:12:  args = {"<<<foo>>>"},

What we choose for the delimiter would matter though to prevent over matching on the telescope side when parsing the entries.

I can work on this.

@matu3ba
Copy link

matu3ba commented May 6, 2023

--json would probably be over kill. There's a lot to parse.

vim.json is fast and makes parsing very simple and now also has documentation on master HEAD. Performance for non-luajit should be quickly benched though, but I dont see (yet) how it would make things slower than the current approach.

@jamestrew
Copy link
Contributor

Yeah I'm reconsidering and leaning towards using --json especially after learning that's basically what burntsushi recommends.
I'm not concerned so much about the performance but rather that reworking live_grep to consume the json data would be a lot more work than using --replace.

And frankly, this repo's been a bit cold in terms of maintainer activity so I don't have a ton of motivation to spend a good chunk of time on something only for it to be forgotten in the stack of PRs.

@jamestrew
Copy link
Contributor

Started working on this. Still have to work on some edge cases and do some clean up but overall fairly straight forward.
image
^notice the filename foo is not getting highlighted.

@fdschmidt93
Copy link
Member

fdschmidt93 commented May 21, 2023

You can also refer to the entry maker of https://github.com/fdschmidt93/telescope-egrepify.nvim as well which uses vim.json to parse info from rg.

@jamestrew
Copy link
Contributor

I did take a peek actually when working on the entry maker 😁

The tricky part is how to deal with certain options that will conflict with --json (rg will throw an error that isn't propagated to telescope), namely --files, --files-with-matches, --files-without-match, --count and --count-matches.

For the first three file related flags, we should be able to use the gen_from_file entry maker and not use the --json flag.

The count flags we won't be able to support (and we never did) without it's own entry maker I think. I'm not going to bother with that but I probably want to throw an error message at least.

@jamestrew jamestrew linked a pull request May 25, 2023 that will close this issue
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants