Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix stops sorting function #5

Merged
merged 1 commit into from
Sep 13, 2021
Merged

Conversation

pbogut
Copy link
Contributor

@pbogut pbogut commented Sep 13, 2021

Fix for stops sorting function. In some cases Lua is complaining that the function is invalid.

Fixed issues:

  • is_before and is_after functions were exactly the same, is_after is not used anywere as far as I can tell, but still.
  • above functions were also broken, as they were basically just checking if line is lower, if not, it was checking if column of first step is the same as column of the same step, which was always true, and then if line was lower (same check as at beginning).
  • sort_step itself was also wrong as, for my snippet it generated steps like this:
    s1:{
      endpos = { 3, 7 },
      id = 4,
      startpos = { 3, 3 },
      type = "placeholder"
    }
    s2:{
      endpos = { 4, 1 },
      id = 3,
      placeholder = "",
      startpos = { 4, 1 },
      type = "tabstop"
    }
    
    Given previous logic sorting function was returning true when (s1, s2) was passed because s1 (line 3) is before s2 (line 4). It was also true when (s2, s1) was called as s2.id < s1.id. Lua apparently don't like that behaviour and is throwing exception "invalid order function for sorting".

Snippet that was causing me the issues (stripped down version):

${1:var1}
${2:var2} ${3:var3}
$2 $3 $3 ${4:var4}
$3 $3 $4

@dcampos
Copy link
Owner

dcampos commented Sep 13, 2021

Great! Thank you for looking into this and providing the fix. I'll see if I can add the snippet you provided as a test case later, when I get around to improving the tests.

@dcampos dcampos merged commit ece921e into dcampos:master Sep 13, 2021
@pbogut
Copy link
Contributor Author

pbogut commented Sep 14, 2021

I was looking into adding tests myself, but I'm not familiar with lua / vim testing and simply didn't know how to start tests locally to play around with them. If you could point me at some documentation of stuff you using here I would appreciate it.

@dcampos
Copy link
Owner

dcampos commented Sep 14, 2021

If you could point me at some documentation of stuff you using here I would appreciate it.

Of course. I wrote the (functional) tests following this guide: https://github.com/KillTheMule/KillTheMule.github.io/blob/master/test_plugins_from_neovim.md

Also, feel free to use the existing tests as a starting point. Running the functional tests locally is a matter of cloning neovim and vim-snippets on the same level as snippy, like so:

neovim
nvim-snippy
vim-snippets

Then cd neovim and run TEST_FILE=../nvim-snippy/test/snippy_spec.lua make functionaltest.

There are also some unit tests for parser stuff, which I'm planning to expand to cover other areas of the code eventually.

dcampos added a commit that referenced this pull request Oct 4, 2021
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