* bug#66431: [PATCH] Fix reset treesit--explorer-last-node when explorer buffer was killed @ 2023-10-10 6:02 nvp 2023-10-14 8:17 ` Eli Zaretskii 2023-10-14 17:08 ` Yuan Fu 0 siblings, 2 replies; 10+ messages in thread From: nvp @ 2023-10-10 6:02 UTC (permalink / raw) To: 66431 [-- Attachment #1.1: Type: text/plain, Size: 1049 bytes --] Tags: patch Bug: After `treesit-explorer-mode` is enabled in a buffer and its associated `treesit--explorer-buffer` is killed, a subsequent call to `treesit-explorer-mode` initially displays an empty explorer buffer b/c `treesit--explorer-refresh` sees old value for `treesit--explorer-last-node`. * lisp/treesit.el (treesit-explorer-mode): reset `treesit--explorer-last-node` when `treesit--explorer-buffer` was killed In GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.33, cairo version 1.16.0) of 2023-10-05 built on noah-X580VD Repository revision: 505c80623049d9e181918acdac8229c9a2041b1e Repository branch: master Windowing system distributor 'The X.Org Foundation', version 11.0.12101004 System Description: Ubuntu 22.04.3 LTS Configured using: 'configure --prefix=/usr/local --with-modules --with-tree-sitter --with-threads --with-x-toolkit=gtk3 --with-xwidgets --with-gnutls --with-json --with-mailutils --with-jpeg --with-png --with-rsvg --with-tiff --with-xml2 --with-xpm --with-imagemagick CC=gcc-12 CXX=gcc-12' [-- Attachment #1.2: Type: text/html, Size: 1190 bytes --] [-- Attachment #2: fix-treesit-explorer-last-node.patch --] [-- Type: text/x-patch, Size: 841 bytes --] From f0d420133b58d0506173f2f1539867b2d9e7f014 Mon Sep 17 00:00:00 2001 From: nverno <noah.v.peart@gmail.com> Date: Mon, 9 Oct 2023 22:52:38 -0700 Subject: [PATCH] Fix reset treesit--explorer-last-node --- lisp/treesit.el | 1 + 1 file changed, 1 insertion(+) diff --git a/lisp/treesit.el b/lisp/treesit.el index 402417c6ca9..e798abac079 100644 --- a/lisp/treesit.el +++ b/lisp/treesit.el @@ -3192,6 +3192,7 @@ treesit-explore-mode (format "*tree-sitter explorer for %s*" (buffer-name)))) (setq-local treesit--explorer-language language) + (setq-local treesit--explorer-last-node nil) (with-current-buffer treesit--explorer-buffer (treesit--explorer-tree-mode))) (display-buffer treesit--explorer-buffer -- 2.34.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* bug#66431: [PATCH] Fix reset treesit--explorer-last-node when explorer buffer was killed 2023-10-10 6:02 bug#66431: [PATCH] Fix reset treesit--explorer-last-node when explorer buffer was killed nvp @ 2023-10-14 8:17 ` Eli Zaretskii 2023-10-14 17:08 ` Yuan Fu 2023-10-14 17:08 ` Yuan Fu 1 sibling, 1 reply; 10+ messages in thread From: Eli Zaretskii @ 2023-10-14 8:17 UTC (permalink / raw) To: nvp, Yuan Fu; +Cc: 66431 > From: nvp <noah.v.peart@gmail.com> > Date: Mon, 9 Oct 2023 23:02:48 -0700 > > Tags: patch > > Bug: After `treesit-explorer-mode` is enabled in a buffer and its > associated `treesit--explorer-buffer` is killed, a subsequent call > to `treesit-explorer-mode` initially displays an empty explorer > buffer b/c `treesit--explorer-refresh` sees old value for > `treesit--explorer-last-node`. > > * lisp/treesit.el (treesit-explorer-mode): reset > `treesit--explorer-last-node` when `treesit--explorer-buffer` was killed > > In GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version > 3.24.33, cairo version 1.16.0) of 2023-10-05 built on noah-X580VD > Repository revision: 505c80623049d9e181918acdac8229c9a2041b1e > Repository branch: master > Windowing system distributor 'The X.Org Foundation', version 11.0.12101004 > System Description: Ubuntu 22.04.3 LTS Yuan, could you please look into this? Is the patch OK to go in, and if so, should it be installed on emacs-29? ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#66431: [PATCH] Fix reset treesit--explorer-last-node when explorer buffer was killed 2023-10-14 8:17 ` Eli Zaretskii @ 2023-10-14 17:08 ` Yuan Fu 2023-10-15 4:20 ` nvp 0 siblings, 1 reply; 10+ messages in thread From: Yuan Fu @ 2023-10-14 17:08 UTC (permalink / raw) To: Eli Zaretskii; +Cc: nvp, 66431 > On Oct 14, 2023, at 1:17 AM, Eli Zaretskii <eliz@gnu.org> wrote: > >> From: nvp <noah.v.peart@gmail.com> >> Date: Mon, 9 Oct 2023 23:02:48 -0700 >> >> Tags: patch >> >> Bug: After `treesit-explorer-mode` is enabled in a buffer and its >> associated `treesit--explorer-buffer` is killed, a subsequent call >> to `treesit-explorer-mode` initially displays an empty explorer >> buffer b/c `treesit--explorer-refresh` sees old value for >> `treesit--explorer-last-node`. >> >> * lisp/treesit.el (treesit-explorer-mode): reset >> `treesit--explorer-last-node` when `treesit--explorer-buffer` was killed >> >> In GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version >> 3.24.33, cairo version 1.16.0) of 2023-10-05 built on noah-X580VD >> Repository revision: 505c80623049d9e181918acdac8229c9a2041b1e >> Repository branch: master >> Windowing system distributor 'The X.Org Foundation', version 11.0.12101004 >> System Description: Ubuntu 22.04.3 LTS > > Yuan, could you please look into this? Is the patch OK to go in, and > if so, should it be installed on emacs-29? I’ll look at it, thanks for the reminder. Yuan ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#66431: [PATCH] Fix reset treesit--explorer-last-node when explorer buffer was killed 2023-10-14 17:08 ` Yuan Fu @ 2023-10-15 4:20 ` nvp 2023-10-19 4:35 ` Yuan Fu 0 siblings, 1 reply; 10+ messages in thread From: nvp @ 2023-10-15 4:20 UTC (permalink / raw) To: Yuan Fu; +Cc: Eli Zaretskii, 66431 [-- Attachment #1: Type: text/plain, Size: 3257 bytes --] Hi, The patch is supposed to reset `treesit--explorer-last-node` in the source buffer, just before the `(with-current-buffer treesit--explorer-buffer ...)`. Upon trying to reproduce it now, I realized it's harder to reproduce than I had thought -- sorry about that. I noticed the bug (if it is a bug) initially when I was adding a function to jump b/w source and explorer buffers, like the following: (defun my-treesit-explorer-jump () "Pop b/w source and explorer buffers." (interactive) (let ((buf (cond ((eq major-mode 'treesit--explorer-tree-mode) (when (buffer-live-p treesit--explorer-source-buffer) treesit--explorer-source-buffer)) (t (unless (and treesit-explore-mode (buffer-live-p treesit--explorer-buffer)) ;; *** Without the reset here, the explorer buffer doesn't ;; get redrawn the first time, when treesit--explorer-last-node ;; is non-nil in the source buffer *** ;; (setq-local treesit--explorer-last-node nil) (cl-letf (((symbol-function (function completing-read)) (lambda (&rest _) (symbol-name (treesit-language-at (point)))))) (treesit-explore-mode 1))) treesit--explorer-buffer)))) (pop-to-buffer buf))) Let me give a more precise recipe to reproduce: 1. From a c++-ts-mode buffer, call `treesit-explorer-mode`, select `cpp`. Now there should be an explorer buffer. 2. Kill the associated explorer buffer. 3. Now, back in the c++-ts-mode buffer, `treesit--explorer-last-node` should still have a value. 4. From that c++-ts-mode buffer, call `my-treesit-explorer-jump`, and the explorer buffer should be empty, until switching back to the source buffer. This seems to me to be caused by `treesit--explorer-post-command` not running until the source buffer is active again. On Sat, Oct 14, 2023 at 10:08 AM Yuan Fu <casouri@gmail.com> wrote: > > > > On Oct 14, 2023, at 1:17 AM, Eli Zaretskii <eliz@gnu.org> wrote: > > > >> From: nvp <noah.v.peart@gmail.com> > >> Date: Mon, 9 Oct 2023 23:02:48 -0700 > >> > >> Tags: patch > >> > >> Bug: After `treesit-explorer-mode` is enabled in a buffer and its > >> associated `treesit--explorer-buffer` is killed, a subsequent call > >> to `treesit-explorer-mode` initially displays an empty explorer > >> buffer b/c `treesit--explorer-refresh` sees old value for > >> `treesit--explorer-last-node`. > >> > >> * lisp/treesit.el (treesit-explorer-mode): reset > >> `treesit--explorer-last-node` when `treesit--explorer-buffer` was killed > >> > >> In GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version > >> 3.24.33, cairo version 1.16.0) of 2023-10-05 built on noah-X580VD > >> Repository revision: 505c80623049d9e181918acdac8229c9a2041b1e > >> Repository branch: master > >> Windowing system distributor 'The X.Org Foundation', version > 11.0.12101004 > >> System Description: Ubuntu 22.04.3 LTS > > > > Yuan, could you please look into this? Is the patch OK to go in, and > > if so, should it be installed on emacs-29? > > I’ll look at it, thanks for the reminder. > > Yuan > > [-- Attachment #2: Type: text/html, Size: 4209 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#66431: [PATCH] Fix reset treesit--explorer-last-node when explorer buffer was killed 2023-10-15 4:20 ` nvp @ 2023-10-19 4:35 ` Yuan Fu 2023-10-20 21:22 ` nvp 0 siblings, 1 reply; 10+ messages in thread From: Yuan Fu @ 2023-10-19 4:35 UTC (permalink / raw) To: nvp; +Cc: Eli Zaretskii, 66431 [-- Attachment #1: Type: text/plain, Size: 2414 bytes --] > On Oct 14, 2023, at 9:20 PM, nvp <noah.v.peart@gmail.com> wrote: > > Hi, > The patch is supposed to reset `treesit--explorer-last-node` in the source buffer, just before the `(with-current-buffer treesit--explorer-buffer ...)`. > Upon trying to reproduce it now, I realized it's harder to reproduce than I had thought -- sorry about that. > I noticed the bug (if it is a bug) initially when I was adding a function to jump b/w source and explorer buffers, like the following: > > (defun my-treesit-explorer-jump () > "Pop b/w source and explorer buffers." > (interactive) > (let ((buf > (cond > ((eq major-mode 'treesit--explorer-tree-mode) > (when (buffer-live-p treesit--explorer-source-buffer) > treesit--explorer-source-buffer)) > (t > (unless (and treesit-explore-mode > (buffer-live-p treesit--explorer-buffer)) > ;; *** Without the reset here, the explorer buffer doesn't > ;; get redrawn the first time, when treesit--explorer-last-node > ;; is non-nil in the source buffer *** > ;; (setq-local treesit--explorer-last-node nil) > (cl-letf (((symbol-function (function completing-read)) > (lambda (&rest _) (symbol-name (treesit-language-at (point)))))) > (treesit-explore-mode 1))) > treesit--explorer-buffer)))) > (pop-to-buffer buf))) > > Let me give a more precise recipe to reproduce: > 1. From a c++-ts-mode buffer, call `treesit-explorer-mode`, select `cpp`. Now there should be an explorer buffer. > 2. Kill the associated explorer buffer. > 3. Now, back in the c++-ts-mode buffer, `treesit--explorer-last-node` should still have a value. > 4. From that c++-ts-mode buffer, call `my-treesit-explorer-jump`, and the explorer buffer should be empty, until > switching back to the source buffer. > > This seems to me to be caused by `treesit--explorer-post-command` not running until the source > buffer is active again. Thank you, I think I see the problem now. Could you try the below patch and see it fixes your problem? Also, sorry for the late reply, I meant to reply sooner but couldn’t find the time to figure out what exact was the cause :-) I was initially a bit confused since we already do set last-node to nil. Yuan [-- Attachment #2: fix-last-node.diff --] [-- Type: application/octet-stream, Size: 912 bytes --] diff --git a/lisp/treesit.el b/lisp/treesit.el index c73ac9912d6..9f7e8bacd35 100644 --- a/lisp/treesit.el +++ b/lisp/treesit.el @@ -3198,13 +3198,13 @@ treesit-explore-mode (treesit--explorer-tree-mode))) (display-buffer treesit--explorer-buffer (cons nil '((inhibit-same-window . t)))) + (setq-local treesit--explorer-last-node nil) (treesit--explorer-refresh) ;; Set up variables and hooks. (add-hook 'post-command-hook #'treesit--explorer-post-command 0 t) (add-hook 'kill-buffer-hook #'treesit--explorer-kill-explorer-buffer 0 t) - (setq-local treesit--explorer-last-node nil) ;; Tell `desktop-save' to not save explorer buffers. (when (boundp 'desktop-modes-not-to-save) (unless (memq 'treesit--explorer-tree-mode [-- Attachment #3: Type: text/plain, Size: 2 bytes --] ^ permalink raw reply related [flat|nested] 10+ messages in thread
* bug#66431: [PATCH] Fix reset treesit--explorer-last-node when explorer buffer was killed 2023-10-19 4:35 ` Yuan Fu @ 2023-10-20 21:22 ` nvp 2023-10-21 18:33 ` Yuan Fu 0 siblings, 1 reply; 10+ messages in thread From: nvp @ 2023-10-20 21:22 UTC (permalink / raw) To: Yuan Fu; +Cc: Eli Zaretskii, 66431 [-- Attachment #1: Type: text/plain, Size: 3374 bytes --] That fixes the problem! However, the reason I initially put the reset inside the `(unless (buffer-live-p treesit--explorer-buffer) ...)` in `treesit-explore-mode` was b/c it looked like there was an optimization happening in `treesit--explorer-refresh` where it does this check ;; If we didn't edit the buffer nor change the top-level ;; node, don't redraw the whole syntax tree. (highlight-only (treesit-node-eq top-level treesit--explorer-last-node)) I don't know if that is something you'd want to keep, but just pointing it out in case. I think the initial patch works as well, but still allows that check to work when the explorer buffer hasn't been killed. Thankyou! On Wed, Oct 18, 2023 at 9:36 PM Yuan Fu <casouri@gmail.com> wrote: > > > > On Oct 14, 2023, at 9:20 PM, nvp <noah.v.peart@gmail.com> wrote: > > > > Hi, > > The patch is supposed to reset `treesit--explorer-last-node` in the > source buffer, just before the `(with-current-buffer > treesit--explorer-buffer ...)`. > > Upon trying to reproduce it now, I realized it's harder to reproduce > than I had thought -- sorry about that. > > I noticed the bug (if it is a bug) initially when I was adding a > function to jump b/w source and explorer buffers, like the following: > > > > (defun my-treesit-explorer-jump () > > "Pop b/w source and explorer buffers." > > (interactive) > > (let ((buf > > (cond > > ((eq major-mode 'treesit--explorer-tree-mode) > > (when (buffer-live-p treesit--explorer-source-buffer) > > treesit--explorer-source-buffer)) > > (t > > (unless (and treesit-explore-mode > > (buffer-live-p treesit--explorer-buffer)) > > ;; *** Without the reset here, the explorer buffer doesn't > > ;; get redrawn the first time, when > treesit--explorer-last-node > > ;; is non-nil in the source buffer *** > > ;; (setq-local treesit--explorer-last-node nil) > > (cl-letf (((symbol-function (function completing-read)) > > (lambda (&rest _) (symbol-name > (treesit-language-at (point)))))) > > (treesit-explore-mode 1))) > > treesit--explorer-buffer)))) > > (pop-to-buffer buf))) > > > > Let me give a more precise recipe to reproduce: > > 1. From a c++-ts-mode buffer, call `treesit-explorer-mode`, select > `cpp`. Now there should be an explorer buffer. > > 2. Kill the associated explorer buffer. > > 3. Now, back in the c++-ts-mode buffer, `treesit--explorer-last-node` > should still have a value. > > 4. From that c++-ts-mode buffer, call `my-treesit-explorer-jump`, and > the explorer buffer should be empty, until > > switching back to the source buffer. > > > > This seems to me to be caused by `treesit--explorer-post-command` not > running until the source > > buffer is active again. > > Thank you, I think I see the problem now. Could you try the below patch > and see it fixes your problem? Also, sorry for the late reply, I meant to > reply sooner but couldn’t find the time to figure out what exact was the > cause :-) I was initially a bit confused since we already do set last-node > to nil. > > Yuan > > > > [-- Attachment #2: Type: text/html, Size: 4223 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#66431: [PATCH] Fix reset treesit--explorer-last-node when explorer buffer was killed 2023-10-20 21:22 ` nvp @ 2023-10-21 18:33 ` Yuan Fu 2023-10-22 0:40 ` nvp 0 siblings, 1 reply; 10+ messages in thread From: Yuan Fu @ 2023-10-21 18:33 UTC (permalink / raw) To: nvp; +Cc: Eli Zaretskii, 66431 > On Oct 20, 2023, at 2:22 PM, nvp <noah.v.peart@gmail.com> wrote: > > That fixes the problem! > > However, the reason I initially put the reset inside the `(unless (buffer-live-p treesit--explorer-buffer) ...)` > in `treesit-explore-mode` was b/c it looked like there was an optimization happening in > `treesit--explorer-refresh` where it does this check > > ;; If we didn't edit the buffer nor change the top-level > ;; node, don't redraw the whole syntax tree. > (highlight-only (treesit-node-eq > top-level treesit--explorer-last-node)) > > I don't know if that is something you'd want to keep, but just pointing it out in case. I think > the initial patch works as well, but still allows that check to work when the explorer buffer hasn't > been killed. Oh that’s fine, treesit-explore-mode always wipes everything and start from a clean slate. That optimization is for when the user moves point in the source buffer when explore-mode is on. If you’d like to send a patch that does roughly what I did in the patch I sent, I’d love to merge it. Otherwise I can fix it myself, too. The initial patch could be a bit confusing to the readers since it sets last-node twice, and it’s not clear why. Thanks, Yuan ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#66431: [PATCH] Fix reset treesit--explorer-last-node when explorer buffer was killed 2023-10-21 18:33 ` Yuan Fu @ 2023-10-22 0:40 ` nvp 2023-10-22 3:35 ` Yuan Fu 0 siblings, 1 reply; 10+ messages in thread From: nvp @ 2023-10-22 0:40 UTC (permalink / raw) To: Yuan Fu; +Cc: Eli Zaretskii, 66431 [-- Attachment #1: Type: text/plain, Size: 1564 bytes --] Ok that makes sense, thanks for clearing that up for me, your fix looks good. I'm loving this package! On Sat, Oct 21, 2023 at 11:33 AM Yuan Fu <casouri@gmail.com> wrote: > > > > On Oct 20, 2023, at 2:22 PM, nvp <noah.v.peart@gmail.com> wrote: > > > > That fixes the problem! > > > > However, the reason I initially put the reset inside the `(unless > (buffer-live-p treesit--explorer-buffer) ...)` > > in `treesit-explore-mode` was b/c it looked like there was an > optimization happening in > > `treesit--explorer-refresh` where it does this check > > > > ;; If we didn't edit the buffer nor change the top-level > > ;; node, don't redraw the whole syntax tree. > > (highlight-only (treesit-node-eq > > top-level treesit--explorer-last-node)) > > > > I don't know if that is something you'd want to keep, but just pointing > it out in case. I think > > the initial patch works as well, but still allows that check to work > when the explorer buffer hasn't > > been killed. > > Oh that’s fine, treesit-explore-mode always wipes everything and start > from a clean slate. That optimization is for when the user moves point in > the source buffer when explore-mode is on. If you’d like to send a patch > that does roughly what I did in the patch I sent, I’d love to merge it. > Otherwise I can fix it myself, too. > > The initial patch could be a bit confusing to the readers since it sets > last-node twice, and it’s not clear why. > > Thanks, > Yuan [-- Attachment #2: Type: text/html, Size: 2039 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#66431: [PATCH] Fix reset treesit--explorer-last-node when explorer buffer was killed 2023-10-22 0:40 ` nvp @ 2023-10-22 3:35 ` Yuan Fu 0 siblings, 0 replies; 10+ messages in thread From: Yuan Fu @ 2023-10-22 3:35 UTC (permalink / raw) To: nvp; +Cc: 66431-done, Eli Zaretskii > On Oct 21, 2023, at 5:40 PM, nvp <noah.v.peart@gmail.com> wrote: > > Ok that makes sense, thanks for clearing that up for me, your fix looks good. I'm loving this package! Thanks, I pushed the fix to emacs-29. I’m glad you are finding it useful! Yuan ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#66431: [PATCH] Fix reset treesit--explorer-last-node when explorer buffer was killed 2023-10-10 6:02 bug#66431: [PATCH] Fix reset treesit--explorer-last-node when explorer buffer was killed nvp 2023-10-14 8:17 ` Eli Zaretskii @ 2023-10-14 17:08 ` Yuan Fu 1 sibling, 0 replies; 10+ messages in thread From: Yuan Fu @ 2023-10-14 17:08 UTC (permalink / raw) To: nvp; +Cc: 66431 > On Oct 9, 2023, at 11:02 PM, nvp <noah.v.peart@gmail.com> wrote: > > Tags: patch > > > Bug: After `treesit-explorer-mode` is enabled in a buffer and its > associated `treesit--explorer-buffer` is killed, a subsequent call > to `treesit-explorer-mode` initially displays an empty explorer > buffer b/c `treesit--explorer-refresh` sees old value for > `treesit--explorer-last-node`. > > * lisp/treesit.el (treesit-explorer-mode): reset > `treesit--explorer-last-node` when `treesit--explorer-buffer` was killed Hmm, I can’t reproduce what you described. Besides, treesit--explorer-last-node is only set in the source buffer, not the explorer buffer. But the patch tries to reset it for the explorer buffer. Also treesit--explorer-last-node is reset at the end of treesit-explore-mode. We can start from reliably reproducing the bug you are seeing, and see what’s the true cause of it. Yuan ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-10-22 3:35 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-10-10 6:02 bug#66431: [PATCH] Fix reset treesit--explorer-last-node when explorer buffer was killed nvp 2023-10-14 8:17 ` Eli Zaretskii 2023-10-14 17:08 ` Yuan Fu 2023-10-15 4:20 ` nvp 2023-10-19 4:35 ` Yuan Fu 2023-10-20 21:22 ` nvp 2023-10-21 18:33 ` Yuan Fu 2023-10-22 0:40 ` nvp 2023-10-22 3:35 ` Yuan Fu 2023-10-14 17:08 ` Yuan Fu
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.