bug#8150: 23.2; cperl-uncomment-region is NOT an inverse of cperl-comment-region

[Lawrence Mitchell <wence@gmx.li>]

Dima Kogan wrote:
> I can and I do, but the current behavior seems wrong. If
> (comment-region beg end -1) is no longer supported, it should generate
> an error instead of succeeding badly. Should we simply call
> (uncomment-region beg end 1) in that case? I can make the patch, but
> would need to know what the current plan is.

Note that the problem is that (uncomment-region A B 1) is not the
inverse of (comment-region A B 1).  Irrespective of the major
mode in effect:

emacs -Q

C-x h
C-1 M-x comment-region RET
C-x h
C-1 M-x uncomment-region RET

Note how there is now a space at the beginning of every line in
the *scratch* buffer.

This is due to the following code in uncomment-region-default:

|	  (if (null arg) (delete-region (point-min) (point))
|	    (skip-syntax-backward " ")
|	    (delete-char (- numarg))

   # foo bar
     ^ point is here when the above code is called.

If an argument is supplied, arg is non-nil and so to delete the
comment we first move backwards over whitespace and then delete
as many comment characters as specified by the argument (1 in
this case).

This patch appears to do the right thing from my minimal testing,
but I don't know if it will have any unfortunate side-effects:

2011-03-03  Lawrence Mitchell  <wence@gmx.li>

	Make uncomment-region the inverse of comment-region.

	* newcomment.el (uncomment-region-default): Delete
          skipped-over whitespace if all comment characters have
          been removed.

	This makes M-1 M-x uncomment-region RET the inverse of
	M-1 M-x comment-region RET.  Previously, commenting and
	then uncommenting a region with an argument would leave a
	space at the beginning of each line.

diff --git a/lisp/newcomment.el b/lisp/newcomment.el
index d88b76a..bcdc1e6 100644
--- a/lisp/newcomment.el
+++ b/lisp/newcomment.el
@@ -871,15 +871,20 @@ comment markers."
 	  (when (and sre (looking-at (concat "\\s-*\n\\s-*" srei)))
 	    (goto-char (match-end 0)))
 	  (if (null arg) (delete-region (point-min) (point))
-	    (skip-syntax-backward " ")
-	    (delete-char (- numarg))
-	    (unless (or (bobp)
-			(save-excursion (goto-char (point-min))
-					(looking-at comment-start-skip)))
-	      ;; If there's something left but it doesn't look like
-	      ;; a comment-start any more, just remove it.
-	      (delete-region (point-min) (point))))
-
+            (let* ((opoint (point-marker))
+                   (nchar (skip-syntax-backward " ")))
+              (delete-char (- numarg))
+              (unless (or (bobp)
+                          (save-excursion (goto-char (point-min))
+                                          (looking-at comment-start-skip)))
+                ;; If there's something left but it doesn't look like
+                ;; a comment-start any more, just remove it.
+                (delete-region (point-min) (point)))
+              (save-excursion
+                (goto-char (point-min))
+                (unless (looking-at comment-start-skip)
+                  (goto-char opoint)
+                  (delete-char nchar)))))
 	  ;; Remove the end-comment (and leading padding and such).
 	  (goto-char (point-max)) (comment-enter-backward)
 	  ;; Check for special `=' used sometimes in comment-box.


-- 
Lawrence Mitchell <wence@gmx.li>