feat(git): notify when not in a git repo instead of error

[antoinemadec]

Description

Fixes #2180

Type of change

Please delete options that are not relevant.

• New feature (non-breaking change which adds functionality)

How Has This Been Tested?

• git_files in git repo: works as before
• git_files outside git repo: notification pops-up, Telescope prompt does not show up, no more error

Configuration:

• Neovim version (nvim --version): NVIM v0.8.0-dev-1200-gf9228577e
• Operating system and version: Linux Manjaro 22

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)

[Conni2461]

Thanks :)

[Conni2461]

Hmm this broke a use case i forgot about: #410 (comment)

[antoinemadec]

@Conni2461 , this is how to do it know: #2183 (comment)

It seems reasonable and rather intuitive to me, what do you think ?

Some users were recommanding updating the Wiki with this new receipe.

[sQVe]

@antoinemadec @Conni2461 This change complicates my use-case by quite a bit. I run the following function:

M.find_files = function(use_buffer_cwd)
  local builtin = require('telescope.builtin')
  local utils = require('telescope.utils')
  local opts = {
    follow = true,
    hidden = true,
    show_untracked = true,
    use_git_root = false,
  }

  if use_buffer_cwd then
    opts.cwd = utils.buffer_dir()
  end

  local ok = pcall(builtin.git_files, opts)
  if not ok then
    builtin.find_files(opts)
  end
end

This allows me to run find_files for the cwd of the current buffer. Now I would have to feed the cwd into a systemlist call, instead of being able to rely on the built-in functionality.

I think this should be reverted. There's a lot of power in it erroring when failing. If you want to be notified instead you can always wrap the call yourself.

[Conni2461]

I agree, i dont consider calling vim.fn.systemlist"git rev-parse --is-inside-work-tree"[1] == 'true' a acceptable "workaround". its doing the same work twice, which is less elegant compared to before.

I was thinking about returning a bool if we started the picker, but this would still result in the vim.notify error message, which isn't ideal either. In fact you can no longer suppress the vim.notify error message easily (at all, without suppressing all notify messages with level error, or providing a no op for vim.notify). This isn't great either.

So i was thinking about reverting because other people could always just do

local wrap = function(builtin, opts)
  local ok, msg = pcall(builtin, opts)
  if not ok then
    vim.notify(msg)
  end
end

wrap(require("telescope.builtin").git_files, {})

something like this would achieves the same as the pr (not tested, written in github right now)

So yeah, i'll need to think about it for a day or so. If you need that feature rn, please checkout a early commit or checkout branch 0.1.x

[antoinemadec]

The "not doing the same check twice" argument is sound.

On my end, I like to be able to chose from find_files and git_files myself (when you are looking for untracked or ignored files in your repo for instance), and I'd rather have a default behavior that notifies me rather than errors out.

I think it just boils down to:

• how many people are negatively impacted by keeping or removing this commit
• consistency of the erroring out behavior across the pickers
• personal taste

Could also be a config option ?

[sQVe]

@Conni2461 👋🏼

Have you possibly landed in a decision here? I've been pushing off the required changes in my config but this issue is getting more frustrating.

[Conni2461]

Yeah. I know i put the decision of because its a very opinionated issue.

We could do a config option and i probably will do it on a more global level. Like what i am currently thinking is doing error everywhere, which enables users to do scripting things and then providing a no_error option (or so) and this option then pcalls everything. you can then obviously do no_error = true on a global level and override it for Telescope git_files no_error=false to bring errors back. #2216

I will revert this commit now and then work on that issue this weekend. I hope this works for everyone :)

[Conni2461]

#2225 does implement config value / option error_mode with the following values "modes":

• "lua" (default, because current behavior)
• "notify"
• "silent"

Telescope git_files error_mode=notify or :lua require("telescope.builtin").git_files { error_mode = "notify" } or configure it in the default table in telescope.setup

feedback is appreciated.

Edit: Its not done yet, because i have to go over all builtins and make error handling consistent.