Re: address@hidden: [patch] factor out comment-or-uncomment feature from comment-dwim]

[Stefan Monnier]

> Would you please DTRT here?

The patch looks fine to me.

On a related note:
how about a transient form of transient-mark-mode ?
We agreed that it would be desirable a while back, but the concrete details
weren't clear at the time.  I have a more concrete proposal now:

--- simple.el   7 Apr 2002 10:10:23 -0000       1.533
+++ simple.el   8 Apr 2002 22:06:00 -0000
@@ -2233,11 +2270,19 @@
   "Deactivate the mark by setting `mark-active' to nil.
 \(That makes a difference only in Transient Mark mode.)
 Also runs the hook `deactivate-mark-hook'."
-  (if transient-mark-mode
-      (progn
+  (cond
+   ((eq transient-mark-mode 'lambda)
+    (setq transient-mark-mode nil))
+   (transient-mark-mode
        (setq mark-active nil)
        (run-hooks 'deactivate-mark-hook))))
 
+(defun mark-and-highlight ()
+  "Set the mark and temporarily activate `transient-mark-mode'."
+  (interactive)
+  (setq transient-mark-mode 'lambda)
+  (push-mark nil nil t))
+
 (defun set-mark (pos)
   "Set this buffer's mark to POS.  Don't use this function!
 That is to say, don't use this function unless you want


--- keyboard.c  3 Apr 2002 08:36:56 -0000       1.669
+++ keyboard.c  8 Apr 2002 22:05:22 -0000
@@ -1718,9 +1721,15 @@
        {
          if (!NILP (Vdeactivate_mark) && !NILP (Vtransient_mark_mode))
            {
+             /* We could also call `deactivate'mark'.  */
+             if (EQ (Vtransient_mark_mode, Qlambda))
+               Vtransient_mark_mode = Qnil;
+             else
+               {
              current_buffer->mark_active = Qnil;
              call1 (Vrun_hooks, intern ("deactivate-mark-hook"));
            }
+           }
          else if (current_buffer != prev_buffer || MODIFF != prev_modiff)
            call1 (Vrun_hooks, intern ("activate-mark-hook"));
        }


An alternative would be to provide

  (defun activate-and-highlight-region ()
    (setq transient-mark-mode 'lambda))

Of course we can have both activate-and-highlight-region as well as
mark-and-highlight.  Also I'm wondering where we could/should bind
these commands.  It would be nice to have it as something like
C-u C-SPC (except that this one is already taken, obviously).


-- Stefan


> ------- Start of forwarded message -------
> X-Authentication-Warning: gamma.cis.ohio-state.edu: rutt set sender to 
> address@hidden using -f
> To: address@hidden
> Subject: [patch] factor out comment-or-uncomment feature from comment-dwim
> From: Benjamin Rutt <address@hidden>
> Mail-Followup-To: address@hidden
> Sender: address@hidden
> Date: Sat, 06 Apr 2002 23:19:45 -0500
> 
> During a discussion today on gnu.emacs.help, I lamented the fact that
> only emacs users who were using `transient-mark-mode' could take
> advantage of `comment-dwim's ability to detect whether a region is
> commented, and uncomment the region if so and comment the region if
> not.  I personally don't use `transient-mark-mode', but I would hate
> to lose out on `comment-dwim's neat feature just because I don't use
> `transient-mark-mode'.
> 
> An idea that came out of that discussion was to factor the
> comment-or-uncomment feature out of `comment-dwim' and place that code
> in a new function named `comment-or-uncomment-region' that would be
> callable interactively.  And then have `comment-dwim' make a call to
> `comment-or-uncomment-region' in the appropriate place.  But since
> `comment-or-uncomment-region' can also be called interactively, this
> change would make me very happy because I could gain the "comment or
> uncomment" feature of `comment-dwim' without having to use
> `transient-mark-mode'.
> 
> So I guess the question is, have I convinced the maintainer of
> newcomment.el that this change is worthwhile?
> 
> BTW, if necessary, I have signed papers for emacs already.  Here
> begins the patch, against tonight's cvs:
> 
> *** newcomment.el.~1.45.~     Sun Mar  3 20:10:55 2002
> - --- newcomment.el   Sat Apr  6 22:00:07 2002
> ***************
> *** 893,898 ****
> - --- 893,911 ----
>       (comment-region beg end (+ comment-add arg))))
>   
>   ;;;###autoload
> + (defun comment-or-uncomment-region (beg end &optional arg)
> +   "Call `comment-region', unless the region only consists of comments,
> + in which case call `uncomment-region'.  If a prefix arg is given, it
> + is passed on to the respective function."
> +   (interactive "*r\nP")
> +   (if (save-excursion ;; check for already commented region
> +     (goto-char beg)
> +     (comment-forward (point-max))
> +     (<= end (point)))
> +       (uncomment-region beg end arg)
> +     (comment-region beg end arg)))
> + 
> + ;;;###autoload
>   (defun comment-dwim (arg)
>     "Call the comment command you want (Do What I Mean).
>   If the region is active and `transient-mark-mode' is on, call
> ***************
> *** 906,917 ****
>     (if (and mark-active transient-mark-mode)
>         (let ((beg (min (point) (mark)))
>           (end (max (point) (mark))))
> !     (if (save-excursion ;; check for already commented region
> !           (goto-char beg)
> !           (comment-forward (point-max))
> !           (<= end (point)))
> !         (uncomment-region beg end arg)
> !       (comment-region beg end arg)))
>       (if (save-excursion (beginning-of-line) (not (looking-at "\\s-*$")))
>       ;; FIXME: If there's no comment to kill on this line and ARG is
>       ;; specified, calling comment-kill is not very clever.
> - --- 919,925 ----
>     (if (and mark-active transient-mark-mode)
>         (let ((beg (min (point) (mark)))
>           (end (max (point) (mark))))
> !     (comment-or-uncomment-region beg end))
>       (if (save-excursion (beginning-of-line) (not (looking-at "\\s-*$")))
>       ;; FIXME: If there's no comment to kill on this line and ARG is
>       ;; specified, calling comment-kill is not very clever.
> 
> - -- 
> Benjamin
> 
> _______________________________________________
> Emacs-devel mailing list
> address@hidden
> http://mail.gnu.org/mailman/listinfo/emacs-devel
> ------- End of forwarded message -------
>