Jump to previous/next math zone

[ghost]

Fixes #1810.

[lervag]

I've made some relatively minor comments to the code. As a general comment: Good work! I think you are quite close.

I think it would be very good to add some tests. I think it should not be so hard. Something like this:

1. Create the file test/tests/test-motions/test-math-motions.tex and add example content.
2. Create the file test/tests/test-motions/test-math-motions.vim and write tests similar to this.

It should suffice, and you should be able to run the tests with make (runs all) or make test-math-motions. It will use neovim by default, if you want to run tests with Vim, you can do it with MYVIM="vim -T dumb --not-a-term -n" make ....

[ghost]

@lervag I have hit an issue with the following tex source

\begin{equation}
    1 + 1 = 2
\end{equation}

\( 1 + 1 = 2 \)

\[
 1 + 1 = 2
\]

Notation used to represent position: (line, col)

• Goto the beginning of the source and press ]N it correctly takes you to (3,1)
• Press ]N again and instead of jumping to \) on (5,14) it jumps to the \] on (9,1)
• If it works correctly, move the cursor around on the same line (3,1) and try jumping again.
• I think the problem is with synstack call in function vimtex#syntax#stack. When a call is made to get the syntax items of the position (5,12) (two columns left to the \) on (5,14), it returns an empty list, however when I call synstack manually for (5,12) it returns an integer.

Here's an asciinema showing the same.

P.S. I used the following modified version vimtex#syntax#stack to print the synstack and I have checked the behavior is the same on both vim and neovim.

function! vimtex#syntax#stack(...) abort " {{{1
  let l:pos = a:0 > 0 ? [a:1, a:2] : [line('.'), col('.')]
  if mode() ==# 'i'
    let l:pos[1] -= 1
  endif
  call map(l:pos, 'max([v:val, 1])')
  if a:0 > 0
      echo "In syntax#stack" l:pos synstack(l:pos[0], l:pos[1])
  endif
  return map(synstack(l:pos[0], l:pos[1]), "synIDattr(v:val, 'name')")
endfunction

[lervag]

@lervag I have hit an issue with the following tex source ...

• I think the problem is with synstack call in function vimtex#syntax#stack. When a call is made to get the syntax items of the position (5,12) (two columns left to the \) on (5,14), it returns an empty list, however when I call synstack manually for (5,12) it returns an integer.

I'll look into it!

I've also made a couple of very minor, pedantic comments (sorry!).

[lervag]

If the current "match" is any of \] and \) (or similar for beginnings), then we don't need to check the syntax. The syntax check is only necessary for $ and \end. This should fix the issue you mention, or, at least the exact issue. The same problem may still be present, but this is exactly why I think it is necessary/useful to add the tests.

[ghost]

If the current "match" is any of \] and \) (or similar for beginnings), then we don't need to check the syntax. The syntax check is only necessary for $ and \end. This should fix the issue you mention, or, at least the exact issue.
The same problem may still be present, but this is exactly why I think it is necessary/useful to add the tests.

Yes, I was thinking the same. While writing the tests I realized that \], \) and \end are easy to handle. '$' and '$$' on the other hand create some corner cases, which arise mainly because we need to check if previous positions are in math zone to find the ending delimiter. Consider the following text for example (line numbers not part of the text)

1 $\alpha + \beta$
2 $\gamma$

If the cursor is on the last column of line 2 (ending delimiter), then [N should take us to the last column on line 1 (ending delimiter). But what happens is that the search takes us to the first column of line 2 (opening delimiter), for which the previous-previous position is (1,15) and it is in math zone. The function then sends us incorrectly to line 2 column 1. In this case then we need to check first if the current search position is an opening delimiter itself and skip if it is.

Another example

1 $\alpha + \beta$
2
3 $$\gamma$$

In this case if the cursor is on the last column of line 3 (right delimiter), the search takes us one column left because $$ itself matches and then the previous-previous position is in math zone. To handle this case then we also need to check that we have at least moved more than one column left or otherwise continue searching.

I have pushed a commit, where I am handling the edge cases by using regex subgroups and a branching logic, and I think I have done it in a crude way.

If there's a better way of handling these cases, or if there are other edge cases you can think of, please let me know.

[ghost]

@lervag I have added tests as you suggested. When I check them manually, they run fine but when I run make test-math-motions, the jump doesn't happen and the assertion fails. I have checked that my mappings exist while the test runs.

[lervag]

Yes, I was thinking the same. While writing the tests I realized that \], \) and \end are easy to handle. '$' and '$$' on the other hand create some corner cases, ...

Yes, I expected these. These have been head scratchers in many other cases as well. I'm glad to see you've figured out a good way to solve it!

I have pushed a commit, where I am handling the edge cases by using regex subgroups and a branching logic, and I think I have done it in a crude way.

I appreciate the use of the p flag for search(), I've never used that before my self! Neat! I don't immediately see any important ways to improve this, except one: The motion does not properly presever the last position in the jump list. That is, as it is now, doing e.g. 3]n, then <c-o>, you may notice that it does not return to the original position. This can be easily fixed by adding ``normal! m``` in the beginning of the function, e.g. after the if a:visual block.

@lervag I have added tests as you suggested. When I check them manually, they run fine but when I run make test-math-motions, the jump doesn't happen and the assertion fails. I have checked that my mappings exist while the test runs.

The solution is obvious when you see it: this feature requires syntax on. So, simply add syntax on after filetype plugin on in the test file. :)

This leaves two minor fixes as mentioned above, and then finally the documentation:

• Add motion to feature list in README.md.
• Add motion to the corresponding list in doc/vimtex.txt.
• Add the mappings to the default map list and to the "map definitions" section (just search for ]m to see how it is documented and use the same format).

[ghost]

I appreciate the use of the p flag for search(), I've never used that before my self! Neat!

Thanks a lot.

I don't immediately see any important ways to improve this, except one: The motion does not properly presever the last position in the jump list. That is, as it is now, doing e.g. 3]n, then <c-o>, you may notice that it does not return to the original position. This can be easily fixed by adding ``normal! m``` in the beginning of the function, e.g. after the if a:visual block.

Fixed.

The solution is obvious when you see it: this feature requires syntax on. So, simply add syntax on after filetype plugin on in the test file. :)

Aah, I should have realized that. Fixed.

• Add motion to feature list in README.md.
• Add motion to the corresponding list in doc/vimtex.txt.
• Add the mappings to the default map list and to the "map definitions" section (just search for ]m to see how it is documented and use the same format).

Please let me know if you want me to make anymore changes.

I am doing some fine tuning, will let you know once I think it is ready to be merged.

:)

[lervag]

I don't immediately see any important ways to improve this, except one: The motion does not properly presever the last position in the jump list. That is, as it is now, doing e.g. 3]n, then <c-o>, you may notice that it does not return to the original position. This can be easily fixed by adding ``normal! m``` in the beginning of the function, e.g. after the if a:visual block.

Fixed.

Did you fix it in a different manner, perhaps?

Please let me know if you want me to make anymore changes.

No, this looks good. Thanks!

I am doing some fine tuning, will let you know once I think it is ready to be merged.

Sounds good.

[ghost]

@lervag I have fixed a regex bug that I had introduced while changing the implementation.

I have also changed the behavior of the motion when the count is larger than the total available math zones. Earlier, the motion didn't occur unless we find the count number of math zones. I have modified it to jump to the last available math zone, which is the same as the behavior of ]m and similar motions. Also, the for loop exists the first time the while loop fails, to prevent unexpected jumps when the count is larger than one.

I have checked the motions on some of my latex files as well, and I think it is ready to be merged now. Please let me know what you think of it.

[lervag]

@lervag I have fixed a regex bug that I had introduced while changing the implementation.

I have also changed the behavior of the motion when the count is larger than the total available math zones. Earlier, the motion didn't occur unless we find the count number of math zones. I have modified it to jump to the last available math zone, which is the same as the behavior of ]m and similar motions. Also, the for loop exists the first time the while loop fails, to prevent unexpected jumps when the count is larger than one.

I have checked the motions on some of my latex files as well, and I think it is ready to be merged now. Please let me know what you think of it.

Thanks! I've now merged this. It seems to work well also in my tests, and at least it should not break anything.

I've made some minor modifications/changes, nothing essential. Again, thank you very much for this, it seems to be a useful feature and your work is of high quality! Very well done!

[ghost]

Thanks! I've now merged this. It seems to work well also in my tests, and at least it should not break anything.

I've made some minor modifications/changes, nothing essential. Again, thank you very much for this, it seems to be a useful feature and your work is of high quality! Very well done!

Your modifications are indeed improvements on what I wrote. Thanks for helping me in adding the feature. 👍

[lervag]

My pleasure :)