From unknown Tue Jun 17 21:55:06 2025 X-Loop: help-debbugs@gnu.org Subject: bug#73533: [PATCH] Rewrite speedbar expansion for all descendants Resent-From: Morgan Willcock Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 28 Sep 2024 20:02:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: report 73533 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch To: 73533@debbugs.gnu.org X-Debbugs-Original-To: bug-gnu-emacs@gnu.org Received: via spool by submit@debbugs.gnu.org id=B.17275536675320 (code B ref -1); Sat, 28 Sep 2024 20:02:02 +0000 Received: (at submit) by debbugs.gnu.org; 28 Sep 2024 20:01:07 +0000 Received: from localhost ([127.0.0.1]:43274 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1sudcp-0001Nj-5N for submit@debbugs.gnu.org; Sat, 28 Sep 2024 16:01:07 -0400 Received: from lists.gnu.org ([209.51.188.17]:50560) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1sudcj-0001MY-QE for submit@debbugs.gnu.org; Sat, 28 Sep 2024 16:01:03 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1sucl5-0008Jv-QD for bug-gnu-emacs@gnu.org; Sat, 28 Sep 2024 15:05:35 -0400 Received: from relay4-d.mail.gandi.net ([2001:4b98:dc4:8::224]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1sucl3-0001GE-2r for bug-gnu-emacs@gnu.org; Sat, 28 Sep 2024 15:05:35 -0400 Received: by mail.gandi.net (Postfix) with ESMTPSA id D22BBE0002 for ; Sat, 28 Sep 2024 19:05:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ice9.digital; s=gm1; t=1727550327; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type; bh=0lpE61LCFx+yDgjVjJm4fg5kzoBcQv4GlKyA5Q8hpy8=; b=THFnAtf4gb+DZMtfPZ1OHcV6iso+jL/Aznt+QOSNTV4Zw0ETAyOO+oc0WOFdwBw1XTW7bt sEws/XKU6B9n7W+LvzJhBBHaDzVJx2PRGXT367PbqyD/Vx0AW38o4CMQArYUEaC4ekadsp dXlv4JriQGrSVxYQLMJcREMwpX6iypPBU0CTrK5PWNiwxHvJKrx2yawtjcVsBUrjCjJqhd rYNnZ3tMAPpD5FAy5Jctttsa8xrqsV3QaRcxjbAUxp+g0ZdJ/QoXBgLGzfJTxxGoJ+43+R FQUIaafiXyZAJH3/pcXRDY675C8Bn6f8Sh5tl8klaebUZJJy/VJRdnP12u1BqQ== From: Morgan Willcock Date: Sat, 28 Sep 2024 20:05:26 +0100 Message-ID: <87y13blhu1.fsf@ice9.digital> User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-GND-Sasl: morgan@ice9.digital Received-SPF: pass client-ip=2001:4b98:dc4:8::224; envelope-from=morgan@ice9.digital; helo=relay4-d.mail.gandi.net X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-Spam-Score: -1.7 (-) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -2.7 (--) --=-=-= Content-Type: text/plain Tags: patch Attached is a patch which rewrites 'speedbar-expand-line-descendants'. The previous version could get into an infinite loop by reaching the maximum recursion depth, although in practice the slow speed meant that most people would probably abort the operation before reaching that point. The majority of the slowdown was because the motion commands being used were the variants which looked up information for every entry, displayed the information as a message, and adjusted the cursor position. The messages were not readable because of being continually overwritten. Here is a way to demonstrate that stack depth was increasing for items at the same level, that the messages were not readable, and how slow the whole process was: rm -rf /tmp/project mkdir /tmp/project for i in $(seq 1 50); do echo "(defun fun-$i ())" >> /tmp/project/file1.el; done for i in $(seq 1 50); do echo "(defun fun-$i ())" >> /tmp/project/file2.el; done emacs -Q \ --eval="(find-file \"/tmp/project/file1.el\")" \ --eval "(speedbar-get-focus)" \ --eval "(profiler-start 'cpu)" \ --eval "(speedbar-expand-line-descendants)" \ --eval "(profiler-stop)" \ --eval "(profiler-report)" ...that should expand every entry in file1.el and not touch the entries in file2.el. The replacement function is significantly faster. Messages are only used to indicate that the function is running and when it is finished - the result is similar to manually clicking every node open. Thanks, Morgan In GNU Emacs 30.0.91 (build 2, x86_64-pc-linux-gnu, X toolkit, cairo version 1.16.0, Xaw3d scroll bars) of 2024-09-12 built on inspiron Windowing system distributor 'The X.Org Foundation', version 11.0.12101007 System Description: Debian GNU/Linux 12 (bookworm) Configured using: 'configure --with-native-compilation=aot --with-xml2 --with-x-toolkit=lucid' --=-=-= Content-Type: text/patch Content-Disposition: attachment; filename=0001-Rewrite-speedbar-expansion-for-all-descendants.patch >From 0e25d28bfbef31c20ec22c2e508933b3824a8172 Mon Sep 17 00:00:00 2001 From: Morgan Willcock Date: Sat, 28 Sep 2024 19:11:11 +0100 Subject: [PATCH] Rewrite speedbar expansion for all descendants Rewrite 'speedbar-expand-line-descendants' to avoid getting into an infinite loop by reaching max-lisp-eval-depth. The new method avoids querying and displaying information for every movement, instead using a single message to indicate that expansion is in progress, and so is significantly faster. * lisp/speedbar.el (speedbar-expand-line-descendants): Use simpler line motion and no recursion. Output messages indicating when expansion is in progress and when it is completed. --- lisp/speedbar.el | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/lisp/speedbar.el b/lisp/speedbar.el index c13c977938b..11e11e1e56c 100644 --- a/lisp/speedbar.el +++ b/lisp/speedbar.el @@ -3172,21 +3172,22 @@ speedbar-expand-line-descendants "Expand the line under the cursor and all descendants. Optional argument ARG indicates that any cache should be flushed." (interactive "P") - (save-restriction - (narrow-to-region (line-beginning-position) - (line-beginning-position 2)) - (speedbar-expand-line arg) - ;; Now, inside the area expanded here, expand all subnodes of - ;; the same descendant type. - (save-excursion - (speedbar-next 1) ;; Move into the list. - (let ((err nil)) - (while (not err) - (condition-case nil - (progn - (speedbar-expand-line-descendants arg) - (speedbar-restricted-next 1)) - (error (setq err t)))))))) + (dframe-message "Expanding all descendants...") + (save-excursion + (with-restriction + ;; Narrow around the top-level item. + (line-beginning-position) + (condition-case nil + (save-excursion + (speedbar-restricted-move 1) + (line-beginning-position)) + (error (line-beginning-position 2))) + ;; Expand every line until the end of the restriction. + (while (zerop (progn + (speedbar-expand-line arg) + (forward-line 1)))))) + (dframe-message "Expanding all descendants...done") + (speedbar-position-cursor-on-line)) (defun speedbar-contract-line-descendants () "Expand the line under the cursor and all descendants." -- 2.39.5 --=-=-=-- From unknown Tue Jun 17 21:55:06 2025 X-Loop: help-debbugs@gnu.org Subject: bug#73533: [PATCH] Rewrite speedbar expansion for all descendants Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sun, 29 Sep 2024 04:47:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 73533 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch To: Morgan Willcock Cc: 73533@debbugs.gnu.org Received: via spool by 73533-submit@debbugs.gnu.org id=B73533.172758521115211 (code B ref 73533); Sun, 29 Sep 2024 04:47:02 +0000 Received: (at 73533) by debbugs.gnu.org; 29 Sep 2024 04:46:51 +0000 Received: from localhost ([127.0.0.1]:58927 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1sulpb-0003xG-6J for submit@debbugs.gnu.org; Sun, 29 Sep 2024 00:46:51 -0400 Received: from eggs.gnu.org ([209.51.188.92]:55196) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1sulpZ-0003wz-Fg for 73533@debbugs.gnu.org; Sun, 29 Sep 2024 00:46:50 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1sulox-0005cY-1k; Sun, 29 Sep 2024 00:46:11 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date: mime-version; bh=JB/dWYtCR0N1LRXjD1GkBCNChvP0s8lC3++1IE6BO78=; b=QNwQ/g3P3gB8 rbgLlZm9/st9H/XNyoBwYJYti0oSUNHS0bCeOz6OT7QP0yn2kaAIS159Wd1cJdyjWtjlHGGqAWwRX yd15W2F9o5pHQGg0EfHlabwcVbF2vF9N11rraDQYtSpxmbZ1tjO14DLFNK1RxrmfW7AWM1k4hEByS War8AD9WIswvHvIlB7bz4b17YX9xXHAeAEsT44lM80zmw+ByLeP4Cw0Oh9kMjnr3IcJjLchOJuvp7 OBVV2DWFrog5TGxm6yPPuVJ8Zmo1cD52wE0eazKXCKQvfUr7kdIwvQnuglM9me6eS4/6QOHfI9s6X DbjPBxCpI9qV4/Ao0rLG/A==; Date: Sun, 29 Sep 2024 07:46:08 +0300 Message-Id: <86a5fr5apb.fsf@gnu.org> From: Eli Zaretskii In-Reply-To: <87y13blhu1.fsf@ice9.digital> (message from Morgan Willcock on Sat, 28 Sep 2024 20:05:26 +0100) References: <87y13blhu1.fsf@ice9.digital> X-Spam-Score: -2.3 (--) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) > From: Morgan Willcock > Date: Sat, 28 Sep 2024 20:05:26 +0100 > > Attached is a patch which rewrites 'speedbar-expand-line-descendants'. > > The previous version could get into an infinite loop by reaching the > maximum recursion depth, although in practice the slow speed meant that > most people would probably abort the operation before reaching that > point. > > The majority of the slowdown was because the motion commands being used > were the variants which looked up information for every entry, displayed > the information as a message, and adjusted the cursor position. The > messages were not readable because of being continually overwritten. > > Here is a way to demonstrate that stack depth was increasing for items > at the same level, that the messages were not readable, and how slow the > whole process was: > > rm -rf /tmp/project > mkdir /tmp/project > for i in $(seq 1 50); do echo "(defun fun-$i ())" >> /tmp/project/file1.el; done > for i in $(seq 1 50); do echo "(defun fun-$i ())" >> /tmp/project/file2.el; done > emacs -Q \ > --eval="(find-file \"/tmp/project/file1.el\")" \ > --eval "(speedbar-get-focus)" \ > --eval "(profiler-start 'cpu)" \ > --eval "(speedbar-expand-line-descendants)" \ > --eval "(profiler-stop)" \ > --eval "(profiler-report)" > > ...that should expand every entry in file1.el and not touch the entries > in file2.el. > > The replacement function is significantly faster. Messages are only > used to indicate that the function is running and when it is finished - > the result is similar to manually clicking every node open. Thanks. Unfortunately, speedbar doesn't have a test suite, so I would like to ask how you tested this rewrite, and whether we could have some tests for this added to the test suite. Regardless, it would be good to have both old and the new code annotated with explanations of what each non-trivial line does, to allow independent verification of the correctness of the rewrite by people who are not familiar with speedbar code and don't immediately understand the effects of a call to speedbar-naxt or speedbar-restricted-move. Also, this comment in the old code bothers me: > - ;; Now, inside the area expanded here, expand all subnodes of > - ;; the same descendant type. What does it mean by "the same descendant type", and how does the old and the new code make sure they expand only those descendants? From unknown Tue Jun 17 21:55:06 2025 X-Loop: help-debbugs@gnu.org Subject: bug#73533: [PATCH] Rewrite speedbar expansion for all descendants Resent-From: Morgan Willcock Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sun, 29 Sep 2024 09:54:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 73533 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch To: Eli Zaretskii Cc: 73533@debbugs.gnu.org Received: via spool by 73533-submit@debbugs.gnu.org id=B73533.172760360114138 (code B ref 73533); Sun, 29 Sep 2024 09:54:02 +0000 Received: (at 73533) by debbugs.gnu.org; 29 Sep 2024 09:53:21 +0000 Received: from localhost ([127.0.0.1]:40123 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1suqcC-0003fx-Gf for submit@debbugs.gnu.org; Sun, 29 Sep 2024 05:53:21 -0400 Received: from relay6-d.mail.gandi.net ([217.70.183.198]:47767) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1suqby-0003cn-Ez for 73533@debbugs.gnu.org; Sun, 29 Sep 2024 05:53:18 -0400 Received: by mail.gandi.net (Postfix) with ESMTPSA id D0A77C0002; Sun, 29 Sep 2024 09:52:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ice9.digital; s=gm1; t=1727603529; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=sQruOnqH9CM49bTgvXCsLGFFI3VJlTZaz+Z6NXMWBP0=; b=PwZWxWJgub0b1PFdhB5XkDw/Z7xwCPA2CFv9KSD3ntsWvJvMrkJtTs74gWZa+v2aC1OOdo 1N/jUJ+k7O1MKK33ELyZ9IV68UuGwNeBSvmxPMDoa+KwzH57CTX9a4litO+oj4Kfj0UAs+ 9q4nR51066hj4o5acg66xC6NeT610pSgYnjNdZZTfVjV1Lt8Mx08p/229iLP2zQ4rlnLfe y8ZUabyoADZDUGGgr+H+zMNJxYUO54EOVcrhXSxVVyXI9wGE5UpCpmgdFi9FLZHEOs6erQ c9sO+sd1Zx3+UTUzn2Z101UDTzU5rKgCpV66SMzJZ5YVy8E1Pu1SgBeVzOkaKA== From: Morgan Willcock In-Reply-To: <86a5fr5apb.fsf@gnu.org> (Eli Zaretskii's message of "Sun, 29 Sep 2024 07:46:08 +0300") References: <87y13blhu1.fsf@ice9.digital> <86a5fr5apb.fsf@gnu.org> Date: Sun, 29 Sep 2024 10:52:07 +0100 Message-ID: <87v7yeeqig.fsf@ice9.digital> User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: text/plain X-GND-Sasl: morgan@ice9.digital X-Spam-Score: 0.0 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) Eli Zaretskii writes: > Thanks. Unfortunately, speedbar doesn't have a test suite, so I would > like to ask how you tested this rewrite, and whether we could have > some tests for this added to the test suite. I've been testing by expanding all descendent items interactively, either with test files which generate a lot of items or within the Emacs source tree. I wouldn't be against adding tests but it doesn't look like there are many ways to query the speedbar state, which is probably why the original code is not testing whether nodes are expandable before trying to expand them. Both the original and the new code and fundamentally doing the same thing by assuming that each new line may be expanded and trying to expand it - moving "into" a sub-list is just moving forward because if expansion was possible it has occurred. I could probably craft a test which would demonstrate the old code hitting a recursion limit, but fundamentally the problem was the speed and excessive message output. > Regardless, it would be good to have both old and the new code > annotated with explanations of what each non-trivial line does, to > allow independent verification of the correctness of the rewrite by > people who are not familiar with speedbar code and don't immediately > understand the effects of a call to speedbar-naxt or > speedbar-restricted-move. speedbar-next = forward-line + display message for item + reposition cursor speedbar-restricted-move = move to the next item in the list at the current level or signal an error if the end of the list is reached speedbar-restricted-next = speedbar-restricted-move + display message for item Here is the original code my comments instead of the original comments: (defun speedbar-expand-line-descendants (&optional arg) "Expand the line under the cursor and all descendants. Optional argument ARG indicates that any cache should be flushed." (interactive "P") (save-restriction ;; The recursion of the original code means that sibling items below ;; the first item were also being accidentally expanded. Narrow the ;; buffer around the current item to prevent opening all sibling ;; items. bug#35014 (narrow-to-region (line-beginning-position) (line-beginning-position 2)) ;; Assume that the current line can be expanded. (speedbar-expand-line arg) (save-excursion ;; Move forward. Where current line did not expand this ;; effectively moves to the next sibling or the end of the buffer ;; restriction. Display a description of the newly moved to item ;; as a message. Readjust the column position of point. (speedbar-next 1) ;; Loop across all siblings until an error is signalled. (let ((err nil)) (while (not err) (condition-case nil (progn ;; Assume that the item is expandable and recursively ;; expand it. (speedbar-expand-line-descendants arg) ;; Move to the next item at the current level or signal ;; an error. Display a description of the newly moved ;; to item as a message. (speedbar-restricted-next 1)) (error (setq err t)))))))) To stop the continual stream of messages and cursor repositioning of calling speedbar-next, it can just be replaced with forward-line because that is all speedbar-next does to move. To stop the continual stream of messages from speedbar-restricted-next it can just be replaced with speedbar-restricted-move which is exactly the same thing but without the message output. So the majority of the speedup is just done by those two replacements, and all that should have changed is that now no messages are being produced. To make it non-recursive and add the status message to indicate that something is happening, what I've actually done is change the fix which was applied for bug#35014. Instead of narrowing recursively to avoid accidentally running into siblings, the narrowing is setup once by using speedbar-restricted-move to find the next sibling. The logic for expansion is effectively still the same as a depth first traversal but without the messages being generated: (defun speedbar-expand-line-descendants (&optional arg) "Expand the line under the cursor and all descendants. Optional argument ARG indicates that any cache should be flushed." (interactive "P") (dframe-message "Expanding all descendants...") (save-excursion (with-restriction ;; Narrow around the top-level item to ensure that later sibling ;; items will not be entered. (line-beginning-position) (condition-case nil (save-excursion (speedbar-restricted-move 1) ;; The line beginning position of the next sibling item. (line-beginning-position)) ;; This was the last sibling item so just apply the ;; restriction around the current item, in the same way that ;; the previous fix for bug#35014 did. (error (line-beginning-position 2))) ;; Expand every line until the end of the restriction. (while (zerop (progn ;; Assume that the line will expand and try to ;; expand it. (speedbar-expand-line arg) ;; Moving forwards will be moving into the ;; expanded lists if one opened, or moving to a ;; sibling or end of restriction if there was no ;; expansion. (forward-line 1)))))) (dframe-message "Expanding all descendants...done") ;; Reposition point to match the previous behavior. (speedbar-position-cursor-on-line)) > Also, this comment in the old code bothers me: > >> - ;; Now, inside the area expanded here, expand all subnodes of >> - ;; the same descendant type. > > What does it mean by "the same descendant type", and how does the old > and the new code make sure they expand only those descendants? I don't know what it means by "type" because the code only deals with siblings and children. I think it means depth-first expansion of all siblings. The old code pre-bug#35014 did not ensure that it only expanded descendants and just ran until the end of the buffer. The old code with the fix for bug#35014 narrowed each item recursively with the narrowing around the top level preventing a top-level sibling being reached. The new code just sets up the narrowing once around the top-level item and then effectively runs the same expansion logic but without the item look and message output. The messages were being generated by calling speedbar-item-info, and I cannot see how calling that repeatedly during expansion is required when someone can just click the nodes open with the mouse and also skip those calls. I can re-send the patch with additional comments (more similar to the above) if that helps. Thanks, Morgan -- Morgan Willcock From debbugs-submit-bounces@debbugs.gnu.org Sun Sep 29 21:36:12 2024 Received: (at control) by debbugs.gnu.org; 30 Sep 2024 01:36:12 +0000 Received: from localhost ([127.0.0.1]:43455 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1sv5Kd-0007o1-IE for submit@debbugs.gnu.org; Sun, 29 Sep 2024 21:36:12 -0400 Received: from mail-ed1-f46.google.com ([209.85.208.46]:51244) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1sv5KP-0007k5-Dh for control@debbugs.gnu.org; Sun, 29 Sep 2024 21:35:58 -0400 Received: by mail-ed1-f46.google.com with SMTP id 4fb4d7f45d1cf-5c874f6f119so4975457a12.2 for ; Sun, 29 Sep 2024 18:35:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1727660060; x=1728264860; darn=debbugs.gnu.org; h=to:subject:message-id:date:mime-version:from:from:to:cc:subject :date:message-id:reply-to; bh=yBjk0znSNK1FUAMtc3o3ibC+jcLFAIAIqFFmtgyAmSo=; b=YJeB1fIC7qMGJYshszvQtUJYe0FEcBl0uFMfpOC0zmRQ/iTkuiJYp1wsvLn19R0FkC gC7vvyCkJ5W9aV7WQLm0bFnXIm6APdvZmQuTVmLpj9dj1T4Cqh8x8L8Ab6NNZ7ikVU0/ soED56FCe2m40+zEksPTYGObBeSMuKyktKuMWqrxjPa14hd2Sjud3zVMxIayuiPW2MYZ 26xso/R+woMjZl1sdoZ9eQ5XMvhIkaLEbDD5AqbByPaIy+AFaUGCMzttv3jDKcyUNb9q emONX0QIBxOg7rZBexF8WJDuyTadaTSpEw/WE+wG16Y1qkBxyMIZ9Dujj/1VmYCbuXfq n68Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1727660060; x=1728264860; h=to:subject:message-id:date:mime-version:from:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=yBjk0znSNK1FUAMtc3o3ibC+jcLFAIAIqFFmtgyAmSo=; b=AyMyKNzduNdTdUMDCLLdVtnIE49meQHqSL6NlaIHk3revX37TlWUcOEENZfxnfhONn wBIG6epKDuHSuTPu5WZ0hu17usPOcaqIj16HnI7EUwtU59oSWfncuJdb3d5FHlQGSYnA nS6fFnd+DtB+awUyRDg8HkLjCZmxWE5Q73+W1A7DUBEufz6xC4ZygrH4e1/h67oo2TCs x4IKRB7Xs/KXoPLCq9ND7tcByiHjzlw0eCUAUsT8bLRBAobJDclxz4GK+xXd98m51R7I W49KELZYmANJTHqgqV9rFsbHGTnEbfJBVcuDEgFjtVZezjmHHKatGbj6VMbUMOeXrDvz TOmA== X-Gm-Message-State: AOJu0YzFEIQVHFEkbMO5nDdgKXum2zMpUDuIX+XNwaUddRGR1brUIIxO j7MwpwVsiXssLSinbQTvMTenIlj1bHOWx9+BjeQbBpCCkyklAvkSUIYBGJjSCQeJOCdwz+RSzvI 4vnpEEtrFyzK0Ycjp1Gcx3FBjBUrDRw== X-Google-Smtp-Source: AGHT+IHQ8NIBabdIA29DOJzzAhw3Jurse91wbM7cMLc1VaF7j+MTCtaBHuzYSbRy7bDadR7gKhZBM+/oaU8J76JNJ+A= X-Received: by 2002:a05:6402:24a4:b0:5c8:79fa:2e3e with SMTP id 4fb4d7f45d1cf-5c882603bd9mr11131771a12.28.1727660059526; Sun, 29 Sep 2024 18:34:19 -0700 (PDT) Received: from 753933720722 named unknown by gmailapi.google.com with HTTPREST; Sun, 29 Sep 2024 18:34:19 -0700 From: Stefan Kangas MIME-Version: 1.0 Date: Sun, 29 Sep 2024 18:34:19 -0700 Message-ID: Subject: control message for bug #73533 To: control@debbugs.gnu.org Content-Type: text/plain; charset="UTF-8" X-Spam-Score: -0.9 (/) X-Debbugs-Envelope-To: control X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.9 (-) severity 73533 wishlist quit From unknown Tue Jun 17 21:55:06 2025 X-Loop: help-debbugs@gnu.org Subject: bug#73533: [PATCH] Rewrite speedbar expansion for all descendants Resent-From: Morgan Willcock Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 30 Sep 2024 09:05:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 73533 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch To: Eli Zaretskii Cc: 73533@debbugs.gnu.org Received: via spool by 73533-submit@debbugs.gnu.org id=B73533.172768706126918 (code B ref 73533); Mon, 30 Sep 2024 09:05:02 +0000 Received: (at 73533) by debbugs.gnu.org; 30 Sep 2024 09:04:21 +0000 Received: from localhost ([127.0.0.1]:44701 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1svCKL-000706-92 for submit@debbugs.gnu.org; Mon, 30 Sep 2024 05:04:21 -0400 Received: from relay7-d.mail.gandi.net ([217.70.183.200]:47913) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1svCKI-0006zx-Uv for 73533@debbugs.gnu.org; Mon, 30 Sep 2024 05:04:19 -0400 Received: by mail.gandi.net (Postfix) with ESMTPSA id A39212000E; Mon, 30 Sep 2024 09:03:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ice9.digital; s=gm1; t=1727687000; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=c9j6lHEM6M8Cdj2l5kENzQJbKHKkw2qso0xQKsV8Qwc=; b=Vmm3Szg0y8+53fB5joJWmYfOLOaDZYXVI+vY0KVAAaTkeHABiURG1K+QrR+YdnylSTX/+b ZiKIj3F6kqNE1l4ujpTAADMbtkUO2U2UrMNaRhPES1QkEbIVC7hJD1ikQOt7X+EdCEEChi SHE1rqVuXqldzLrm4TOoPI5a+EE6dhGKhip8yi6Ry3HLxeypqIcRr7dPh3uuvzCbUlZIoX HUt/0sHl1KFeAst+08X2Bjgvmmc4ipmiHDBY6EV9k1bI7sDSy7uUQ/8vbsIriIQI+9iV2p Eaizaow5+vsqXMym/mJXZol4QzxzIuEB0KhGW4Vpr8LgmIFbM09351BAqry3Bg== From: Morgan Willcock In-Reply-To: <87v7yeeqig.fsf@ice9.digital> (Morgan Willcock's message of "Sun, 29 Sep 2024 10:52:07 +0100") References: <87y13blhu1.fsf@ice9.digital> <86a5fr5apb.fsf@gnu.org> <87v7yeeqig.fsf@ice9.digital> Date: Mon, 30 Sep 2024 10:03:18 +0100 Message-ID: <87wmitfr8p.fsf@ice9.digital> User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-GND-Sasl: morgan@ice9.digital X-Spam-Score: 0.0 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) --=-=-= Content-Type: text/plain Morgan Willcock writes: > The old code with the fix for bug#35014 narrowed each item recursively > with the narrowing around the top level preventing a top-level sibling > being reached. The change that was made in bug#35014 looks like it broke the expansion when the item was already partially expanded, so attached is a modified version of the patch that completely removes the narrowing-per-item approach and also includes more comments. Thanks, Morgan -- Morgan Willcock --=-=-= Content-Type: text/x-diff Content-Disposition: attachment; filename=0001-Rewrite-speedbar-expansion-for-all-descendants.patch >From aa17e562b9844a50b8b01777c83830d3c4ead963 Mon Sep 17 00:00:00 2001 From: Morgan Willcock Date: Sat, 28 Sep 2024 19:11:11 +0100 Subject: [PATCH] Rewrite speedbar expansion for all descendants Rewrite 'speedbar-expand-line-descendants' to avoid getting into an infinite loop by reaching max-lisp-eval-depth. The new method avoids querying and displaying information for every movement, instead using a single message to indicate that expansion is in progress, and so is significantly faster. The narrowing per item introduced by the fix for bug#35014 is removed because it prevented expanded descendant items when the top-level item was already expanded. * lisp/speedbar.el (speedbar-expand-line-descendants): Use simpler line motion and no recursion. Output messages indicating when expansion is in progress and when it is completed. Fix expansion of descendants where the top-level item was already expanded. --- lisp/speedbar.el | 45 ++++++++++++++++++++++++++++++--------------- 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/lisp/speedbar.el b/lisp/speedbar.el index c13c977938b..723a5595854 100644 --- a/lisp/speedbar.el +++ b/lisp/speedbar.el @@ -3172,21 +3172,36 @@ speedbar-expand-line-descendants "Expand the line under the cursor and all descendants. Optional argument ARG indicates that any cache should be flushed." (interactive "P") - (save-restriction - (narrow-to-region (line-beginning-position) - (line-beginning-position 2)) - (speedbar-expand-line arg) - ;; Now, inside the area expanded here, expand all subnodes of - ;; the same descendant type. - (save-excursion - (speedbar-next 1) ;; Move into the list. - (let ((err nil)) - (while (not err) - (condition-case nil - (progn - (speedbar-expand-line-descendants arg) - (speedbar-restricted-next 1)) - (error (setq err t)))))))) + (dframe-message "Expanding all descendants...") + (save-excursion + (with-restriction + ;; Narrow around the top-level item to ensure that later sibling + ;; items will not be entered. + (line-beginning-position) + (condition-case nil + (save-excursion + (speedbar-restricted-move 1) + ;; Use the line beginning position of the next sibling + ;; item to apply the restriction. + (line-beginning-position)) + ;; This was the last sibling item so just apply the + ;; restriction to the end of the buffer. This fixes the + ;; change applied in bug#35014 which prevented the top-level + ;; item from having its descendants expanded if it was already + ;; expanded. + (error (point-max))) + ;; Expand every line until the end of the restriction is reached. + (while (zerop (progn + ;; Assume that the line will expand and try to + ;; expand it. + (speedbar-expand-line arg) + ;; Moving forwards will be moving into the + ;; expanded lists if one opened, into an already + ;; expanded list if it was already open, to a + ;; sibling, or to the end of restriction. + (forward-line 1)))))) + (dframe-message "Expanding all descendants...done") + (speedbar-position-cursor-on-line)) (defun speedbar-contract-line-descendants () "Expand the line under the cursor and all descendants." -- 2.39.5 --=-=-=-- From unknown Tue Jun 17 21:55:06 2025 X-Loop: help-debbugs@gnu.org Subject: bug#73533: [PATCH] Rewrite speedbar expansion for all descendants Resent-From: Morgan Willcock Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 30 Sep 2024 18:32:03 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 73533 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch To: Eli Zaretskii Cc: 73533@debbugs.gnu.org Received: via spool by 73533-submit@debbugs.gnu.org id=B73533.172772107230396 (code B ref 73533); Mon, 30 Sep 2024 18:32:03 +0000 Received: (at 73533) by debbugs.gnu.org; 30 Sep 2024 18:31:12 +0000 Received: from localhost ([127.0.0.1]:46258 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1svLAp-0007tW-5c for submit@debbugs.gnu.org; Mon, 30 Sep 2024 14:31:11 -0400 Received: from relay4-d.mail.gandi.net ([217.70.183.196]:55811) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1svL8j-00075i-AD for 73533@debbugs.gnu.org; Mon, 30 Sep 2024 14:29:40 -0400 Received: by mail.gandi.net (Postfix) with ESMTPSA id DD0C4E0002; Mon, 30 Sep 2024 18:28:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ice9.digital; s=gm1; t=1727720895; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=pE6Le+bgXmhtAa+VqUzqvOhqgW1751b6Do8UKqTj0AA=; b=UFYmBcxHgQGBJ36Gb6LRYewxjHfetRqljBCsOhSH1y6RKus8evC/clqNeOPdPUo7PatNv1 LmrOHoWyKEztuANxA7YzC+S29CQlE89PSHEherhOQGWinH9AVBxNgeHSnM8EPhLSYryp69 GC8WGGg51AAoCOGjHCCMMysv7oDSzRLZTntciHfdtb2CnnEwfb4Bt9ZSUcoXO1Urd6gXfW 18kSSJtflFgqwcWUQqZX5GK1+VjW0wE5hWD++FMTwneRj5YeI5OGNP/oY2Wzt9s/4G0c0/ OMo2wMxZZGehd1Zaig9Wbze4thAxP1LdQEqy7iDPZKWmkJZrBFGQXNFTMMvQ+g== From: Morgan Willcock In-Reply-To: <87wmitfr8p.fsf@ice9.digital> (Morgan Willcock's message of "Mon, 30 Sep 2024 10:03:18 +0100") References: <87y13blhu1.fsf@ice9.digital> <86a5fr5apb.fsf@gnu.org> <87v7yeeqig.fsf@ice9.digital> <87wmitfr8p.fsf@ice9.digital> Date: Mon, 30 Sep 2024 19:28:12 +0100 Message-ID: <8734lhgfnn.fsf@ice9.digital> User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: text/plain X-GND-Sasl: morgan@ice9.digital X-Spam-Score: 0.0 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) Morgan Willcock writes: > The change that was made in bug#35014 looks like it broke the expansion > when the item was already partially expanded, so attached is a modified > version of the patch that completely removes the narrowing-per-item > approach and also includes more comments. Unfortunately the narrowing trick causes problems in some situations, either the previous way from bug#35014 or using the result of point-max (either too few entries or expanded to too many). I'll try to an alternative way to move through the entries. -- Morgan Willcock From unknown Tue Jun 17 21:55:06 2025 X-Loop: help-debbugs@gnu.org Subject: bug#73533: [PATCH] Rewrite speedbar expansion for all descendants Resent-From: Morgan Willcock Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 08 Oct 2024 18:37:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 73533 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch To: Eli Zaretskii Cc: 73533@debbugs.gnu.org Received: via spool by 73533-submit@debbugs.gnu.org id=B73533.172841258627188 (code B ref 73533); Tue, 08 Oct 2024 18:37:01 +0000 Received: (at 73533) by debbugs.gnu.org; 8 Oct 2024 18:36:26 +0000 Received: from localhost ([127.0.0.1]:54429 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1syF4L-00074R-N4 for submit@debbugs.gnu.org; Tue, 08 Oct 2024 14:36:26 -0400 Received: from relay2-d.mail.gandi.net ([217.70.183.194]:43881) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1syF4I-00074A-Ke for 73533@debbugs.gnu.org; Tue, 08 Oct 2024 14:36:24 -0400 Received: by mail.gandi.net (Postfix) with ESMTPSA id CFC5440003; Tue, 8 Oct 2024 18:36:06 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ice9.digital; s=gm1; t=1728412567; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=gzj8wSgPkKc3Q1NRnbmuCuhINchtvTSF3D1vjR26i3A=; b=CoFPnl6sCbfE6vuzk5GeRkUk0I02YgmmFJCaOTC3l6j7KT1RxTf76KrW0fOcKV/aWmXYlB aOgOLOnx4Q7gk4ZXhvDsqkeLDVa/exJy5Y3rQMF5eHh5EASMIQa0dJn8zruofd8HxNNOOE w3O1VP7OAeO2rLfZc2EkWtw4kYys2xy5O94ErKQTroJqU8KYcpiX27DbAV7S1m4BXDw7cX moxV4eqIQlmALf5yqZyXv1DBQqktEYQS4pUJCRGGmxV5bA1H3R8YOq73Mbyk5V8RJwcJlX oZMAB2HvuJo7b8DMhL0bir8+qI706WvBzYJKfZGRy0lzyN18vITSDBWdpLmpgg== From: Morgan Willcock In-Reply-To: <8734lhgfnn.fsf@ice9.digital> (Morgan Willcock's message of "Mon, 30 Sep 2024 19:28:12 +0100") References: <87y13blhu1.fsf@ice9.digital> <86a5fr5apb.fsf@gnu.org> <87v7yeeqig.fsf@ice9.digital> <87wmitfr8p.fsf@ice9.digital> <8734lhgfnn.fsf@ice9.digital> Date: Tue, 08 Oct 2024 19:36:06 +0100 Message-ID: <87ttdm4f3d.fsf@ice9.digital> User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-GND-Sasl: morgan@ice9.digital X-Spam-Score: -0.7 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.7 (-) --=-=-= Content-Type: text/plain Attached is a patch for a replacement function and an additional patch which adds tests for expanding all descendants. I couldn't see a direct way to test a Speedbar without creating a frame, and because the Speedbar code doesn't implement any type of hierarchy itself I've used eieio-speedbar to make something minimal to test. Each test runs by converting a list of strings into objects which implement a Speedbar display, making state changes for the functions being tested, and then converting the objects back into strings - this was the simplest way I could find to create a reusable interface for tests. The conversion back to a string is customizable to allow whatever state change is under test to be represented. If the tests are problematic or considered too complicated, perhaps they can just be used to test the current function and the replacement function and not committed. The current function fails 3/9 tests. -- Morgan Willcock --=-=-= Content-Type: text/x-diff Content-Disposition: attachment; filename=0001-Rewrite-speedbar-expansion-for-all-descendants.patch >From 50c623de5e4ac3a101a2fa19de97f75132063554 Mon Sep 17 00:00:00 2001 From: Morgan Willcock Date: Sat, 5 Oct 2024 18:33:51 +0100 Subject: [PATCH 1/2] Rewrite speedbar expansion for all descendants Rewrite 'speedbar-expand-line-descendants' to avoid getting into an infinite loop by reaching max-lisp-eval-depth. The new method avoids querying and displaying information for every movement, instead using a single message to indicate that expansion is in progress, and so is significantly faster. The narrowing per item introduced by the fix for bug#35014 is removed because it prevented expanded descendant items when the top-level item was already expanded. * lisp/speedbar.el (speedbar--get-line-indent-level): New function to return the indentation level of the current line. (speedbar-expand-line-descendants): Use simpler line motion and no recursion. Output messages indicating when expansion is in progress and when it is completed. Fix expansion of descendants where the top-level item was already expanded. --- lisp/speedbar.el | 37 ++++++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/lisp/speedbar.el b/lisp/speedbar.el index c13c977938b..38fb641acf7 100644 --- a/lisp/speedbar.el +++ b/lisp/speedbar.el @@ -3168,25 +3168,32 @@ speedbar-toggle-line-expansion (speedbar-do-function-pointer)) (error (speedbar-position-cursor-on-line)))) +(defun speedbar--get-line-indent-level () + "Return the indentation level of the current line." + (save-excursion + (beginning-of-line) + (if (looking-at "[0-9]+:") + (string-to-number (match-string 0)) + 0))) + (defun speedbar-expand-line-descendants (&optional arg) "Expand the line under the cursor and all descendants. Optional argument ARG indicates that any cache should be flushed." (interactive "P") - (save-restriction - (narrow-to-region (line-beginning-position) - (line-beginning-position 2)) - (speedbar-expand-line arg) - ;; Now, inside the area expanded here, expand all subnodes of - ;; the same descendant type. - (save-excursion - (speedbar-next 1) ;; Move into the list. - (let ((err nil)) - (while (not err) - (condition-case nil - (progn - (speedbar-expand-line-descendants arg) - (speedbar-restricted-next 1)) - (error (setq err t)))))))) + (dframe-message "Expanding all descendants...") + (save-excursion + (let ((top-depth (speedbar--get-line-indent-level))) + ;; Attempt to expand the top-level item. + (speedbar-expand-line arg) + ;; Move forwards, either into the newly expanded list, onto an + ;; already expanded list, onto a sibling item, or to the end of + ;; the buffer. + (while (and (zerop (forward-line 1)) + (not (eobp)) + (> (speedbar--get-line-indent-level) top-depth) + (speedbar-expand-line arg))))) + (dframe-message "Expanding all descendants...done") + (speedbar-position-cursor-on-line)) (defun speedbar-contract-line-descendants () "Expand the line under the cursor and all descendants." -- 2.39.5 --=-=-= Content-Type: text/x-diff Content-Disposition: attachment; filename=0002-Add-initial-Speedbar-tests.patch >From eb4c17fbe64542a9877cd16753cda29188c700a0 Mon Sep 17 00:00:00 2001 From: Morgan Willcock Date: Tue, 8 Oct 2024 17:34:20 +0100 Subject: [PATCH 2/2] Add initial Speedbar tests Add initial Speedbar tests which test the operation of 'speedbar-expand-line-descendants'. * test/lisp/speedbar-tests.el (speedbar-tests-container) (eieio-speedbar-object-children) (speedbar-tests-item) (speedbar-tests--make-object) (speedbar-tests--setup-strings) (speedbar-tests--object-hierarchy) (speedbar-tests--base-items) (speedbar-tests--clean-up) (speedbar-tests--initialize) (speedbar-tests--object-name-expanded) (speedbar-tests--object-name-function) (speedbar-tests--objects-as-strings) (speedbar-tests--state-test) (speedbar-tests--expand-descendants-single) (speedbar-tests--expand-descendants-nested) (speedbar-tests--expand-descendants-nested-wide) (speedbar-tests--expand-descendants-of-first) (speedbar-tests--expand-descendants-of-first-expanded) (speedbar-tests--expand-descendants-of-last) (speedbar-tests--expand-descendants-of-last-expanded) (speedbar-tests--expand-descendants-of-middle) (speedbar-tests--expand-descendants-of-middle-expanded): Test 'speedbar-expand-line-descendants'. --- test/lisp/speedbar-tests.el | 318 ++++++++++++++++++++++++++++++++++++ 1 file changed, 318 insertions(+) create mode 100644 test/lisp/speedbar-tests.el diff --git a/test/lisp/speedbar-tests.el b/test/lisp/speedbar-tests.el new file mode 100644 index 00000000000..5450d211b1a --- /dev/null +++ b/test/lisp/speedbar-tests.el @@ -0,0 +1,318 @@ +;;; speedbar-tests.el --- Tests for speedbar.el -*- lexical-binding: t -*- + +;; Copyright (C) 2024 Free Software Foundation, Inc. + +;; This file is part of GNU Emacs. + +;; GNU Emacs is free software: you can redistribute it and/or modify +;; it under the terms of the GNU General Public License as published by +;; the Free Software Foundation, either version 3 of the License, or +;; (at your option) any later version. + +;; GNU Emacs is distributed in the hope that it will be useful, +;; but WITHOUT ANY WARRANTY; without even the implied warranty of +;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +;; GNU General Public License for more details. + +;; You should have received a copy of the GNU General Public License +;; along with GNU Emacs. If not, see . + +;;; Commentary: + +;;; Code: + +(require 'ert) +(require 'eieio-base) +(require 'eieio-speedbar) + +(defclass speedbar-tests-container (eieio-named eieio-speedbar-file-button) + ((child-items :initarg :child-items + :type list)) + "An expandable Speedbar item which can contain other items.") + +(cl-defmethod eieio-speedbar-object-children ((item speedbar-tests-container)) + "Return the list of child items for ITEM." + (slot-value item 'child-items)) + +(defclass speedbar-tests-item (eieio-named eieio-speedbar) + nil + "A Speedbar item which cannot contain other items.") + +(defun speedbar-tests--make-object (item-spec) + "Return an object representing a Speedbar item. + +The object is constructed based on the specification ITEM-SPEC which +should be a cons pair of the form (NAME . CHILD-ITEMS). NAME is a +string which will be used for display purposes. CHILD-ITEMS is a list +of additional ITEM-SPEC values which will be referenced as children." + (let ((name (car item-spec)) + (child-items (cdr item-spec))) + (unless (stringp name) + (error "Item name must be a string")) + (unless (listp child-items) + (error "Child-items must be a list")) + (if child-items + (speedbar-tests-container + :object-name name + :child-items (mapcar #'speedbar-tests--make-object + child-items)) + (speedbar-tests-item + :object-name name)))) + +(defvar speedbar-tests--setup-strings nil + "An alist of strings which represents a hierarchy of Speedbar items.") + +(defvar speedbar-tests--object-hierarchy nil + "The current object hierarchy for the Speedbar being tested.") + +(defun speedbar-tests--base-items (_directory) + "Return the list of top-level objects for the Speedbar." + (setq speedbar-tests--object-hierarchy + (mapcar #'speedbar-tests--make-object + speedbar-tests--setup-strings))) + +(eieio-speedbar-create #'eieio-speedbar-make-map + 'eieio-speedbar-key-map + 'eieio-speedbar-menu + "Test" + #'speedbar-tests--base-items) + +(defun speedbar-tests--clean-up () + "Clean-up after Speedbar test." + (when (framep speedbar-frame) + (delete-frame speedbar-frame))) + +(defun speedbar-tests--initialize () + "Initialize a Speedbar for testing." + (speedbar-get-focus) + (speedbar-change-initial-expansion-list "Test")) + +(defun speedbar-tests--object-name-expanded (object) + "Return the string name of OBJECT when it is expanded." + (let ((name (eieio-speedbar-object-buttonname object))) + (if (slot-value object 'expanded) + (concat name "+") + name))) + +(defvar speedbar-tests--object-name-function + #'eieio-speedbar-object-buttonname + "The function which returns the string representation of an object.") + +(defun speedbar-tests--objects-as-strings (object-list) + "Return the object hierarchy OBJECT-LIST as an alist of strings. + +The string used to represent the object is determined by the function +bound to `speedbar-tests--object-name-function' is a function, which +should accept the object as the only argument and return a string to use +as the name." + (mapcar (lambda (object) + (let ((name (funcall speedbar-tests--object-name-function + object)) + (child-items (eieio-speedbar-object-children + object))) + (cons name (speedbar-tests--objects-as-strings + child-items)))) + object-list)) + +(cl-defmacro speedbar-tests--state-test + ((&optional &key setup final name-function) &rest body) + "Evaluate BODY and verify the Speedbar is in an expected state. + +`:setup' specifies an alist of strings which will be used to create an +object hierarchy used for the Speedbar display. + +`:final' specifies an alist of strings which should represent the final +Speedbar state once BODY has been evaluated and the object hierarchy has +been converted back to an alist of strings. `:name-function' specifies +the function to use to generate a string from an object, which should +accept the object as an argument and return a string which represents +the object as well as its state." + (declare (indent 1)) + (let ((let-vars `((speedbar-tests--setup-strings ',setup)))) + (when name-function + (push `(speedbar-tests--object-name-function #',name-function) + let-vars)) + `(unwind-protect + (let ,let-vars + (speedbar-tests--initialize) + (should (equal (speedbar-tests--objects-as-strings + speedbar-tests--object-hierarchy) + ',setup)) + ,@body + (should (equal (speedbar-tests--objects-as-strings + speedbar-tests--object-hierarchy) + ',final))) + (speedbar-tests--clean-up)))) + +(ert-deftest speedbar-tests--expand-descendants-single () + "Expand the first item." + (skip-when noninteractive) + (speedbar-tests--state-test + ( :setup (("A" . (("A1")))) + :final (("A+" . (("A1")))) + :name-function speedbar-tests--object-name-expanded) + (with-current-buffer speedbar-buffer + (goto-char (point-min)) + (should (string-equal "A" (speedbar-line-text))) + (speedbar-expand-line-descendants 'nocache)))) + +(ert-deftest speedbar-tests--expand-descendants-nested () + "Expand the first item and its only child." + (skip-when noninteractive) + (speedbar-tests--state-test + ( :setup (("A" . (("A1" . (("A1A")))))) + :final (("A+" . (("A1+" . (("A1A")))))) + :name-function speedbar-tests--object-name-expanded) + (with-current-buffer speedbar-buffer + (goto-char (point-min)) + (should (string-equal "A" (speedbar-line-text))) + (speedbar-expand-line-descendants 'nocache)))) + +(ert-deftest speedbar-tests--expand-descendants-nested-wide () + "Expand all descendants of first item which has multiple children." + (skip-when noninteractive) + (speedbar-tests--state-test + ( :setup (("A" . (("A1" . (("A1A"))) + ("A2" . (("A2A")))))) + :final (("A+" . (("A1+" . (("A1A"))) + ("A2+" . (("A2A")))))) + :name-function speedbar-tests--object-name-expanded) + (with-current-buffer speedbar-buffer + (goto-char (point-min)) + (should (string-equal "A" (speedbar-line-text))) + (speedbar-expand-line-descendants 'nocache)))) + +(ert-deftest speedbar-tests--expand-descendants-of-first () + "Expand the first item and all descendants." + (skip-when noninteractive) + (speedbar-tests--state-test + ( :setup (("A" . (("A1" . (("A1A"))) + ("A2" . (("A2A"))))) + ("B" . (("B1" . (("B1B"))) + ("B2" . (("B2B")))))) + :final (("A+" . (("A1+" . (("A1A"))) + ("A2+" . (("A2A"))))) + ("B" . (("B1" . (("B1B"))) + ("B2" . (("B2B")))))) + :name-function speedbar-tests--object-name-expanded) + (with-current-buffer speedbar-buffer + (goto-char (point-min)) + (should (string-equal "A" (speedbar-line-text))) + (speedbar-expand-line-descendants 'nocache)))) + +(ert-deftest speedbar-tests--expand-descendants-of-first-expanded () + "Expand the already expanded first item and all descendants." + (skip-when noninteractive) + (speedbar-tests--state-test + ( :setup (("A" . (("A1" . (("A1A"))) + ("A2" . (("A2A"))))) + ("B" . (("B1" . (("B1B"))) + ("B2" . (("B2B")))))) + :final (("A+" . (("A1+" . (("A1A"))) + ("A2+" . (("A2A"))))) + ("B" . (("B1" . (("B1B"))) + ("B2" . (("B2B")))))) + :name-function speedbar-tests--object-name-expanded) + (with-current-buffer speedbar-buffer + (goto-char (point-min)) + (should (string-equal "A" (speedbar-line-text))) + (speedbar-expand-line 'nocache) + (speedbar-expand-line-descendants 'nocache)))) + +(ert-deftest speedbar-tests--expand-descendants-of-last () + "Expand the last item and all descendants." + (skip-when noninteractive) + (speedbar-tests--state-test + ( :setup (("A" . (("A1" . (("A1A"))) + ("A2" . (("A2A"))))) + ("B" . (("B1" . (("B1B"))) + ("B2" . (("B2B")))))) + :final (("A" . (("A1" . (("A1A"))) + ("A2" . (("A2A"))))) + ("B+" . (("B1+" . (("B1B"))) + ("B2+" . (("B2B")))))) + :name-function speedbar-tests--object-name-expanded) + (with-current-buffer speedbar-buffer + (goto-char (point-min)) + (forward-line) + (should (string-equal "B" (speedbar-line-text))) + (speedbar-expand-line-descendants 'nocache)))) + +(ert-deftest speedbar-tests--expand-descendants-of-last-expanded () + "Expand the already expanded last item and all descendants." + (skip-when noninteractive) + (speedbar-tests--state-test + ( :setup (("A" . (("A1" . (("A1A"))) + ("A2" . (("A2A"))))) + ("B" . (("B1" . (("B1B"))) + ("B2" . (("B2B")))))) + :final (("A" . (("A1" . (("A1A"))) + ("A2" . (("A2A"))))) + ("B+" . (("B1+" . (("B1B"))) + ("B2+" . (("B2B")))))) + :name-function speedbar-tests--object-name-expanded) + (with-current-buffer speedbar-buffer + (goto-char (point-min)) + (save-excursion + (forward-line) + (should (string-equal "B" (speedbar-line-text))) + (speedbar-expand-line 'nocache)) + (save-excursion + (forward-line) + (should (string-equal "B" (speedbar-line-text))) + (speedbar-expand-line-descendants 'nocache))))) + +(ert-deftest speedbar-tests--expand-descendants-of-middle () + "Expand the middle item and all descendants." + (skip-when noninteractive) + (speedbar-tests--state-test + ( :setup (("A" . (("A1" . (("A1A"))) + ("A2" . (("A2A"))))) + ("B" . (("B1" . (("B1B"))) + ("B2" . (("B2B"))))) + ("C" . (("C1" . (("C1C"))) + ("C2" . (("C2C")))))) + :final (("A" . (("A1" . (("A1A"))) + ("A2" . (("A2A"))))) + ("B+" . (("B1+" . (("B1B"))) + ("B2+" . (("B2B"))))) + ("C" . (("C1" . (("C1C"))) + ("C2" . (("C2C")))))) + :name-function speedbar-tests--object-name-expanded) + (with-current-buffer speedbar-buffer + (goto-char (point-min)) + (goto-char (point-min)) + (forward-line) + (should (string-equal "B" (speedbar-line-text))) + (speedbar-expand-line-descendants 'nocache)))) + +(ert-deftest speedbar-tests--expand-descendants-of-middle-expanded () + "Expand the already expanded middle item and all descendants." + (skip-when noninteractive) + (speedbar-tests--state-test + ( :setup (("A" . (("A1" . (("A1A"))) + ("A2" . (("A2A"))))) + ("B" . (("B1" . (("B1B"))) + ("B2" . (("B2B"))))) + ("C" . (("C1" . (("C1C"))) + ("C2" . (("C2C")))))) + :final (("A" . (("A1" . (("A1A"))) + ("A2" . (("A2A"))))) + ("B+" . (("B1+" . (("B1B"))) + ("B2+" . (("B2B"))))) + ("C" . (("C1" . (("C1C"))) + ("C2" . (("C2C")))))) + :name-function speedbar-tests--object-name-expanded) + (with-current-buffer speedbar-buffer + (goto-char (point-min)) + (save-excursion + (forward-line) + (should (string-equal "B" (speedbar-line-text))) + (speedbar-expand-line 'nocache)) + (save-excursion + (forward-line) + (should (string-equal "B" (speedbar-line-text))) + (speedbar-expand-line-descendants 'nocache))))) + +(provide 'speedbar-tests) +;;; speedbar-tests.el ends here -- 2.39.5 --=-=-=-- From unknown Tue Jun 17 21:55:06 2025 X-Loop: help-debbugs@gnu.org Subject: bug#73533: [PATCH] Rewrite speedbar expansion for all descendants Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 19 Oct 2024 07:44:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 73533 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch To: Morgan Willcock Cc: 73533@debbugs.gnu.org Received: via spool by 73533-submit@debbugs.gnu.org id=B73533.17293237981716 (code B ref 73533); Sat, 19 Oct 2024 07:44:02 +0000 Received: (at 73533) by debbugs.gnu.org; 19 Oct 2024 07:43:18 +0000 Received: from localhost ([127.0.0.1]:41180 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1t247J-0000Rb-Ot for submit@debbugs.gnu.org; Sat, 19 Oct 2024 03:43:18 -0400 Received: from eggs.gnu.org ([209.51.188.92]:34394) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1t247H-0000RO-9m for 73533@debbugs.gnu.org; Sat, 19 Oct 2024 03:43:16 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1t246n-0006Qg-Fb; Sat, 19 Oct 2024 03:42:45 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date: mime-version; bh=q2cdl2RaTpzSl+6qjnZHV8a+pIH5PV1h0sXS/GxA9Qk=; b=nQDOu2EsdOu7 Wtcg0Dci1E5oT+pSWBKLDolhbjPx5y4MfdC4C6ceb48+U+1z/A42VhooByKpz/BIWJgMze/tO2g/r jXn1lH74qie5UHMdid2eEBt+Rwf2n9TE8G6MB4oZqjserrHtc/L1uNA2w8glhxh82L1uLv+4sKR/h QU2CxYEKkBGhEJ14K+SxQsh+thjrqTsyRfxfO76SnQsPkUpbOHpgdF/omGIED/Zx+p4d96LzDAy9R yYQ9W1mWa7sMdPI2apOSvTDT7VIV6+IVuJG7c3/BfCUQmWNDM3lsI3nyvTxvVcmTkoJdBHpi585dc Mpp/UIrqlRLCnyRyQVMJ7Q==; Date: Sat, 19 Oct 2024 10:42:41 +0300 Message-Id: <86y12klery.fsf@gnu.org> From: Eli Zaretskii In-Reply-To: <87ttdm4f3d.fsf@ice9.digital> (message from Morgan Willcock on Tue, 08 Oct 2024 19:36:06 +0100) References: <87y13blhu1.fsf@ice9.digital> <86a5fr5apb.fsf@gnu.org> <87v7yeeqig.fsf@ice9.digital> <87wmitfr8p.fsf@ice9.digital> <8734lhgfnn.fsf@ice9.digital> <87ttdm4f3d.fsf@ice9.digital> X-Spam-Score: -2.3 (--) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) > From: Morgan Willcock > Cc: 73533@debbugs.gnu.org > Date: Tue, 08 Oct 2024 19:36:06 +0100 > > Attached is a patch for a replacement function and an additional patch > which adds tests for expanding all descendants. > > I couldn't see a direct way to test a Speedbar without creating a frame, > and because the Speedbar code doesn't implement any type of hierarchy > itself I've used eieio-speedbar to make something minimal to test. > > Each test runs by converting a list of strings into objects which > implement a Speedbar display, making state changes for the functions > being tested, and then converting the objects back into strings - this > was the simplest way I could find to create a reusable interface for > tests. The conversion back to a string is customizable to allow > whatever state change is under test to be represented. > > If the tests are problematic or considered too complicated, perhaps they > can just be used to test the current function and the replacement > function and not committed. The current function fails 3/9 tests. Thanks, I installed these patches on master. There seems to be still a problem with some files, in that speedbar-expand-line-descendants takes a very long time, and if I interrupt it with C-g, I see nested DEFUNs, something that shouldn't happen, because DEFUNs are never nested. E.g., try this: $ cd /path/to/emacs/src $ ./emacs -Q M-x speedbar RET At this point you should see all the files in the Emacs src directory. Go to androidfns.c and type '['. After about 10 sec type C-g. You should see nested DEFUns in the Speedbar frame. Could you please look into this? From unknown Tue Jun 17 21:55:06 2025 X-Loop: help-debbugs@gnu.org Subject: bug#73533: [PATCH] Rewrite speedbar expansion for all descendants Resent-From: Morgan Willcock Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 19 Oct 2024 17:31:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 73533 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch To: Eli Zaretskii Cc: 73533@debbugs.gnu.org Received: via spool by 73533-submit@debbugs.gnu.org id=B73533.172935904811235 (code B ref 73533); Sat, 19 Oct 2024 17:31:02 +0000 Received: (at 73533) by debbugs.gnu.org; 19 Oct 2024 17:30:48 +0000 Received: from localhost ([127.0.0.1]:44662 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1t2DHr-0002v9-Nz for submit@debbugs.gnu.org; Sat, 19 Oct 2024 13:30:48 -0400 Received: from relay9-d.mail.gandi.net ([217.70.183.199]:59389) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1t2DHp-0002uq-1H for 73533@debbugs.gnu.org; Sat, 19 Oct 2024 13:30:46 -0400 Received: by mail.gandi.net (Postfix) with ESMTPSA id A9AB4FF804; Sat, 19 Oct 2024 17:30:14 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ice9.digital; s=gm1; t=1729359014; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=SUtVZJ7RKjTOd15eC+tXNL16DP8+489+apXcisdrcFk=; b=k0d6QDiL70odTi2oUiQXSFIzY4Px8QRN0DBS2qFngz8OgzCjYV0jSMFEwxjAv7Dpl6AlpH RDZR/y9n+GmX65cB0vnmi3gySl6whU0iFrnzl27bwqoZTSK4sYTgM53B+lwR2uhM0TRP9c K1418fyUR1MEZQfAplBID/z/V5iWUrauT0JUBWMKWwlw/NMT4pCzlI6vmlZpFme0dWqmDM je6jeuElwoJeXQJJ02DDlZft/2DS3a6vccT9n+0o5G7pJOH6UWsQVXZO7PKaDGpObs3Z44 aSZ8Nps6Jy9igwMK2/5yp/2tLCdsLExJZr1EvXDD/4mDYnc8wTr1qnzChBnPmg== From: Morgan Willcock In-Reply-To: <86y12klery.fsf@gnu.org> (Eli Zaretskii's message of "Sat, 19 Oct 2024 10:42:41 +0300") References: <87y13blhu1.fsf@ice9.digital> <86a5fr5apb.fsf@gnu.org> <87v7yeeqig.fsf@ice9.digital> <87wmitfr8p.fsf@ice9.digital> <8734lhgfnn.fsf@ice9.digital> <87ttdm4f3d.fsf@ice9.digital> <86y12klery.fsf@gnu.org> Date: Sat, 19 Oct 2024 18:30:13 +0100 Message-ID: <87a5f0c862.fsf@ice9.digital> User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: text/plain X-GND-Sasl: morgan@ice9.digital X-Spam-Score: -0.7 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.7 (-) Eli Zaretskii writes: > There seems to be still a problem with some files, in that > speedbar-expand-line-descendants takes a very long time, and if I > interrupt it with C-g, I see nested DEFUNs, something that shouldn't > happen, because DEFUNs are never nested. E.g., try this: > > $ cd /path/to/emacs/src > $ ./emacs -Q > M-x speedbar RET > > At this point you should see all the files in the Emacs src directory. > Go to androidfns.c and type '['. After about 10 sec type C-g. You > should see nested DEFUns in the Speedbar frame. > > Could you please look into this? This looks like an unrelated issue where the logic to modify the item hierarchy does not make sense under certain conditions. It only seems to trigger when particular limits are met, but the limits can be lowered for an easier demonstration: (require 'speedbar) (setq speedbar-tag-regroup-maximum-length 2) (setq speedbar-tag-split-minimum-length 2) (speedbar-prefix-group-tag-hierarchy '(("DEFUN" . 1) ("DEFUN" . 5) ("DEFUN" . 10))) => (("DEFUN" ("DEFUN" . 1) ("DEFUN" . 5) ("DEFUN" . 10))) i.e. speedbar-prefix-group-tag-hierarchy (which is meant to restructure the Imenu data to make better use of the available space in the Speedbar) is generating a new parent item which contains the items that were meant to be inserted. It is probably best to open a separate bug report for this because working out exactly how and why speedbar-prefix-group-tag-hierarchy applies particular sorting actions will probably take someone a while. -- Morgan Willcock From unknown Tue Jun 17 21:55:06 2025 MIME-Version: 1.0 X-Mailer: MIME-tools 5.505 (Entity 5.505) X-Loop: help-debbugs@gnu.org From: help-debbugs@gnu.org (GNU bug Tracking System) To: Morgan Willcock Subject: bug#73533: closed (Re: bug#73533: [PATCH] Rewrite speedbar expansion for all descendants) Message-ID: References: <86msj0j8ha.fsf@gnu.org> <87y13blhu1.fsf@ice9.digital> X-Gnu-PR-Message: they-closed 73533 X-Gnu-PR-Package: emacs X-Gnu-PR-Keywords: patch Reply-To: 73533@debbugs.gnu.org Date: Sat, 19 Oct 2024 17:43:02 +0000 Content-Type: multipart/mixed; boundary="----------=_1729359782-13183-1" This is a multi-part message in MIME format... ------------=_1729359782-13183-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" Your bug report #73533: [PATCH] Rewrite speedbar expansion for all descendants which was filed against the emacs package, has been closed. The explanation is attached below, along with your original report. If you require more details, please reply to 73533@debbugs.gnu.org. --=20 73533: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=3D73533 GNU Bug Tracking System Contact help-debbugs@gnu.org with problems ------------=_1729359782-13183-1 Content-Type: message/rfc822 Content-Disposition: inline Content-Transfer-Encoding: 7bit Received: (at 73533-done) by debbugs.gnu.org; 19 Oct 2024 17:42:12 +0000 Received: from localhost ([127.0.0.1]:44687 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1t2DSt-0003PN-RA for submit@debbugs.gnu.org; Sat, 19 Oct 2024 13:42:12 -0400 Received: from eggs.gnu.org ([209.51.188.92]:40544) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1t2DSr-0003P7-QV for 73533-done@debbugs.gnu.org; Sat, 19 Oct 2024 13:42:10 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1t2DSN-0003BK-FL; Sat, 19 Oct 2024 13:41:39 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date: mime-version; bh=gGCGK5nkfbJaHxQ24bKc6qp9SfVvcfZnfbXwgcC32DI=; b=ZtWTa9sGxds5 MXPrNQ0gheNRm5vG1XdPFTvCrCzEUdyHNOJfK07PTXteIJ7Fbyw3SJqfkaC1PR0QQMpdObZmKYpzJ +qbhyTy+vEVKB/GBO6ZrBKekrKaZse/AHkg/P89gC09TLiuSpE6g/usjEHCwtQPF9OaI8lonWj27b inBRXZJTpuxAPvRedti3oD4h3TPWmAcZltJBr3rYYF4ZZmXp/ASM+ABPrPmBFYNt4pjdvcnTSG4lZ 200Rk3XyLb0eb2xwazBWuKS18MS/MdJxf++WxVwYDDSzKg0uwIduasiechzdmb6lWbWgvm+q0MHpX B+yv7Fylbe3OzHDNhzufbw==; Date: Sat, 19 Oct 2024 20:41:37 +0300 Message-Id: <86msj0j8ha.fsf@gnu.org> From: Eli Zaretskii To: Morgan Willcock In-Reply-To: <87a5f0c862.fsf@ice9.digital> (message from Morgan Willcock on Sat, 19 Oct 2024 18:30:13 +0100) Subject: Re: bug#73533: [PATCH] Rewrite speedbar expansion for all descendants References: <87y13blhu1.fsf@ice9.digital> <86a5fr5apb.fsf@gnu.org> <87v7yeeqig.fsf@ice9.digital> <87wmitfr8p.fsf@ice9.digital> <8734lhgfnn.fsf@ice9.digital> <87ttdm4f3d.fsf@ice9.digital> <86y12klery.fsf@gnu.org> <87a5f0c862.fsf@ice9.digital> X-Spam-Score: -2.3 (--) X-Debbugs-Envelope-To: 73533-done Cc: 73533-done@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) > From: Morgan Willcock > Cc: 73533@debbugs.gnu.org > Date: Sat, 19 Oct 2024 18:30:13 +0100 > > Eli Zaretskii writes: > > > There seems to be still a problem with some files, in that > > speedbar-expand-line-descendants takes a very long time, and if I > > interrupt it with C-g, I see nested DEFUNs, something that shouldn't > > happen, because DEFUNs are never nested. E.g., try this: > > > > $ cd /path/to/emacs/src > > $ ./emacs -Q > > M-x speedbar RET > > > > At this point you should see all the files in the Emacs src directory. > > Go to androidfns.c and type '['. After about 10 sec type C-g. You > > should see nested DEFUns in the Speedbar frame. > > > > Could you please look into this? > > This looks like an unrelated issue where the logic to modify the item > hierarchy does not make sense under certain conditions. It only seems > to trigger when particular limits are met, but the limits can be lowered > for an easier demonstration: > > (require 'speedbar) > (setq speedbar-tag-regroup-maximum-length 2) > (setq speedbar-tag-split-minimum-length 2) > > (speedbar-prefix-group-tag-hierarchy > '(("DEFUN" . 1) > ("DEFUN" . 5) > ("DEFUN" . 10))) > > => (("DEFUN" ("DEFUN" . 1) ("DEFUN" . 5) ("DEFUN" . 10))) > > i.e. speedbar-prefix-group-tag-hierarchy (which is meant to restructure > the Imenu data to make better use of the available space in the > Speedbar) is generating a new parent item which contains the items that > were meant to be inserted. > > It is probably best to open a separate bug report for this because > working out exactly how and why speedbar-prefix-group-tag-hierarchy > applies particular sorting actions will probably take someone a while. Done, and closing. Thanks. ------------=_1729359782-13183-1 Content-Type: message/rfc822 Content-Disposition: inline Content-Transfer-Encoding: 7bit Received: (at submit) by debbugs.gnu.org; 28 Sep 2024 20:01:07 +0000 Received: from localhost ([127.0.0.1]:43274 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1sudcp-0001Nj-5N for submit@debbugs.gnu.org; Sat, 28 Sep 2024 16:01:07 -0400 Received: from lists.gnu.org ([209.51.188.17]:50560) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1sudcj-0001MY-QE for submit@debbugs.gnu.org; Sat, 28 Sep 2024 16:01:03 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1sucl5-0008Jv-QD for bug-gnu-emacs@gnu.org; Sat, 28 Sep 2024 15:05:35 -0400 Received: from relay4-d.mail.gandi.net ([2001:4b98:dc4:8::224]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1sucl3-0001GE-2r for bug-gnu-emacs@gnu.org; Sat, 28 Sep 2024 15:05:35 -0400 Received: by mail.gandi.net (Postfix) with ESMTPSA id D22BBE0002 for ; Sat, 28 Sep 2024 19:05:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ice9.digital; s=gm1; t=1727550327; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type; bh=0lpE61LCFx+yDgjVjJm4fg5kzoBcQv4GlKyA5Q8hpy8=; b=THFnAtf4gb+DZMtfPZ1OHcV6iso+jL/Aznt+QOSNTV4Zw0ETAyOO+oc0WOFdwBw1XTW7bt sEws/XKU6B9n7W+LvzJhBBHaDzVJx2PRGXT367PbqyD/Vx0AW38o4CMQArYUEaC4ekadsp dXlv4JriQGrSVxYQLMJcREMwpX6iypPBU0CTrK5PWNiwxHvJKrx2yawtjcVsBUrjCjJqhd rYNnZ3tMAPpD5FAy5Jctttsa8xrqsV3QaRcxjbAUxp+g0ZdJ/QoXBgLGzfJTxxGoJ+43+R FQUIaafiXyZAJH3/pcXRDY675C8Bn6f8Sh5tl8klaebUZJJy/VJRdnP12u1BqQ== From: Morgan Willcock To: bug-gnu-emacs@gnu.org Subject: [PATCH] Rewrite speedbar expansion for all descendants Date: Sat, 28 Sep 2024 20:05:26 +0100 Message-ID: <87y13blhu1.fsf@ice9.digital> User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-GND-Sasl: morgan@ice9.digital Received-SPF: pass client-ip=2001:4b98:dc4:8::224; envelope-from=morgan@ice9.digital; helo=relay4-d.mail.gandi.net X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-Spam-Score: -1.7 (-) X-Debbugs-Envelope-To: submit X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -2.7 (--) --=-=-= Content-Type: text/plain Tags: patch Attached is a patch which rewrites 'speedbar-expand-line-descendants'. The previous version could get into an infinite loop by reaching the maximum recursion depth, although in practice the slow speed meant that most people would probably abort the operation before reaching that point. The majority of the slowdown was because the motion commands being used were the variants which looked up information for every entry, displayed the information as a message, and adjusted the cursor position. The messages were not readable because of being continually overwritten. Here is a way to demonstrate that stack depth was increasing for items at the same level, that the messages were not readable, and how slow the whole process was: rm -rf /tmp/project mkdir /tmp/project for i in $(seq 1 50); do echo "(defun fun-$i ())" >> /tmp/project/file1.el; done for i in $(seq 1 50); do echo "(defun fun-$i ())" >> /tmp/project/file2.el; done emacs -Q \ --eval="(find-file \"/tmp/project/file1.el\")" \ --eval "(speedbar-get-focus)" \ --eval "(profiler-start 'cpu)" \ --eval "(speedbar-expand-line-descendants)" \ --eval "(profiler-stop)" \ --eval "(profiler-report)" ...that should expand every entry in file1.el and not touch the entries in file2.el. The replacement function is significantly faster. Messages are only used to indicate that the function is running and when it is finished - the result is similar to manually clicking every node open. Thanks, Morgan In GNU Emacs 30.0.91 (build 2, x86_64-pc-linux-gnu, X toolkit, cairo version 1.16.0, Xaw3d scroll bars) of 2024-09-12 built on inspiron Windowing system distributor 'The X.Org Foundation', version 11.0.12101007 System Description: Debian GNU/Linux 12 (bookworm) Configured using: 'configure --with-native-compilation=aot --with-xml2 --with-x-toolkit=lucid' --=-=-= Content-Type: text/patch Content-Disposition: attachment; filename=0001-Rewrite-speedbar-expansion-for-all-descendants.patch >From 0e25d28bfbef31c20ec22c2e508933b3824a8172 Mon Sep 17 00:00:00 2001 From: Morgan Willcock Date: Sat, 28 Sep 2024 19:11:11 +0100 Subject: [PATCH] Rewrite speedbar expansion for all descendants Rewrite 'speedbar-expand-line-descendants' to avoid getting into an infinite loop by reaching max-lisp-eval-depth. The new method avoids querying and displaying information for every movement, instead using a single message to indicate that expansion is in progress, and so is significantly faster. * lisp/speedbar.el (speedbar-expand-line-descendants): Use simpler line motion and no recursion. Output messages indicating when expansion is in progress and when it is completed. --- lisp/speedbar.el | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/lisp/speedbar.el b/lisp/speedbar.el index c13c977938b..11e11e1e56c 100644 --- a/lisp/speedbar.el +++ b/lisp/speedbar.el @@ -3172,21 +3172,22 @@ speedbar-expand-line-descendants "Expand the line under the cursor and all descendants. Optional argument ARG indicates that any cache should be flushed." (interactive "P") - (save-restriction - (narrow-to-region (line-beginning-position) - (line-beginning-position 2)) - (speedbar-expand-line arg) - ;; Now, inside the area expanded here, expand all subnodes of - ;; the same descendant type. - (save-excursion - (speedbar-next 1) ;; Move into the list. - (let ((err nil)) - (while (not err) - (condition-case nil - (progn - (speedbar-expand-line-descendants arg) - (speedbar-restricted-next 1)) - (error (setq err t)))))))) + (dframe-message "Expanding all descendants...") + (save-excursion + (with-restriction + ;; Narrow around the top-level item. + (line-beginning-position) + (condition-case nil + (save-excursion + (speedbar-restricted-move 1) + (line-beginning-position)) + (error (line-beginning-position 2))) + ;; Expand every line until the end of the restriction. + (while (zerop (progn + (speedbar-expand-line arg) + (forward-line 1)))))) + (dframe-message "Expanding all descendants...done") + (speedbar-position-cursor-on-line)) (defun speedbar-contract-line-descendants () "Expand the line under the cursor and all descendants." -- 2.39.5 --=-=-=-- ------------=_1729359782-13183-1--