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

Recursive stackop #15

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 25 additions & 1 deletion autoload/sexp.vim
Original file line number Diff line number Diff line change
Expand Up @@ -1558,6 +1558,9 @@ function! sexp#stackop(mode, last, capture)
if a:mode ==? 'v'
execute "normal! \<Esc>"
let marks = s:get_visual_marks()
else
silent! normal! ix
silent! normal! x
endif

" Move to element tail first so we can skip leading macro chars
Expand All @@ -1581,18 +1584,39 @@ function! sexp#stackop(mode, last, capture)

if !(a:capture ? s:stackop_capture(a:last, pos, bpos)
\ : s:stackop_emit(a:last, pos, bpos))
throw 'sexp-error'
throw 'sexp-noop-error'
endif

if a:mode ==? 'v'
call sexp#select_current_element('n', 1)
else
let newchar = getline(cursorline)[cursorcol - 1]
if newchar != char
let cursorcol += (a:last ? 1 : -1)
endif

call cursor(cursorline, cursorcol)
endif
catch /sexp-noop-error/
if a:mode !=? 'v'
silent! undo
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Emit with a count doesn't work properly when a count is used to emit all elements from a list, and cursor starts on the head element.
Example: Starting with...
(((|foo bar baz)))
...emit 3 elements, 2 different ways:

  1. with a count of 3
    (((|foo bar) baz))
  2. with 3 consecutive applications of emit (no count)
    (((|foo) bar) baz)

Root Cause: The :undo invoked in the non-visual "sexp-noop-error" handler undoes all the changes made in the docount loop thus far. If cursor doesn't start on the first element, it will already have escaped the list before the head element is reached, thereby preventing the "sexp-noop-error" from being thrown.
Question: What exactly is being undone in the noop case? Has anything even been done?

endif

call sexp#move_to_nearest_bracket(a:mode, a:last)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stack overflow occurs when you attempt to emit the only element of a list and there are no elements in ancestor lists to emit.
Example: At top level...
(((|foo)))

E132: Function call depth is higher than 'maxfuncdepth'

Root Cause: Attempt to emit the only element of a list results in "sexp-noop-error", whose catch handler simply attempts to move to containing bracket and retry. But when there are no elements in any ancestor lists to emit, this unconditional retry results in an infinite loop, since attempting to find containing bracket when you're already on a toplevel bracket doesn't move the cursor, and the catch handler doesn't check for this...

call sexp#stackop(a:mode, a:last, a:capture)
if a:mode ==? 'v'
call s:set_visual_marks(marks)
normal! gv
else
call cursor(cursorline, cursorcol)
endif
catch /sexp-error/
" Cleanup after error
if a:mode ==? 'v'
call s:set_visual_marks(marks)
normal! gv
else
silent! undo
call cursor(cursorline, cursorcol)
endif
endtry
Expand Down