Skip to content

Conversation

@lelutin
Copy link
Collaborator

@lelutin lelutin commented Jul 30, 2025

Those are all of the examples that match the reported indentation errors.

The very strange thing is that all of the new tests fail, but they fail differently in vader than how they do in an interactive (n)vim session. The result that we get in those tests is usually that the string contents get wrongly moved to fit indentation. However, when I run the indentation command in the editor, the string contents don't change but things afterwards get indented at the wrong level. I'm not sure why the tests don't behave in the same way.

…ation (rodjek#159)

Those are all of the examples that match the reported indentation
errors.

The very strange thing is that all of the new tests fail, but they fail
differently in vader than how they do in an interactive (n)vim session.
The result that we get in those tests is usually that the string
contents get wrongly moved to fit indentation. However, when I run the
indentation command in the editor, the string contents don't change but
things afterwards get indented at the wrong level. I'm not sure why the
tests don't behave in the same way.
@lelutin lelutin marked this pull request as draft July 30, 2025 20:38
@lelutin
Copy link
Collaborator Author

lelutin commented Jul 30, 2025

@alexjfisher fyi I've created new tests with the reproducing examples you reported in #159

like I described in the commit message vader.vim is failing on all of these, but not in the same way that we've both seen :( I wonder if I got something wrong. but if I run indentation with = on the test files I still see the same behavior that we both saw, not what the tests report as their result.

@alexjfisher
Copy link

For the life of me, I can't work out what's going on in the tests, and why it's different.

@alexjfisher
Copy link

Now I have even less of a clue as to what's going on.
Why would the following change the behaviour of the new tests??

--- a/indent/puppet.vim
+++ b/indent/puppet.vim
@@ -47,6 +47,7 @@ function! s:OpenBrace(lnum)
 endfunction

 function! s:InsideMultilineString(lnum)
+    let g:wtf = synID(line("$"), 1, 0)
     return synIDattr(synID(a:lnum, 1, 0), 'name') =~? 'string'
 endfunction

With this simple test:

Given puppet (indented heredoc):
 class blah {
   $value = @(HERE)
     Simple indented heredoc
     | HERE
 }

Do (full text indent with '='):
 gg=G

Expect puppet (should remain the same):
 class blah {
   $value = @(HERE)
     Simple indented heredoc
     | HERE
 }

without my seemingly pointless change...

Starting Vader: 1 suite(s), 1 case(s)
Starting Vader: /home/alex/test-vim-puppet/vim-puppet/test/indent/indented_heredoc.vader
  (1/1) [  GIVEN] indented heredoc
  (1/1) [     DO] full text indent with '='
  (1/1) [ EXPECT] (X) should remain the same
    - Expected:
        class blah {
          $value = @(HERE)
            Simple indented heredoc
            | HERE
        }
    - Got:
        class blah {
          $value = @(HERE)
          Simple indented heredoc
          | HERE
        }
Success/Total: 0/1
Success/Total: 0/1 (assertions: 0/0)

But with the dumb patch

Starting Vader: 1 suite(s), 1 case(s)
  Starting Vader: /home/alex/test-vim-puppet/vim-puppet/test/indent/indented_heredoc.vader
    (1/1) [  GIVEN] indented heredoc
    (1/1) [     DO] full text indent with '='
    (1/1) [ EXPECT] should remain the same
  Success/Total: 1/1
Success/Total: 1/1 (assertions: 0/0)

@lelutin
Copy link
Collaborator Author

lelutin commented Aug 3, 2025

wat.. I'm thouroughly confused as to what the unit test plugin is doing here :(

it's too bad since it was working fine for most other tests. also from what I've seen so far, it's pretty much the only one that lets us test what (n)vim is doing when running actual commands on a test buffer.

I've also tried to load nvim with the test vimrc file with nvim -u test/init.vim and doing the indentation manually with gg=G on a copy of one of our new tests and I'm still getting the behaviour we've both observed, not the one that's displayed by vader.vim.

we may have stumbled upon a vader.vim bug there

@alexjfisher
Copy link

I think it's just a race condition. The syntax hasn't been evaluated by the time gg=G is run.

lelutin and others added 2 commits August 4, 2025 09:34
Try and wait for syntax to be fully evaluated before running the indexing command

Co-authored-by: Alexander Fisher <[email protected]>
Things really did change with the addition of the wait for the syntax.
Apparently as @alexjfisher said, the format function needs its calls to
get the correct syntax ID so we need the syntax file to be fully loaded
before trying to format things.
@lelutin
Copy link
Collaborator Author

lelutin commented Aug 4, 2025

@alexjfisher nice that seems to help, thanks! and your reasoning for the need to call that sync to wait for syntax is sound. it's been a long while since I touched the code in this module.. I've implemented the same thing on all tests.

now the new mystery is ... well all of the tests are now passing which is not what we were expecting so far: the tests are not replicating the odd removals of spaces on empty lines and the odd indentation after a string with an empty line 🤔

Note: the vim 8 tests that fail seem to be unrelated to this MR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants