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

vimgrep entry maker using --json #2536

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jamestrew
Copy link
Contributor

@jamestrew jamestrew commented May 25, 2023

Description

Applies --json to vimgrep arguments for live_grep and grep_string allowing for correct highlighting of submatches in the entry display.

image
^notice the filename foo is not getting highlighted matching the submatch highlight directly from rg

More specific changes:

  • vimgrep_arguments now { "rg", "--smart-case", "--json" }
  • Rewrite of gen_from_vimgrep entry maker to support parsing of json output
  • Refactor and consolidation of rg options, pattern and search paths generation for live_grep and grep_string

Closes #2272
Closes #934

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list relevant details about your configuration

  • Pretty rigorous testing of both live_grep and grep_string pickers using all options
  1. touch foo.md
  2. echo "foo\nfoobar bar\n\tfoo" > foo.md
  3. touch test.py
  4. echo "def bar():\n\treturn 'foobar'" > test.py
  5. run through both pickers with various picker options and rg flags via additional_args (notably -l, --files-with-matches, --files, ie. those documented by rg to be incompatible with --json)

For file_encoding option:

  1. create file with acentuação
  2. write file with latin1 encoding :write ++enc=latin1
  3. test pickers with :Telescope <picker> file_encoding=latin1 and verify previewer/entry

Configuration:

  • Neovim version (nvim --version): NVIM v0.10.0-dev-410+gc48f94d1f
  • Operating system and version: Linux archlinux 6.3.2-arch1-1

Checklist:

  • My code follows the style guidelines of this project (stylua)
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (lua annotations)

@jamestrew jamestrew changed the title wip vimgrep entry maker using --json May 25, 2023
@jamestrew jamestrew force-pushed the feat/vimgrep--json branch 2 times, most recently from 66170f7 to f31994a Compare May 29, 2023 00:22
@jamestrew jamestrew marked this pull request as ready for review May 29, 2023 00:24
@jamestrew
Copy link
Contributor Author

jamestrew commented May 29, 2023

Maybe worth discussing is that with the currently implementation of the PR, for grep_string the rg submatch highlights aren't applied to de-conflict with grep_string's sorter matches.

image
^ only the prompt matches highlight in cyan (TelescopeMatching).

Maybe it's better to use a different highlight for the rg submatches for both pickers.
image
^ here notices the bar submatches are probably highlighted in red while the grep_string prompt matches are still highlighted in the cyan.

@Conni2461
Copy link
Member

thats awesome thank you very much. there are a couple of things i have to think about like: gen_from_vimgrep now working differently and if we want to keep the old one and provide a new make_entry, if we dont do that we kinda have a breaking change (not that big of a deal but i have to think about it ^^).

Another question i need to think about is if displayer is the best way to do highlighting here.

But i've tested it and it already works great. Thank you very much for implementing it :)

@jamestrew
Copy link
Contributor Author

Yeah I'm not opposed to keeping gen_from_vimgrep and calling this new entry_maker something else for backwards compatibility reasons.

Another question i need to think about is if displayer is the best way to do highlighting here.

Yeah I don't love creating a new displayer for each entry. I'm open to ideas.

@Marskey
Copy link
Contributor

Marskey commented Jul 18, 2023

when this will be merged into master, I realy love this new feature

@Marskey
Copy link
Contributor

Marskey commented Jul 20, 2023

" Telescope pickers will occur error:
image

@Conni2461 Conni2461 marked this pull request as draft July 26, 2023 07:48
@Conni2461
Copy link
Member

Yeah I don't love creating a new displayer for each entry. I'm open to ideas.

Displayer is just a wrapper around the actual interface, i'll going to rewrite that section.

I am also making this PR a Draft for the time being. I know its almost done but then we might have less ppl asking 😆

@jamestrew
Copy link
Contributor Author

Sounds good.

" Telescope pickers will occur error: image

I also gotta fix this eventually as well

@jamestrew
Copy link
Contributor Author

Note to self: vim.base64?

@Marskey
Copy link
Contributor

Marskey commented Feb 1, 2024

" Telescope pickers will occur error: image

function make_entry.gen_from_vimgrep_json(opts)
  opts = opts or {}
  local cwd = vim.fn.expand(opts.cwd or vim.loop.cwd() or "")

  local function bytes_or_text_to_str(data)
    if data.bytes ~= nil then
      return require("telescope.algos.base64").decode(data.bytes)
    else
      return data.text
    end
  end

  local mt_vimgrep_entry = {}
  mt_vimgrep_entry.display = function(entry)
    local display_filename = utils.transform_path(opts, entry.filename)

    local coordinates = ":"
    if not opts.disable_coordinates then
      if entry.lnum then
        if entry.col then
          coordinates = string.format(":%s:%s:", entry.lnum, entry.col)
        else
          coordinates = string.format(":%s:", entry.lnum)
        end
      end
    end

    local file_pos, hl_group, icon = utils.transform_devicons(
      entry.filename,
      string.format("%s%s", display_filename, coordinates),
      opts.disable_devicons
    )

    return file_pos .. entry.text,
      (function()
        if opts.__finder == "grep_string" then
          return {}
        end

        local highlights = { { { 0, #icon }, hl_group or "" } }
        local file_pos_len = #file_pos
        for _, submatch in ipairs(entry.submatches) do
          table.insert(
            highlights,
            { { submatch["start"] + file_pos_len, submatch["end"] + file_pos_len }, "TelescopeMatching" }
          )
        end
        return highlights
      end)()
  end

  mt_vimgrep_entry.__index = function(t, k)
    local override = handle_entry_index(opts, t, k)
    if override then
      return override
    end

    local raw = rawget(mt_vimgrep_entry, k)
    if raw then
      return raw
    end

    if k == "path" then
      local path = Path:new(t.filename)
      if path:is_absolute() then
        return t.filename
      end
      return Path:new({ cwd, t.filename }):absolute()
    end

    if k == "text" then
      return t.value
    end

    if k == "ordinal" then
      local text = t.text
      return opts.only_sort_text and text or text .. " " .. t.filename
    end
  end

  return function(line)
    local msg = vim.json.decode(line)
    if msg == nil or msg.type ~= "match" then
      return
    end

    local text = bytes_or_text_to_str(msg.data.lines):gsub("%s+$", "")
    return setmetatable({
      value = text,
      filename = bytes_or_text_to_str(msg.data.path),
      lnum = msg.data.line_number,
      col = #msg.data.submatches ~= 0 and msg.data.submatches[1].start + 1 or nil,
      submatches = msg.data.submatches,
    }, mt_vimgrep_entry)
  end
end

@jamestrew this should fix it, entry_display.create called in display makes this error occurred

@jamestrew jamestrew marked this pull request as ready for review February 2, 2024 04:00
@jamestrew
Copy link
Contributor Author

@Marskey thanks I've incorporated your snippets. looks to be working well so far.

@Marskey
Copy link
Contributor

Marskey commented Feb 2, 2024

    return setmetatable({
      value = text,
      filename = bytes_or_text_to_str(msg.data.path),
      lnum = msg.data.line_number,
      col = #msg.data.submatches ~= 0 and msg.data.submatches[1].start + 1 or nil,
      colend = #msg.data.submatches ~= 0 and msg.data.submatches[1]["end"] + 1 or nil,
      submatches = msg.data.submatches,
    }, mt_vimgrep_entry)

Please consider whether colend needs to be included.

@jamestrew
Copy link
Contributor Author

I also rebased everything.
I switched out to using vim.base64 over some copy/pasted code so this is now requires neovim 0.10. I think the intention was to make telescope 0.2.0 require nvim 0.10 so I'm leaning on keeping this.

wochap added a commit to wochap/telescope.nvim that referenced this pull request Mar 28, 2024
@humblehacker
Copy link

This fix has been working for me. Any chance we can get this merged?

@Marskey
Copy link
Contributor

Marskey commented Apr 2, 2024

til neovim 0.10 released, or you can merged by youself if you are using pre-release version 0.10

@npearson72
Copy link

Neovim 0.10 has already been released. Any reason this PR still can't be merged?

@Conni2461
Copy link
Member

I still need to review this, thats missing.

@Conni2461 Conni2461 self-requested a review May 29, 2024 16:54
@jamestrew
Copy link
Contributor Author

We're still supporting 0.9 on the master branch as well.

wochap added a commit to wochap/telescope.nvim that referenced this pull request Jul 8, 2024
vimgrep entry maker using --json
nvim-telescope#2536
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: To Do
5 participants