Re: comment-kill can't deal with following situation

Re: comment-kill can't deal with following situation

[lgfang]

Hi,

comment-kill is defined in newcomment.el.  It is supposed to kill
comments (refer to its document).  But it can't deal with the cases in
which there are more than one comment in a line.  An example is:

--------8<----------------8<-------- 
void f(){
    int i; /* this will be killed */ int j; /* oops, this
                                               one won't be killed */
}
--------8<----------------8<--------

I check the code and think it is due to lack of loop.  It seems that
following fix works.  But maybe there are better fixes.

--------8<------Fixed comment-kill ----------8<--------
(defun comment-kill (arg)
  "Kill the comment on this line, if any.
With prefix ARG, kill comments on that many lines starting with this one."
  (interactive "P")
  (comment-normalize-vars)
  (dotimes (_ (prefix-numeric-value arg))
    (save-excursion
      (beginning-of-line)
      (let ((line-num (line-number-at-pos))
            (cs (comment-search-forward (line-end-position) t)))
	(while cs
	  (goto-char cs)
	  (skip-syntax-backward " ")
	  (setq cs (point))
	  (comment-forward)
	  (kill-region cs (if (bolp) (1- (point)) (point)))
	  (indent-according-to-mode)
      (when (= line-num (line-number-at-pos)) ; still in the line
        (setq cs (comment-search-forward (line-end-position)  t))))))
    (if arg (forward-line 1))))
--------8<----------------8<--------

In GNU Emacs 22.1.1 (i386-mingw-nt5.1.2600) of 2007-06-02 on
RELEASE Windowing system distributor `Microsoft Corp.',
version 5.1.2600 configured using `configure --with-gcc
(3.4) --cflags -Ic:/gnuwin32/include'

Regards,

Fang, lungang

Re: comment-kill can't deal with following situation

[Stefan Monnier]

> comment-kill is defined in newcomment.el.  It is supposed to kill
> comments (refer to its document).  But it can't deal with the cases in
> which there are more than one comment in a line.  An example is:

If you do not provide a prefix arg, comment-kill should only kill
1 comment.  Indeed when providing a prefix arg, the behavior is probably
not right.  Actually, when I wrote it I just tried to reproduce the
previous behavior.  But in the presence of multi-line comments, the
behavior doesn't match the docstring either.

I don't know of anybody who uses the prefix arg of comment-kill, so if
you could explain how you use it, it might help decide what the behavior
should be.


        Stefan

Re: comment-kill can't deal with following situation

[lgfang]

>>>>> "Stefan" == Stefan Monnier <monnier@iro.umontreal.ca> writes:

    >> comment-kill is defined in newcomment.el.  It is supposed to
    >> kill comments (refer to its document).  But it can't deal with
    >> the cases in which there are more than one comment in a line.
    >> An example is:

    Stefan> If you do not provide a prefix arg, comment-kill should
    Stefan> only kill 1 comment.  Indeed when providing a prefix arg,

The document says:

    Kill the comment on this line, if any. With prefix ARG, kill
    comments on that many lines starting with this one.

So, take following C code for example:
int j; /* comment 1 */ int k; /* comment 2*/

I think comment-kill should kill both comment 1 and comment 2 even
without prefix ARG.  But in fact, it only kills the first one.

    Stefan> But in the presence of multi-line comments, the behavior
    Stefan> doesn't match the docstring either.

Even if all comments reside in one line and call comment-kill either
with/without prefix arg, the behavior is in-correct so long as there
are more than one comments.

Regards,

-- 
Fang, lungang

Re: comment-kill can't deal with following situation

[Andreas Röhler]

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

Am Sonntag, 2. März 2008 14:49 schrieb lgfang:
> int j; /* comment 1 */ int k; /* comment 2*/


Herewith a patch. It relies at the fact, that if a
comment was removed, variable `cs' was set before with
`(cs (comment-search-forward`.

Performs a recursive call as long that's true on a
given line.

As the argument only make sense as a numeric one, I
used interactive lesser p. That prevents another bug,
which would occur with this fix otherwise.

HTH

Andreas Röhler
 

[-- Attachment #2: comment-kill.diff --]
[-- Type: text/x-diff, Size: 987 bytes --]

--- old/newcomment.el	2008-01-08 21:44:45.000000000 +0100
+++ new/newcomment.el	2008-03-02 16:45:34.000000000 +0100
@@ -679,9 +679,9 @@
 (defun comment-kill (arg)
   "Kill the comment on this line, if any.
 With prefix ARG, kill comments on that many lines starting with this one."
-  (interactive "P")
+  (interactive "p")
   (comment-normalize-vars)
-  (dotimes (_ (prefix-numeric-value arg))
+  (dotimes (_ arg)
     (save-excursion
       (beginning-of-line)
       (let ((cs (comment-search-forward (line-end-position) t)))
@@ -691,8 +691,10 @@
 	  (setq cs (point))
 	  (comment-forward)
 	  (kill-region cs (if (bolp) (1- (point)) (point)))
-	  (indent-according-to-mode))))
-    (if arg (forward-line 1))))
+	  (indent-according-to-mode)
+	  (comment-kill 1))))
+    (setq arg (1- arg)) 
+    (when (< 0 arg) (forward-line 1))))
 
 (defun comment-padright (str &optional n)
   "Construct a string composed of STR plus `comment-padding'.

Diff finished.  Sun Mar  2 16:45:53 2008

Re: comment-kill can't deal with following situation

[Stefan Monnier]

>>> comment-kill is defined in newcomment.el.  It is supposed to
>>> kill comments (refer to its document).  But it can't deal with
>>> the cases in which there are more than one comment in a line.
>>> An example is:

Stefan> If you do not provide a prefix arg, comment-kill should
Stefan> only kill 1 comment.  Indeed when providing a prefix arg,

> The document says:

>     Kill the comment on this line, if any. With prefix ARG, kill
>     comments on that many lines starting with this one.

> So, take following C code for example:
> int j; /* comment 1 */ int k; /* comment 2*/

> I think comment-kill should kill both comment 1 and comment 2 even
> without prefix ARG.

Here I disagree.  I find it more useful to only kill one comment at
a time (you can always choose to run the command 2 times, but you cannot
run it 1/2 a time).  Also the docstring says "kill *the comment* on this
line", which admittedly is ambiguous when there are more than
1 comments, but indicates a clear intention to only kill 1 comment.

Stefan> But in the presence of multi-line comments, the behavior
Stefan> doesn't match the docstring either.

> Even if all comments reside in one line and call comment-kill either
> with/without prefix arg, the behavior is in-correct so long as there
> are more than one comments.

We should not make a design decision based on the behavior in one
particular kind of circumstances (only single-line comments), since
that's how we got into this mess in the first place.

So please reply to my previous request:

   I don't know of anybody who uses the prefix arg of comment-kill, so
   if you could explain how you use it, it might help decide what the
   behavior should be.


-- Stefan

Re: comment-kill can't deal with following situation

[lgfang]

>>>>> "Stefan" == Stefan Monnier <monnier@iro.umontreal.ca> writes:
    Stefan> If you do not provide a prefix arg, comment-kill should
    Stefan> only kill 1 comment.  Indeed when providing a prefix arg,

    >> The document says:

    >> Kill the comment on this line, if any. With prefix ARG, kill
    >> comments on that many lines starting with this one.

    >> So, take following C code for example: int j; /* comment 1 */
    >> int k; /* comment 2*/

    >> I think comment-kill should kill both comment 1 and comment 2
    >> even without prefix ARG.

    Stefan> Here I disagree.  I find it more useful to only kill one
    Stefan> comment at a time (you can always choose to run the
    Stefan> command 2 times, but you cannot run it 1/2 a time).  Also

I agree with you.  I didn't think of that.  So, it is not a bug.  I
just misunderstood the document.

    Stefan> the docstring says "kill *the comment* on this line",
    Stefan> which admittedly is ambiguous when there are more than 1
    Stefan> comments, but indicates a clear intention to only kill 1
    Stefan> comment.

Maybe need to update document ? :)

    Stefan> So please reply to my previous request:

    Stefan>    I don't know of anybody who uses the prefix arg of
    Stefan> comment-kill, so if you could explain how you use it, it
    Stefan> might help decide what the behavior should be.

In case you are interested, I'm NOT calling comment-kill
interactively.  I wrote a elisp script to count NCSL.  The basic idea
is to delete all comment lines using comment-kill and delete all blank
lines using flush-lines. Then, the number of lines is just the number
of NCSL.

(let ((file nil) (sum 0))
  (condition-case nil
      (while (setq file (read-string ""))
        (find-file file)
        (let ((buffer-read-only nil))
          (goto-char (point-min))
          (comment-kill (line-number-at-pos (point-max)))
          (flush-lines "^[[:blank:]]*$" (point-min) (point-max))
          (let ((linum (- (line-number-at-pos (point-max)) 1)))
            ;; -1 since always end with empty line
              (princ (format "%d %s\n" linum file))
              (setq sum (+ sum linum))))
        (kill-buffer nil))
    (error (princ (format "%s total\n" sum)))))

But it turns out that that script is slow and in-correct.  At last, I
end up with something like following:

(require 'hideif)
(require 'newcomment)

(let ((file nil) (sum 0) (ncsl 0))
  (condition-case nil
      (while (setq file (read-string "")) ; for each file
        (find-file file)
        (setq ncsl 0)

        ;; for C/c++ files, hide if 0
        (when (or (equal 'c-mode major-mode) (equal 'c++-mode
        major-mode))
          (goto-char (point-min))
          (while (re-search-forward "^[ \t]*#if[ \t]*0" nil t)
            (hide-ifdef-block)))

        (goto-char (point-min))
        (while (not (eobp))
          ;; suppose there won't be 10000 consecutive comments
          (forward-comment 10000)
          (setq ncsl (+ 1 ncsl))
          ;; In case a comment starts at current line and spans several
          ;; lines.
          (let* ((eol (line-end-position)) (cs (comment-search-forward
        eol t)))
            (while cs
              (goto-char cs)
              (forward-comment 10000)
              (setq cs (comment-search-forward eol t)))
            (unless (> (point) eol)     ; unless already a new line
              (forward-visible-line 1))))
        (setq sum (+ sum ncsl))
        (kill-buffer nil)
        (princ (format "%d %s\n" ncsl file)))
    (error (princ (format "%s total\n" sum)))))

-- 
Fang, lungang

Re: comment-kill can't deal with following situation

[Stefan Monnier]

Stefan> Here I disagree.  I find it more useful to only kill one
Stefan> comment at a time (you can always choose to run the
Stefan> command 2 times, but you cannot run it 1/2 a time).  Also

> I agree with you.  I didn't think of that.  So, it is not a bug.  I
> just misunderstood the document.

Glad we agree.  I'll improve the docstring to remove this ambiguity.

> In case you are interested, I'm NOT calling comment-kill
> interactively.  I wrote a elisp script to count NCSL.  The basic idea
> is to delete all comment lines using comment-kill and delete all blank
> lines using flush-lines. Then, the number of lines is just the number
> of NCSL.

I see.  I guess what we be better is to change `comment-kill' (when
called with an argument) such that it kills all the comments on the
N following lines.  Better yet: let it take a region as argument.


        Stefan