Include option parameters with environments in folding

[gjgress]

I added an option variable in case down the road it is deemed worthwhile to create a more robust parse_option() function, or get options for frames/tables. I also did it this way so that way the options from tables and frames wouldn't be included (as they are often parameters and hence not pertinent information in a fold).

I chose to use a lookahead and add a space between the environment for aesthetic purposes. Hence,

\begin{defn}[Holomorphic]
Lorem Ipsum
\end{defn}

will be folded as

\begin{defn} Holomorphic ----------------

If instead you prefer

\begin{defn}[Holomorphic] ---------------

then replace the command on line 79 from

let option = matchstr(a:line, '\[\zs.*\ze\]')

to

let option = matchstr(a:line, '\[.*\]')

[gjgress]

Some other considerations:

Reading the main branch code, I realized there is already functionality for labels and captions.

• One could argue that this isn't necessary if you use labeling (which you should, I suppose). However, the labelling is usually less nicely formatted than the options, so I still think this is worth implementing
• Currently, the code captures comments made after the environment, i.e. \begin{defn} % Example Comment. This is pretty useful, but currently incompatible with the current implementation. As written, the comments are only captured if no options are given.
  • The regex should probably be rewritten to at least allow options to be given, but still ignored. Should be simple, I can give it a shot tomorrow if you think this is a good idea
  • I'm not sure whether options AND comments should both be displayed. On my screen, there's plenty of space, but including both options and comments might fill up the fold bar pretty quickly. If you think both should be displayed-- let me know and I can rewrite the comments regex

Finally, I'm not familiar with all the spacing stuff with regards to width_env, width_label, etc... I understand their purpose, but I didn't want to change anything because frankly, I'd be more likely to mess up the folding spacing. There might be an issue if your options are long and don't get truncated... I'll play around with it and see what happens

[lervag]

First, for future self and others, see #2083 for more context regarding this PR.

Sorry for the delay in replying.

I added an option variable in case down the road it is deemed worthwhile to create a more robust parse_option() function, or get options for frames/tables. I also did it this way so that way the options from tables and frames wouldn't be included (as they are often parameters and hence not pertinent information in a fold).

Sounds good!

I chose to use a lookahead and add a space between the environment for aesthetic purposes.

That seems nice in some cases, but not so good in others. I would prefer not to have this space and the aesthetic alteration, because in many cases, the environment options are really options, not a title string or similar. E.g. \begin{figure}[htpb] or \begin{axis}[xwidth=12cm,...].

Reading the main branch code, I realized there is already functionality for labels and captions.

Yes; I believe options and labels can coexist, but if there are captions, I think we should choose between the caption text and the option group. Either that, or we need to adjust the widths accordingly.

• Currently, the code captures comments made after the environment, i.e. \begin{defn} % Example Comment. This is pretty useful, but currently incompatible with the current implementation. As written, the comments are only captured if no options are given.

I agree with your choice here.

• I'm not sure whether options AND comments should both be displayed. On my screen, there's plenty of space, but including both options and comments might fill up the fold bar pretty quickly. If you think both should be displayed-- let me know and I can rewrite the comments regex

This seems like a similar comment as above - I think it is sufficient to show either options or comments, not both.

• The regex should probably be rewritten to at least allow options to be given, but still ignored. Should be simple, I can give it a shot tomorrow if you think this is a good idea

Not quite sure what you mean here. If you give it a shot, I might better understand by looking at the code.

Finally, I'm not familiar with all the spacing stuff with regards to width_env, width_label, etc... I understand their purpose, but I didn't want to change anything because frankly, I'd be more likely to mess up the folding spacing. There might be an issue if your options are long and don't get truncated... I'll play around with it and see what happens

Sounds good. I can also probably make some adjustments if I find it necessary.

One more thing which would be good here is to provide an example document similar to what you added in #2083 for testing. It can be nice to add some tests under test/test-folding/. I can do this, as it requires some adjustment of the Makefile, but if you could add the example file under e.g. test/test-folding/test-env-options.tex which shows several variations and perhaps adds a comment above/below to describe what you expect the foldtext to look like?

[gjgress]

That seems nice in some cases, but not so good in others. I would prefer not to have this space and the aesthetic alteration, because in many cases, the environment options are really options, not a title string or similar. E.g. \begin{figure}[htpb] or \begin{axis}[xwidth=12cm,...].

Makes sense. I have fixed it again, I'll send in the commit soon.

Yes; I believe options and labels can coexist, but if there are captions, I think we should choose between the caption text and the option group. Either that, or we need to adjust the widths accordingly.

I've been giving this some thought... Captions are very limited in their use-case. It is pretty much exclusively used for figures and tables. And in both of those cases, the options are not really worth displaying.

I would have to test it, but I bet we can simply include the 'figure' environment with the 'table' option. That is,

...
  elseif env ==# 'table' || env ==# 'figure'
    let option = ''
    let label = self.parse_label()
    let caption = self.parse_caption_table(a:line)
  else ...

Then the chance that there is a option/caption fold conflict would be very slim. Let me know if you like this idea, and I'll run it that way instead.

One more thing which would be good here is to provide an example document similar to what you added in #2083 for testing. It can be nice to add some tests under test/test-folding/. I can do this, as it requires some adjustment of the Makefile, but if you could add the example file under e.g. test/test-folding/test-env-options.tex which shows several variations and perhaps adds a comment above/below to describe what you expect the foldtext to look like?

Will do. Once I hear back from you in regards to the above changes, I'll implement these fixes and create the test files. Let me know if there's anything else you'd like done before pulling in the commits.

[lervag]

...
Then the chance that there is a option/caption fold conflict would be very slim. Let me know if you like this idea, and I'll run it that way instead.

Seems good, except I would not be surprised if there are other environments that also have captions. However, let's not aim for perfect. If someone notices this and wants that feature, it is trivial to add it in a similar way. So let's run with this idea.

Will do. Once I hear back from you in regards to the above changes, I'll implement these fixes and create the test files. Let me know if there's anything else you'd like done before pulling in the commits.

Great! I can't think of anything more right now, but of course, I'll let you know if I have thoughts. Also, I'll try to reply and review the changes faster. I'm probably active once every evening (UTC+2) the next week.

[gjgress]

Done. Although, in the process of adding a few examples, I stumbled across a bug...

Turns out that the functions that are used to search for and parse 'labels' and 'captions' scan subfolds. Worse, even if the main fold has labels and captions, it will be overwritten by the subfolds' labels and captions. For example:

\begin{thm}[Theorem 1]
\label{thm:theorem_1}
\caption{theorem 1 caption}
...
\begin{figure}
\label{fig:figure_1}
...
\caption{figure 1 caption}
\end{figure}
...
\end{thm}

is currently folded as:

\begin{thm}[Theorem 1]     figure 1 caption                                             (fig:figure_1)

instead of the proper folding:

\begin{thm}[Theorem 1]     theorem 1 caption                                         (thm:theorem_1)

It has to do with the function's search method here:

vimtex/autoload/vimtex/fold/envs.vim

Lines 118 to 142 in a064c48

	function! s:folder.parse_label() abort dict " {{{1
	let i = v:foldend
	while i >= v:foldstart
	if getline(i) =~# '^\s*\\label'
	return matchstr(getline(i), '^\s*\\label\%(\[.*\]\)\?{\zs.*\ze}')
	end
	let i -= 1
	endwhile
	return ''
	endfunction
	" }}}1
	function! s:folder.parse_caption(line) abort dict " {{{1
	let i = v:foldend
	while i >= v:foldstart
	if getline(i) =~# '^\s*\\caption'
	return matchstr(getline(i),
	\ '^\s*\\caption\(\[.*\]\)\?{\zs.\{-1,}\ze\(}\s*\)\?$')
	end
	let i -= 1
	endwhile
	" If no caption found, check for a caption comment
	return matchstr(a:line,'\\begin\*\?{.*}\s*%\s*\zs.*')
	endfunction

I might be able to investigate/fix this later, but I do not have time at the moment. I can also open this as a new issue if that is preferred.

[lervag]

Done. Although, in the process of adding a few examples, I stumbled across a bug...

Haha... the story of my (VimTeX) life... :p

Turns out that the functions that are used to search for and parse 'labels' and 'captions' scan subfolds. Worse, even if the main fold has labels and captions, it will be overwritten by the subfolds' labels and captions. For example: ...

Yes, this is true. I've partly known of this, but never had a reason to fix it myself. The fix is probably nontrivial, but of course, not impossible. It makes it harder that the \label and \caption can often come at different places in an environment (e.g. top or bottom).

I might be able to investigate/fix this later, but I do not have time at the moment. I can also open this as a new issue if that is preferred.

I would propose to open a new issue for this and then either I can try to fix it, or if you have time and you want to try, you can open a new PR. This means we can focus on the present topic in this PR and get it resolved faster. Is that OK by you?

[gjgress]

I would propose to open a new issue for this and then either I can try to fix it, or if you have time and you want to try, you can open a new PR. This means we can focus on the present topic in this PR and get it resolved faster. Is that OK by you?

Done, created as issue #2117 . I may try and tackle it myself later in a new PR, but I have other work I must prioritize at the moment.

In that case, I've done all that I can for this PR. I think the modifications to the make file in the test-folding folder still need done, but I'll let you manage that.

Thanks for working with me to resolve this!

[lervag]

Ok, I've merged this now. I've also improved the output format and added tests. There is one failing test due to #2117 which is commented out. That test should pass after we fix the issue.

Let me know what you think!

[gjgress]

Nice touch with the rewriting of the folding spacing stuff, it's much easier to understand.

Happy with how this turned out! Won't be able to pull the latest branch for a few days, but I'm looking forward to trying it out.