comment-dwim on blank lines

comment-dwim on blank lines

[Glenn Morris]

If comment-dwim is called on a non-blank line, it calls comment-indent
to do the work of either inserting or indenting a comment. On a blank
line, it does the work of inserting a new comment itself. Is there a
reason for this difference in behaviour?

It means that there isn't really any way for modes to customize what
comment-dwim does on blank lines, other than by abusing
indent-line-function. In particular, comment-insert-comment-function
does not get called, which seems to contradict its doc-string.

Re: comment-dwim on blank lines

[Stefan Monnier]

> If comment-dwim is called on a non-blank line, it calls comment-indent
> to do the work of either inserting or indenting a comment. On a blank
> line, it does the work of inserting a new comment itself. Is there a
> reason for this difference in behaviour?

> It means that there isn't really any way for modes to customize what
> comment-dwim does on blank lines, other than by abusing
> indent-line-function. In particular, comment-insert-comment-function
> does not get called, which seems to contradict its doc-string.

It's clearly an oversight.  comment-insert-comment-function was added due to
a specific need and whoever implemented didn't look at other related cases.
Feel free to fix it.


        Stefan

Re: comment-dwim on blank lines

[Glenn Morris]

Stefan Monnier wrote:

> comment-insert-comment-function was added due to a specific need and
> whoever implemented didn't look at other related cases. Feel free to
> fix it.

Will do. I'll also fix comment-indent, which as it stands does not
work when comment-insert-comment-function is set. I thought it was
suspicious that comment-insert-comment-function isn't actually used
anywhere in Emacs...

Re: comment-dwim on blank lines

[martin rudalics]

[-- Attachment #1: Type: text/plain, Size: 618 bytes --]

Glen, if you're going to spend some time on newcomment please have a
look at the below.  I once asked Stefan but apparently he forgot.

With emacs -Q customize `comment-style' to box or box-multi.  In
*scratch* insert

(foo bar baz)

set the region around bar and do `comment-region'.  Gets you

(foo ;; bar ;; baz)

instead of

(foo ;; bar ;;
      baz)

Responsible is the

     (when block (unless ce (setq ce (comment-string-reverse cs))))

line in `comment-region-internal' which makes the subsequent

       (unless (or ce (eolp)) (insert "\n") (indent-according-to-mode))

fail.  I proposed the attached patch.

[-- Attachment #2: new-comment.patch --]
[-- Type: text/plain, Size: 3162 bytes --]

*** newcomment.el.~1.101.~	Mon Apr  2 07:45:08 2007
--- newcomment.el	Thu May 10 10:38:22 2007
***************
*** 926,941 ****
  the region rather than at left margin."
    ;;(assert (< beg end))
    (let ((no-empty (not (or (eq comment-empty-lines t)
! 			   (and comment-empty-lines (zerop (length ce)))))))
      ;; Sanitize CE and CCE.
      (if (and (stringp ce) (string= "" ce)) (setq ce nil))
      (if (and (stringp cce) (string= "" cce)) (setq cce nil))
      ;; If CE is empty, multiline cannot be used.
      (unless ce (setq ccs nil cce nil))
      ;; Should we mark empty lines as well ?
      (if (or ccs block lines) (setq no-empty nil))
      ;; Make sure we have end-markers for BLOCK mode.
!     (when block (unless ce (setq ce (comment-string-reverse cs))))
      ;; If BLOCK is not requested, we don't need CCE.
      (unless block (setq cce nil))
      ;; Continuation defaults to the same as CS and CE.
--- 926,943 ----
  the region rather than at left margin."
    ;;(assert (< beg end))
    (let ((no-empty (not (or (eq comment-empty-lines t)
! 			   (and comment-empty-lines (zerop (length ce))))))
! 	ce-sanitized)
      ;; Sanitize CE and CCE.
      (if (and (stringp ce) (string= "" ce)) (setq ce nil))
+     (setq ce-sanitized ce)
      (if (and (stringp cce) (string= "" cce)) (setq cce nil))
      ;; If CE is empty, multiline cannot be used.
      (unless ce (setq ccs nil cce nil))
      ;; Should we mark empty lines as well ?
      (if (or ccs block lines) (setq no-empty nil))
      ;; Make sure we have end-markers for BLOCK mode.
!     (when (and block (not ce)) (setq ce (comment-string-reverse cs)))
      ;; If BLOCK is not requested, we don't need CCE.
      (unless block (setq cce nil))
      ;; Continuation defaults to the same as CS and CE.
***************
*** 945,951 ****
        (goto-char end)
        ;; If the end is not at the end of a line and the comment-end
        ;; is implicit (i.e. a newline), explicitly insert a newline.
!       (unless (or ce (eolp)) (insert "\n") (indent-according-to-mode))
        (comment-with-narrowing beg end
  	(let ((min-indent (point-max))
  	      (max-indent 0))
--- 947,954 ----
        (goto-char end)
        ;; If the end is not at the end of a line and the comment-end
        ;; is implicit (i.e. a newline), explicitly insert a newline.
!       (unless (or ce-sanitized (eolp))
! 	(insert "\n") (indent-according-to-mode))
        (comment-with-narrowing beg end
  	(let ((min-indent (point-max))
  	      (max-indent 0))
***************
*** 1163,1169 ****
                               (buffer-substring (point)
                                                 (progn (move-to-left-margin)
                                                        (point)))))))))))))
!                     

  ;;;###autoload
  (defun comment-indent-new-line (&optional soft)
--- 1166,1172 ----
                               (buffer-substring (point)
                                                 (progn (move-to-left-margin)
                                                        (point)))))))))))))
! 

  ;;;###autoload
  (defun comment-indent-new-line (&optional soft)

[-- Attachment #3: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

Re: comment-dwim on blank lines

[Glenn Morris]

martin rudalics wrote:

> I proposed the attached patch.

FWIW, looks OK to me. Apart from the gratuitous formatting changes... :)

Are you going to install your view-quit patch, BTW?

Re: comment-dwim on blank lines

[Glenn Morris]

Glenn Morris wrote:

> martin rudalics wrote:
>
>> I proposed the attached patch.
>
> FWIW, looks OK to me.

I suggest installing it.

Re: comment-dwim on blank lines

[Glenn Morris]

Glenn Morris wrote:

> I suggest installing it.

Rather than send another reminder, I installed it.

Re: comment-dwim on blank lines

[martin rudalics]

> Rather than send another reminder, I installed it.

Thanks.