unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* comment-dwim on blank lines
@ 2007-11-02  1:09 Glenn Morris
  2007-11-02  1:14 ` Stefan Monnier
  0 siblings, 1 reply; 8+ messages in thread
From: Glenn Morris @ 2007-11-02  1:09 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel


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.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: comment-dwim on blank lines
  2007-11-02  1:09 comment-dwim on blank lines Glenn Morris
@ 2007-11-02  1:14 ` Stefan Monnier
  2007-11-02  1:47   ` Glenn Morris
  0 siblings, 1 reply; 8+ messages in thread
From: Stefan Monnier @ 2007-11-02  1:14 UTC (permalink / raw)
  To: Glenn Morris; +Cc: emacs-devel

> 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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: comment-dwim on blank lines
  2007-11-02  1:14 ` Stefan Monnier
@ 2007-11-02  1:47   ` Glenn Morris
  2007-11-02  6:45     ` martin rudalics
  0 siblings, 1 reply; 8+ messages in thread
From: Glenn Morris @ 2007-11-02  1:47 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

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...

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: comment-dwim on blank lines
  2007-11-02  1:47   ` Glenn Morris
@ 2007-11-02  6:45     ` martin rudalics
  2007-11-07 23:27       ` Glenn Morris
  0 siblings, 1 reply; 8+ messages in thread
From: martin rudalics @ 2007-11-02  6:45 UTC (permalink / raw)
  To: Glenn Morris; +Cc: Stefan Monnier, emacs-devel

[-- 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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: comment-dwim on blank lines
  2007-11-02  6:45     ` martin rudalics
@ 2007-11-07 23:27       ` Glenn Morris
  2007-11-22  3:08         ` Glenn Morris
  0 siblings, 1 reply; 8+ messages in thread
From: Glenn Morris @ 2007-11-07 23:27 UTC (permalink / raw)
  To: martin rudalics; +Cc: Stefan Monnier, emacs-devel

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?

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: comment-dwim on blank lines
  2007-11-07 23:27       ` Glenn Morris
@ 2007-11-22  3:08         ` Glenn Morris
  2007-11-28  5:21           ` Glenn Morris
  0 siblings, 1 reply; 8+ messages in thread
From: Glenn Morris @ 2007-11-22  3:08 UTC (permalink / raw)
  To: martin rudalics; +Cc: emacs-devel

Glenn Morris wrote:

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

I suggest installing it.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: comment-dwim on blank lines
  2007-11-22  3:08         ` Glenn Morris
@ 2007-11-28  5:21           ` Glenn Morris
  2007-11-28  9:15             ` martin rudalics
  0 siblings, 1 reply; 8+ messages in thread
From: Glenn Morris @ 2007-11-28  5:21 UTC (permalink / raw)
  To: martin rudalics; +Cc: emacs-devel

Glenn Morris wrote:

> I suggest installing it.

Rather than send another reminder, I installed it.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: comment-dwim on blank lines
  2007-11-28  5:21           ` Glenn Morris
@ 2007-11-28  9:15             ` martin rudalics
  0 siblings, 0 replies; 8+ messages in thread
From: martin rudalics @ 2007-11-28  9:15 UTC (permalink / raw)
  To: Glenn Morris; +Cc: emacs-devel

> Rather than send another reminder, I installed it.

Thanks.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2007-11-28  9:15 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-11-02  1:09 comment-dwim on blank lines Glenn Morris
2007-11-02  1:14 ` Stefan Monnier
2007-11-02  1:47   ` Glenn Morris
2007-11-02  6:45     ` martin rudalics
2007-11-07 23:27       ` Glenn Morris
2007-11-22  3:08         ` Glenn Morris
2007-11-28  5:21           ` Glenn Morris
2007-11-28  9:15             ` martin rudalics

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).