Add syntax fold support

[breed808]

This uses the existing regions defined in the syntax file to provide fold support when foldmethod = syntax.

Before:

After (with top-level fold opened):

[lelutin]

nice! thanks for the contribution :)

would you be comfortable writing a test for the feature? this project is using vader.vim:

https://github.com/junegunn/vader.vim/blob/master/README.md#syntax-of-vader-file

otherwise, no problem. we can add the test on top of your work

[breed808]

I've amended the commit; is the basic test OK or would you like a more in-depth test as well?

[lelutin]

thanks for the added test, it's looking good :+1:

I wonder what just happened to travis.. it looks like it didn't run the tests.

it seems as though 6 tests break down with this change.

the added folding is present and behaves as expected, so that's good.

I've tried keeping the test but manually changing back the syntax file (e.g. removing "fold" on the three lines) and I got back to all tests passing, except for the one you sent -- which is normal if the change is not present. so this confirms that the break in the tests happens because of the added "fold" on the regions.

I presume that by adding the folding property to the regions, we're changing how vim behaves for some things and the assumptions built up in some of the other features of this plugin probably don't hold.

so to make it all work we'll have to fix up parts of the plugin that break up. I'll try figuring out just now if there's something obvious happening.

here's what I see when I run the tests:

$ ./test/run-tests.sh 
+ TEST_FILE=
+++ readlink -f ./test/run-tests.sh
++ dirname /home/gabster/.vim/bundle/vim-puppet/test/run-tests.sh
+ SCRIPT_FOLDER=/home/gabster/.vim/bundle/vim-puppet/test
+ '[' '' == vim ']'
+ '[' '' == nvim ']'
++ command -v nvim
+ '[' -x '' ']'
++ command -v vim
+ '[' -x /usr/bin/vim ']'
+ RUNVIM=vim
+ cd /home/gabster/.vim/bundle/vim-puppet/test/..
+ '[' '!' -d vader.vim ']'
+ '[' -z ']'
+ TEST_SUITE='test/**/*.vader'
+ vim -u test/init.vim -c 'Vader! test/**/*.vader'
Vim: Warning: Output is not to a terminal
Vader note: cannot print to stderr reliably/directly.  Please consider using Vim's -es/-Es option (mode=n).
VIM - Vi IMproved 8.2 (2019 Dec 12, compiled Jan 17 2021 15:53:54)
Included patches: 1-2367
Modified by team+vim@tracker.debian.org
Compiled by team+vim@tracker.debian.org
Huge version without GUI.  Features included (+) or not (-):
+acl               -farsi             +mouse_sgr         +tag_binary
+arabic            +file_in_path      -mouse_sysmouse    -tag_old_static
+autocmd           +find_in_path      +mouse_urxvt       -tag_any_white
+autochdir         +float             +mouse_xterm       +tcl
-autoservername    +folding           +multi_byte        +termguicolors
-balloon_eval      -footer            +multi_lang        +terminal
+balloon_eval_term +fork()            -mzscheme          +terminfo
-browse            +gettext           +netbeans_intg     +termresponse
++builtin_terms    -hangul_input      +num64             +textobjects
+byte_offset       +iconv             +packages          +textprop
+channel           +insert_expand     +path_extra        +timers
+cindent           +ipv6              +perl              +title
-clientserver      +job               +persistent_undo   -toolbar
-clipboard         +jumplist          +popupwin          +user_commands
+cmdline_compl     +keymap            +postscript        +vartabs
+cmdline_hist      +lambda            +printer           +vertsplit
+cmdline_info      +langmap           +profile           +virtualedit
+comments          +libcall           -python            +visual
+conceal           +linebreak         +python3           +visualextra
+cryptv            +lispindent        +quickfix          +viminfo
+cscope            +listcmds          +reltime           +vreplace
+cursorbind        +localmap          +rightleft         +wildignore
+cursorshape       +lua               +ruby              +wildmenu
+dialog_con        +menu              +scrollbind        +windows
+diff              +mksession         +signs             +writebackup
+digraphs          +modify_fname      +smartindent       -X11
-dnd               +mouse             +sound             -xfontset
-ebcdic            -mouseshape        +spell             -xim
+emacs_tags        +mouse_dec         +startuptime       -xpm
+eval              +mouse_gpm         +statusline        -xsmp
+ex_extra          -mouse_jsbterm     -sun_workshop      -xterm_clipboard
+extra_search      +mouse_netterm     +syntax            -xterm_save
   system vimrc file: "$VIM/vimrc"
     user vimrc file: "$HOME/.vimrc"
 2nd user vimrc file: "~/.vim/vimrc"
      user exrc file: "$HOME/.exrc"
       defaults file: "$VIMRUNTIME/defaults.vim"
  fall-back for $VIM: "/usr/share/vim"
Compilation: gcc -c -I. -Iproto -DHAVE_CONFIG_H -Wdate-time -g -O2 -ffile-prefix-map=/build/vim-UdUp9z/vim-8.2.2367=. -fstack-protector-strong -Wformat -Werror=format-security -D_REENTRANT -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1 
Linking: gcc -L. -Wl,-z,relro -Wl,-z,now -fstack-protector-strong -rdynamic -Wl,-export-dynamic -Wl,-E -Wl,-z,relro -Wl,-z,now -Wl,--as-needed -o vim -lm -ltinfo -lselinux -lcanberra -lacl -lattr -lgpm -ldl -L/usr/lib -llua5.2 -Wl,-E -fstack-protector-strong -L/usr/local/lib -L/usr/lib/x86_64-linux-gnu/perl/5.32/CORE -lperl -ldl -lm -lpthread -lcrypt -L/usr/lib/python3.9/config-3.9-x86_64-linux-gnu -lpython3.9 -lcrypt -lpthread -ldl -lutil -lm -lm -L/usr/lib/x86_64-linux-gnu -ltcl8.6 -ldl -lz -lpthread -lm -lruby-2.7 -lm -L/usr/lib 

Starting Vader: 14 suite(s), 38 case(s)
  Starting Vader: /home/gabster/.vim/bundle/vim-puppet/test/format/hashrocket.vader
    (1/2) [  GIVEN] simple resource with autoformat while writing
    (1/2) [     DO] format resource
    (1/2) [ EXPECT] (X) formated resource
      - Expected:
          file { 'foo':
            ensure => present,
            force  => true,
          }
      - Got:
          file { 'foo':
            ensure => present,
          }
    (2/2) [  GIVEN] simple resource with gq
    (2/2) [     DO] format resource
    (2/2) [ EXPECT] formated resource
  Success/Total: 1/2
  Starting Vader: /home/gabster/.vim/bundle/vim-puppet/test/format/textwidth.vader
    (1/2) [  GIVEN] long line
    (1/2) [     DO] format all text
    (1/2) [ EXPECT] nothing changed
    (2/2) [  GIVEN] long line
    (2/2) [     DO] format all text with textwidth set
    (2/2) [ EXPECT] comment is wrapped into more lines
  Success/Total: 2/2
  Starting Vader: /home/gabster/.vim/bundle/vim-puppet/test/indent/basic.vader
    (1/3) [  GIVEN] class with includes
    (1/3) [     DO] full text indent with '='
    (1/3) [ EXPECT] indented class
    (2/3) [  GIVEN] class with includes
    (2/3) [     DO] full text indent with 'gq' in Visual Mode
    (2/3) [ EXPECT] indented class
    (3/3) [  GIVEN] class with includes and resources
    (3/3) [     DO] full text indent by gq with motion
    (3/3) [ EXPECT] (X) indented class
      - Expected:
          class foo::boo {
            file { 'boo':
              ensure => present,
            }
            include foo::moo
            include foo::moo::boo
          }
      - Got:
          class foo::boo {
          file { 'boo':
          ensure => present,
          }
          include foo::moo
          include foo::moo::boo
          }
  Success/Total: 2/3
  Starting Vader: /home/gabster/.vim/bundle/vim-puppet/test/indent/comments_strings.vader
    (1/4) [  GIVEN] comments before closing brace
    (1/4) [     DO] full text indent with '='
    (1/4) [ EXPECT] closing brace back aligned with start of resource block
    (2/4) [  GIVEN] closing brace after comment that contains closing brace
    (2/4) [     DO] full text indent with '='
    (2/4) [ EXPECT] closing brace back aligned with start of resource block
    (3/4) [  GIVEN] multi-line string
    (3/4) [     DO] full text indent with '='
    (3/4) [ EXPECT] (X) multi-line string does not move and line after resumes indentation
      - Expected:
          class something {
            $var="
          This text on new line
           should not move
          "
            this::defined_type { 'foo':
              content => $var,
            }
          }
      - Got:
          class something {
            $var="
            This text on new line
            should not move
          "
                this::defined_type { 'foo':
          content => $var,
          }
              }
    (4/4) [  GIVEN] square bracket and hash sign
    (4/4) [     DO] New line
    (4/4) [ EXPECT] indent stays the same
  Success/Total: 3/4
  Starting Vader: /home/gabster/.vim/bundle/vim-puppet/test/indent/include.vader
    (1/3) [  GIVEN] include of multiple classes over multiple lines
    (1/3) [     DO] full text indent with '='
    (1/3) [ EXPECT] indented include
    (2/3) [  GIVEN] resource with param named include
    (2/3) [     DO] full text indent with '='
    (2/3) [ EXPECT] (X) properly aligned parameters
      - Expected:
          some::resouce { 'namehere':
            include => ['s1', 's2'],
            otherparam => 'value',
      - Got:

    (3/3) [  GIVEN] resource with param named include contained in class
    (3/3) [     DO] full text indent with '='
    (3/3) [ EXPECT] (X) second contained resource should align with the first one
      - Expected:
          class module::blah {
            some::resouce { 'namehere':
              include => ['s1', 's2'],
              otherparam => 'value',
            }
            following::resource { 'identifier':
      - Got:

  Success/Total: 1/3
  Starting Vader: /home/gabster/.vim/bundle/vim-puppet/test/syntax/array.vader
    (1/4) [  GIVEN] simple array
    (1/4) [EXECUTE] syntax is good
    (2/4) [  GIVEN] empty array
    (2/4) [EXECUTE] syntax is good
    (3/4) [  GIVEN] nested array
    (3/4) [EXECUTE] syntax is good
    (4/4) [  GIVEN] array with nested hash
    (4/4) [EXECUTE] syntax is good
  Success/Total: 4/4
  Starting Vader: /home/gabster/.vim/bundle/vim-puppet/test/syntax/boolean.vader
    (1/2) [  GIVEN] true
    (1/2) [EXECUTE] syntax is good
    (2/2) [  GIVEN] false
    (2/2) [EXECUTE] syntax is good
  Success/Total: 2/2
  Starting Vader: /home/gabster/.vim/bundle/vim-puppet/test/syntax/embeddedpuppet.vader
    (1/3) [  GIVEN] template with litteral content puppet tags
    (1/3) [EXECUTE] litteral content syntax must be correct
    (2/3) [  GIVEN] template with litteral content puppet tags
    (2/3) [EXECUTE] epp delimiter syntax must be correct
    (3/3) [  GIVEN] template with litteral content puppet tags
    (3/3) [EXECUTE] puppet syntax must be correct
  Success/Total: 3/3
  Starting Vader: /home/gabster/.vim/bundle/vim-puppet/test/syntax/fold.vader
    (1/1) [  GIVEN] basic class
    (1/1) [EXECUTE] fold level # in code block
  Success/Total: 1/1
  Starting Vader: /home/gabster/.vim/bundle/vim-puppet/test/syntax/number.vader
    (1/4) [  GIVEN] hexidecimal
    (1/4) [EXECUTE] syntax is good
    (2/4) [  GIVEN] integer
    (2/4) [EXECUTE] syntax is good
    (3/4) [  GIVEN] octal
    (3/4) [EXECUTE] syntax is good
    (4/4) [  GIVEN] floating point
    (4/4) [EXECUTE] syntax is good
  Success/Total: 4/4
  Starting Vader: /home/gabster/.vim/bundle/vim-puppet/test/syntax/regex.vader
    (1/1) [  GIVEN] division operation
    (1/1) [EXECUTE] syntax is good
  Success/Total: 1/1
  Starting Vader: /home/gabster/.vim/bundle/vim-puppet/test/syntax/strings.vader
    (1/2) [  GIVEN] heredoc with trailing comma on first line
    (1/2) [EXECUTE] syntax is undisturbed by trailing comma or apostrophe
    (1/2) [EXECUTE] (X) 'puppetString' should be equal to ''
    (2/2) [  GIVEN] heredoc with simple closing symbol
    (2/2) [EXECUTE] closing symbol properly ends syntax group
  Success/Total: 1/2
  Starting Vader: /home/gabster/.vim/bundle/vim-puppet/test/syntax/variable.vader
    (1/3) [  GIVEN] local variables
    (1/3) [EXECUTE] syntax is good
    (2/3) [  GIVEN] global scope variables
    (2/3) [EXECUTE] syntax is good
    (3/3) [  GIVEN] out of scope qualified name variables
    (3/3) [EXECUTE] syntax is good
  Success/Total: 3/3
  Starting Vader: /home/gabster/.vim/bundle/vim-puppet/test/syntax/variable_in_string.vader
    (1/4) [  GIVEN] interpolated top scope variable
    (1/4) [EXECUTE] syntax is good
    (2/4) [  GIVEN] local variable
    (2/4) [EXECUTE] syntax is good
    (3/4) [  GIVEN] unenclosed local variable
    (3/4) [EXECUTE] syntax is good
    (4/4) [  GIVEN] unenclosed top scope variable
    (4/4) [EXECUTE] syntax is good
  Success/Total: 4/4
Success/Total: 32/38 (assertions: 246/247)
Elapsed time: 1.598200 sec.
+ vader_exit=1
+ '[' -n '' ']'
+ exit 1

[lelutin]

so I've just tried manually doing all of the failed tests (e.g. copying the "Given" text in a buffer that's detected as puppet, then running the commands in "Do". and everything seems to be behaving as expected.

so apparently it's vader.vim that's breaking up with the presence of folds. I'm wondering if we need to disable something more in init.vim (the basic vimrc used to run the tests).

[lelutin]

if I remove just this one "fold", I'm back with all tests working but the new one -- removing any of the other two "fold" lines didn't have any effect on the test results (edit: except for breaking the new test as well):

diff --git a/syntax/puppet.vim b/syntax/puppet.vim
index b1e573d..f0411d7 100644
--- a/syntax/puppet.vim
+++ b/syntax/puppet.vim
@@ -29,7 +29,7 @@ syn match  puppetOperator "+=\|-=\|==\|!=\|=\~\|!\~\|>=\|<=\|<-\|<\~\|=>\|+>\|->
 syn match  puppetOperator "<<|\||>>"

 syn region puppetBracketOperator matchgroup=puppetDelimiter start="\[\s*" end="\s*]" fold contains=ALLBUT,@puppetNotTop
-syn region puppetBraceOperator matchgroup=puppetDelimiter start="{\s*" end="\s*}" fold contains=ALLBUT,@puppetNotTop
+syn region puppetBraceOperator matchgroup=puppetDelimiter start="{\s*" end="\s*}" contains=ALLBUT,@puppetNotTop
 syn region puppetParenOperator matchgroup=puppetDelimiter start="(\s*" end="\s*)" fold contains=ALLBUT,@puppetNotTop

 " Expression Substitution and Backslash Notation {{{1

so now we can try to identify why that line is causing us trouble

[lelutin]

found it! by adding this to test/init.vim, the tests behave as expected and they all pass. e.g.;

diff --git a/test/init.vim b/test/init.vim
index 3246d3e..dd77520 100644
--- a/test/init.vim
+++ b/test/init.vim
@@ -6,3 +6,6 @@ set rtp+=.
 set rtp+=after
 filetype plugin indent on
 syntax enable
+" Avoid closing up any fold since it results in some tests skipping lines and
+" their output ends up diverging from what we expect them to be.
+set foldlevel=99

by default foldlevel is set to 0, closing all folds. because of this some commands skip some text and the output is not the expected one.

I will add this up on top of your work and merge.

sorry if I was being noisy, but I was kind of walking blindly around the test issue.

[lelutin]

thanks for this new feature :)

[breed808]

No worries! I think the tests were passing successfully on my machine due to some local configuration differences, though the only relevant configuration entries in my Neovim init.vim are set foldenable and set foldmethod=syntax :man_shrugging:

Thanks for investigating and resolving the testing issue :+1: