* Re: Improvements to adjust-parens
2013-11-26 1:51 ` Improvements to adjust-parens Barry OReilly
@ 2013-11-26 1:52 ` Ryan
2013-11-26 2:26 ` Stefan Monnier
2013-11-26 6:57 ` Ryan
2 siblings, 0 replies; 7+ messages in thread
From: Ryan @ 2013-11-26 1:52 UTC (permalink / raw)
To: Barry OReilly; +Cc: emacs-devel
I believe I have already completed the copyright assignment.
On Mon Nov 25 17:51:54 2013, Barry OReilly wrote:
>
> I have recently started to use your adjust-parens package for
> Emacs. I have made a few changes to my copy that I thought you
> might want to incorporate upstream. I have attached a patch of all
> my changes; below are the ChangeLog entries for my changes (also
> inclued in the patch):
>
> * adjust-parens.el (adjust-parens-mode): Convert to minor mode
> (global-adjust-parens-mode): Add globalized mode. Uses a custom
> var to determine which modes to enable the mode in.
> (adjust-parens-p): Only adjust parens after fixing indentation and
> moving point to end of indentation.
> (adjust-parens-and-indent): Correct handling of prefix arg. Now
> only explicit numeric prefix arguments (i.e. not `C-u') affect the
> number of parens adjusted, and negative & zero prefix are handled
> correctly.
>
> * adjust-parens-tests.el (apt-mode-test): Add tests for minor mode
> enabling/disabling
> (apt-indent-dedent-test): Add tests for behavior when indentation
> is incorrect or point is not at end of indentation
>
> The to main changes of interest are the conversion of
> adjust-parens into a minor mode that can be enabled and disabled,
> and the changing the initial behavior of the TAB key when the
> current line is wrongly indented or the point is at the wrong
> position. I hope you find this patch useful. Please let me know
> what you think of it.
>
>
> Thanks. adjust-parens is in ELPA which requires a copyright assignment
> to FSF for non tiny changes. I'm not sure how to check whether you're
> on the list of those who have completed the paperwork.
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Improvements to adjust-parens
2013-11-26 1:51 ` Improvements to adjust-parens Barry OReilly
2013-11-26 1:52 ` Ryan
@ 2013-11-26 2:26 ` Stefan Monnier
2013-11-26 6:57 ` Ryan
2 siblings, 0 replies; 7+ messages in thread
From: Stefan Monnier @ 2013-11-26 2:26 UTC (permalink / raw)
To: Barry OReilly; +Cc: Ryan, emacs-devel
> Thanks. adjust-parens is in ELPA which requires a copyright assignment
> to FSF for non tiny changes. I'm not sure how to check whether you're
> on the list of those who have completed the paperwork.
Ryan did sign his paperwork, thank you,
Stefan
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: Improvements to adjust-parens
2013-11-26 1:51 ` Improvements to adjust-parens Barry OReilly
2013-11-26 1:52 ` Ryan
2013-11-26 2:26 ` Stefan Monnier
@ 2013-11-26 6:57 ` Ryan
2013-11-26 23:04 ` Barry OReilly
2 siblings, 1 reply; 7+ messages in thread
From: Ryan @ 2013-11-26 6:57 UTC (permalink / raw)
To: Barry OReilly; +Cc: emacs-devel
[-- Attachment #1: Type: text/plain, Size: 1919 bytes --]
Oh, I forgot to mention, the same changes are in a Github repo, in case
you want to cherry-pick things:
https://github.com/DarwinAwardWinner/adjust-parens
On 11/25/13, 5:51 PM, Barry OReilly wrote:
>
>
> I have recently started to use your adjust-parens package for
> Emacs. I have made a few changes to my copy that I thought you
> might want to incorporate upstream. I have attached a patch of all
> my changes; below are the ChangeLog entries for my changes (also
> inclued in the patch):
>
> * adjust-parens.el (adjust-parens-mode): Convert to minor mode
> (global-adjust-parens-mode): Add globalized mode. Uses a custom
> var to determine which modes to enable the mode in.
> (adjust-parens-p): Only adjust parens after fixing indentation and
> moving point to end of indentation.
> (adjust-parens-and-indent): Correct handling of prefix arg. Now
> only explicit numeric prefix arguments (i.e. not `C-u') affect the
> number of parens adjusted, and negative & zero prefix are handled
> correctly.
>
> * adjust-parens-tests.el (apt-mode-test): Add tests for minor mode
> enabling/disabling
> (apt-indent-dedent-test): Add tests for behavior when indentation
> is incorrect or point is not at end of indentation
>
> The to main changes of interest are the conversion of
> adjust-parens into a minor mode that can be enabled and disabled,
> and the changing the initial behavior of the TAB key when the
> current line is wrongly indented or the point is at the wrong
> position. I hope you find this patch useful. Please let me know
> what you think of it.
>
>
> Thanks. adjust-parens is in ELPA which requires a copyright assignment
> to FSF for non tiny changes. I'm not sure how to check whether you're
> on the list of those who have completed the paperwork.
>
[-- Attachment #2: Type: text/html, Size: 3725 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Improvements to adjust-parens
2013-11-26 6:57 ` Ryan
@ 2013-11-26 23:04 ` Barry OReilly
2013-11-27 1:04 ` Ryan C. Thompson
0 siblings, 1 reply; 7+ messages in thread
From: Barry OReilly @ 2013-11-26 23:04 UTC (permalink / raw)
To: Ryan; +Cc: emacs-devel
[-- Attachment #1: Type: text/plain, Size: 3391 bytes --]
(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))))))))
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.
+ (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))
+ ;; 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>.
+(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)
[-- Attachment #2: Type: text/html, Size: 3878 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: Improvements to adjust-parens
2013-11-26 23:04 ` Barry OReilly
@ 2013-11-27 1:04 ` Ryan C. Thompson
2013-11-27 20:00 ` Barry OReilly
0 siblings, 1 reply; 7+ messages in thread
From: Ryan C. Thompson @ 2013-11-27 1:04 UTC (permalink / raw)
To: Barry OReilly; +Cc: emacs-devel
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".
^ permalink raw reply [flat|nested] 7+ messages in thread