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

Option to disable Copilot for certain directories / files whose path matches certain patterns #74

Open
ouuan opened this issue Nov 2, 2022 · 17 comments · Fixed by #127

Comments

@ouuan
Copy link

ouuan commented Nov 2, 2022

Motivation

I want to disable Copilot for my university assignments to avoid the risk of cheating. So I want to disable Copilot for all files in a particular directory.

Feature Request

Add an option to disable Copilot for certain directories or files whose path matches certain patterns.

Or provide example config scripts if this can be done by the user without modifying the plugin. I tried running setup conditionally and using autocmd but with no luck.

@zbirenbaum
Copy link
Owner

zbirenbaum commented Nov 2, 2022

I'll think about making this an official feature where users provide a pattern to be matched, but I think that may be overkill for something that can be done in a relatively simple autocmd. This code works perfectly for me:

local disable_dir = vim.fn.expand('$HOME/.config/nvim_configs') .. '/*'

vim.api.nvim_create_autocmd({"LspAttach"}, {
  pattern = disable_dir,
  callback = function (args)
    if not args['data'] or not args.data['client_id'] then return end

    local client = vim.lsp.get_client_by_id(args.data.client_id)
    if client.name == 'copilot' then
      vim.lsp.stop_client(client.id, true)
    end
  end
})

Obviously you will need to change the disable_dir path to be wherever it is you do your work as well as run the above before copilot is launched

@MunifTanjim
Copy link
Collaborator

Might be worth adding:

  • :Copilot disable to stop the client
  • :Copilot enable to start the client (only if copilot.setup() was already called, otherwise it won't know the config)

@ouuan
Copy link
Author

ouuan commented Feb 24, 2023

As there's no documentation for #127, I'm wondering how it resolves this issue.

@MunifTanjim
Copy link
Collaborator

MunifTanjim commented Feb 24, 2023

That PR just adds the :Copilot enable and :Copilot disable command, same as the original copilot.vim plugin.

You can use that to completely disable copilot. But the logic of when to disable copilot is still up to you.

You can manually execute that command, or use that with DirChanged autocmd.

Something like:

  local function should_disable_copilot(dir)
    return dir == "/some/specific/dir"
  end

  vim.api.nvim_create_autocmd("DirChanged", {
    callback = function(params)
      if should_disable_copilot(params.file) then
        vim.cmd("Copilot disable")
      end
    end,
  })

  -- for initial directory neovim was started with
  if should_disable_copilot(vim.loop.cwd()) then
    vim.cmd("Copilot disable")
  end

It's up to you to implement your preferred logic.

@MunifTanjim
Copy link
Collaborator

Or you can use the 'exrc' option from latest neovim.

And for your specific repo, create a file called .nvim.lua and do:

vim.cmd("Copilot disable")

@ouuan
Copy link
Author

ouuan commented Feb 24, 2023

A working solution is already provided in @zbirenbaum's comment, and this issue is supposed to be tracking a built-in solution. I'm fine if the built-in solution is not implemented right now, but I don't think this issue should be closed as a result of Copilot disable.

BTW, the LspAttach and lsp.stop_client based solution seems better to me. It directly hooks the LSP event instead of relying on the somehow indirect DirChanged event, and different buffers can have different LSPs attached.

@MunifTanjim
Copy link
Collaborator

MunifTanjim commented Feb 24, 2023

That comment is quite old and not really what you want. Even if you do lsp.stop_client, when you open another file it'll start the server again. Copilot will be attached to the new buffer, LspAttach will be triggered again, you'll do lsp.stop_client again... and it'll go on for each new file you open. So the plugin is not completely disabled/stopped.

On the other hand :Copilot disable completely disables the plugin. It does lsp.stop_client under the hood along with other stuffs, e.g. cleaning up autocmds created by copilot, to prevent it from automatically starting again.

But you don't specifically need to use DirChanged, you can do the same thing with LspAttach autocmd. It'll run when copilot is initially attached to a buffer, and immediately disable it.

I don't want to add some super specific config option for disabling copilot for certain directories/files patterns. The :Copilot disable solution is flexible enough to satisfy any use-cases. I hope that makes sense.

@zbirenbaum what do you think?

@ouuan
Copy link
Author

ouuan commented Feb 24, 2023

That comment is quite old and not really what you want. Even if you do lsp.stop_client, when you open another file it'll start the server again. Copilot will be attached to the new buffer, LspAttach will be triggered again, you'll do lsp.stop_client again... and it'll go on for each new file you open. So the plugin is not completely disabled/stopped.

I don't want to completely disable it. I just want it to be disabled when the file is in some directory. If I open a file in another directory, Copilot should be working there.

@MunifTanjim
Copy link
Collaborator

I don't want to completely disable it. I just want it to be disabled when the file is in some directory. If I open a file in another directory, Copilot should be working there.

Not sure about the use-case. Can you explain a bit about how you're using neovim?

- dir-a
  - dir-x
    - file-x-1
    - file-x-2
  - dir-y
    - file-y-1
    - file-y-2
- dir-b
  - file-b-1
  - file-b-2
  • Open Neovim in dir-a, then you want copilot to be disabled for files inside dir-a/dir-x but enabled for files inside dir-a/dir-y?
  • Open Neovim in dir-a, you want copilot to be disabled for all files inside it. Open another Neovim in dir-b, you want copilot to be enabled for all files inside it?
  • Open Neovim in dir-a/dir-x, you want copilot to be disabled. Then change the directory using :cd ../dir-y command to dir-a/dir-y and want copilot to be enabled?

Or something else?

@ouuan
Copy link
Author

ouuan commented Feb 24, 2023

Ideally, whether Copilot is working or not should depend only on the path of the file being edited (whether the path matches a pattern), not where I initially opened neovim, not which files I opened before, and not where the PWD is.

I don't understand your example, because you didn't specify which directory is the directory where I want to disable Copilot.

BTW, the "another directory" in my previous comment means a directory that doesn't match the pattern, not an arbitrary directory.

@ouuan
Copy link
Author

ouuan commented Feb 24, 2023

I'm commenting here because the "closed this as completed in #127" made me think a built-in solution was added in #127 and I missed it. I'm happy with the lsp.stop_client based solution, so it's OK for me to close this as a no-fix, but not as implemented.

@MunifTanjim MunifTanjim reopened this Feb 24, 2023
@MunifTanjim
Copy link
Collaborator

MunifTanjim commented Feb 24, 2023

Hmm... I think I misunderstood your problem. Now I understand exactly what you want.

I'd suggest to modify the solution from #74 (comment) to this:

  local augroup = vim.api.nvim_create_augroup("copilot-disable-patterns", { clear = true })
  for _, pattern in ipairs({ "/dir-a/dir-x/*", "/dir-b/*" }) do
    vim.api.nvim_create_autocmd("LspAttach", {
      group = augroup,
      pattern = pattern,
      callback = vim.schedule_wrap(function(args)
        local client = vim.lsp.get_client_by_id(args.data.client_id)
        if client.name == "copilot" then
          vim.cmd("Copilot detach")
        end
      end),
    })
  end

vim.lsp.stop_client kills the whole server. So all the files copilot is currently attached will get detached, not just the specific file from your pattern. :Copilot detach will only affect the current buffer.

Whether this will be included in the plugin's config is up to @zbirenbaum .

The downside of adding it to plugin's config is that the patterns will need to be hardcoded in config. And directory structure is different for different machines (i.e. config won't be portable between machines).

Keeping it outside plugin's config let's you use it from .nvim.lua file (i.e. with 'exrc' option). So you can add these autocommand only for a specific repo.

I'm keeping the issue open for zbirenbaum's decision.

@ouuan
Copy link
Author

ouuan commented Feb 24, 2023

vim.lsp.stop_client kills the whole server. So all the files copilot is currently attached will get detached, not just the specific file from your pattern. :Copilot detach will only affect the current buffer.

I realized this but I thought it was not a big problem in my workflow, and I didn't notice the new :Copilot detach. Thanks for the new config as it does look much better.

@ouuan
Copy link
Author

ouuan commented Feb 24, 2023

I changed the callback to this:

    callback = function(args)
      local client = vim.lsp.get_client_by_id(args.data.client_id)
      if client.name == 'copilot' then
        vim.defer_fn(function()
          vim.cmd("silent Copilot detach")
        end, 0)
      end
    end,

Without vim.defer_fn it shows "not attached", and the command output is noisy so silent.

@richchurcher
Copy link

Came here looking for exactly this. I could see an option to ignore certain paths being compelling to users or companies who are nervous about snippets of code being released outside their network, even with GitHub's assurance that they are not retained.

@FullStackAlex
Copy link

Based on the functions suggested by @MunifTanjim, @ouuan and @zbirenbaum I wrote this one to exclude a number of directories and empty buffers with cwd matching any of the disable_dirs:

local augroup = vim.api.nvim_create_augroup("copilot-disable-patterns", { clear = true })
local disable_dirs = {
    vim.fn.expand "$HOME/Documents" .. "/*",
}
for _, pattern in ipairs(disable_dirs) do
    vim.api.nvim_create_autocmd("LspAttach", {
        group = augroup,
        pattern = "*", -- This pattern will match all files, including new buffers
        callback = function(args)
            -- Check if the buffer has a name (file associated) or if CWD starts with /home/user/Documents
            local bufname = vim.api.nvim_buf_get_name(0)
            local cwd = vim.fn.getcwd()

            if bufname == "" and cwd:match("^" .. pattern) or bufname:match(pattern) then
                local client = vim.lsp.get_client_by_id(args.data.client_id)
                if client.name == "copilot" then
                    vim.defer_fn(function()
                        vim.cmd "silent Copilot detach"
                    end, 0)
                end
            end
        end,
    })
end

But the "silent" part seems not to work - I get 202 "Detached buffer(id:1) from client (id:1)" messages.
Apart from the noise - why does the plugin/copilot tries repeatedly to attach again to the buffer after being detached?

@LarsMichelsen
Copy link

Many of the recent approaches react to the LspAttach event. This means the LSP client was started and attached already.

I am asking myself: Does this already provide information to the LSP we would rather not reveal?

Also not knowing the overall architecture the vim.defer_fn look suspicious. Why is this needed? Is there a short time where data might reach the LSP?

If the goal is to prevent data be handled by copilot, I think we should cut it of as soon as possible. Scanning through the code the plugin, I see https://github.com/zbirenbaum/copilot.lua/blob/b03617a6dc4bc88b65ab5deac1631da9a9c2dcaf/lua/copilot/util.lua#L99C12-L99C25 which can prevent the LSP client from actually being attached. I personally would rather like to see a feature hooking in there. That would give me a way better feeling that the exclusion actually works as intended.

Do you think it would be possible to provide a customizable API, something which allows configurations like this?

require("copilot").setup({
    -- ... other options ...
    should_attach = function()
       -- Custom conditions come here. 
       return true
   end
})

It would be very handy to have access to the current buffers filename and the cwd. To enable filtering based on paths.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants