Port to new advice mechanism

[gongzhitaao]

According to Porting Old Advice, advice such as crux-with-region-or-line may need to be updated.

1. Here is a proposed new advice syntax.

   ;; This is the advising function
   (defun me--ad-with-region-or-line (&rest r)
   "Operate on line or region."
   (if (use-region-p)
       (list (region-beginning) (region-end))
     (list (line-beginning-position) (line-end-position))))

   Either (personally prefer this style)

   (defmacro crux-with-region-or-line (func)
   "When called with no active region, call FUNC on current line."
   `(advice-add ,func :filter-args #'me--ad-with-region-or-line))
   
   (crux-with-region-or-line #'comment-or-uncomment-region)

   Or

   (defmacro crux-with-region-or-line (func)
   "When called with no active region, call FUNC on current line."
   (advice-add func :filter-args #'me--ad-with-region-or-line))
   
   (crux-with-region-or-line comment-or-uncomment-region)

2. On top of that, since crux-with-region-or-line adds advice, i.e., changes the behaviour of the function, it might be convenient to provide an option to remove the advice.

   (defmacro crux-with-region-or-line (func &optional remove)
   "If not REMOVE, add advice to FUNC, i.e., when called with no
   active region, call FUNC on current line.  Otherwise remove
   advice."
   (if remove
       `(advice-remove ,func #'me--ad-with-region-or-line)
     `(advice-add ,func :filter-args #'me--ad-with-region-or-line)))

   So (crux-with-region-or-line #'comment-or-uncomment-region) adds advice to this function while (crux-with-region-or-line #'comment-or-uncomment-region t) removes the advice.

[bbatsov]

I'm fine with your suggestions, so PR welcome! :-)

The only small downside is that the new mechanism was introduced in Emacs 24.4, but I'm guessing the majority of people are using it anyways.

[jxs]

Hi,
his is the status of this? I'm using emacs 25.3.1 and crux-with-region-or-buffer and crux-with-region-or-line are not working, it's related to this?

[bbatsov]

I think @gongzhitaao never sent this PR, however, the old advice mechanism should work just fine with newer Emacsen.

[bbatsov]

Ah, actually the PR got stalled by some upstream Emacs bug. My bad.

[gongzhitaao]

Really sorry about this dangling submission ☹️ Because of the old bug (if I remember correctly)

you have to activate and deactivate a random mark in a buffer before you can use any function advised by this crux-with-region-or-buffer, otherwise, it will report errors.

And yes, it has something to do with the upstream Emacs implementation.

It is not a big deal but really annoying. And I use some hard-code workaround now

(defun me--ad-with-region-or-line (args)
  "Operate on line or region."
  (if (region-active-p)
      args
    (let ((bol (+ (line-beginning-position) (current-indentation)))
          (eol (line-end-position)))
      (push-mark bol)
      (goto-char eol)
      (list bol eol (nth 2 args)))))

If you need, I could test it and submit a PR, probably sometime next weekend since I'm running a dealine now 😄

[gongzhitaao]

Now I use this advice again. It seems the new version of emacs still has this bug LOL. So I have to manually push a mark as a workaround...