[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."