emacs-devel
[Top][All Lists]
Advanced

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

Re: Improvements to adjust-parens


From: Ryan C. Thompson
Subject: Re: Improvements to adjust-parens
Date: Tue, 26 Nov 2013 17:04:56 -0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.1.1

Replies in line below:

On 11/26/2013 03:04 PM, Barry OReilly wrote:
       (and (not (use-region-p))
-           (<= orig-pos (point))))))
+           ;; Corrent line is properly indented
+           (= (current-column)
+              (let ((indent (calculate-lisp-indent)))
+                (if (listp indent)
+                    (car indent)
+                  indent)))
+           ;; Point at indentation
+           (= orig-pos (point))))))

Revising the diff to account for nil indent and to put the most
expensive check last:

-      (and (not (use-region-p))
-           (<= orig-pos (point))))))
+      (and (= orig-pos (point))
+           (not (use-region-p))
+           ;; Corrent line is properly indented
+           (let ((indent (calculate-lisp-indent)))
+             (and indent
+                  (= (current-column)
+                     (if (listp indent)
+                         (car indent)
+                       indent))))))))


Sounds good. Also change "corrent" to "current". That's probably a typo on my part.

Regarding:

+       ;; Only pass an explicit numeric prefix, not `C-u' prefix.
+       (unless (listp parg)
+         (prefix-numeric-value parg)))

What sort of use cases do prefix args as lists normally serve? From
what I see, they come in powers of four, which doesn't seem applicable
to adjust-parens. The Elisp manual entry doesn't really cover the
"why" of this kind of prefix arg.

I believe the "C-u" prefix is passed as a list just so you can tell the difference between e.g. "C-4" and "C-u", i.e. a numeric vs generic prefix argument. I don't think there is ever more than one element in the list. In any case, you can make the prefix arguments work however you want.


+  (cl-callf or parg 1)
+  ;; Negative prefix arg inverts the behavior
+  (when (< parg 0)
+    (setq parg (- parg)
+          adjust-function
+          (cl-case adjust-function
+ (adjust-close-paren-for-indent 'adjust-close-paren-for-dedent) + (adjust-close-paren-for-dedent 'adjust-close-paren-for-indent) + (otherwise (error "Unknown adjust-function: %s" adjust-function)))))
+  (when (> parg 0)

Processing negative prefix arg makes sense. I think it is cleaner to
do it one level higher and pass the right function as the
adjust-function argument. eg:

  (adjust-parens-and-indent
   (if (< parg 0)
       'adjust-close-paren-for-dedent
     'adjust-close-paren-for-indent)
   (abs parg))

That's fine. I just did the first thing that worked.


+    ;; Move forward (but not back) to end of indentation (but don't
+    ;; change the indentation unlike `indent-for-tab-command'.
+    (when (< (current-column) (current-indentation))
+      (back-to-indentation))))

I figured since <backtab> is unbound in 'emacs -Q', adjust-parens
should do nothing in this case. The analogous <backtab> in Python
appears to also do nothing if it does not dedent.

It might be easier to decide what to do here with better perspective
about how people use <backtab>.

I forget what my logic was here. Feel free to do whatever you think is appropriate. I think backtab is actually not used in many modes.

+(define-globalized-minor-mode global-adjust-parens-mode adjust-parens-mode
           (lambda ()
-            (local-set-key (kbd "TAB") 'lisp-indent-adjust-parens)
- (local-set-key (kbd "<backtab>") 'lisp-dedent-adjust-parens)))
+    ;; Add or remove hook
+    (funcall (if global-adjust-parens-mode #'add-hook #'remove-hook)
+             'after-change-major-mode-hook
+             #'adjust-parens-after-change-mm-hook)
+    ;; Run the hook in existing buffers to enable/disable the mode
+    (dolist (buf (buffer-list))
+      (with-current-buffer buf
+        (adjust-parens-after-change-mm-hook)))))

adjust-parens doesn't seem global in nature, so I'm uncertain about
the choice of a global minor mode. I think a local minor mode makes
sense though. Then the user would enable it in major modes by eg:

  (add-hook 'emacs-lisp-mode-hook #'adjust-parens-mode)


The global mode isn't really "global". It only activates in buffers with approved major modes. This is essentially a way for users who do all their customization through M-x customize to auto-enable adjust-parens in specific modes, by enabling the global mode and then customizing "adjust-parens-enabled-major-modes".



reply via email to

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