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.

Sign up for GitHub

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

Recursive stackop #15

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a2d62ad
Do not move cursor with emit or capture
Apr 16, 2015
8547d38
recursively capture elements
Apr 16, 2015
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
21 changes: 15 additions & 6 deletions autoload/sexp.vim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1584,21 +1584,30 @@ 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
if a:last
let cursorcol += 1
else
let cursorcol -= 1
endif
let cursorcol += (a:last ? 1 : -1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Cursor not adjusted correctly for bracket displacement in some cases.
Root Cause: The test for bracket displacement is too simplistic: e.g., it doesn't work when char adjacent to cursor position happens to be same as original cursor char.
Example: Note the difference in final cursor position for the following 2 emits, which differ only in the value being emitted.
(+ 100 (foo 4|2)) => (+ 100 (foo) 4|2)
(+ 100 (foo 4|4)) => (+ 100 (foo) |44)

Copy link
Collaborator

Choose a reason for hiding this comment

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

The cursor positioning logic discussed in the preceding comment also leads to unexpected results in captures such as this one:

 (let
   ((|a b)))

=>

 ((let
   (a| b)))

The problem is that the call to stackop that actually captures the "let" is a recursive call made from the "sexp-noop-error" catch handler. Although this recursive call does in fact adjust cursorcol by negative 1 as desired, the adjustment is thrown away, since it's the adjustment performed in the toplevel call to stackop that determines the final cursor position.

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/
Expand Down