unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#64283: 29.0.91; js-mode's mark-defun does not work correctly when functions have a comment on top
       [not found] <m1o7l3d74m.fsf.ref@yahoo.es>
@ 2023-06-25 13:05 ` Daniel Martín via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-06-25 13:38   ` Daniel Martín via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 8+ messages in thread
From: Daniel Martín via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-06-25 13:05 UTC (permalink / raw)
  To: 64283

This is a regression from Emacs 28.2.

emacs -Q
C-x b sample.js RET
M-x js-mode
Paste the following code:

function foo() {
  var value = 1;
}

/* Hello */
function bar() {
  var value = 1;
}

Place the point inside function bar.
C-M-h

Expected result:

Function bar is marked.

Actual result:

Comment "/* Hello */" is marked.

This is probably related to the following regression:

When point is between function foo and function bar, C-M-e moves point
to the _beginning_ of bar, not to the end of bar.

In GNU Emacs 29.0.91 (build 8, aarch64-apple-darwin21.6.0, NS
 appkit-2113.60 Version 12.6 (Build 21G115)) of 2023-06-07 built on
 Daniels-MacBook-Pro.local
Repository revision: bcc222251e1a750a11e365f2faa641cc56c1169d
Repository branch: emacs-29
Windowing system distributor 'Apple', version 10.3.2113
System Description:  macOS 12.6.7

Configured using:
 'configure 'CFLAGS=-O0 -g3 -fsanitize=address'
 CPPFLAGS=-I/opt/homebrew/opt/openjdk@11/include'

Configured features:
ACL GNUTLS JSON LCMS2 LIBXML2 MODULES NOTIFY KQUEUE NS PDUMPER PNG
SQLITE3 THREADS TOOLKIT_SCROLL_BARS WEBP ZLIB

Important settings:
  value of $LC_CTYPE: UTF-8
  locale-coding-system: utf-8-unix

Major mode: Shell

Minor modes in effect:
  shell-dirtrack-mode: t
  comint-fontify-input-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  undelete-frame-mode: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
None found.

Features:
(shadow emacsbug ido cc-langs whitespace python pcase reposition edebug
gnus-msg sort find-dired kmacro cus-start cus-load info-look info
apropos display-line-numbers re-builder dabbrev pp mail-extr goto-addr
view smerge-mode diff log-view pcvs-util org-element org-persist org-id
org-refile avl-tree oc-basic ol-eww ol-rmail ol-mhe ol-irc ol-info
ol-gnus nnselect gnus-art mm-uu mml2015 mm-view mml-smime smime gnutls
dig gnus-sum gnus-group gnus-undo gnus-start gnus-dbus dbus gnus-cloud
nnimap nnmail mail-source utf7 nnoo parse-time gnus-spec gnus-int
gnus-range message sendmail rfc822 mml mml-sec epa derived epg rfc6068
epg-config mm-decode mm-bodies mm-encode mail-parse rfc2231 rfc2047
rfc2045 ietf-drums mailabbrev gmm-utils mailheader gnus-win ol-docview
ol-bibtex bibtex iso8601 ol-bbdb ol-w3m ol-doi org-link-doi org ob
ob-tangle ob-ref ob-lob ob-table ob-exp org-macro org-src ob-comint
org-pcomplete org-list org-footnote org-faces org-entities ob-emacs-lisp
ob-core ob-eval org-cycle org-table ol org-fold org-fold-core org-keys
oc org-loaddefs cal-menu calendar cal-loaddefs org-version org-compat
org-macs make-mode ffap grep pcmpl-gnu pcmpl-unix novice etags fileloop
generator pulse xref project shortdoc cl-extra proced noutline outline
icons add-log rng-xsd xsd-regexp rng-cmpct rng-nxml rng-valid rng-loc
rng-uri rng-parse nxml-parse rng-match rng-dt rng-util rng-pttrn nxml-ns
nxml-mode nxml-outln nxml-rap nxml-util nxml-enc xmltok yank-media
mhtml-mode css-mode eww xdg url-queue thingatpt shr pixel-fill kinsoku
url-file svg xml browse-url url url-proxy url-privacy url-expand
url-methods url-history url-cookie generate-lisp-file url-domsuf
url-util url-parse auth-source eieio eieio-core cl-macs password-cache
url-vars mailcap puny mm-url gnus nnheader gnus-util mail-utils range
wid-edit mm-util mail-prsvr color sgml-mode facemenu dom vc
bug-reference octave misearch multi-isearch js c-ts-common json map
cc-mode cc-fonts cc-guess cc-menus cc-cmds cc-styles cc-align cc-engine
cc-vars cc-defs ediff ediff-merg ediff-mult ediff-wind ediff-diff
ediff-help ediff-init ediff-util format-spec imenu doc-view filenotify
jka-compr image-mode exif vc-git diff-mode easy-mmode vc-dispatcher
sh-script rx smie treesit cl-seq executable files-x shell pcomplete
compile text-property-search comint ansi-osc ansi-color ring dired-aux
dired dired-loaddefs time-date subr-x help-fns radix-tree cl-print
byte-opt gv bytecomp byte-compile debug backtrace help-mode find-func
cl-loaddefs cl-lib rmc iso-transl tooltip cconv eldoc paren electric
uniquify ediff-hook vc-hooks lisp-float-type elisp-mode mwheel
term/ns-win ns-win ucs-normalize mule-util term/common-win tool-bar dnd
fontset image regexp-opt fringe tabulated-list replace newcomment
text-mode lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow
isearch easymenu timer select scroll-bar mouse jit-lock font-lock syntax
font-core term/tty-colors frame minibuffer nadvice seq simple cl-generic
indonesian philippine cham georgian utf-8-lang misc-lang vietnamese
tibetan thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek
romanian slovak czech european ethiopic indian cyrillic chinese
composite emoji-zwj charscript charprop case-table epa-hook
jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button loaddefs
theme-loaddefs faces cus-face macroexp files window text-properties
overlay sha1 md5 base64 format env code-pages mule custom widget keymap
hashtable-print-readable backquote threads kqueue cocoa ns lcms2
multi-tty make-network-process emacs)

Memory information:
((conses 16 939191 235686)
 (symbols 48 45772 49)
 (strings 32 194496 17389)
 (string-bytes 1 6108451)
 (vectors 16 74154)
 (vector-slots 8 2095032 169146)
 (floats 8 489 580)
 (intervals 56 136872 727)
 (buffers 976 123))





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#64283: 29.0.91; js-mode's mark-defun does not work correctly when functions have a comment on top
  2023-06-25 13:05 ` bug#64283: 29.0.91; js-mode's mark-defun does not work correctly when functions have a comment on top Daniel Martín via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-06-25 13:38   ` Daniel Martín via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-06-25 15:05     ` Eli Zaretskii
  0 siblings, 1 reply; 8+ messages in thread
From: Daniel Martín via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-06-25 13:38 UTC (permalink / raw)
  To: 64283

Daniel Martín via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs@gnu.org> writes:

> This is a regression from Emacs 28.2.
>

This is the first commit where this bug happens:

commit 4489450f37deafb013b1f0fc00c89f0973fda14a
Author: Yuan Fu <casouri@gmail.com>
Date:   Thu Nov 10 15:00:29 2022 -0800

    In end-of-defun, terminate early if no further defun exists
    
    Before this change, end-of-defun calls end-of-defun-function even if
    point is not necessarily at the beginning of a defun (contrary to what
    end-of-defun-function's docstring claims).  Now it terminates early
    and doesn't call end-of-defun-function.
    
    * lisp/emacs-lisp/lisp.el (beginning-of-defun-raw): Update docstring
    clarifying the return value.
    (end-of-defun): Terminate early if beginning-of-defun-raw returns nil.





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#64283: 29.0.91; js-mode's mark-defun does not work correctly when functions have a comment on top
  2023-06-25 13:38   ` Daniel Martín via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-06-25 15:05     ` Eli Zaretskii
  2023-06-25 20:49       ` Daniel Martín via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 8+ messages in thread
From: Eli Zaretskii @ 2023-06-25 15:05 UTC (permalink / raw)
  To: Daniel Martín, Yuan Fu; +Cc: Dmitry Gutov, 64283

> Date: Sun, 25 Jun 2023 15:38:12 +0200
> From:  Daniel Martín via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> Daniel Martín via "Bug reports for GNU Emacs, the Swiss army knife of
> text editors" <bug-gnu-emacs@gnu.org> writes:
> 
> > This is a regression from Emacs 28.2.
> >
> 
> This is the first commit where this bug happens:
> 
> commit 4489450f37deafb013b1f0fc00c89f0973fda14a
> Author: Yuan Fu <casouri@gmail.com>
> Date:   Thu Nov 10 15:00:29 2022 -0800
> 
>     In end-of-defun, terminate early if no further defun exists
>     
>     Before this change, end-of-defun calls end-of-defun-function even if
>     point is not necessarily at the beginning of a defun (contrary to what
>     end-of-defun-function's docstring claims).  Now it terminates early
>     and doesn't call end-of-defun-function.
>     
>     * lisp/emacs-lisp/lisp.el (beginning-of-defun-raw): Update docstring
>     clarifying the return value.
>     (end-of-defun): Terminate early if beginning-of-defun-raw returns nil.

Yuan, could you please look into this?





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#64283: 29.0.91; js-mode's mark-defun does not work correctly when functions have a comment on top
  2023-06-25 15:05     ` Eli Zaretskii
@ 2023-06-25 20:49       ` Daniel Martín via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-06-27  1:42         ` Yuan Fu
  0 siblings, 1 reply; 8+ messages in thread
From: Daniel Martín via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-06-25 20:49 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Dmitry Gutov, Yuan Fu, 64283

[-- Attachment #1: Type: text/plain, Size: 1911 bytes --]

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Sun, 25 Jun 2023 15:38:12 +0200
>> From:  Daniel Martín via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
>> 
>> Daniel Martín via "Bug reports for GNU Emacs, the Swiss army knife of
>> text editors" <bug-gnu-emacs@gnu.org> writes:
>> 
>> > This is a regression from Emacs 28.2.
>> >
>> 
>> This is the first commit where this bug happens:
>> 
>> commit 4489450f37deafb013b1f0fc00c89f0973fda14a
>> Author: Yuan Fu <casouri@gmail.com>
>> Date:   Thu Nov 10 15:00:29 2022 -0800
>> 
>>     In end-of-defun, terminate early if no further defun exists
>>     
>>     Before this change, end-of-defun calls end-of-defun-function even if
>>     point is not necessarily at the beginning of a defun (contrary to what
>>     end-of-defun-function's docstring claims).  Now it terminates early
>>     and doesn't call end-of-defun-function.
>>     
>>     * lisp/emacs-lisp/lisp.el (beginning-of-defun-raw): Update docstring
>>     clarifying the return value.
>>     (end-of-defun): Terminate early if beginning-of-defun-raw returns nil.
>
> Yuan, could you please look into this?

What I see is that, after 4489450f37deafb013b1f0fc00c89f0973fda14a,
defun movement may be subtly broken if beginning-of-defun-function does
not return non-nil when it found the beginning of a defun.  One of the
affected modes is js-mode, but who knows if there are more out there.

We could either revert 4489450f37deafb013b1f0fc00c89f0973fda14a, because
of the incompatibilities it may cause (Yuan, what is the bug it tries to
fix?), or maybe adjust js-mode so that it follows the documentation of
beginning-of-defun-function and returns non-nil when it found the
beginning of a defun.  I've attached a patch that follows this second
approach, with some unit tests.  It fixes the bug on my side.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Make-js-beginning-of-defun-return-non-nil-on-success.patch --]
[-- Type: text/x-patch, Size: 5214 bytes --]

From dbd0a48fba90b9a7f2fb291a3d279b6adf3117f3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Mart=C3=ADn?= <mardani29@yahoo.es>
Date: Sun, 25 Jun 2023 22:17:14 +0200
Subject: [PATCH] Make js-beginning-of-defun return non-nil on success

* lisp/progmodes/js.el (js-beginning-of-defun): The docstring of
beginning-of-defun-function says that this function shall return
non-nil when it found the beginning of a defun.  This is specially
important because the calling code decides when to move point
depending on the return value.  Make js-beginning-of-defun return
non-nil when it found the beginning of a defun.
(js--beginning-of-defun-flat): Same for this helper.
* test/lisp/progmodes/js-tests.el (js-mode-end-of-defun): Add a unit
test.
---
 lisp/progmodes/js.el            | 61 ++++++++++++++++++---------------
 test/lisp/progmodes/js-tests.el | 51 +++++++++++++++++++++++++++
 2 files changed, 85 insertions(+), 27 deletions(-)

diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index 414b6eb2baf..a05bd758dbc 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -1024,38 +1024,45 @@ js--beginning-of-defun-flat
   "Helper function for `js-beginning-of-defun'."
   (let ((pstate (js--beginning-of-defun-raw)))
     (when pstate
-      (goto-char (js--pitem-h-begin (car pstate))))))
+      (goto-char (js--pitem-h-begin (car pstate)))
+      t)))
 
 (defun js-beginning-of-defun (&optional arg)
   "Value of `beginning-of-defun-function' for `js-mode'."
   (setq arg (or arg 1))
-  (while (and (not (eobp)) (< arg 0))
-    (cl-incf arg)
-    (when (and (not js-flat-functions)
-               (or (eq (js-syntactic-context) 'function)
-                   (js--function-prologue-beginning)))
-      (js-end-of-defun))
-
-    (if (js--re-search-forward
-         "\\_<function\\_>" nil t)
-        (goto-char (js--function-prologue-beginning))
-      (goto-char (point-max))))
-
-  (while (> arg 0)
-    (cl-decf arg)
-    ;; If we're just past the end of a function, the user probably wants
-    ;; to go to the beginning of *that* function
-    (when (eq (char-before) ?})
-      (backward-char))
-
-    (let ((prologue-begin (js--function-prologue-beginning)))
-      (cond ((and prologue-begin (< prologue-begin (point)))
-             (goto-char prologue-begin))
+  (let ((found))
+    (while (and (not (eobp)) (< arg 0))
+      (cl-incf arg)
+      (when (and (not js-flat-functions)
+                 (or (eq (js-syntactic-context) 'function)
+                     (js--function-prologue-beginning)))
+        (js-end-of-defun))
+
+      (if (js--re-search-forward
+           "\\_<function\\_>" nil t)
+          (progn (goto-char (js--function-prologue-beginning))
+                 (setq found t))
+        (goto-char (point-max))
+        (setq found nil)))
+
+    (while (> arg 0)
+      (cl-decf arg)
+      ;; If we're just past the end of a function, the user probably wants
+      ;; to go to the beginning of *that* function
+      (when (eq (char-before) ?})
+        (backward-char))
 
-            (js-flat-functions
-             (js--beginning-of-defun-flat))
-            (t
-             (js--beginning-of-defun-nested))))))
+      (let ((prologue-begin (js--function-prologue-beginning)))
+        (cond ((and prologue-begin (< prologue-begin (point)))
+               (goto-char prologue-begin)
+               (setq found t))
+
+              (js-flat-functions
+               (setq found (js--beginning-of-defun-flat)))
+              (t
+               (when (js--beginning-of-defun-nested)
+                 (setq found t))))))
+    found))
 
 (defun js--flush-caches (&optional beg _ignored)
   "Flush the `js-mode' syntax cache after position BEG.
diff --git a/test/lisp/progmodes/js-tests.el b/test/lisp/progmodes/js-tests.el
index 00fa78e8891..5db92b08f8a 100644
--- a/test/lisp/progmodes/js-tests.el
+++ b/test/lisp/progmodes/js-tests.el
@@ -237,6 +237,57 @@ "jsx-unclosed-1.jsx"
 (js-deftest-indent "jsx-unclosed-2.jsx")
 (js-deftest-indent "jsx.jsx")
 
+;;;; Navigation tests.
+
+(ert-deftest js-mode-beginning-of-defun ()
+  (with-temp-buffer
+    (insert "function foo() {
+  var value = 1;
+}
+
+/** A comment. */
+function bar() {
+  var value = 1;
+}
+")
+    (js-mode)
+    ;; Move point inside `foo'.
+    (goto-char 18)
+    (beginning-of-defun)
+    (should (bobp))
+    ;; Move point between the two functions.
+    (goto-char 37)
+    (beginning-of-defun)
+    (should (bobp))
+    ;; Move point inside `bar'.
+    (goto-char 73)
+    (beginning-of-defun)
+    ;; Point should move to the beginning of `bar'.
+    (should (equal (point) 56))))
+
+(ert-deftest js-mode-end-of-defun ()
+  (with-temp-buffer
+    (insert "function foo() {
+  var value = 1;
+}
+
+/** A comment. */
+function bar() {
+  var value = 1;
+}
+")
+    (js-mode)
+    (goto-char (point-min))
+    (end-of-defun)
+    ;; end-of-defun from the beginning of the buffer should go to the
+    ;; end of `foo'.
+    (should (equal (point) 37))
+    ;; Move point to the beginning of /** A comment. */
+    (goto-char 38)
+    (end-of-defun)
+    ;; end-of-defun should move point to eob.
+    (should (eobp))))
+
 (provide 'js-tests)
 
 ;;; js-tests.el ends here
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* bug#64283: 29.0.91; js-mode's mark-defun does not work correctly when functions have a comment on top
  2023-06-25 20:49       ` Daniel Martín via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-06-27  1:42         ` Yuan Fu
  2023-06-27 11:01           ` Eli Zaretskii
  0 siblings, 1 reply; 8+ messages in thread
From: Yuan Fu @ 2023-06-27  1:42 UTC (permalink / raw)
  To: Daniel Martín; +Cc: Dmitry Gutov, Eli Zaretskii, 64283

> 
> What I see is that, after 4489450f37deafb013b1f0fc00c89f0973fda14a,
> defun movement may be subtly broken if beginning-of-defun-function does
> not return non-nil when it found the beginning of a defun.  One of the
> affected modes is js-mode, but who knows if there are more out there.
> 
> We could either revert 4489450f37deafb013b1f0fc00c89f0973fda14a, because
> of the incompatibilities it may cause (Yuan, what is the bug it tries to
> fix?), or maybe adjust js-mode so that it follows the documentation of
> beginning-of-defun-function and returns non-nil when it found the
> beginning of a defun.  I've attached a patch that follows this second
> approach, with some unit tests.  It fixes the bug on my side.
> 
> <0001-Make-js-beginning-of-defun-return-non-nil-on-success.patch>

The original problem that I tried to solve is that sometimes end-of-defun-function was called when point isn’t at the beginning of a defun, contrary to what the documentation claims. 

I first find out about it when writing defun movement functions for tree-sitter, but if you revert the commit now tree-sitter defun functions wouldn’t break: they have change quite a bit since then and treesit-end-of-defun don’t need to be called at the beginning of the defun anymore.

Yuan




^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#64283: 29.0.91; js-mode's mark-defun does not work correctly when functions have a comment on top
  2023-06-27  1:42         ` Yuan Fu
@ 2023-06-27 11:01           ` Eli Zaretskii
  2023-06-28 20:11             ` Yuan Fu
  0 siblings, 1 reply; 8+ messages in thread
From: Eli Zaretskii @ 2023-06-27 11:01 UTC (permalink / raw)
  To: Yuan Fu; +Cc: dmitry, 64283, mardani29

> From: Yuan Fu <casouri@gmail.com>
> Date: Mon, 26 Jun 2023 18:42:41 -0700
> Cc: Eli Zaretskii <eliz@gnu.org>,
>  Dmitry Gutov <dmitry@gutov.dev>,
>  64283@debbugs.gnu.org
> 
> > 
> > What I see is that, after 4489450f37deafb013b1f0fc00c89f0973fda14a,
> > defun movement may be subtly broken if beginning-of-defun-function does
> > not return non-nil when it found the beginning of a defun.  One of the
> > affected modes is js-mode, but who knows if there are more out there.
> > 
> > We could either revert 4489450f37deafb013b1f0fc00c89f0973fda14a, because
> > of the incompatibilities it may cause (Yuan, what is the bug it tries to
> > fix?), or maybe adjust js-mode so that it follows the documentation of
> > beginning-of-defun-function and returns non-nil when it found the
> > beginning of a defun.  I've attached a patch that follows this second
> > approach, with some unit tests.  It fixes the bug on my side.
> > 
> > <0001-Make-js-beginning-of-defun-return-non-nil-on-success.patch>
> 
> The original problem that I tried to solve is that sometimes end-of-defun-function was called when point isn’t at the beginning of a defun, contrary to what the documentation claims. 
> 
> I first find out about it when writing defun movement functions for tree-sitter, but if you revert the commit now tree-sitter defun functions wouldn’t break: they have change quite a bit since then and treesit-end-of-defun don’t need to be called at the beginning of the defun anymore.

Thanks.

Do you (or anyone else) see a problem with the alternative proposed by
Daniel?  If not, I'd prefer not to revert at this stage, but instead
to apply the simple fix Daniel suggested.





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#64283: 29.0.91; js-mode's mark-defun does not work correctly when functions have a comment on top
  2023-06-27 11:01           ` Eli Zaretskii
@ 2023-06-28 20:11             ` Yuan Fu
  2023-06-29  5:41               ` Eli Zaretskii
  0 siblings, 1 reply; 8+ messages in thread
From: Yuan Fu @ 2023-06-28 20:11 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Dmitry Gutov, 64283, Daniel Martín



> On Jun 27, 2023, at 4:01 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> 
>> From: Yuan Fu <casouri@gmail.com>
>> Date: Mon, 26 Jun 2023 18:42:41 -0700
>> Cc: Eli Zaretskii <eliz@gnu.org>,
>> Dmitry Gutov <dmitry@gutov.dev>,
>> 64283@debbugs.gnu.org
>> 
>>> 
>>> What I see is that, after 4489450f37deafb013b1f0fc00c89f0973fda14a,
>>> defun movement may be subtly broken if beginning-of-defun-function does
>>> not return non-nil when it found the beginning of a defun.  One of the
>>> affected modes is js-mode, but who knows if there are more out there.
>>> 
>>> We could either revert 4489450f37deafb013b1f0fc00c89f0973fda14a, because
>>> of the incompatibilities it may cause (Yuan, what is the bug it tries to
>>> fix?), or maybe adjust js-mode so that it follows the documentation of
>>> beginning-of-defun-function and returns non-nil when it found the
>>> beginning of a defun.  I've attached a patch that follows this second
>>> approach, with some unit tests.  It fixes the bug on my side.
>>> 
>>> <0001-Make-js-beginning-of-defun-return-non-nil-on-success.patch>
>> 
>> The original problem that I tried to solve is that sometimes end-of-defun-function was called when point isn’t at the beginning of a defun, contrary to what the documentation claims. 
>> 
>> I first find out about it when writing defun movement functions for tree-sitter, but if you revert the commit now tree-sitter defun functions wouldn’t break: they have change quite a bit since then and treesit-end-of-defun don’t need to be called at the beginning of the defun anymore.
> 
> Thanks.
> 
> Do you (or anyone else) see a problem with the alternative proposed by
> Daniel?  If not, I'd prefer not to revert at this stage, but instead
> to apply the simple fix Daniel suggested.

I don’t see any problem :-)

Yuan




^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#64283: 29.0.91; js-mode's mark-defun does not work correctly when functions have a comment on top
  2023-06-28 20:11             ` Yuan Fu
@ 2023-06-29  5:41               ` Eli Zaretskii
  0 siblings, 0 replies; 8+ messages in thread
From: Eli Zaretskii @ 2023-06-29  5:41 UTC (permalink / raw)
  To: Yuan Fu; +Cc: dmitry, 64283-done, mardani29

> From: Yuan Fu <casouri@gmail.com>
> Date: Wed, 28 Jun 2023 13:11:43 -0700
> Cc: Daniel Martín <mardani29@yahoo.es>,
>  Dmitry Gutov <dmitry@gutov.dev>,
>  64283@debbugs.gnu.org
> 
> 
> 
> > On Jun 27, 2023, at 4:01 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> > 
> >> From: Yuan Fu <casouri@gmail.com>
> >> Date: Mon, 26 Jun 2023 18:42:41 -0700
> >> Cc: Eli Zaretskii <eliz@gnu.org>,
> >> Dmitry Gutov <dmitry@gutov.dev>,
> >> 64283@debbugs.gnu.org
> >> 
> >>> 
> >>> What I see is that, after 4489450f37deafb013b1f0fc00c89f0973fda14a,
> >>> defun movement may be subtly broken if beginning-of-defun-function does
> >>> not return non-nil when it found the beginning of a defun.  One of the
> >>> affected modes is js-mode, but who knows if there are more out there.
> >>> 
> >>> We could either revert 4489450f37deafb013b1f0fc00c89f0973fda14a, because
> >>> of the incompatibilities it may cause (Yuan, what is the bug it tries to
> >>> fix?), or maybe adjust js-mode so that it follows the documentation of
> >>> beginning-of-defun-function and returns non-nil when it found the
> >>> beginning of a defun.  I've attached a patch that follows this second
> >>> approach, with some unit tests.  It fixes the bug on my side.
> >>> 
> >>> <0001-Make-js-beginning-of-defun-return-non-nil-on-success.patch>
> >> 
> >> The original problem that I tried to solve is that sometimes end-of-defun-function was called when point isn’t at the beginning of a defun, contrary to what the documentation claims. 
> >> 
> >> I first find out about it when writing defun movement functions for tree-sitter, but if you revert the commit now tree-sitter defun functions wouldn’t break: they have change quite a bit since then and treesit-end-of-defun don’t need to be called at the beginning of the defun anymore.
> > 
> > Thanks.
> > 
> > Do you (or anyone else) see a problem with the alternative proposed by
> > Daniel?  If not, I'd prefer not to revert at this stage, but instead
> > to apply the simple fix Daniel suggested.
> 
> I don’t see any problem :-)

Thanks.  So I've now installed Daniel's patch on the released branch,
and I'm therefore closing this bug.





^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2023-06-29  5:41 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <m1o7l3d74m.fsf.ref@yahoo.es>
2023-06-25 13:05 ` bug#64283: 29.0.91; js-mode's mark-defun does not work correctly when functions have a comment on top Daniel Martín via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-06-25 13:38   ` Daniel Martín via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-06-25 15:05     ` Eli Zaretskii
2023-06-25 20:49       ` Daniel Martín via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-06-27  1:42         ` Yuan Fu
2023-06-27 11:01           ` Eli Zaretskii
2023-06-28 20:11             ` Yuan Fu
2023-06-29  5:41               ` Eli Zaretskii

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).