lervag / vimtex

VimTeX: A modern Vim and neovim filetype plugin for LaTeX files.
MIT License
5.45k stars 388 forks source link

folding newenvironment #604

Closed Konfekt closed 7 years ago

Konfekt commented 7 years ago

With vimtex expression folding, the command in the preamble of a TeX file

    \newenvironment{ccd}
      {\begin{centered}\begin{tikzcd}}
      {\end{tikzcd}\end{centered}}

causes everything below up to

\begin{document}

to be folded.

Expected: Nothing, or perhaps the two lines below \newenvironment{ccd} are folded.

lervag commented 7 years ago

Yes, see :h g:vimtex_fold_newcommands. This was added quite recently, but it will also be exchanged with an improved command folding feature soon, see #597.

You can either

  1. Move the last closing brace to the next line with the same indent level as the opening part, or
  2. let g:vimtex_fold_newcommands = 0.
Konfekt commented 7 years ago

Ok, still there seems little reason why it should fold up to begin document when the last brace is not on a new line.

lervag commented 7 years ago

Perhaps you're right. Since I've already stated that the fold depends on the indentation, it doesn't really need to consider the ending brace.

I'll update when I get the time.

lervag commented 7 years ago

As I thought, it is simple to allow the fold to end at the last indented line. However, the current implementation does not combine fold regions. That is, the provided example will not work, because the \begin{center}....\end{center} is folded as well. This particular case is actually not that trivial, and so I am very tempted to insist on the current implementation that demands the ending brace. That way, I think the folding should be consistent and work well with no updates.

If I were to change this, then I would have to add rules for how to combine the additions/subtractions of fold levels. I think that looks quite difficult, at least without a lot of rewriting. I hope you can accept the current version, even though you have to either disable that fold type, or change your way of writing the commands...

Konfekt commented 7 years ago

Ok. Then it should be off by default.

lervag commented 7 years ago

@kiryph What do you think: Do you agree with @Konfekt to remove folding of \newcommands and similar (the single_opt types) by default? I'm personally not quite sure, as I find it pretty convenient. But at the same time, I think it may be the right thing to do if it leads to unexpected behaviour for people.

kiryph commented 7 years ago

I think folding dependent on indentation would be a separate feature. This is not what command folding tries to implement (:h vimtex_fold_commands).

In this particular case raised by @Konfekt, I would propose to be more strict with the regular expression matching the start of 'multi-commands' such as newenvironment.

I've modified the regex in following diff. Now it is mandatory that there is an opening square or curly brace which is followed immediately only by whitespace and/or a comment otherwise it simply is not folded:

❯ git diff
diff --git a/autoload/vimtex/fold.vim b/autoload/vimtex/fold.vim
index ec7c729..bd314ce 100644
--- a/autoload/vimtex/fold.vim
+++ b/autoload/vimtex/fold.vim
@@ -442,7 +442,7 @@ function! s:cmd_multi(cmds) " {{{1

   let l:fold = {}
   let l:fold.re = {
-        \ 'start' : l:re,
+        \ 'start' : l:re.'.*(\{|\[)\s*(\%.*)?$',
         \ 'end' : '^\s*}\s*$',
         \ 'text' : l:re . '\{[^}]*\}'
         \}

Following MWE

\documentclass{article}

\newenvironment{ccd}{%
  \begin{centered}\begin{tikzcd}}
  {\end{tikzcd}\end{centered}%
}

\newenvironment{ccd}
  {\begin{centered}\begin{tikzcd}}
  {\end{tikzcd}\end{centered}}

% this should not be folded.
\begin{document}

\end{document}

folds to demo5 which I think resolves the raised issue.

Update I've noticed that I haven't read the full description in the helpfile I have referenced myself 😞 . @lervag you mentioned that folding for 'multi' is based on indentation. However, I think this is in contrast to the requirement, that there should be a single closing brace on a separate line. Together with my diff, I would definitely rephrase it to something about unmatched opening brace on start line and a single closing brace on a separate closing line. As I said folding based on indentation would be a separate feature. This is possibly not a bad idea, but not what I want. E.g. in python it would be an option to use indentation based folding however I want to have meaningful foldtexts and that is difficult to implement based purely on indentation.

If the environment centered should also not be folded, this would be another issue.

lervag commented 7 years ago

Sorry for taking so long! It's been a very busy life (and all that jazz), and I've tried to keep the issue count low and therefore prioritized "simpler" issues as they are raised.

So, back to this. I think you are right that I can solve this by improving the start regex and then closing on the last remaining ending brace. I've added a test based on your example and update the code.

@Konfekt Let me know if this fixes your issue.

Konfekt commented 7 years ago

Good work, skimming through a few TeX preambles, the folding seems just fine. 👍

lervag commented 7 years ago

Great! Thanks :)