akinsho / git-conflict.nvim

A plugin to visualise and resolve merge conflicts in neovim
1k stars 35 forks source link

Pre PR disabling git-conflict on demand #46

Closed ttytm closed 1 year ago

ttytm commented 1 year ago

Before putting in the work to finish a PR I wanted to reach out real quick.

It's about adding a way to disable and enabling the plugin on demand. Reason for this is having experienced git-conflict.nvim-errors and performance recessions when using it in conjunction with diffview.nvim.

As far as I can see it could just be done by moving all the aus out of the setup() into their own accessible function. Then we could easily clear and re-apply them.

At first I thought about a ignore_filetype option but from my side I would prefer just making a function accessible or adding a command to toggle git-conflict. It's not only a bit less work, it would also provide enough that users can make their own way to filetype ignoring.

Any input is welcome :+1:

akinsho commented 1 year ago

@tobealive thanks for raising the issue to discuss first. Tbh there's kind of a lot here that I'm entirely unaware of has never been raised that I think is worth discussing first

Reason for this is having experienced git-conflict.nvim-errors and performance recessions when using it in conjunction with diffview.nvim.

I really don't change this plugin very much so there being performance regressions is strange since I'm not changing much that should be causing this at all, unless your talking in span of year+ in which case there have been some changes.

I know that Sindrets kindly pointed to this plugin from diffview.nvim but I've never used them together so I don't really know what using it in the context of diffview would even look like or if there's an interaction there. Would be good to know exactly what about this isn't working

As far as I can see it could just be done by moving all the aus out of the setup() into their own accessible function. Then we could easily clear and re-apply them.

This seems like just a valuable thing to have I guess being able to more tidily apply and clear the autocommands. To my mind I've never envisioned a flow where a person would be doing this or why since the plugin is just designed to sit in the background and watch for merge conflicts so no autocommands is kind of completely besides the point of this plugin 🤷🏿

So before going into a disable/enable solution which seems kind of local to the issue you are facing I'd love to just clarify this entire workflow exactly and what is wrong specifically since it doesn't really sound at all like how I use the plugin or imagined it being used

ttytm commented 1 year ago

Thank for the reply @akinsho!

I intended to say a general impairment with the term "recessions", not a "regression" resulting from recent changes. As a non-native speaker, I may have chosen the wrong word though.

I'm currently incorporating diffview more into my workflow. It was primarily neogit and lazygit until now. So I do not have an extensive time horizon to compare the two plugins side by side.

This week, while diffing repositories with a medium to large set of changes, I encountered two scenarios where the file switching in diffview took some processing time and git-conflict.nvim errors were thrown at me. This was without having conflicts in the repo. Although this issue did not occur consistently and as not-random something is in programming, it felt random.

However, I didn't experience the issue when disabling git-conflict, and I didn't dig deeper. Now, without knowing the exact friction points or having an exact reproducible scenario, I figured it would work to just disable the watcher while working in diffview.

Edit: I'm just realizing that it probably would be no problem to just delete the augroup and call the setup again to achieve the described behavior.

akinsho commented 1 year ago

Thanks for explaining, tbh from my perspective that would be quite a blunt solution since rather than fixing the issue it would just mask it. So I'd have no way of resolving it properly. Tbh I've never used the two together. Although I use diffview extensively I only ever use it for diffing and never for handling merge conflicts since I generally don't really like that paradigm anyway, I'm much too used to years of just operating on the conflicts directly.

If you happen to get any error messages please post them here and I can dig in, otherwise I can try and use the two together myself and see what comes up.

TLDR I'd like to solve for the real underlying issue rather than code that allows bypassing it and that just becoming the way it's dealt with

ttytm commented 1 year ago

Okay, we have a plan. I'll keep track of errors and come back to this :+1: .

ttytm commented 1 year ago

I see there have been some improvements in error handling and parsing on the diffviews side.

Having not re-experience this issue yet and admitting that I lacked the motivation to produce git conflicts, or just partially conflicting sections, in order to forcefully investigate the mentioned errors we can close it from my side. If something comes up, we can still reopen it.