bug#62158: treesit-end-of-defun error

[Yuan Fu]

> On Mar 13, 2023, at 12:28 AM, Juri Linkov <juri@linkov.net> wrote:
> 
> X-Debbugs-Cc: Yuan Fu <casouri@gmail.com>, Dmitry Gutov <dgutov@yandex.ru>
> 
> Since this is a separate problem, I'm closing bug#62086
> and opening a new bug report:
> 
>>>>> I don't know if the second bug is related to this, but while
>>>>> in the same file, also type 'C-M-l' ('reposition-window').
>>>>> It raises the error:
>>>>>  Debugger entered--Lisp error: (wrong-type-argument number-or-marker-p 
>>>>> nil)
>>>>>    treesit-end-of-defun()
>>>>>    end-of-defun(-1)
>>>>>    reposition-window(nil nil)
>>>>>    reposition-window(nil 89)
>>>>>    funcall-interactively(reposition-window nil 89)
>>>>>    command-execute(reposition-window)
>>> I see it only in some files in test/lisp/progmodes/ruby-mode-resources/
>>> e.g. ruby-parenless-call-arguments-indent.rb, ruby-method-call-indent.rb,
>>> ruby-block-indent.rb.  But not in e.g. ruby-after-operator-indent.rb.
>>> Also everywhere in 
>>> test/lisp/progmodes/js-resources/js-indent-init-dynamic.js,
>>> js-indent-init-t.js.  But not in e.g. js-chain.js.
>> 
>> Thanks, I can repro. I might have been trying the wrong binding at the end
>> last night (C-l instead of C-M-l).
>> 
>> The fix seems to be easy:
>> 
>> diff --git a/lisp/treesit.el b/lisp/treesit.el
>> index c118f5d52a4..b271a1f0c4b 100644
>> --- a/lisp/treesit.el
>> +++ b/lisp/treesit.el
>> @@ -1882,6 +1882,7 @@ treesit-end-of-defun
>> `treesit-defun-skipper'."
>>   (interactive "^p\nd")
>>   (let ((orig-point (point)))
>> +    (if (or (null arg) (= arg 0)) (setq arg 1))
>>     (catch 'done
>>       (dotimes (_ 2) ; Not making progress is better than infloop.
>> 
>> But I'm not quite sure if that is what we want to do.

This looks good to me.

>> 
>> More naturally, I think, would be to remove the argument from
>> treesit-end-of-defun altogether (and adjust the code accordingly), because
>> end-of-defun-function is documented to take no arguments.
>> 
>> The only other place where treesit-end-of-defun seems to be used is the
>> <remap> <end-of-defun> binding set up by treesit-major-mode-setup.
>> 
>> Why not keep the default bindings for these? When
>> beginning-of-defun-function and end-of-defun-function are set
>> appropriately, they should work fine. Don't they?

We tried that initially, but end-of-defun doesn’t have the notion of nested 
defuns, which leads to problems when end-of-defun-function recognizes nested 
defuns. In the following code

(defun xxx ()
  |
  (defun yyy () ...)

  (defun zzz () ...)
  )

If point is at “|” and you call end-of-defun, you’d expect point to move to the 
end of yyy, but instead it moves to the end of xxx. That’s because end-of-defun 
first runs (beginning-of-defun -1) followed by (end-of-defun 1) to check if the 
starting point is in a defun or between two defuns. This is fine in non-nested 
defuns, but in this example, the point first goes to the beginning of xxx, then 
goes to the end of xxx. And end-of-defun thinks that we started in a defun and 
now is at an end of defun, job’s done, and finishes.

The plan is to improve end-of-defun to support nested defuns in Emacs 30. For 
now we rebind end-of-defun to treesit-end-of-defun.

Yuan