emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Copyright assignment and questions about package submission


From: Stefan Monnier
Subject: Re: Copyright assignment and questions about package submission
Date: Sat, 26 Dec 2020 17:13:51 -0500
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux)

>>> Please fill the form below and send it to the FSF as instructed so they
>>> can send you the relevant paperwork to sign.
>> Thanks, I've sent the form.
>
> BTW, I just looked at your package.
>
> The code looks clean, but when I tried to enable it in my main Emacs
> session it got stuck in `swsw-update`, apparently more specifically
> inside `swsw--get-possible-ids` (see sample backtrace below obtained
> with `debug-on-quit`).
>
> It looks like your algorithm has a bad asymptotic complexity (my session
> has currently 88 frames, most of them with a single window in it).

Actually, it looks like a simple error where you used division instead
of logarithm, thus constructing a `sws-ids` that's exponentially longer
than the one you need.  The patch below makes it usable for me.

[ I'd still argue that you shouldn't bother to construct the whole
  `sws-ids` list but instead keep a counter which lets you build the
  "next" id on the fly.  But that's up to you.  ]


        Stefan


diff --git a/swsw.el b/swsw.el
index 7f26b63..e4bb743 100644
--- a/swsw.el
+++ b/swsw.el
@@ -66,12 +66,10 @@
 
 (defcustom swsw-id-chars '(?a ?s ?d ?f ?g ?h ?j ?k ?l)
   "Base set of characters from which window IDs are constructed."
-  :group 'swsw
   :type '(repeat character))
 
 (defcustom swsw-minibuffer-id ?m
   "ID reserved for the minibuffer."
-  :group 'swsw
   :type '(character))
 
 (defcustom swsw-scope t
@@ -81,7 +79,6 @@ t means consider all windows on all existing frames.
   iconified frames.
 ‘visible’ means consider all windows on all visible frames.
 ‘current’ means consider only the currently selected frame."
-  :group 'swsw
   :type '(radio (const :tag "All windows on all frames" t)
                 (const
                  :tag "All windows on all visible and iconified frames." 0)
@@ -109,7 +106,6 @@ sole argument (turning it on)."
 This function is called with t as the sole argument when enabling
 ‘swsw-mode’, and with nil as the sole argument when disabling it.
 If set to ‘lighter’, use the mode line lighter of ‘swsw-mode’."
-  :group 'swsw
   :type '(radio (const :tag "Mode line lighter" lighter)
                 (function :tag "Display function"))
   :set #'swsw--set-display-function)
@@ -117,7 +113,6 @@ If set to ‘lighter’, use the mode line lighter of 
‘swsw-mode’."
 (defcustom swsw-id-format " <%s>"
   "Format string for the window ID.
 %s is replaced with a representation of the window's ID."
-  :group 'swsw
   :type '(string))
 
 ;;;; Simple window switching minor mode:
@@ -147,12 +142,8 @@ If set to ‘lighter’, use the mode line lighter of 
‘swsw-mode’."
 (defun swsw--get-id-length ()
   "Return the current length of a window ID."
   (let* ((windows (length (window-list-1 nil nil (swsw--get-scope))))
-         (chars (length swsw-id-chars))
-         (div (/ windows chars)))
-    ;; Check the remainder to avoid returning a longer length than necessary.
-    (if (= 0 (mod windows chars))
-        div
-      (1+ div))))
+         (chars (length swsw-id-chars)))
+    (ceiling (log windows chars))))
 
 (defun swsw-update-window (window)
   "Update information for WINDOW."




reply via email to

[Prev in Thread] Current Thread [Next in Thread]