[nongnu] elpa/git-commit 4bcb303154 12/26: Use and-let* for side-effects even more

[ELPA Syncer]

branch: elpa/git-commit
commit 4bcb3031542deee77e5a5899e266fbe61f958ced
Author: Jonas Bernoulli <jonas@bernoul.li>
Commit: Jonas Bernoulli <jonas@bernoul.li>

    Use and-let* for side-effects even more
    
    The previous commit also replaces many instances of `when-let' with
    `and-let'.  In those cases I did not hesitate.  This commit addresses
    instances where side-effects are involved.
    
    My current reasoning is that if the returned value matters, then
    `and'/`and-let*' should be used, and whether there are side-effects is
    only a secondary consideration.  Of course there is a gray area; if it
    is really important to emphasize the side-effect, then one might still
    want to use `when'/`when-let*'.
    
    The simplest case is the change to `magit-diff-visit--hunk'.  That
    function doesn't actually visit anything; instead it returns the hunk
    which is relevant when visiting.  But the function name could give the
    false impression that it somehow visits a hunk.  Preventing that false
    impression is a good reason to use `and-let*'.  Additionally the
    side-effect of the first form in BODY is local to the BODY, to the
    outside code this is an irrelevant implementation detail.
    
    `magit-diff--dwim' call `deactivate-mark', which clearly is a
    side-effect that affects the global buffer state.  However this
    side-effect is not relevant to the direct outside code, which on the
    other hand cares very much about the returned value.
    
    Finally `magit-hunk-goto-successor', which is very much about the
    side-effect of "going to the successor".  This is so obvious that I
    don't think it is necessary to signal the side-effect-ness.  Again the
    returned value is used for flow-control, which should be made obvious,
    so `and-let*' it is.
    
    Unfortunately, due to a bug in Emacs 26, we cannot actually use
    `and-let*' in some of these cases.
---
 lisp/magit-diff.el | 91 +++++++++++++++++++++++++++---------------------------
 lisp/magit-git.el  |  6 ++--
 lisp/magit-refs.el |  2 +-
 3 files changed, 50 insertions(+), 49 deletions(-)

diff --git a/lisp/magit-diff.el b/lisp/magit-diff.el
index f942b42bdf..1141973675 100644
--- a/lisp/magit-diff.el
+++ b/lisp/magit-diff.el
@@ -1090,9 +1090,10 @@ The information can be in three forms:
 
 If no DWIM context is found, nil is returned."
   (cond
-   ((--when-let (magit-region-values '(commit branch) t)
+   ((when-let* ((commits (magit-region-values '(commit branch) t)))
+      ;; Cannot use and-let* because of debbugs#31840.
       (deactivate-mark)
-      (concat (car (last it)) ".." (car it))))
+      (concat (car (last commits)) ".." (car commits))))
    (magit-buffer-refname
     (cons 'commit magit-buffer-refname))
    ((derived-mode-p 'magit-stash-mode)
@@ -1141,26 +1142,26 @@ If no DWIM context is found, nil is returned."
         (t range)))
 
 (defun magit-diff--region-range (&optional interactive mbase)
-  (when-let ((commits (magit-region-values '(commit branch) t)))
-    (let ((revA (car (last commits)))
-          (revB (car commits)))
-      (when interactive
-        (deactivate-mark))
-      (if mbase
-          (let ((base (magit-git-string "merge-base" revA revB)))
-            (cond
-             ((string= (magit-rev-parse revA) base)
-              (format "%s..%s" revA revB))
-             ((string= (magit-rev-parse revB) base)
-              (format "%s..%s" revB revA))
-             (interactive
-              (let ((main (magit-completing-read "View changes along"
-                                                 (list revA revB)
-                                                 nil t nil nil revB)))
-                (format "%s...%s"
-                        (if (string= main revB) revA revB) main)))
-             (t "%s...%s" revA revB)))
-        (format "%s..%s" revA revB)))))
+  (when-let* ((commits (magit-region-values '(commit branch) t)) ;debbugs#31840
+              (revA (car (last commits)))
+              (revB (car commits)))
+    (when interactive
+      (deactivate-mark))
+    (if mbase
+        (let ((base (magit-git-string "merge-base" revA revB)))
+          (cond
+           ((string= (magit-rev-parse revA) base)
+            (format "%s..%s" revA revB))
+           ((string= (magit-rev-parse revB) base)
+            (format "%s..%s" revB revA))
+           (interactive
+            (let ((main (magit-completing-read "View changes along"
+                                               (list revA revB)
+                                               nil t nil nil revB)))
+              (format "%s...%s"
+                      (if (string= main revB) revA revB) main)))
+           (t "%s...%s" revA revB)))
+      (format "%s..%s" revA revB))))
 
 (defun magit-diff-read-range-or-commit (prompt &optional secondary-default 
mbase)
   "Read range or revision with special diff range treatment.
@@ -1677,24 +1678,24 @@ the Magit-Status buffer for DIRECTORY."
       (list buf nil))))
 
 (defun magit-diff-visit--hunk ()
-  (when-let ((scope (magit-diff-scope)))
-    (let ((section (magit-current-section)))
-      (cl-case scope
-        ((file files)
-         (setq section (car (oref section children))))
-        (list
-         (setq section (car (oref section children)))
-         (when section
-           (setq section (car (oref section children))))))
-      (and
-       ;; Unmerged files appear in the list of staged changes
-       ;; but unlike in the list of unstaged changes no diffs
-       ;; are shown here.  In that case `section' is nil.
-       section
-       ;; Currently the `hunk' type is also abused for file
-       ;; mode changes, which we are not interested in here.
-       (not (equal (oref section value) '(chmod)))
-       section))))
+  (when-let* ((scope (magit-diff-scope)) ;debbugs#31840
+             (section (magit-current-section)))
+    (cl-case scope
+      ((file files)
+       (setq section (car (oref section children))))
+      (list
+       (setq section (car (oref section children)))
+       (when section
+         (setq section (car (oref section children))))))
+    (and
+     ;; Unmerged files appear in the list of staged changes
+     ;; but unlike in the list of unstaged changes no diffs
+     ;; are shown here.  In that case `section' is nil.
+     section
+     ;; Currently the `hunk' type is also abused for file
+     ;; mode changes, which we are not interested in here.
+     (not (equal (oref section value) '(chmod)))
+     section)))
 
 (defun magit-diff-visit--goto-from-p (section in-worktree)
   (and magit-diff-visit-previous-blob
@@ -2843,7 +2844,7 @@ It the SECTION has a different type, then do nothing."
 
 (defun magit-hunk-goto-successor (section arg)
   (and (magit-hunk-section-p section)
-       (when-let ((parent (magit-get-section
+       (and-let* ((parent (magit-get-section
                            (magit-section-ident
                             (oref section parent)))))
          (let* ((children (oref parent children))
@@ -3191,10 +3192,10 @@ are highlighted."
     (cond
      ((not magit-diff-adjust-tab-width)
       tab-width)
-     ((--when-let (find-buffer-visiting file)
-        (cache (buffer-local-value 'tab-width it))))
-     ((--when-let (assoc file magit-diff--tab-width-cache)
-        (or (cdr it)
+     ((and-let* ((buffer (find-buffer-visiting file)))
+        (cache (buffer-local-value 'tab-width buffer))))
+     ((and-let* ((elt (assoc file magit-diff--tab-width-cache)))
+        (or (cdr elt)
             tab-width)))
      ((or (eq magit-diff-adjust-tab-width 'always)
           (and (numberp magit-diff-adjust-tab-width)
diff --git a/lisp/magit-git.el b/lisp/magit-git.el
index 46fc791ca4..d5b8d8aa6c 100644
--- a/lisp/magit-git.el
+++ b/lisp/magit-git.el
@@ -2119,9 +2119,9 @@ Return a list of two integers: (A>B B>A)."
                     (if rev (concat rev "^{commit}") "HEAD") "--"))
 
 (defun magit-format-rev-summary (rev)
-  (--when-let (magit-rev-format "%h %s" rev)
-    (magit--put-face 0 (string-match " " it) 'magit-hash it)
-    it))
+  (when-let* ((str (magit-rev-format "%h %s" rev))) ;debbugs#31840
+    (magit--put-face 0 (string-match " " str) 'magit-hash str)
+    str))
 
 (defvar magit-ref-namespaces
   '(("\\`HEAD\\'"                  . magit-head)
diff --git a/lisp/magit-refs.el b/lisp/magit-refs.el
index c6f9ef0598..c1216f919c 100644
--- a/lisp/magit-refs.el
+++ b/lisp/magit-refs.el
@@ -359,7 +359,7 @@ Type \\[magit-reset] to reset `HEAD' to the commit at point.
      ((eq major-mode 'magit-refs-mode)
       (setq args magit-buffer-arguments))
      ((and (memq use-buffer-args '(always selected))
-           (when-let ((buffer (magit-get-mode-buffer
+           (when-let* ((buffer (magit-get-mode-buffer ;debbugs#31840
                                'magit-refs-mode nil
                                (eq use-buffer-args 'selected))))
              (setq args (buffer-local-value 'magit-buffer-arguments buffer))