Konfekt / FastFold

Speed up Vim by updating folds only when called-for.
708 stars 24 forks source link

Support diff mode #35

Closed lervag closed 8 years ago

lervag commented 8 years ago

I'm curious if it would be possible to add some support for diff mode. That is, the FastFold "system" breaks down if I for instance do the following:

  1. Open a LaTeX file
  2. Start diffmode with a different version or file
  3. Exit diffmode

I am currently using lawrencium for Mercurial support, but I think this is unrelated. It should be enough to add two minimal TeX files with minor differences, open one, then do diffsplit with the other one. Since this sets fdm=diff, it tends to break the FastFold magic. For reference, I've also asked the lawrencium developer to support FastFold, but I am now sure if that's a solution: I think FastFold should "just work". (Reference: https://github.com/ludovicchabant/vim-lawrencium/issues/10#issuecomment-212251257).

Perhaps a solution is to "ignore" the diff foldmethod? I.e., when one sets fdm=diff, then FastFold keeps the old foldmethod in memory and uses that when refreshing?

Konfekt commented 8 years ago

Ah, ok. You mean without FastFold vim would set foldmethod to the value set before diff, whereas FastFold does not update its equivalent w:lastfdm but the user has to do it manually?

Konfekt commented 8 years ago

About your proposed solution, that would imply that for example za refreshes the folds with the last foldmethod, undesired in diff mode.

But presuming the computation of diff folds is lightweight, I simply ignore it for now and you tell me if it's fast nuff?

lervag commented 8 years ago

Ironically, I always urge people to define a minimal and reproducible example for issues on vimtex, but I don't follow my principle for issues other places. Sorry! In this case, it is because I'm at work and don't have time to create the minimal example. I'll make a more concrete example later when I get the time.

In the meantime: I propose that when diffmode changes the foldmethod, then the old foldmethod is lost when I change between the diffed windows. For some reason, it seems that w:lastfdm and b:lastfdm is cleared. Thus, when I exit the diff mode, then the foldmethod returns to manual, but FastFold no longer knows the "real" fold method.

Konfekt commented 8 years ago

Wait, but FastFold does already currently ignore the diff foldmethod as it assumes it to be fast enough. See

https://github.com/Konfekt/FastFold/blob/master/plugin/fastfold.vim#L111

Konfekt commented 8 years ago

Ah, ok, true, even so FastFold stops functioning, it might clear w:lastfdm.

Konfekt commented 8 years ago

Yep, this happens in https://github.com/Konfekt/FastFold/blob/master/plugin/fastfold.vim#L37

Konfekt commented 8 years ago

Ok, but if w:lastfdm is cleared, then the actual &l:foldmethod is diff. So when diff mode stops, does Vim not set &l:foldmethod to that defined for the FileType and FastFold should notice by one of its many autocmds? If you could check that setlocal fdm? gives diff, and what happens for echo w:lastfdm and setlocal fdm? after diff mode entered.

lervag commented 8 years ago

Ah, yes, exactly. That must be the problem. It seems this also happens for b:lastfdm.

Btw: Could I propose that you write the header in english instead of french? Not important, but I think english is accessible to more people.

lervag commented 8 years ago

No, &l:foldmethod is set to the previous foldmethod, i.e. manual.

lervag commented 8 years ago

And this is exactly why it is a problem. I think perhaps diff mode should be treated specially. This might be solved by checking the &diff option, which is local to the current window. E.g. if &diff is on, then I propose that FastFold not interfer, and that it preserves w:lastfdm and similar.

Konfekt commented 8 years ago

Yeah, ok, cleanest would be to set fdm to lastfdm before starting diff mode. Perhaps the OptionSet autocmd for diff allows to hook into it.

Konfekt commented 8 years ago

Oh well, I see no good autocmd for checking thay diffsplit or alike was issued. And then even without fast fold the value of fold method after twice :diffoff is empty. There's a workaround for fugitive diffs but in general that's something that fast fold has to create from scratch it seems. But how to hook into a diffoff command?

Konfekt commented 8 years ago

Ok, check out the latest commit. Works fine for diffsplit and diffoff but not for fugitive diffs.

Konfekt commented 8 years ago

Issue resolved by latest commit. Please check it out, and tell me if it's working for you.

lervag commented 8 years ago

Yes, this seems to fix the issue for me, both with fugitive and with lawrencium. Although, for some reason, I need to refresh the folds twice after I exit diffmode, both with fugitive and with lawrencium.

lervag commented 8 years ago

This seems to be because this part:

function! s:EnterWin()
  if exists('w:predifffdm')
    if empty(&l:foldmethod) || &l:foldmethod is# 'manual'
      let w:lastfdm = w:predifffdm
      setlocal foldmethod=manual
      return
    elseif &l:foldmethod isnot# 'diff'
      unlet w:predifffdm
    endif
  endif

needs to be evaluated before the s:LeaveWin function.

Konfekt commented 8 years ago

Should be fixed now.

lervag commented 8 years ago

Great! Very nice work! One final request, although I'm not sure if it is a good one: Would it be possible to automatically recalculate/refresh folds when exiting diff mode? It seems to me that this is one particular case where it is "natural" to autorefresh, else the diff folds will remain.

Alternatively, do you see a good way to solve this manually?

Konfekt commented 8 years ago

Ok. There is no trigger for catching :diffoff but if a split is closed, like after fugitive diff, or more generally the window or buffer switched, then now folds update automatically. Thanks for all the Suggestions and help.

lervag commented 8 years ago

Thanks! Now things work very nicely, I think. Great work!