rg: file_ignore_patterns will be ignored in searches

[flunderpero]

Description

This is my setup:

local telescope = require("telescope")
telescope.setup {
    defaults = {
        file_sorter = require "telescope.sorters".get_fzy_sorter,
        generic_sorter = require "telescope.sorters".get_fzy_sorter,
        vimgrep_arguments = {
            "rg",
            "--color=never",
            "--no-heading",
            "--with-filename",
            "--line-number",
            "--column",
            "--smart-case",
            "--hidden"
        },
        file_ignore_patterns = {
            "node_modules",
            ".work/.*",
            ".cache/.*",
            ".idea/.*",
            "dist/.*",
            ".git/.*"
        },
       ... some other stuff ...
    }
}

When I use live_grep and search for work.build nothing is found. I have a lot of files including the string .work/build/.... When I remove the pattern ".work/.*" from file_ignore_patterns I find all the matches and the output matches that of rg run in a shell.

Perhaps I am doing something wrong here.

Neovim version

NVIM v0.6.0-dev+nightly-353-gff0833cb4

Operating system and version

macOS 11.5

Steps to reproduce

See above.

Expected behavior

file_ignore_patterns should only affect the files that are searched.

Actual behavior

See above.

Minimal config

See above.

[fdschmidt93]

If I see it correctly, this is working as intended.

The option is gathered from your global defaults

telescope.nvim/lua/telescope/pickers.lua

Line 94 in dcee6c8

file_ignore_patterns = get_default(opts.file_ignore_patterns, config.values.file_ignore_patterns),

and then run on your picker if applicable

telescope.nvim/lua/telescope/pickers.lua

Lines 1013 to 1027 in dcee6c8

	log.trace("Processing result... ", entry)
	for _, v in ipairs(self.file_ignore_patterns or {}) do
	local file = type(entry.value) == "string" and entry.value or entry.filename
	if file then
	if string.find(file, v) then
	log.trace("SKIPPING", entry.value, "because", v)
	self:_decrement "processed"
	return
	end
	end
	end
	self.sorter:score(prompt, entry, cb_add, cb_filter)
	end
	end

I think you either want to (a) pass file_ignore_patterns directly to builtin.find_files, (b) the file_ignore_patterns in your .gitignore so it's picked up adequately, or maybe (c) use builtin.git_filesinstead.

[flunderpero]

@fdschmidt93 thank you for your quick response.

I see that the code does that, but how can the actual behavior be intended?
If I have foo in my file_ignore_patterns I cannot search for foo in live_grep. Every file with a name that contains foo should be ignored, but not every file containing the word foo.

I think the code is wrong. It should only skip a file based on entry.filename, shouldn't it?

Edit: Fixed typo. :-) Again...

[fdschmidt93]

Every file with a name that contains foo should be ignored, but not every file containing the word foo.

Sorry, I read / thought about this a bit to fast and misunderstood your original issue -- you are of course right :)

I think the code is wrong. It should only skip a file based on entry.filename, shouldn't it?

Generally, yes. I think the better default is to turn it around and prefer entry.filename and fall back to entry.value which, for instance, would apply to the builtin.file_browser. builtin.live_grep is the situation where entry.value is a string and then your bug occurs though we have entry.filename.

Would you be happy to submit the fix via a PR? :) Otherwise I'll do that some time this week.

As a side note, more generally, we probably should unify all entry.{filename, value, path} into a unified attribute, possibly wrapped in Plenary's path module. In my preliminary tests tracing inconsistencies was just confusing 😅 My hunch as to why we're not doing that yet is speed in eg live grep. But we can take care of that some time else.

[flunderpero]

I could provide a PR that changes

local file = type(entry.value) == "string" and entry.value or entry.filename

to

local file = type(entry.value) == "string" and entry.filename or entry.value

I am completely new to the code base and I don't see any tests for live_grep. How would I test this (besides trying it out in my local nvim)?

[fdschmidt93]

Apologies, I didn't provide sufficient background, I meant something like this

local file = vim.F.if_nil(entry.filename, type(entry.value) == "string" and entry.value) -- false if none is true

entry.filename is always a string and entry.value iirc is just a copy/reference of the original input which most commonly is a table but may be a string. Probably could be implemented more elegantly ;)

I am completely new to the code base and I don't see any tests for live_grep. How would I test this (besides trying it out in my local nvim)?

Good question. That part I'm not so familiar with myself. I guess we'd have to come up with something that isolates file_ignore_patterns and live_grep within the telescope repo inspired by this spec: https://github.com/nvim-telescope/telescope.nvim/blob/master/lua/tests/automated/pickers/find_files_spec.lua

But I'd personally be fine fixing this without a test for the time being.

[TheLeoP]

Hi, @fdschmidt93

First question: how could I try my changes if I'm also using Telescope myself? Should I just use another neovim config?

I could include my fork on my runtimepath, but that will cause conflicts with Telescope, right? If I do require('telescope') for example.

[fdschmidt93]

There's probably a nicer way of doing that but I just point packer to my fork and switch branches when needed with telescope. The primary downside of this is you have to update yourself with git fetch upstream master and git rebase upstream master. It's one of the things that doesn't bother me so much though. Maybe another maintainer or reddit has better answers though.

[TheLeoP]

A couple of things: first I just copy the line you suggested. Should I make some teaks? I'm not sure about how could I improve this.

Also, I'm kinda lost going from .lua to .lua. I was looking up the neovim help, but I wasn't able to find help about functions like Picker:get_result_processor. Are there some docs I'm not finding? Or should I just keep looking upt the code?

And, finally, while tryng out the change I foung that, on Windows, the pattern ".work/.*" doesn't ignore files like .\.work\.should_not_show_up but I have to use the pattern .work\.* instead. That happens because Telescope is ignoring files using string.pattern() and Windows uses backslashes when Linux uses slashes for paths. Is that working as intended?

[fdschmidt93]

A couple of things: first I just copy the line you suggested. Should I make some teaks? I'm not sure about how could I improve this.

Just make a PR how you see fit and I'll have a look at the diff then :)

Also, I'm kinda lost going from .lua to .lua. I was looking up the neovim help, but I wasn't able to find help about functions like Picker:get_result_processor. Are there some docs I'm not finding? Or should I just keep looking upt the code?

Is that required for this PR? Mid-to-long-term, if you want to continue contributing on more challenging matters I guess yes, I'm afraid you'll have to go through it largely by yourself to understand what's going on. However, there's also #927 at which you can look to give you a better idea.

And, finally, while tryng out the change I foung that, on Windows, the pattern ".work/." doesn't ignore files like ..work.should_not_show_up but I have to use the pattern .work. instead. That happens because Telescope is ignoring files using string.pattern() and Windows uses backslashes when Linux uses slashes for paths. Is that working as intended?

Wouldn't Windows users then just use backslashes? Am I missing something?

[TheLeoP]

Ok, so, I think I have made my first PR #1243 . Tell me if I have done something wrong.

Is that required for this PR?

Not really, but I was kinda excited, so I was trying to start understanding all the code base.

However, ther's also #927 ar which you can look to gie you a better idea.

Thanks, I will definetly take a look at it :3.

Wouldn't Windows users then just use backslashes? Am I missing something?

Yeah, that makes sense, I was just wondering if that was intended because I prefer using normal slashes as much as possible for paths as a Windows user.

[Conni2461]

I think this is fixed now. Closing. If this is still an issue, feel free to reopen :)