comment-indent and tail comments

comment-indent and tail comments

[Yoni Rabkin Katzenell]

I'm using GNU Emacs 22.0.50.1 (i686-pc-linux-gnu, X toolkit, Xaw3d
scroll bars) of 2006-07-23 on ardbeg.

I've noticed that sometimes indenting a region of Lisp code will modify
the buffer (as seen by the ** in the mode line) without actually
changing the contents of the buffer.

After looking at it I got to Line 604 in the function `comment-indent'
which is defined in the file "newcomment.el". Line 604 runs
`delete-region', which modifies the buffer and then runs `indent-to'
which intents to the exact same place as the comment was before.

This only happens with tail comments positioned after a certain column.

I arrived at this conclusion by changing `comment-indent' so that it
would modify the buffer only if the indentation would actually modify
the line in question.

Here is the "instrumented" version of `comment-indent', which I used to
find the problem, the seemingly problematic part starts at "(unless (=
(current-column) indent)", most of the way down the function:

(defun comment-indent (&optional continue)
  "Indent this line's comment to `comment-column', or insert an empty comment.
If CONTINUE is non-nil, use the `comment-continue' markers if any."
  (interactive "*")
  (comment-normalize-vars)
  (let* ((empty (save-excursion (beginning-of-line)
				(looking-at "[ \t]*$")))
	 (before 0)
	 (after 0)
	 (isolate "")
	 (starter (or (and continue comment-continue)
		      (and empty block-comment-start) comment-start))
	 (ender (or (and continue comment-continue "")
		    (and empty block-comment-end) comment-end)))
    (unless starter (error "No comment syntax defined"))
    (beginning-of-line)
    (let* ((eolpos (line-end-position))
	   (begpos (comment-search-forward eolpos t))
	   cpos indent)
      ;; An existing comment?
      (if begpos
	  (progn
	    (if (and (not (looking-at "[\t\n ]"))
		     (looking-at comment-end-skip))
		;; The comment is empty and we have skipped all its space
		;; and landed right before the comment-ender:
		;; Go back to the middle of the space.
		(forward-char (/ (skip-chars-backward " \t") -2)))
	    (setq cpos (point-marker)))
	;; If none, insert one.
	(if comment-insert-comment-function
	    (funcall comment-insert-comment-function)
	  (save-excursion
	    ;; Some `comment-indent-function's insist on not moving
	    ;; comments that are in column 0, so we first go to the
	    ;; likely target column.
	    (indent-to comment-column)
	    ;; Ensure there's a space before the comment for things
	    ;; like sh where it matters (as well as being neater).
	    (unless (memq (char-before) '(nil ?\n ?\t ?\ ))
	      (insert ?\ ))
	    (setq begpos (point))
	    (insert starter)
	    (setq cpos (point-marker))
	    (insert ender))))
      (goto-char begpos)
      ;; Compute desired indent.
      (setq indent (save-excursion (funcall comment-indent-function)))
      ;; If `indent' is nil and there's code before the comment, we can't
      ;; use `indent-according-to-mode', so we default to comment-column.
      (unless (or indent (save-excursion (skip-chars-backward " \t") (bolp)))
	(setq indent comment-column))
      (if (not indent)
	  ;; comment-indent-function refuses: delegate to line-indent.
	  (indent-according-to-mode)
	;; If the comment is at the left of code, adjust the indentation.
	(unless (save-excursion (skip-chars-backward " \t") (bolp))
	  ;; Avoid moving comments past the fill-column.
	  (let ((max (+ (current-column)
			(- (or comment-fill-column fill-column)
			   (save-excursion (end-of-line) (current-column))))))
	    (if (<= max indent)
		(setq indent max)    ;Don't move past the fill column.
	      ;; We can choose anywhere between indent..max.
	      ;; Let's try to align to a comment on the previous line.
	      (let ((other nil)
		    (min (max indent
			      (save-excursion (skip-chars-backward " \t")
					      (1+ (current-column))))))
		(save-excursion
		  (when (and (zerop (forward-line -1))
			     (setq other (comment-search-forward
					  (line-end-position) t)))
		    (goto-char other) (setq other (current-column))))
		(if (and other (<= other max) (>= other min))
		    ;; There is a comment and it's in the range: bingo.
		    (setq indent other)
		  ;; Let's try to align to a comment on the next line, then.
		  (let ((other nil))
		    (save-excursion
		      (when (and (zerop (forward-line 1))
				 (setq other (comment-search-forward
					      (line-end-position) t)))
			(goto-char other) (setq other (current-column))))
		    (if (and other (<= other max) (> other min))
			;; There is a comment and it's in the range: bingo.
			(setq indent other))))))))
	(unless (= (current-column) indent)
	  ;; If that's different from current, change it.
	  ;;
	  ;; since this line is not like the one before or after it,
	  ;; we can deal with it in isolation.
	  (setq before (current-column))
	  (save-excursion
	    (setq isolate 
		  (buffer-substring (point-at-bol) (point-at-eol))))
	  (with-temp-buffer
	    (insert isolate)
	    (goto-char before)
	    (delete-region (point) (progn (skip-chars-backward " \t") (point)))
	    (indent-to (if (bolp) indent
			 (max indent (1+ (current-column)))))
	    (setq after (current-column))
	    (setq isolate
		  (buffer-substring (point-at-bol) (point-at-eol))))
	  (when (not (= before after))
	    (delete-region (point) (progn (skip-chars-backward " \t") (point)))
	    (indent-to (if (bolp) indent
			 (max indent (1+ (current-column))))))))
      (goto-char cpos)
      (set-marker cpos nil))))

-- 
   "Cut your own wood and it will warm you twice"

Re: comment-indent and tail comments

[Richard Stallman]

    After looking at it I got to Line 604 in the function `comment-indent'
    which is defined in the file "newcomment.el". Line 604 runs
    `delete-region', which modifies the buffer and then runs `indent-to'
    which intents to the exact same place as the comment was before.

Do you mean this code?

	(unless (= (current-column) indent)
	  ;; If that's different from current, change it.
	  (delete-region (point) (progn (skip-chars-backward " \t") (point)))
	  (indent-to (if (bolp) indent
		       (max indent (1+ (current-column)))))))

Does this fix it?

*** newcomment.el	11 May 2006 14:33:16 -0400	1.94
--- newcomment.el	09 Aug 2006 19:23:27 -0400	
***************
*** 599,609 ****
  		    (if (and other (<= other max) (> other min))
  			;; There is a comment and it's in the range: bingo.
  			(setq indent other))))))))
  	(unless (= (current-column) indent)
- 	  ;; If that's different from current, change it.
  	  (delete-region (point) (progn (skip-chars-backward " \t") (point)))
! 	  (indent-to (if (bolp) indent
! 		       (max indent (1+ (current-column)))))))
        (goto-char cpos)
        (set-marker cpos nil))))
  
--- 599,614 ----
  		    (if (and other (<= other max) (> other min))
  			;; There is a comment and it's in the range: bingo.
  			(setq indent other))))))))
+ 	;; Update INDENT to leave at least one space
+ 	;; after other nonwhite text on the line.
+ 	(save-excursion
+ 	  (skip-chars-backward " \t") 
+ 	  (unless (bolp)
+ 	    (setq indent (max indent (1+ (current-column))))))
+ 	;; If that's different from comment's current position, change it.
  	(unless (= (current-column) indent)
  	  (delete-region (point) (progn (skip-chars-backward " \t") (point)))
! 	  (indent-to indent)))
        (goto-char cpos)
        (set-marker cpos nil))))

Re: comment-indent and tail comments

[Yoni Rabkin Katzenell]

Richard Stallman <rms@gnu.org> writes:

> Do you mean this code?
>
> 	(unless (= (current-column) indent)
> 	  ;; If that's different from current, change it.
> 	  (delete-region (point) (progn (skip-chars-backward " \t") (point)))
> 	  (indent-to (if (bolp) indent
> 		       (max indent (1+ (current-column)))))))

Yes.

> Does this fix it?

Yes, your patch fixes the problem.

-- 
   "Cut your own wood and it will warm you twice"