Add condition before using return value

[conao3]

To solve tumashu/posframe#29 .

persp-curr and persp-last returns nil sometimes. Some places use class accessors with proper return value checking, but some places don't.
I added a code to check it mechanically. You might want to issue a warning or print a message to a persp-mode specific debug buffer, but I didn't mention it because I'm not familiar with its implementation.

[gcv]

Adapting my comment from tumashu/posframe#29:

This is definitely a bug in perspective-el. Thank you for looking into it.

I worry, however, that the approach in this patch leaves open the possibility of someone forgetting the check the return value in new code. What about changing the functions which may return nil to return valid but empty values instead? Please see https://github.com/gcv/perspective-el/commit/dd67e90821ba94549395c290d4fb2cc8e2cbcddd — what do you think?

[conao3]

I thought about what to do if a method can't return a meaningful value, and added a return value check in all places this PR.
Some approaches, such as the @gcv approach, return a some value, while others give up execution by signal. Which method do @nex3 prefer?

[nnicandro]

Hi, I bumped into this issue when trying to use company-posframe.

I did the same thing as @gcv to get around it. However, in that approach, I'm unsure if there are places in the code that may, in addition to expecting a valid perspective for something like persp-curr, expect persp-name to be a valid string or something like that. We may also have to set better defaults for the struct fields as well.

I agree that the approach in this pull request adds a much greater maintenance burden, but we could possibly use it if we move all of the work into a macro, something like persp-defun, that protects the body of the functions in the way described in this pull request.

A third option may be to have a way of suppressing perspective in a frame. Detecting if a frame is a bare bones frame, i.e. a posframe, somehow and disabling the perspective advice for that frame. Maybe the logic can be: if any frame, F, has a parent-frame parameter that is a frame with a non-nil perspectives-hash, disable the advice for frame F. We can add this to persp-protect so that it doesn't evaluate its body when perspective should be disabled and treat the other advise that don't use persp-protect accordingly.

[gcv]

I really like your third option, @DZoP. I had a similar idea when I first ran into the problem, but I couldn't (hastily) come up with a good heuristic for distinguishing bare-bones frames from full-featured ones. Just checking for persp-related frame parameters seemed like it would have subtle edge cases, though I'm now struggling to remember the specific one I thought of at the time. 👍 to the idea of putting that logic into persp-protect.

[zenspider]

I have some minor feedback but then a big existential question.

[zenspider]

This diffs MUCH better when you toggle the whitespace flag.

Seems like almost all of these could be refactored into a helper macro to make it a bit more clear...

I'm not sure about the persp-last one. You're mostly just verifying that there is a perspective so it could probably be the same?

[zenspider]

(defmacro guard-perspecive (&rest body)
  (declare (indent 0))
  `(when (persp-curr)
     @,body))

(no clue what the name should be in)

[zenspider]

This all seems like a lot.. would it not be "cleaner" if we guarded the struct accessors instead? Or have some sort of "null perspective"? I dunno. This is clearly infiltrating nearly every function and makes it really hard to audit as a result.

[conao3]

This PR is outdated. And now @gcv is maintainer, this issue has solved maybe.
Close PR.

Note:
I will delete the branch. To restore this, please use below patch

From 004781e3e106cef5af0c8fc95853898547820128 Mon Sep 17 00:00:00 2001
From: conao3 <conao3@gmail.com>
Date: Sat, 25 May 2019 21:02:32 +0900
Subject: [PATCH 1/3] add persp-curr condition before using persp-curr return
 value

---
 perspective.el | 45 ++++++++++++++++++++++++++++++---------------
 1 file changed, 30 insertions(+), 15 deletions(-)

diff --git a/perspective.el b/perspective.el
index 1f6f1fb..d63e85c 100644
--- a/perspective.el
+++ b/perspective.el
@@ -524,24 +524,26 @@ See `persp-switch', `persp-get-quick'."
 (defun persp-next ()
   "Switch to next perspective (to the right)."
   (interactive)
+  (when (persp-curr)
   (let* ((names (persp-names))
          (pos (cl-position (persp-name (persp-curr)) names)))
     (cond
      ((null pos) (persp-find-some))
      ((= pos (1- (length names)))
       (if persp-switch-wrap (persp-switch (nth 0 names))))
-     (t (persp-switch (nth (1+ pos) names))))))
+     (t (persp-switch (nth (1+ pos) names)))))))
 
 (defun persp-prev ()
   "Switch to previous perspective (to the left)."
   (interactive)
+  (when (persp-curr)
   (let* ((names (persp-names))
          (pos (cl-position (persp-name (persp-curr)) names)))
     (cond
      ((null pos) (persp-find-some))
      ((= pos 0)
       (if persp-switch-wrap (persp-switch (nth (1- (length names)) names))))
-     (t (persp-switch (nth (1- pos) names))))))
+     (t (persp-switch (nth (1- pos) names)))))))
 
 (defun persp-find-some ()
   "Return the name of a valid perspective.
@@ -573,9 +575,10 @@ See also `persp-switch' and `persp-remove-buffer'."
    (list
     (let ((read-buffer-function nil))
       (read-buffer "Add buffer to perspective: "))))
+  (when (persp-curr)
   (let ((buffer (get-buffer buffer)))
     (unless (memq buffer (persp-buffers (persp-curr)))
-      (push buffer (persp-buffers (persp-curr))))))
+      (push buffer (persp-buffers (persp-curr)))))))
 
 (defun persp-set-buffer (buffer-name)
   "Associate BUFFER-NAME with the current perspective and remove it from any other."
@@ -599,7 +602,8 @@ perspective that has the buffer.
 Prefers perspectives in the selected frame."
   (cl-loop for frame in (sort (frame-list) (lambda (frame1 frame2) (eq frame2 (selected-frame))))
            do (cl-loop for persp being the hash-values of (frame-parameter frame 'persp--hash)
-                       if (and (not (and (equal frame (selected-frame))
+                       if (and (persp-curr frame)
+                               (not (and (equal frame (selected-frame))
                                          (equal (persp-name persp) (persp-name (persp-curr frame)))))
                                (memq buffer (persp-buffers persp)))
                        do (cl-return-from persp-buffer-in-other-p
@@ -612,13 +616,14 @@ Prefers perspectives in the selected frame."
    (list
     (let ((read-buffer-function nil))
       (read-buffer-to-switch "Switch to buffer: "))))
+  (when (persp-curr)
   (let ((buffer (window-normalize-buffer-to-switch-to buffer-or-name)))
     (if (memq buffer (persp-buffers (persp-curr)))
         (switch-to-buffer buffer)
       (let ((other-persp (persp-buffer-in-other-p buffer)))
         (when (eq (car-safe other-persp) (selected-frame))
           (persp-switch (cdr other-persp)))
-        (switch-to-buffer buffer)))))
+        (switch-to-buffer buffer))))))
 
 (defun persp-remove-buffer (buffer)
   "Disassociate BUFFER with the current perspective.
@@ -640,7 +645,8 @@ See also `persp-switch' and `persp-add-buffer'."
                ;; the loop because otherwise it will go on infinitely.
                (setq window (unless (eq window new-window) new-window))))))
         (t (bury-buffer buffer)))
-  (setf (persp-buffers (persp-curr)) (remq buffer (persp-buffers (persp-curr)))))
+  (when (persp-curr)
+  (setf (persp-buffers (persp-curr)) (remq buffer (persp-buffers (persp-curr))))))
 
 (defun persp-kill (name)
   "Kill the perspective given by NAME.
@@ -648,6 +654,7 @@ See also `persp-switch' and `persp-add-buffer'."
 Killing a perspective means that all buffers associated with that
 perspective and no others are killed."
   (interactive "i")
+  (when (persp-curr)
   (if (null name) (setq name (persp-prompt (persp-name (persp-curr)) t)))
   (with-perspective name
     (run-hooks 'persp-killed-hook)
@@ -666,17 +673,18 @@ perspective and no others are killed."
   (when (or (not (persp-curr)) (equal name (persp-name (persp-curr))))
     ;; Don't let persp-last get set to the deleted persp.
     (persp-let-frame-parameters ((persp--last (persp-last)))
-      (persp-switch (persp-find-some)))))
+      (persp-switch (persp-find-some))))))
 
 (defun persp-rename (name)
   "Rename the current perspective to NAME."
   (interactive "sNew name: ")
+  (when (persp-curr)
   (if (gethash name (perspectives-hash))
       (persp-error "Perspective `%s' already exists" name)
     (remhash (persp-name (persp-curr)) (perspectives-hash))
     (puthash name (persp-curr) (perspectives-hash))
     (setf (persp-name (persp-curr)) name)
-    (persp-update-modestring)))
+    (persp-update-modestring))))
 
 (cl-defun persp-all-get (name not-frame)
   "Returns the list of buffers for a perspective named NAME from any
@@ -720,12 +728,13 @@ With a prefix arg, uses the old `read-buffer' instead."
 
 (defun persp-complete-buffer ()
   "Perform completion on all buffers within the current perspective."
+  (when (persp-curr)
   (lexical-let ((persp-names (mapcar 'buffer-name (persp-buffers (persp-curr)))))
     (apply-partially 'completion-table-with-predicate
                      (or minibuffer-completion-table 'internal-complete-buffer)
                      (lambda (name)
                        (member (if (consp name) (car name) name) persp-names))
-                     nil)))
+                     nil))))
 
 (cl-defun persp-import (name &optional dont-switch)
   "Import a perspective named NAME from another frame.  If DONT-SWITCH
@@ -790,6 +799,7 @@ See also `persp-add-buffer'."
          (old-buffer (window-buffer window)))
     ad-do-it
 
+    (when (persp-curr)
     (let ((buffer (window-buffer window)))
       (with-selected-frame frame
         (unless (memq buffer (persp-buffers (persp-curr)))
@@ -804,12 +814,13 @@ See also `persp-add-buffer'."
               (setq name (concat "*scratch*  (" (persp-name (persp-curr)) ")")))
             (with-selected-window window
               (switch-to-buffer name)
-              (funcall initial-major-mode))))))))
+              (funcall initial-major-mode)))))))))
 
 (defadvice recursive-edit (around persp-preserve-for-recursive-edit)
   "Preserve the current perspective when entering a recursive edit."
   (persp-protect
     (persp-save)
+    (when (persp-curr)
     (persp-let-frame-parameters ((persp--recursive (persp-curr)))
       (let ((old-hash (copy-hash-table (perspectives-hash))))
         ad-do-it
@@ -820,7 +831,7 @@ See also `persp-add-buffer'."
                      (when persp
                        (setf (persp-buffers persp) (persp-buffers new-persp)))))
                  (perspectives-hash))
-        (set-frame-parameter nil 'persp--hash old-hash)))))
+        (set-frame-parameter nil 'persp--hash old-hash))))))
 
 (defadvice exit-recursive-edit (before persp-restore-after-recursive-edit)
   "Restore the old perspective when exiting a recursive edit."
@@ -846,7 +857,8 @@ named collections of buffers and window configurations."
         (add-hook 'ido-make-buffer-list-hook 'persp-set-ido-buffers)
         (setq read-buffer-function 'persp-read-buffer)
         (mapc 'persp-init-frame (frame-list))
-        (setf (persp-buffers (persp-curr)) (buffer-list))
+        (when (persp-curr)
+        (setf (persp-buffers (persp-curr)) (buffer-list)))
 
         (run-hooks 'persp-mode-hook))
     (ad-deactivate-regexp "^persp-.*")
@@ -893,11 +905,12 @@ This means that whenever a new perspective is switched into, the
 variable will take on its local value for that perspective.  When
 a new perspective is created, the variable will inherit its value
 from the current perspective at time of creation."
+  (when (persp-curr)
   (unless (assq variable (persp-local-variables (persp-curr)))
     (let ((entry (list variable (symbol-value variable))))
       (dolist (frame (frame-list))
         (cl-loop for persp being the hash-values of (frame-parameter frame 'persp--hash)
-                 do (push entry (persp-local-variables persp)))))))
+                 do (push entry (persp-local-variables persp))))))))
 
 (defmacro persp-setup-for (name &rest body)
   "Add code that should be run to set up the perspective named NAME.
@@ -907,7 +920,8 @@ it. In addition, if one exists already, runs BODY in it immediately."
   `(progn
      (add-hook 'persp-created-hook
                (lambda ()
-                 (when (string= (persp-name (persp-curr)) ,name)
+                 (when (and (persp-curr)
+                            (string= (persp-name (persp-curr)) ,name))
                    ,@body))
                'append)
      (when (gethash ,name (perspectives-hash))
@@ -915,6 +929,7 @@ it. In addition, if one exists already, runs BODY in it immediately."
 
 (defun persp-set-ido-buffers ()
   "Restrict the ido buffer to the current perspective."
+  (when (persp-curr)
   (let ((persp-names
          (remq nil (mapcar 'buffer-name (persp-buffers (persp-curr)))))
         (indices (make-hash-table :test 'equal)))
@@ -925,7 +940,7 @@ it. In addition, if one exists already, runs BODY in it immediately."
           (sort (intersection persp-names ido-temp-list)
                 (lambda (a b)
                   (< (gethash a indices)
-                     (gethash b indices)))))))
+                     (gethash b indices))))))))
 
 (defun quick-perspective-keys ()
   "Bind quick key commands to switch to perspectives.

From 2470ef1defc1fe7c49f4ab9ef6e01285f5c6e5e4 Mon Sep 17 00:00:00 2001
From: conao3 <conao3@gmail.com>
Date: Sat, 25 May 2019 21:05:43 +0900
Subject: [PATCH 2/3] add persp-last condition before using persp-last return
 value

---
 perspective.el | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/perspective.el b/perspective.el
index d63e85c..5cc11c8 100644
--- a/perspective.el
+++ b/perspective.el
@@ -343,6 +343,7 @@ REQUIRE-MATCH can take the same values as in `completing-read'."
   (declare (indent 1))
   (let ((old (cl-gensym)))
     `(progn
+       (when (persp-last)
        (let ((,old (when (persp-curr) (persp-name (persp-curr))))
              (last-persp-cache (persp-last)))
          (unwind-protect
@@ -350,7 +351,7 @@ REQUIRE-MATCH can take the same values as in `completing-read'."
                (persp-switch ,name 'norecord)
                ,@body)
            (when ,old (persp-switch ,old 'norecord)))
-         (set-frame-parameter nil 'persp--last last-persp-cache)))))
+         (set-frame-parameter nil 'persp--last last-persp-cache))))))
 
 (defun persp-reset-windows ()
   "Remove all windows, ensure the remaining one has no window parameters.
@@ -670,7 +671,8 @@ perspective and no others are killed."
             (last (nth 1 names)))
        (when last
          (gethash last (perspectives-hash))))))
-  (when (or (not (persp-curr)) (equal name (persp-name (persp-curr))))
+  (when (and (or (not (persp-curr)) (equal name (persp-name (persp-curr))))
+             (persp-last))
     ;; Don't let persp-last get set to the deleted persp.
     (persp-let-frame-parameters ((persp--last (persp-last)))
       (persp-switch (persp-find-some))))))

From 14d25d6f748975679f6735ca13d5d43525343e77 Mon Sep 17 00:00:00 2001
From: conao3 <conao3@gmail.com>
Date: Sat, 25 May 2019 21:07:09 +0900
Subject: [PATCH 3/3] fix indentation

---
 perspective.el | 226 ++++++++++++++++++++++++-------------------------
 1 file changed, 113 insertions(+), 113 deletions(-)

diff --git a/perspective.el b/perspective.el
index 5cc11c8..a7e9e9d 100644
--- a/perspective.el
+++ b/perspective.el
@@ -344,14 +344,14 @@ REQUIRE-MATCH can take the same values as in `completing-read'."
   (let ((old (cl-gensym)))
     `(progn
        (when (persp-last)
-       (let ((,old (when (persp-curr) (persp-name (persp-curr))))
-             (last-persp-cache (persp-last)))
-         (unwind-protect
-             (progn
-               (persp-switch ,name 'norecord)
-               ,@body)
-           (when ,old (persp-switch ,old 'norecord)))
-         (set-frame-parameter nil 'persp--last last-persp-cache))))))
+         (let ((,old (when (persp-curr) (persp-name (persp-curr))))
+               (last-persp-cache (persp-last)))
+           (unwind-protect
+               (progn
+                 (persp-switch ,name 'norecord)
+                 ,@body)
+             (when ,old (persp-switch ,old 'norecord)))
+           (set-frame-parameter nil 'persp--last last-persp-cache))))))
 
 (defun persp-reset-windows ()
   "Remove all windows, ensure the remaining one has no window parameters.
@@ -427,10 +427,10 @@ Has no effect when `persp-show-modestring' is nil."
           (close (list (nth 1 persp-modestring-dividers)))
           (sep (nth 2 persp-modestring-dividers)))
       (set-frame-parameter nil 'persp--modestring
-           (append open
-                   (persp-intersperse (mapcar 'persp-format-name
-                                              (persp-names)) sep)
-                   close)))))
+                           (append open
+                                   (persp-intersperse (mapcar 'persp-format-name
+                                                              (persp-names)) sep)
+                                   close)))))
 
 (defun persp-format-name (name)
   "Format the perspective name given by NAME for display in the modeline."
@@ -526,25 +526,25 @@ See `persp-switch', `persp-get-quick'."
   "Switch to next perspective (to the right)."
   (interactive)
   (when (persp-curr)
-  (let* ((names (persp-names))
-         (pos (cl-position (persp-name (persp-curr)) names)))
-    (cond
-     ((null pos) (persp-find-some))
-     ((= pos (1- (length names)))
-      (if persp-switch-wrap (persp-switch (nth 0 names))))
-     (t (persp-switch (nth (1+ pos) names)))))))
+    (let* ((names (persp-names))
+           (pos (cl-position (persp-name (persp-curr)) names)))
+      (cond
+       ((null pos) (persp-find-some))
+       ((= pos (1- (length names)))
+        (if persp-switch-wrap (persp-switch (nth 0 names))))
+       (t (persp-switch (nth (1+ pos) names)))))))
 
 (defun persp-prev ()
   "Switch to previous perspective (to the left)."
   (interactive)
   (when (persp-curr)
-  (let* ((names (persp-names))
-         (pos (cl-position (persp-name (persp-curr)) names)))
-    (cond
-     ((null pos) (persp-find-some))
-     ((= pos 0)
-      (if persp-switch-wrap (persp-switch (nth (1- (length names)) names))))
-     (t (persp-switch (nth (1- pos) names)))))))
+    (let* ((names (persp-names))
+           (pos (cl-position (persp-name (persp-curr)) names)))
+      (cond
+       ((null pos) (persp-find-some))
+       ((= pos 0)
+        (if persp-switch-wrap (persp-switch (nth (1- (length names)) names))))
+       (t (persp-switch (nth (1- pos) names)))))))
 
 (defun persp-find-some ()
   "Return the name of a valid perspective.
@@ -577,9 +577,9 @@ See also `persp-switch' and `persp-remove-buffer'."
     (let ((read-buffer-function nil))
       (read-buffer "Add buffer to perspective: "))))
   (when (persp-curr)
-  (let ((buffer (get-buffer buffer)))
-    (unless (memq buffer (persp-buffers (persp-curr)))
-      (push buffer (persp-buffers (persp-curr)))))))
+    (let ((buffer (get-buffer buffer)))
+      (unless (memq buffer (persp-buffers (persp-curr)))
+        (push buffer (persp-buffers (persp-curr)))))))
 
 (defun persp-set-buffer (buffer-name)
   "Associate BUFFER-NAME with the current perspective and remove it from any other."
@@ -592,7 +592,7 @@ See also `persp-switch' and `persp-remove-buffer'."
          (cl-loop for other-persp = (persp-buffer-in-other-p (get-buffer buffer-name))
                   while other-persp
                   do (with-perspective (cdr other-persp)
-                                       (persp-remove-buffer buffer-name))))
+                       (persp-remove-buffer buffer-name))))
         (t (message "buffer %s doesn't exist" buffer-name))))
 
 (cl-defun persp-buffer-in-other-p (buffer)
@@ -618,13 +618,13 @@ Prefers perspectives in the selected frame."
     (let ((read-buffer-function nil))
       (read-buffer-to-switch "Switch to buffer: "))))
   (when (persp-curr)
-  (let ((buffer (window-normalize-buffer-to-switch-to buffer-or-name)))
-    (if (memq buffer (persp-buffers (persp-curr)))
-        (switch-to-buffer buffer)
-      (let ((other-persp (persp-buffer-in-other-p buffer)))
-        (when (eq (car-safe other-persp) (selected-frame))
-          (persp-switch (cdr other-persp)))
-        (switch-to-buffer buffer))))))
+    (let ((buffer (window-normalize-buffer-to-switch-to buffer-or-name)))
+      (if (memq buffer (persp-buffers (persp-curr)))
+          (switch-to-buffer buffer)
+        (let ((other-persp (persp-buffer-in-other-p buffer)))
+          (when (eq (car-safe other-persp) (selected-frame))
+            (persp-switch (cdr other-persp)))
+          (switch-to-buffer buffer))))))
 
 (defun persp-remove-buffer (buffer)
   "Disassociate BUFFER with the current perspective.
@@ -647,7 +647,7 @@ See also `persp-switch' and `persp-add-buffer'."
                (setq window (unless (eq window new-window) new-window))))))
         (t (bury-buffer buffer)))
   (when (persp-curr)
-  (setf (persp-buffers (persp-curr)) (remq buffer (persp-buffers (persp-curr))))))
+    (setf (persp-buffers (persp-curr)) (remq buffer (persp-buffers (persp-curr))))))
 
 (defun persp-kill (name)
   "Kill the perspective given by NAME.
@@ -656,37 +656,37 @@ Killing a perspective means that all buffers associated with that
 perspective and no others are killed."
   (interactive "i")
   (when (persp-curr)
-  (if (null name) (setq name (persp-prompt (persp-name (persp-curr)) t)))
-  (with-perspective name
-    (run-hooks 'persp-killed-hook)
-    (mapc 'persp-remove-buffer (persp-buffers (persp-curr)))
-    (setf (persp-killed (persp-curr)) t))
-  (remhash name (perspectives-hash))
-  (persp-update-modestring)
-  (when (and (persp-last) (equal name (persp-name (persp-last))))
-    (set-frame-parameter
-     nil 'persp--last
-     (let* ((persp-sort-chronologically t)
-            (names (persp-names))
-            (last (nth 1 names)))
-       (when last
-         (gethash last (perspectives-hash))))))
-  (when (and (or (not (persp-curr)) (equal name (persp-name (persp-curr))))
-             (persp-last))
-    ;; Don't let persp-last get set to the deleted persp.
-    (persp-let-frame-parameters ((persp--last (persp-last)))
-      (persp-switch (persp-find-some))))))
+    (if (null name) (setq name (persp-prompt (persp-name (persp-curr)) t)))
+    (with-perspective name
+      (run-hooks 'persp-killed-hook)
+      (mapc 'persp-remove-buffer (persp-buffers (persp-curr)))
+      (setf (persp-killed (persp-curr)) t))
+    (remhash name (perspectives-hash))
+    (persp-update-modestring)
+    (when (and (persp-last) (equal name (persp-name (persp-last))))
+      (set-frame-parameter
+       nil 'persp--last
+       (let* ((persp-sort-chronologically t)
+              (names (persp-names))
+              (last (nth 1 names)))
+         (when last
+           (gethash last (perspectives-hash))))))
+    (when (and (or (not (persp-curr)) (equal name (persp-name (persp-curr))))
+               (persp-last))
+      ;; Don't let persp-last get set to the deleted persp.
+      (persp-let-frame-parameters ((persp--last (persp-last)))
+        (persp-switch (persp-find-some))))))
 
 (defun persp-rename (name)
   "Rename the current perspective to NAME."
   (interactive "sNew name: ")
   (when (persp-curr)
-  (if (gethash name (perspectives-hash))
-      (persp-error "Perspective `%s' already exists" name)
-    (remhash (persp-name (persp-curr)) (perspectives-hash))
-    (puthash name (persp-curr) (perspectives-hash))
-    (setf (persp-name (persp-curr)) name)
-    (persp-update-modestring))))
+    (if (gethash name (perspectives-hash))
+        (persp-error "Perspective `%s' already exists" name)
+      (remhash (persp-name (persp-curr)) (perspectives-hash))
+      (puthash name (persp-curr) (perspectives-hash))
+      (setf (persp-name (persp-curr)) name)
+      (persp-update-modestring))))
 
 (cl-defun persp-all-get (name not-frame)
   "Returns the list of buffers for a perspective named NAME from any
@@ -731,12 +731,12 @@ With a prefix arg, uses the old `read-buffer' instead."
 (defun persp-complete-buffer ()
   "Perform completion on all buffers within the current perspective."
   (when (persp-curr)
-  (lexical-let ((persp-names (mapcar 'buffer-name (persp-buffers (persp-curr)))))
-    (apply-partially 'completion-table-with-predicate
-                     (or minibuffer-completion-table 'internal-complete-buffer)
-                     (lambda (name)
-                       (member (if (consp name) (car name) name) persp-names))
-                     nil))))
+    (lexical-let ((persp-names (mapcar 'buffer-name (persp-buffers (persp-curr)))))
+      (apply-partially 'completion-table-with-predicate
+                       (or minibuffer-completion-table 'internal-complete-buffer)
+                       (lambda (name)
+                         (member (if (consp name) (car name) name) persp-names))
+                       nil))))
 
 (cl-defun persp-import (name &optional dont-switch)
   "Import a perspective named NAME from another frame.  If DONT-SWITCH
@@ -802,38 +802,38 @@ See also `persp-add-buffer'."
     ad-do-it
 
     (when (persp-curr)
-    (let ((buffer (window-buffer window)))
-      (with-selected-frame frame
-        (unless (memq buffer (persp-buffers (persp-curr)))
-          ;; If a buffer from outside this perspective was selected, it's because
-          ;; this perspective is out of buffers. For lack of any better option, we
-          ;; recreate the scratch buffer.
-          ;;
-          ;; If we were just in a scratch buffer, change the name slightly.
-          ;; Otherwise our new buffer will get deleted too.
-          (let ((name (concat "*scratch* (" (persp-name (persp-curr)) ")")))
-            (when (and bury-or-kill (equal name (buffer-name old-buffer)))
-              (setq name (concat "*scratch*  (" (persp-name (persp-curr)) ")")))
-            (with-selected-window window
-              (switch-to-buffer name)
-              (funcall initial-major-mode)))))))))
+      (let ((buffer (window-buffer window)))
+        (with-selected-frame frame
+          (unless (memq buffer (persp-buffers (persp-curr)))
+            ;; If a buffer from outside this perspective was selected, it's because
+            ;; this perspective is out of buffers. For lack of any better option, we
+            ;; recreate the scratch buffer.
+            ;;
+            ;; If we were just in a scratch buffer, change the name slightly.
+            ;; Otherwise our new buffer will get deleted too.
+            (let ((name (concat "*scratch* (" (persp-name (persp-curr)) ")")))
+              (when (and bury-or-kill (equal name (buffer-name old-buffer)))
+                (setq name (concat "*scratch*  (" (persp-name (persp-curr)) ")")))
+              (with-selected-window window
+                (switch-to-buffer name)
+                (funcall initial-major-mode)))))))))
 
 (defadvice recursive-edit (around persp-preserve-for-recursive-edit)
   "Preserve the current perspective when entering a recursive edit."
   (persp-protect
     (persp-save)
     (when (persp-curr)
-    (persp-let-frame-parameters ((persp--recursive (persp-curr)))
-      (let ((old-hash (copy-hash-table (perspectives-hash))))
-        ad-do-it
-        ;; We want the buffer lists that were created in the recursive edit,
-        ;; but not the window configurations
-        (maphash (lambda (key new-persp)
-                   (let ((persp (gethash key old-hash)))
-                     (when persp
-                       (setf (persp-buffers persp) (persp-buffers new-persp)))))
-                 (perspectives-hash))
-        (set-frame-parameter nil 'persp--hash old-hash))))))
+      (persp-let-frame-parameters ((persp--recursive (persp-curr)))
+        (let ((old-hash (copy-hash-table (perspectives-hash))))
+          ad-do-it
+          ;; We want the buffer lists that were created in the recursive edit,
+          ;; but not the window configurations
+          (maphash (lambda (key new-persp)
+                     (let ((persp (gethash key old-hash)))
+                       (when persp
+                         (setf (persp-buffers persp) (persp-buffers new-persp)))))
+                   (perspectives-hash))
+          (set-frame-parameter nil 'persp--hash old-hash))))))
 
 (defadvice exit-recursive-edit (before persp-restore-after-recursive-edit)
   "Restore the old perspective when exiting a recursive edit."
@@ -860,7 +860,7 @@ named collections of buffers and window configurations."
         (setq read-buffer-function 'persp-read-buffer)
         (mapc 'persp-init-frame (frame-list))
         (when (persp-curr)
-        (setf (persp-buffers (persp-curr)) (buffer-list)))
+          (setf (persp-buffers (persp-curr)) (buffer-list)))
 
         (run-hooks 'persp-mode-hook))
     (ad-deactivate-regexp "^persp-.*")
@@ -908,11 +908,11 @@ variable will take on its local value for that perspective.  When
 a new perspective is created, the variable will inherit its value
 from the current perspective at time of creation."
   (when (persp-curr)
-  (unless (assq variable (persp-local-variables (persp-curr)))
-    (let ((entry (list variable (symbol-value variable))))
-      (dolist (frame (frame-list))
-        (cl-loop for persp being the hash-values of (frame-parameter frame 'persp--hash)
-                 do (push entry (persp-local-variables persp))))))))
+    (unless (assq variable (persp-local-variables (persp-curr)))
+      (let ((entry (list variable (symbol-value variable))))
+        (dolist (frame (frame-list))
+          (cl-loop for persp being the hash-values of (frame-parameter frame 'persp--hash)
+                   do (push entry (persp-local-variables persp))))))))
 
 (defmacro persp-setup-for (name &rest body)
   "Add code that should be run to set up the perspective named NAME.
@@ -932,17 +932,17 @@ it. In addition, if one exists already, runs BODY in it immediately."
 (defun persp-set-ido-buffers ()
   "Restrict the ido buffer to the current perspective."
   (when (persp-curr)
-  (let ((persp-names
-         (remq nil (mapcar 'buffer-name (persp-buffers (persp-curr)))))
-        (indices (make-hash-table :test 'equal)))
-    (cl-loop for elt in ido-temp-list
-             for i upfrom 0
-             do (puthash elt i indices))
-    (setq ido-temp-list
-          (sort (intersection persp-names ido-temp-list)
-                (lambda (a b)
-                  (< (gethash a indices)
-                     (gethash b indices))))))))
+    (let ((persp-names
+           (remq nil (mapcar 'buffer-name (persp-buffers (persp-curr)))))
+          (indices (make-hash-table :test 'equal)))
+      (cl-loop for elt in ido-temp-list
+               for i upfrom 0
+               do (puthash elt i indices))
+      (setq ido-temp-list
+            (sort (intersection persp-names ido-temp-list)
+                  (lambda (a b)
+                    (< (gethash a indices)
+                       (gethash b indices))))))))
 
 (defun quick-perspective-keys ()
   "Bind quick key commands to switch to perspectives.