* Re: Some problems in `add-log-current-defun'
@ 2006-12-31 8:28 Herbert Euler
2006-12-31 22:13 ` Richard Stallman
0 siblings, 1 reply; 19+ messages in thread
From: Herbert Euler @ 2006-12-31 8:28 UTC (permalink / raw)
Cc: emacs-devel
[-- Attachment #1: Type: text/plain, Size: 35028 bytes --]
PATCH FOR I, II, AND III
I have tested my approach, it works. The patch, which solves all of
I, II and III, is
*** add-log.el Thu Dec 28 20:49:44 2006
--- add-log.el.1-3 Sun Dec 31 11:33:21 2006
*************** (defun add-log-current-defun ()
*** 813,863 ****
(progn (forward-sexp 1)
(point))))
((memq major-mode add-log-c-like-modes)
! (beginning-of-line)
! ;; See if we are in the beginning part of a function,
! ;; before the open brace. If so, advance forward.
! (while (not (looking-at "{\\|\\(\\s *$\\)"))
! (forward-line 1))
! (or (eobp)
! (forward-char 1))
! (let (maybe-beg)
! ;; Try to find the containing defun.
! (beginning-of-defun)
! (end-of-defun)
! ;; If the defun we found ends before the desired
position,
! ;; see if there's a DEFUN construct
! ;; between that end and the desired position.
! (when (save-excursion
! (and (> location (point))
! (re-search-forward "^DEFUN"
! (save-excursion
! (goto-char location)
! (line-end-position))
! t)
! (re-search-forward "^{" nil t)
! (setq maybe-beg (point))))
! ;; If so, go to the end of that instead.
! (goto-char maybe-beg)
! (end-of-defun)))
! ;; If the desired position is within the defun we found,
! ;; find the function name.
! (when (< location (point))
! ;; Move back over function body.
! (backward-sexp 1)
! (let (beg)
! ;; Skip back over typedefs and arglist.
! ;; Stop at the function definition itself
! ;; or at the line that follows end of function doc
string.
! (forward-line -1)
! (while (and (not (bobp))
! (looking-at "[ \t\n]")
! (not (looking-back "[*]/)\n" (- (point)
4))))
! (forward-line -1))
! ;; If we found a doc string, this must be the DEFUN
macro
! ;; used in Emacs. Move back to the DEFUN line.
! (when (looking-back "[*]/)\n" (- (point) 4))
! (backward-sexp 1)
! (beginning-of-line))
;; Is this a DEFUN construct? And is LOCATION in it?
(if (and (looking-at "DEFUN\\b")
(>= location (point)))
--- 813,868 ----
(progn (forward-sexp 1)
(point))))
((memq major-mode add-log-c-like-modes)
! ;; See whether the point is inside a defun.
! (let* (having-previous-defun
! having-next-defun
! (previous-defun-end (save-excursion
! (setq having-previous-defun
! (c-beginning-of-defun))
! (c-end-of-defun)
! ;; `c-end-of-defun' move
! ;; point to the line
! ;; after the function
! ;; close, but the
! ;; position we prefer
! ;; here is the position
! ;; after the } that
! ;; closes the function.
! (backward-sexp 1)
! (forward-sexp 1)
! (point)))
! (next-defun-beginning (save-excursion
! (setq having-next-defun
! (c-end-of-defun))
! (c-beginning-of-defun)
! (point))))
! (if (and having-next-defun
! (< location next-defun-beginning))
! (skip-syntax-forward " "))
! (if (and having-previous-defun
! (> location previous-defun-end))
! (skip-syntax-backward " "))
! (unless (or
! ;; When there is no previous defun, the
! ;; point is not in a defun if it is not at
! ;; the beginning of the next defun.
! (and (not having-previous-defun)
! (not (= (point)
! next-defun-beginning)))
! ;; When there is no next defun, the point
! ;; is not in a defun if it is not at the
! ;; end of the previous defun.
! (and (not having-next-defun)
! (not (= (point)
! previous-defun-end)))
! ;; If the point is between two defuns, it
! ;; is not in a defun.
! (and (> (point) previous-defun-end)
! (< (point) next-defun-beginning)))
! ;; If the point is already at the beginning of a
! ;; defun, there is no need to move point again.
! (if (not (= (point) next-defun-beginning))
! (c-beginning-of-defun))
;; Is this a DEFUN construct? And is LOCATION in it?
(if (and (looking-at "DEFUN\\b")
(>= location (point)))
This should be more reliable than the previous implementation, since
it parses the file with the functions provided by CC mode, rather than
analysing text features in the file.
The basic idea of these changes is that, move the point to exactly the
same position where the previous implementation moves it to before
evaluating the forms that are not changed (i.e. forms starting from
line 861 in the orignal file, or the forms starting with ``(if (and
(looking-at "DEFUN\\b")'' in the orignal file) in a different way.
With the powerful functions provided by CC mode, the new
implementation is more reliable and cleaner.
There is a problem with this approach yet, because of a bug in CC
mode. The function `c-beginning-of-defun' cannot move point correctly
for functions like
int
main (argc, argv)
int argc;
char *argv[];
{
/* ... */
}
So `add-log-current-defun' gives wrong result for this kind of
functions. However, we can expect this problem to be fixed when the
bug is fixed in CC mode. Please see
(1) http://lists.gnu.org/archive/html/emacs-devel/2006-12/msg01341.html
for more information about this bug.
AGAIN, PATCH FOR IV AND ANOTHER PROBLEM
By the way, problem IV in the beginning of this thread can be applied
to some other code in `add-log-current-defun' too. That is, the
processing of `public', `private', `enum', `struct', `union' and
`class'. For instance, the name below is valid in C but the current
code cannot handle it correctly:
struct
abc {
};
When fixing it, I found that the patch I provided in link (2) to solve
C++ names problem caused another problem, keywords such as `struct' or
`enum' are not recorded when needed:
(2) http://lists.gnu.org/archive/html/emacs-devel/2006-12/msg01095.html
However, my later patch of IV in
(3) http://lists.gnu.org/archive/html/emacs-devel/2006-12/msg01226.html
eliminates the problem raised in applying the patch in link (2). So
the patch below, which fixes both the problem of `enum', `struct',
`union' and `class' (but not `public' and `private') and the problem
created by applying the patch in link (2), is based on the second
patch in link (3):
*** add-log.el Thu Dec 28 20:49:44 2006
--- add-log.el.4_2 Sun Dec 31 14:38:46 2006
*************** (defun add-log-current-defun ()
*** 916,941 ****
;; Include certain keywords if they
;; precede the name.
(setq middle (point))
! ;; Single (forward-sexp -1) invocation is
! ;; not enough for C++ member function defined
! ;; as part of nested class and/or namespace
! ;; like:
! ;;
! ;; void
! ;; foo::bar::baz::bazz ()
! ;; { ...
! ;;
! ;; Here we have to move the point to
! ;; the beginning of foo, not bazz.
! (while (not (looking-back "\\(^\\|[ \t]\\)"))
(forward-sexp -1))
;; Is this C++ method?
(when (and (< 2 middle)
! (string= (buffer-substring (-
middle 2)
! middle)
! "::"))
;; Include "classname::".
! (setq middle (point)))
;; Ignore these subparts of a class decl
;; and move back to the class name itself.
(while (looking-at "public \\|private ")
--- 916,939 ----
;; Include certain keywords if they
;; precede the name.
(setq middle (point))
! ;; Move through C++ nested name.
! (while (looking-back "::[ \t\n]*")
(forward-sexp -1))
+ (forward-sexp -1)
;; Is this C++ method?
(when (and (< 2 middle)
! (save-excursion
! (goto-char middle)
! (looking-back "::[ \t\n]*")))
;; Include "classname::".
! (save-excursion
! ;; The `forward-sexp' form after
! ;; the `while' form above moves
! ;; backward one more sexp, so we
! ;; move forward one.
! (forward-sexp 1)
! (re-search-forward "\\(\\s \\|\n\\)*")
! (setq middle (point))))
;; Ignore these subparts of a class decl
;; and move back to the class name itself.
(while (looking-at "public \\|private ")
*************** (defun add-log-current-defun ()
*** 944,960 ****
(backward-sexp 1)
(setq middle (point))
(forward-word -1))
! (and (bolp)
! (looking-at
! "enum \\|struct \\|union \\|class ")
! (setq middle (point)))
(goto-char end)
(when (eq (preceding-char) ?=)
(forward-char -1)
(skip-chars-backward " \t")
(setq end (point)))
! (buffer-substring-no-properties
! middle end))))))))
((memq major-mode add-log-tex-like-modes)
(if (re-search-backward
"\\\\\\(sub\\)*\\(section\\|paragraph\\|chapter\\)"
--- 942,969 ----
(backward-sexp 1)
(setq middle (point))
(forward-word -1))
! (and
! ;; These defuns are not necessary
! ;; starting at column 0.
! ;; (bolp)
! (looking-at
! "\\(enum\\|struct\\|union\\|class\\)[
\t\n]")
! (setq middle (point)))
(goto-char end)
(when (eq (preceding-char) ?=)
(forward-char -1)
(skip-chars-backward " \t")
(setq end (point)))
! (let ((name (buffer-substring-no-properties
! middle end)))
! (setq name (replace-regexp-in-string
! "\n" " " name)
! name (replace-regexp-in-string
!
"\\(enum\\|struct\\|union\\|class\\)[ \t]+"
! "\\1 "
! name)
! name (replace-regexp-in-string
! "[ \t]*::[ \t]*" "::"
name))))))))))
((memq major-mode add-log-tex-like-modes)
(if (re-search-backward
"\\\\\\(sub\\)*\\(section\\|paragraph\\|chapter\\)"
These changes are guaranteed not to cause other problems during my
test.
PATCH FOR `public' AND `private'
The reason of separating the patch for `public' and `private' is that
it is different from other patches. The body of the `while' form that
contains `public' and `private' is really the same as the forms before
the while form, so it should be a whole iteration, not a new one. So
the patch is based on the file with the patch in the above section:
*** add-log.el.4_2 Sun Dec 31 14:38:46 2006
--- add-log.el.other Sun Dec 31 15:17:03 2006
*************** (defun add-log-current-defun ()
*** 906,947 ****
;; Consistency check: going down and up
;; shouldn't take us back before BEG.
(> (point) beg))
! (let (end middle)
! ;; Don't include any final whitespace
! ;; in the name we use.
! (skip-chars-backward " \t\n")
! (setq end (point))
! (backward-sexp 1)
! ;; Now find the right beginning of the name.
! ;; Include certain keywords if they
! ;; precede the name.
! (setq middle (point))
! ;; Move through C++ nested name.
! (while (looking-back "::[ \t\n]*")
! (forward-sexp -1))
! (forward-sexp -1)
! ;; Is this C++ method?
! (when (and (< 2 middle)
! (save-excursion
! (goto-char middle)
! (looking-back "::[ \t\n]*")))
! ;; Include "classname::".
! (save-excursion
! ;; The `forward-sexp' form after
! ;; the `while' form above moves
! ;; backward one more sexp, so we
! ;; move forward one.
! (forward-sexp 1)
! (re-search-forward "\\(\\s \\|\n\\)*")
! (setq middle (point))))
! ;; Ignore these subparts of a class decl
! ;; and move back to the class name itself.
! (while (looking-at "public \\|private ")
! (skip-chars-backward " \t:")
(setq end (point))
(backward-sexp 1)
(setq middle (point))
! (forward-word -1))
(and
;; These defuns are not necessary
;; starting at column 0.
--- 906,944 ----
;; Consistency check: going down and up
;; shouldn't take us back before BEG.
(> (point) beg))
! (let (end
! middle
! (first-iter t))
! (while (or first-iter
! (looking-at
!
"\\(public\\|protected\\|private\\)[ \t\n]"))
! (setq first-iter nil)
! ;; Skip characters that are not for
identifiers.
! (skip-chars-backward ": \t\n")
(setq end (point))
(backward-sexp 1)
+ ;; Now find the right beginning of the
name.
+ ;; Include certain keywords if they
+ ;; precede the name.
(setq middle (point))
! ;; Move through C++ nested name.
! (while (looking-back "::[ \t\n]*")
! (forward-sexp -1))
! (forward-sexp -1)
! ;; Is this C++ method?
! (when (and (< 2 middle)
! (save-excursion
! (goto-char middle)
! (looking-back "::[ \t\n]*")))
! ;; Include "classname::".
! (save-excursion
! ;; The `forward-sexp' form after
! ;; the `while' form above moves
! ;; backward one more sexp, so we
! ;; move forward one.
! (forward-sexp 1)
! (re-search-forward "\\(\\s \\|\n\\)*")
! (setq middle (point)))))
(and
;; These defuns are not necessary
;; starting at column 0.
Note that `protected' is added here.
PUTTING TOGETHER
Finally I want to put all changes together, since there are too many
changes to make really a new implementation. The patch for solving
all of I, II, III, and IV is
*** add-log.el Thu Dec 28 20:49:44 2006
--- add-log.el.new Sun Dec 31 16:13:48 2006
*************** (defun add-log-current-defun ()
*** 813,863 ****
(progn (forward-sexp 1)
(point))))
((memq major-mode add-log-c-like-modes)
! (beginning-of-line)
! ;; See if we are in the beginning part of a function,
! ;; before the open brace. If so, advance forward.
! (while (not (looking-at "{\\|\\(\\s *$\\)"))
! (forward-line 1))
! (or (eobp)
! (forward-char 1))
! (let (maybe-beg)
! ;; Try to find the containing defun.
! (beginning-of-defun)
! (end-of-defun)
! ;; If the defun we found ends before the desired
position,
! ;; see if there's a DEFUN construct
! ;; between that end and the desired position.
! (when (save-excursion
! (and (> location (point))
! (re-search-forward "^DEFUN"
! (save-excursion
! (goto-char location)
! (line-end-position))
! t)
! (re-search-forward "^{" nil t)
! (setq maybe-beg (point))))
! ;; If so, go to the end of that instead.
! (goto-char maybe-beg)
! (end-of-defun)))
! ;; If the desired position is within the defun we found,
! ;; find the function name.
! (when (< location (point))
! ;; Move back over function body.
! (backward-sexp 1)
! (let (beg)
! ;; Skip back over typedefs and arglist.
! ;; Stop at the function definition itself
! ;; or at the line that follows end of function doc
string.
! (forward-line -1)
! (while (and (not (bobp))
! (looking-at "[ \t\n]")
! (not (looking-back "[*]/)\n" (- (point)
4))))
! (forward-line -1))
! ;; If we found a doc string, this must be the DEFUN
macro
! ;; used in Emacs. Move back to the DEFUN line.
! (when (looking-back "[*]/)\n" (- (point) 4))
! (backward-sexp 1)
! (beginning-of-line))
;; Is this a DEFUN construct? And is LOCATION in it?
(if (and (looking-at "DEFUN\\b")
(>= location (point)))
--- 813,868 ----
(progn (forward-sexp 1)
(point))))
((memq major-mode add-log-c-like-modes)
! ;; See whether the point is inside a defun.
! (let* (having-previous-defun
! having-next-defun
! (previous-defun-end (save-excursion
! (setq having-previous-defun
! (c-beginning-of-defun))
! (c-end-of-defun)
! ;; `c-end-of-defun' move
! ;; point to the line
! ;; after the function
! ;; close, but the
! ;; position we prefer
! ;; here is the position
! ;; after the } that
! ;; closes the function.
! (backward-sexp 1)
! (forward-sexp 1)
! (point)))
! (next-defun-beginning (save-excursion
! (setq having-next-defun
! (c-end-of-defun))
! (c-beginning-of-defun)
! (point))))
! (if (and having-next-defun
! (< location next-defun-beginning))
! (skip-syntax-forward " "))
! (if (and having-previous-defun
! (> location previous-defun-end))
! (skip-syntax-backward " "))
! (unless (or
! ;; When there is no previous defun, the
! ;; point is not in a defun if it is not at
! ;; the beginning of the next defun.
! (and (not having-previous-defun)
! (not (= (point)
! next-defun-beginning)))
! ;; When there is no next defun, the point
! ;; is not in a defun if it is not at the
! ;; end of the previous defun.
! (and (not having-next-defun)
! (not (= (point)
! previous-defun-end)))
! ;; If the point is between two defuns, it
! ;; is not in a defun.
! (and (> (point) previous-defun-end)
! (< (point) next-defun-beginning)))
! ;; If the point is already at the beginning of a
! ;; defun, there is no need to move point again.
! (if (not (= (point) next-defun-beginning))
! (c-beginning-of-defun))
;; Is this a DEFUN construct? And is LOCATION in it?
(if (and (looking-at "DEFUN\\b")
(>= location (point)))
*************** (defun add-log-current-defun ()
*** 906,960 ****
;; Consistency check: going down and up
;; shouldn't take us back before BEG.
(> (point) beg))
! (let (end middle)
! ;; Don't include any final whitespace
! ;; in the name we use.
! (skip-chars-backward " \t\n")
! (setq end (point))
! (backward-sexp 1)
! ;; Now find the right beginning of the name.
! ;; Include certain keywords if they
! ;; precede the name.
! (setq middle (point))
! ;; Single (forward-sexp -1) invocation is
! ;; not enough for C++ member function defined
! ;; as part of nested class and/or namespace
! ;; like:
! ;;
! ;; void
! ;; foo::bar::baz::bazz ()
! ;; { ...
! ;;
! ;; Here we have to move the point to
! ;; the beginning of foo, not bazz.
! (while (not (looking-back "\\(^\\|[ \t]\\)"))
! (forward-sexp -1))
! ;; Is this C++ method?
! (when (and (< 2 middle)
! (string= (buffer-substring (-
middle 2)
! middle)
! "::"))
! ;; Include "classname::".
! (setq middle (point)))
! ;; Ignore these subparts of a class decl
! ;; and move back to the class name itself.
! (while (looking-at "public \\|private ")
! (skip-chars-backward " \t:")
(setq end (point))
(backward-sexp 1)
(setq middle (point))
! (forward-word -1))
! (and (bolp)
! (looking-at
! "enum \\|struct \\|union \\|class ")
! (setq middle (point)))
(goto-char end)
(when (eq (preceding-char) ?=)
(forward-char -1)
(skip-chars-backward " \t")
(setq end (point)))
! (buffer-substring-no-properties
! middle end))))))))
((memq major-mode add-log-tex-like-modes)
(if (re-search-backward
"\\\\\\(sub\\)*\\(section\\|paragraph\\|chapter\\)"
--- 911,971 ----
;; Consistency check: going down and up
;; shouldn't take us back before BEG.
(> (point) beg))
! (let (end
! middle
! (first-iter t))
! (while (or first-iter
! (looking-at
!
"\\(public\\|protected\\|private\\)[ \t\n]"))
! (setq first-iter nil)
! ;; Skip characters that are not for
identifiers.
! (skip-chars-backward ": \t\n")
(setq end (point))
(backward-sexp 1)
+ ;; Now find the right beginning of the
name.
+ ;; Include certain keywords if they
+ ;; precede the name.
(setq middle (point))
! ;; Move through C++ nested name.
! (while (looking-back "::[ \t\n]*")
! (forward-sexp -1))
! (forward-sexp -1)
! ;; Is this C++ method?
! (when (and (< 2 middle)
! (save-excursion
! (goto-char middle)
! (looking-back "::[ \t\n]*")))
! ;; Include "classname::".
! (save-excursion
! ;; The `forward-sexp' form after
! ;; the `while' form above moves
! ;; backward one more sexp, so we
! ;; move forward one.
! (forward-sexp 1)
! (re-search-forward "\\(\\s \\|\n\\)*")
! (setq middle (point)))))
! (and
! ;; These defuns are not necessary
! ;; starting at column 0.
! ;; (bolp)
! (looking-at
! "\\(enum\\|struct\\|union\\|class\\)[
\t\n]")
! (setq middle (point)))
(goto-char end)
(when (eq (preceding-char) ?=)
(forward-char -1)
(skip-chars-backward " \t")
(setq end (point)))
! (let ((name (buffer-substring-no-properties
! middle end)))
! (setq name (replace-regexp-in-string
! "\n" " " name)
! name (replace-regexp-in-string
!
"\\(enum\\|struct\\|union\\|class\\)[ \t]+"
! "\\1 "
! name)
! name (replace-regexp-in-string
! "[ \t]*::[ \t]*" "::"
name))))))))))
((memq major-mode add-log-tex-like-modes)
(if (re-search-backward
"\\\\\\(sub\\)*\\(section\\|paragraph\\|chapter\\)"
And I will provide many test cases. The all-together patch and the
test cases are attached. Most of the cases are passed by the new
implementation. Of course, there are two cases can't be passed. We
can test it again when Alan fix the bug in CC mode (See link (1)).
>From these cases, I can conclude that these changes (i.e. the new
implementation) do not break anything else, and can behave for ``bad
formatted'' or ``not reasonably formatted'' code better than the
previous implementation. It is cleaner and more reliable than the
previous one.
Regards,
Guanpeng Xu
>From: Richard Stallman <rms@gnu.org>
>Reply-To: rms@gnu.org
>To: "Herbert Euler" <herberteuler@hotmail.com>
>CC: emacs-devel@gnu.org
>Subject: Re: Some problems in `add-log-current-defun'
>Date: Fri, 29 Dec 2006 10:44:33 -0500
>
> I have solved I and IV, more or less.
>
>We want to fix I and II and maybe III.
>But not necessarily IV, not if it makes the change more
>complex or risks breaking something else.
>
>
_________________________________________________________________
Don't just search. Find. Check out the new MSN Search!
http://search.msn.click-url.com/go/onm00200636ave/direct/01/
[-- Attachment #2: add-log.el.patch --]
[-- Type: application/octet-stream, Size: 8463 bytes --]
*** add-log.el Thu Dec 28 20:49:44 2006
--- add-log.el.new Sun Dec 31 16:13:48 2006
*************** (defun add-log-current-defun ()
*** 813,863 ****
(progn (forward-sexp 1)
(point))))
((memq major-mode add-log-c-like-modes)
! (beginning-of-line)
! ;; See if we are in the beginning part of a function,
! ;; before the open brace. If so, advance forward.
! (while (not (looking-at "{\\|\\(\\s *$\\)"))
! (forward-line 1))
! (or (eobp)
! (forward-char 1))
! (let (maybe-beg)
! ;; Try to find the containing defun.
! (beginning-of-defun)
! (end-of-defun)
! ;; If the defun we found ends before the desired position,
! ;; see if there's a DEFUN construct
! ;; between that end and the desired position.
! (when (save-excursion
! (and (> location (point))
! (re-search-forward "^DEFUN"
! (save-excursion
! (goto-char location)
! (line-end-position))
! t)
! (re-search-forward "^{" nil t)
! (setq maybe-beg (point))))
! ;; If so, go to the end of that instead.
! (goto-char maybe-beg)
! (end-of-defun)))
! ;; If the desired position is within the defun we found,
! ;; find the function name.
! (when (< location (point))
! ;; Move back over function body.
! (backward-sexp 1)
! (let (beg)
! ;; Skip back over typedefs and arglist.
! ;; Stop at the function definition itself
! ;; or at the line that follows end of function doc string.
! (forward-line -1)
! (while (and (not (bobp))
! (looking-at "[ \t\n]")
! (not (looking-back "[*]/)\n" (- (point) 4))))
! (forward-line -1))
! ;; If we found a doc string, this must be the DEFUN macro
! ;; used in Emacs. Move back to the DEFUN line.
! (when (looking-back "[*]/)\n" (- (point) 4))
! (backward-sexp 1)
! (beginning-of-line))
;; Is this a DEFUN construct? And is LOCATION in it?
(if (and (looking-at "DEFUN\\b")
(>= location (point)))
--- 813,868 ----
(progn (forward-sexp 1)
(point))))
((memq major-mode add-log-c-like-modes)
! ;; See whether the point is inside a defun.
! (let* (having-previous-defun
! having-next-defun
! (previous-defun-end (save-excursion
! (setq having-previous-defun
! (c-beginning-of-defun))
! (c-end-of-defun)
! ;; `c-end-of-defun' move
! ;; point to the line
! ;; after the function
! ;; close, but the
! ;; position we prefer
! ;; here is the position
! ;; after the } that
! ;; closes the function.
! (backward-sexp 1)
! (forward-sexp 1)
! (point)))
! (next-defun-beginning (save-excursion
! (setq having-next-defun
! (c-end-of-defun))
! (c-beginning-of-defun)
! (point))))
! (if (and having-next-defun
! (< location next-defun-beginning))
! (skip-syntax-forward " "))
! (if (and having-previous-defun
! (> location previous-defun-end))
! (skip-syntax-backward " "))
! (unless (or
! ;; When there is no previous defun, the
! ;; point is not in a defun if it is not at
! ;; the beginning of the next defun.
! (and (not having-previous-defun)
! (not (= (point)
! next-defun-beginning)))
! ;; When there is no next defun, the point
! ;; is not in a defun if it is not at the
! ;; end of the previous defun.
! (and (not having-next-defun)
! (not (= (point)
! previous-defun-end)))
! ;; If the point is between two defuns, it
! ;; is not in a defun.
! (and (> (point) previous-defun-end)
! (< (point) next-defun-beginning)))
! ;; If the point is already at the beginning of a
! ;; defun, there is no need to move point again.
! (if (not (= (point) next-defun-beginning))
! (c-beginning-of-defun))
;; Is this a DEFUN construct? And is LOCATION in it?
(if (and (looking-at "DEFUN\\b")
(>= location (point)))
*************** (defun add-log-current-defun ()
*** 906,960 ****
;; Consistency check: going down and up
;; shouldn't take us back before BEG.
(> (point) beg))
! (let (end middle)
! ;; Don't include any final whitespace
! ;; in the name we use.
! (skip-chars-backward " \t\n")
! (setq end (point))
! (backward-sexp 1)
! ;; Now find the right beginning of the name.
! ;; Include certain keywords if they
! ;; precede the name.
! (setq middle (point))
! ;; Single (forward-sexp -1) invocation is
! ;; not enough for C++ member function defined
! ;; as part of nested class and/or namespace
! ;; like:
! ;;
! ;; void
! ;; foo::bar::baz::bazz ()
! ;; { ...
! ;;
! ;; Here we have to move the point to
! ;; the beginning of foo, not bazz.
! (while (not (looking-back "\\(^\\|[ \t]\\)"))
! (forward-sexp -1))
! ;; Is this C++ method?
! (when (and (< 2 middle)
! (string= (buffer-substring (- middle 2)
! middle)
! "::"))
! ;; Include "classname::".
! (setq middle (point)))
! ;; Ignore these subparts of a class decl
! ;; and move back to the class name itself.
! (while (looking-at "public \\|private ")
! (skip-chars-backward " \t:")
(setq end (point))
(backward-sexp 1)
(setq middle (point))
! (forward-word -1))
! (and (bolp)
! (looking-at
! "enum \\|struct \\|union \\|class ")
! (setq middle (point)))
(goto-char end)
(when (eq (preceding-char) ?=)
(forward-char -1)
(skip-chars-backward " \t")
(setq end (point)))
! (buffer-substring-no-properties
! middle end))))))))
((memq major-mode add-log-tex-like-modes)
(if (re-search-backward
"\\\\\\(sub\\)*\\(section\\|paragraph\\|chapter\\)"
--- 911,971 ----
;; Consistency check: going down and up
;; shouldn't take us back before BEG.
(> (point) beg))
! (let (end
! middle
! (first-iter t))
! (while (or first-iter
! (looking-at
! "\\(public\\|protected\\|private\\)[ \t\n]"))
! (setq first-iter nil)
! ;; Skip characters that are not for identifiers.
! (skip-chars-backward ": \t\n")
(setq end (point))
(backward-sexp 1)
+ ;; Now find the right beginning of the name.
+ ;; Include certain keywords if they
+ ;; precede the name.
(setq middle (point))
! ;; Move through C++ nested name.
! (while (looking-back "::[ \t\n]*")
! (forward-sexp -1))
! (forward-sexp -1)
! ;; Is this C++ method?
! (when (and (< 2 middle)
! (save-excursion
! (goto-char middle)
! (looking-back "::[ \t\n]*")))
! ;; Include "classname::".
! (save-excursion
! ;; The `forward-sexp' form after
! ;; the `while' form above moves
! ;; backward one more sexp, so we
! ;; move forward one.
! (forward-sexp 1)
! (re-search-forward "\\(\\s \\|\n\\)*")
! (setq middle (point)))))
! (and
! ;; These defuns are not necessary
! ;; starting at column 0.
! ;; (bolp)
! (looking-at
! "\\(enum\\|struct\\|union\\|class\\)[ \t\n]")
! (setq middle (point)))
(goto-char end)
(when (eq (preceding-char) ?=)
(forward-char -1)
(skip-chars-backward " \t")
(setq end (point)))
! (let ((name (buffer-substring-no-properties
! middle end)))
! (setq name (replace-regexp-in-string
! "\n" " " name)
! name (replace-regexp-in-string
! "\\(enum\\|struct\\|union\\|class\\)[ \t]+"
! "\\1 "
! name)
! name (replace-regexp-in-string
! "[ \t]*::[ \t]*" "::" name))))))))))
((memq major-mode add-log-tex-like-modes)
(if (re-search-backward
"\\\\\\(sub\\)*\\(section\\|paragraph\\|chapter\\)"
[-- Attachment #3: add_log_test.c --]
[-- Type: application/octet-stream, Size: 5047 bytes --]
/* 1. K&R C. */
/* This will not get correct result until the bug in CC mode is fixed.
See
http://lists.gnu.org/archive/html/emacs-devel/2006-12/msg01341.html */
int
main1 (argc, argv)
int argc;
char *argv[];
{
/* ... */
}
int
f1 (arg)
Lisp_Object arg;
{
}
int
f2 (arg)
Lisp_Object arg;
/* The `arg' is a Lisp_Object. */
{
}
DEFUN ("catch", Fcatch1, Scatch, 1, UNEVALLED, 0,
doc: /* Eval BODY allowing nonlocal exits using `throw'.
TAG is evalled to get the tag to use; it must not be nil.
Then the BODY is executed.
Within BODY, a call to `throw' with the same TAG exits BODY and this `catch'.
If no throw happens, `catch' returns the value of the last BODY form.
If a throw happens, it specifies the value to return from `catch'.
usage: (catch TAG BODY...) */)
(args)
Lisp_Object args;
{
register Lisp_Object tag;
struct gcpro gcpro1;
GCPRO1 (args);
tag = Feval (Fcar (args));
UNGCPRO;
return internal_catch (tag, Fprogn, Fcdr (args));
}
/* Now, some badly formatted code. */
int
main2 (argc, argv)
int argc;
char *argv[];
{
/* ... */
}
int
f3
(arg)
Lisp_Object
arg
;
{
}
int f4
(arg)
Lisp_Object arg;
/* The `arg' is a Lisp_Object. */
{
}
DEFUN ("catch", Fcatch2, Scatch, 1, UNEVALLED, 0,
doc: /* Eval BODY allowing nonlocal exits using `throw'.
TAG is evalled to get the tag to use; it must not be nil.
Then the BODY is executed.
Within BODY, a call to `throw' with the same TAG exits BODY and this `catch'.
If no throw happens, `catch' returns the value of the last BODY form.
If a throw happens, it specifies the value to return from `catch'.
usage: (catch TAG BODY...) */)
(args)
Lisp_Object args;
{
register Lisp_Object tag;
struct gcpro gcpro1;
GCPRO1 (args);
tag = Feval (Fcar (args));
UNGCPRO;
return internal_catch (tag, Fprogn, Fcdr (args));
}
/* 2. ANSI/ISO C. */
int
main3 (int argc, char *argv[])
{
}
int
f5 (Lisp_Object arg)
{
}
int
f6 (Lisp_Object arg)
/* The `arg' is a Lisp_Object. */
{
}
DEFUN ("catch", Fcatch3, Scatch, 1, UNEVALLED, 0,
doc: /* Eval BODY allowing nonlocal exits using `throw'.
TAG is evalled to get the tag to use; it must not be nil.
Then the BODY is executed.
Within BODY, a call to `throw' with the same TAG exits BODY and this `catch'.
If no throw happens, `catch' returns the value of the last BODY form.
If a throw happens, it specifies the value to return from `catch'.
usage: (catch TAG BODY...) */)
(Lisp_Object args)
{
register Lisp_Object tag;
struct gcpro gcpro1;
GCPRO1 (args);
tag = Feval (Fcar (args));
UNGCPRO;
return internal_catch (tag, Fprogn, Fcdr (args));
}
/* Now, some badly formatted code. */
int
main4
(int
argc,
char *
argv[])
{
}
int
f7
(
Lisp_Object
arg)
{
}
int
f8
(Lisp_Object
arg)
/* The `arg' is a Lisp_Object. */
{
}
DEFUN ("catch", Fcatch4, Scatch, 1, UNEVALLED, 0,
doc: /* Eval BODY allowing nonlocal exits using `throw'.
TAG is evalled to get the tag to use; it must not be nil.
Then the BODY is executed.
Within BODY, a call to `throw' with the same TAG exits BODY and this `catch'.
If no throw happens, `catch' returns the value of the last BODY form.
If a throw happens, it specifies the value to return from `catch'.
usage: (catch TAG BODY...) */)
(Lisp_Object args)
{
register Lisp_Object tag;
struct gcpro gcpro1;
GCPRO1 (args);
tag = Feval (Fcar (args));
UNGCPRO;
return internal_catch (tag, Fprogn, Fcdr (args));
}
/* 3. C++ functions. */
int
a_b::c_d::e_f1 ()
{
}
int
a_b
::
c_d::e_f2 ()
{
}
int
a_b
::
c_d
::
e_f3
()
{
}
/* 4. struct, enum, class, union; private, protected, public. */
/* The cases of `struct' can represent all `struct', `enum', `class',
and `union'. */
struct struct1
{
};
struct
struct2
{
};
struct struct3
{
};
struct
struct4
{
};
class a_b1 : public c_d
{
};
class a_b2
: public c_d
{
};
class a_b3:
public c_d
{
};
class a_b4
:
public c_d
{
};
class
a_b5 : public c_d
{
};
class a_b6
: public
c_d
{
};
class
a_b7 :
public c_d
{
};
class
a_b8
:
public
c_d
{
};
class a_b::c_d1 : public e_f, public g_h
{
};
class a_b
::
c_d2
: public
e_f
,
public
g_h
{
};
[-- Attachment #4: 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] 19+ messages in thread* Re: Some problems in `add-log-current-defun'
2006-12-31 8:28 Some problems in `add-log-current-defun' Herbert Euler
@ 2006-12-31 22:13 ` Richard Stallman
2007-01-01 1:27 ` Herbert Euler
2007-01-02 23:35 ` Stefan Monnier
0 siblings, 2 replies; 19+ messages in thread
From: Richard Stallman @ 2006-12-31 22:13 UTC (permalink / raw)
Cc: jet, emacs-devel
I would like to install your fix for problems I, II and III,
but it is big enough that we need legal papers for it.
I will send you what is needed to do this, and in the mean time,
I will save that patch.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Some problems in `add-log-current-defun'
2006-12-31 22:13 ` Richard Stallman
@ 2007-01-01 1:27 ` Herbert Euler
2007-01-02 23:35 ` Stefan Monnier
1 sibling, 0 replies; 19+ messages in thread
From: Herbert Euler @ 2007-01-01 1:27 UTC (permalink / raw)
Cc: jet, emacs-devel
>I would like to install your fix for problems I, II and III,
>but it is big enough that we need legal papers for it.
>I will send you what is needed to do this, and in the mean time,
>I will save that patch.
I am trying to get the postal address and will send this mail
later. Thank you.
Here I'd like to say something for the fix of IV. This change
2006-12-26 Guanpeng Xu <herberteuler@hotmail.com> (tiny change)
* add-log.el (add-log-current-defun): Call `forward-sexp'
multiple times to pick a member function name defined as
part of nested classes/namespaces.
created a bug. For the following example,
struct abc {
-!-};
If point is at -!-, `add-log-current-defun' will return ``abc'' rather
than the desired, old ``struct abc''. I found it when trying to
solve IV. So, please consider to either revert the above change,
or install the fix of IV. I think it is necessary to keep the behavior
correct for at least C, and based on my test I think there is little
risk of breaking something else to install the fix of IV.
Regards,
Guanpeng Xu
_________________________________________________________________
FREE pop-up blocking with the new MSN Toolbar - get it now!
http://toolbar.msn.click-url.com/go/onm00200415ave/direct/01/
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: Some problems in `add-log-current-defun'
2006-12-31 22:13 ` Richard Stallman
2007-01-01 1:27 ` Herbert Euler
@ 2007-01-02 23:35 ` Stefan Monnier
2007-01-03 21:11 ` Richard Stallman
1 sibling, 1 reply; 19+ messages in thread
From: Stefan Monnier @ 2007-01-02 23:35 UTC (permalink / raw)
Cc: jet, Herbert Euler, emacs-devel
> I would like to install your fix for problems I, II and III,
> but it is big enough that we need legal papers for it.
> I will send you what is needed to do this, and in the mean time,
> I will save that patch.
BTW, this code should be in cc-mode, not in add-log.el.
add-log.el provides a variable add-log-current-defun-function which cc-mode
should set.
Stefan
^ permalink raw reply [flat|nested] 19+ messages in thread
* Some problems in `add-log-current-defun'
@ 2006-12-27 10:32 Herbert Euler
2006-12-27 10:45 ` David Kastrup
` (3 more replies)
0 siblings, 4 replies; 19+ messages in thread
From: Herbert Euler @ 2006-12-27 10:32 UTC (permalink / raw)
I encountered a problem in add-log.el, and solved it somehow (see
http://lists.gnu.org/archive/html/emacs-devel/2006-09/msg00869.html).
However, after re-reading the implementation of
`add-log-current-defun', I found there are still many problems (bugs)
in `add-log-current-defun' for the C like languages. I'm going to
work on it, but before doing anything, I'd like to write them down.
This message is intended to describe the problems I found, rather than
talking about any proposal. Please help me find whether there are
more problems not found yet.
Please locate to the function `add-log-current-defun' in `add-log.el'
before continuing reading.
I. The end of a function
The point is moved to the start position of a function or an empty
line (lines consist of only white space characters) with the following
code.
;; See if we are in the beginning part of a function,
;; before the open brace. If so, advance forward.
(while (not (looking-at "{\\|\\(\\s *$\\)"))
(forward-line 1))
But this is not reliable. If someone forgets to put a newline after a
function, `add-log-current-defun' will report wrong name. Please
consider the following example:
int
f1 ()
{
/* If point is here `add-log-current-defun' gets wrong result. */
}
int
f2 ()
{
/* ... */
}
When the point is inside the body of `f1', invoking
`add-log-current-defun' will get `f2', rather than `f1'.
II. On the change of `beginning-of-defun' in CC mode
When the point is in the docstring of Emacs C source code, the
following forms are evaluated.
(let (maybe-beg)
;; Try to find the containing defun.
(beginning-of-defun)
(end-of-defun)
But I found the result is wrong with the newest CC mode. Consider the
following Emacs C source code:
DEFUN ("catch", Fcatch, Scatch, 1, UNEVALLED, 0,
doc: /* Eval BODY allowing nonlocal exits using `throw'.
TAG is evalled to get the tag to use; it must not be nil.
Then the BODY is executed.
Within BODY, a call to `throw' with the same TAG exits BODY and this
`catch'.
If no throw happens, `catch' returns the value of the last BODY form.
If a throw happens, it specifies the value to return from `catch'.
usage: (catch TAG BODY...) */)
Now suppose the point is at the beginning of the second paragarph,
i.e. before ``Then''. This is where the point will be before
evaluating the forms given above if one invokes
`add-log-current-defun' when the point is in the first paragraph of
the docstring. In the past, CC mode does not consider the arguments
of DEFUN as a defun, so `beginning-of-defun' will move point to the
beginning of the function that appear before this DEFUN. With the
forms in `add-log-current-defun', the result is correct. But I found
in the newest CC mode considers the arguments (starting with
``("catch"'', ending with ``*/)'') as a defun, so that
`beginning-of-defun' will move point to the beginning of the
arguments, i.e. between the space that is after ``DEFUN'' and the left
paren before ``"catch"''. As a result, one cannot produce correct
change log entry when point is in the first paragraph of this
function, for example, when point is between ``Eval'' and ``BODY'' in
the first paragraph.
III. Different styles
The function skips typedefs and arglist with the following forms.
;; Skip back over typedefs and arglist.
;; Stop at the function definition itself
;; or at the line that follows end of function doc string.
(forward-line -1)
(while (and (not (bobp))
(looking-at "[ \t\n]")
(not (looking-back "[*]/)\n" (- (point) 4))))
(forward-line -1))
This is not general: it cannot process programs in some style. In
section 7.7 of the 3rd edition of The C++ Programming Language by
Bjarne Stroustrup, there is a shell sort implementation:
void ssort(void * base, size_t n, size_t sz, CFT cmp)
/*
Sort the "n" elements of vector "base" into increasing order
using the comparison function pointed to by "cmp".
The elements are of size "sz".
Shell sort (Knuth, Vol3, pg84)
*/
{
/* ... */
}
The current implementation cannot handle programs in this style
correctly.
IV. On the C++ names
And what I tried to fix is not general too. My fix is
(while (not (looking-back "\\(^\\|[ \t]\\)"))
(forward-sexp -1))
This is not general too: C++ permits the nested name to be put in many
lines. For example, the following name is valid:
void
class_1
::
sub_class_2
::
method_3 ()
{
/* ... */
}
The current implementation cannot handle this name correctly.
Regards,
Guanpeng Xu
_________________________________________________________________
Express yourself instantly with MSN Messenger! Download today it's FREE!
http://messenger.msn.click-url.com/go/onm00200471ave/direct/01/
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: Some problems in `add-log-current-defun'
2006-12-27 10:32 Herbert Euler
@ 2006-12-27 10:45 ` David Kastrup
2006-12-27 11:48 ` Masatake YAMATO
2006-12-27 11:55 ` Masatake YAMATO
` (2 subsequent siblings)
3 siblings, 1 reply; 19+ messages in thread
From: David Kastrup @ 2006-12-27 10:45 UTC (permalink / raw)
Cc: emacs-devel
"Herbert Euler" <herberteuler@hotmail.com> writes:
> I encountered a problem in add-log.el, and solved it somehow (see
> http://lists.gnu.org/archive/html/emacs-devel/2006-09/msg00869.html).
> However, after re-reading the implementation of
> `add-log-current-defun', I found there are still many problems (bugs)
> in `add-log-current-defun' for the C like languages.
[...]
> IV. On the C++ names
>
> And what I tried to fix is not general too. My fix is
>
> (while (not (looking-back "\\(^\\|[ \t]\\)"))
> (forward-sexp -1))
>
> This is not general too: C++ permits the nested name to be put in many
> lines. For example, the following name is valid:
>
> void
> class_1
> ::
> sub_class_2
> ::
> method_3 ()
> {
> /* ... */
> }
>
> The current implementation cannot handle this name correctly.
Please note that we are talking about implementing editor support, not
a C++ parser. It is not our task to handle all C++ constructs
perfectly. Instead, it is our task to handle reasonably formatted C++
code. Code like the above may _intentionally_ be formatted in this
manner in order to persuade the editor to interpret things
differently.
--
David Kastrup
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: Some problems in `add-log-current-defun'
2006-12-27 10:45 ` David Kastrup
@ 2006-12-27 11:48 ` Masatake YAMATO
2006-12-27 12:22 ` David Kastrup
0 siblings, 1 reply; 19+ messages in thread
From: Masatake YAMATO @ 2006-12-27 11:48 UTC (permalink / raw)
Cc: herberteuler, emacs-devel
> Please note that we are talking about implementing editor support, not
> a C++ parser. It is not our task to handle all C++ constructs
> perfectly. Instead, it is our task to handle reasonably formatted C++
> code. Code like the above may _intentionally_ be formatted in this
> manner in order to persuade the editor to interpret things
> differently.
Just a question.
Here do you think Emacs's C code is reasonably formatted? I don't
think so. However, this is the special case which
`add-log-current-defun' should handle a source which is not reasonably
formatted.
Masatake YAMATO
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Some problems in `add-log-current-defun'
2006-12-27 11:48 ` Masatake YAMATO
@ 2006-12-27 12:22 ` David Kastrup
0 siblings, 0 replies; 19+ messages in thread
From: David Kastrup @ 2006-12-27 12:22 UTC (permalink / raw)
Cc: herberteuler, emacs-devel
Masatake YAMATO <jet@gyve.org> writes:
>> Please note that we are talking about implementing editor support, not
>> a C++ parser. It is not our task to handle all C++ constructs
>> perfectly. Instead, it is our task to handle reasonably formatted C++
>> code. Code like the above may _intentionally_ be formatted in this
>> manner in order to persuade the editor to interpret things
>> differently.
>
> Just a question.
>
> Here do you think Emacs's C code is reasonably formatted?
I can't parse your message; maybe it is not reasonably formatted...
The examples were C++ code mostly. So what is "here" and "Emacs' C
code" supposed to be? Some code from the previous mail, or code from
within Emacs? An actual quote might be helpful.
> I don't think so. However, this is the special case which
> `add-log-current-defun' should handle a source which is not
> reasonably formatted.
I don't get your point, and I don't have an idea how to parse that
sentence.
--
David Kastrup
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Some problems in `add-log-current-defun'
2006-12-27 10:32 Herbert Euler
2006-12-27 10:45 ` David Kastrup
@ 2006-12-27 11:55 ` Masatake YAMATO
2006-12-27 13:46 ` Masatake YAMATO
2006-12-27 21:17 ` Richard Stallman
3 siblings, 0 replies; 19+ messages in thread
From: Masatake YAMATO @ 2006-12-27 11:55 UTC (permalink / raw)
Cc: emacs-devel
About II. I'd like to hear a comment from David.
> I. The end of a function
>
> The point is moved to the start position of a function or an empty
> line (lines consist of only white space characters) with the following
> code.
>
> ;; See if we are in the beginning part of a function,
> ;; before the open brace. If so, advance forward.
> (while (not (looking-at "{\\|\\(\\s *$\\)"))
> (forward-line 1))
>
> But this is not reliable. If someone forgets to put a newline after a
> function, `add-log-current-defun' will report wrong name. Please
> consider the following example:
>
> int
> f1 ()
> {
> /* If point is here `add-log-current-defun' gets wrong result. */
> }
> int
> f2 ()
> {
> /* ... */
> }
>
> When the point is inside the body of `f1', invoking
> `add-log-current-defun' will get `f2', rather than `f1'.
Reproduced. I think this is case that `add-log-current-defun' cannot handle
reasonably formatted C++ code.
> III. Different styles
>
> The function skips typedefs and arglist with the following forms.
>
> ;; Skip back over typedefs and arglist.
> ;; Stop at the function definition itself
> ;; or at the line that follows end of function doc string.
> (forward-line -1)
> (while (and (not (bobp))
> (looking-at "[ \t\n]")
> (not (looking-back "[*]/)\n" (- (point) 4))))
> (forward-line -1))
>
> This is not general: it cannot process programs in some style. In
> section 7.7 of the 3rd edition of The C++ Programming Language by
> Bjarne Stroustrup, there is a shell sort implementation:
>
> void ssort(void * base, size_t n, size_t sz, CFT cmp)
> /*
> Sort the "n" elements of vector "base" into increasing order
> using the comparison function pointed to by "cmp".
> The elements are of size "sz".
>
> Shell sort (Knuth, Vol3, pg84)
> */
> {
> /* ... */
> }
>
> The current implementation cannot handle programs in this style
> correctly.
The book is very famous. However, in my experience this code is not
reasonably formatted.
> And what I tried to fix is not general too. My fix is
>
> (while (not (looking-back "\\(^\\|[ \t]\\)"))
> (forward-sexp -1))
>
> This is not general too: C++ permits the nested name to be put in many
> lines. For example, the following name is valid:
>
> void
> class_1
> ::
> sub_class_2
> ::
> method_3 ()
> {
> /* ... */
> }
>
> The current implementation cannot handle this name correctly.
I think this is also not reasonably formatted.
Masatake YAMATO
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: Some problems in `add-log-current-defun'
2006-12-27 10:32 Herbert Euler
2006-12-27 10:45 ` David Kastrup
2006-12-27 11:55 ` Masatake YAMATO
@ 2006-12-27 13:46 ` Masatake YAMATO
2006-12-27 21:17 ` Richard Stallman
3 siblings, 0 replies; 19+ messages in thread
From: Masatake YAMATO @ 2006-12-27 13:46 UTC (permalink / raw)
Cc: herberteuler, emacs-devel
> >> Please note that we are talking about implementing editor support, not
> >> a C++ parser. It is not our task to handle all C++ constructs
> >> perfectly. Instead, it is our task to handle reasonably formatted C++
> >> code. Code like the above may _intentionally_ be formatted in this
> >> manner in order to persuade the editor to interpret things
> >> differently.
> >
> > Just a question.
> >
> > Here do you think Emacs's C code is reasonably formatted?
>
> I can't parse your message; maybe it is not reasonably formatted...
I'm very sorry.
What I'd like to write:
Do you think that Emacs's C code is reasonably formatted?
> The examples were C++ code mostly. So what is "here" and "Emacs' C
> code" supposed to be? Some code from the previous mail, or code from
> within Emacs? An actual quote might be helpful.
C source within Emacs. Here I quote the related statements:
> II. On the change of `beginning-of-defun' in CC mode
>
> When the point is in the docstring of Emacs C source code, the
> following forms are evaluated.
>
> (let (maybe-beg)
> ;; Try to find the containing defun.
> (beginning-of-defun)
> (end-of-defun)
>
> But I found the result is wrong with the newest CC mode. Consider the
> following Emacs C source code:
>
> DEFUN ("catch", Fcatch, Scatch, 1, UNEVALLED, 0,
> doc: /* Eval BODY allowing nonlocal exits using `throw'.
> TAG is evalled to get the tag to use; it must not be nil.
>
> Then the BODY is executed.
> Within BODY, a call to `throw' with the same TAG exits BODY and this
> `catch'.
> If no throw happens, `catch' returns the value of the last BODY form.
> If a throw happens, it specifies the value to return from `catch'.
> usage: (catch TAG BODY...) */)
>
> Now suppose the point is at the beginning of the second paragarph,
> i.e. before ``Then''. This is where the point will be before
> evaluating the forms given above if one invokes
> `add-log-current-defun' when the point is in the first paragraph of
> the docstring. In the past, CC mode does not consider the arguments
> of DEFUN as a defun, so `beginning-of-defun' will move point to the
> beginning of the function that appear before this DEFUN. With the
> forms in `add-log-current-defun', the result is correct. But I found
> in the newest CC mode considers the arguments (starting with
> ``("catch"'', ending with ``*/)'') as a defun, so that
> `beginning-of-defun' will move point to the beginning of the
> arguments, i.e. between the space that is after ``DEFUN'' and the left
> paren before ``"catch"''. As a result, one cannot produce correct
> change log entry when point is in the first paragraph of this
> function, for example, when point is between ``Eval'' and ``BODY'' in
> the first paragraph.
Masatake YAMATO
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: Some problems in `add-log-current-defun'
2006-12-27 10:32 Herbert Euler
` (2 preceding siblings ...)
2006-12-27 13:46 ` Masatake YAMATO
@ 2006-12-27 21:17 ` Richard Stallman
2006-12-28 12:41 ` Masatake YAMATO
` (2 more replies)
3 siblings, 3 replies; 19+ messages in thread
From: Richard Stallman @ 2006-12-27 21:17 UTC (permalink / raw)
Cc: emacs-devel
But this is not reliable. If someone forgets to put a newline after a
function, `add-log-current-defun' will report wrong name. Please
consider the following example:
int
f1 ()
{
/* If point is here `add-log-current-defun' gets wrong result. */
}
int
f2 ()
{
/* ... */
}
When the point is inside the body of `f1', invoking
`add-log-current-defun' will get `f2', rather than `f1'.
That bug is worth fixing.
In the past, CC mode does not consider the arguments
of DEFUN as a defun, so `beginning-of-defun' will move point to the
beginning of the function that appear before this DEFUN. With the
forms in `add-log-current-defun', the result is correct. But I found
in the newest CC mode considers the arguments (starting with
``("catch"'', ending with ``*/)'') as a defun,
That change in CC mode would clearly be a change for the better, but
it doesn't work for me.
I updated my sources yesterday around noon EST.
I find that `beginning-of-defun' when given inside the DEFUN args
of Fcatch moves back to the { at the start of the body of Fmacroexpand.
I also find that C-M-a is bound to `beginning-of-defun' in C mode.
Wasn't it supposed to be `c-beginning-of-defun' nowadays?
What is wrong here?
In any case, it would be good to change `add-log-current-defun'
to correspond to the new improved behavior of CC mode.
void ssort(void * base, size_t n, size_t sz, CFT cmp)
/*
Sort the "n" elements of vector "base" into increasing order
using the comparison function pointed to by "cmp".
The elements are of size "sz".
Shell sort (Knuth, Vol3, pg84)
*/
{
/* ... */
}
That seems like common usage -- not weird. It would be nice to handle
that usage correctly. However, if we have never handled it in the
past, I think it is not urgent to fix it now.
Masatake YAMATO <jet@gyve.org> thinks that code is badly formatted.
Masatake, why do you think so?
This is not general too: C++ permits the nested name to be put in many
lines. For example, the following name is valid:
void
class_1
::
sub_class_2
::
method_3 ()
{
/* ... */
}
The current implementation cannot handle this name correctly.
I'd say don't bother with that. At least not now.
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: Some problems in `add-log-current-defun'
2006-12-27 21:17 ` Richard Stallman
@ 2006-12-28 12:41 ` Masatake YAMATO
2006-12-29 15:44 ` Richard Stallman
2006-12-28 12:47 ` Herbert Euler
2006-12-29 3:10 ` Herbert Euler
2 siblings, 1 reply; 19+ messages in thread
From: Masatake YAMATO @ 2006-12-28 12:41 UTC (permalink / raw)
Cc: herberteuler, emacs-devel
> void ssort(void * base, size_t n, size_t sz, CFT cmp)
> /*
> Sort the "n" elements of vector "base" into increasing order
> using the comparison function pointed to by "cmp".
> The elements are of size "sz".
>
> Shell sort (Knuth, Vol3, pg84)
> */
> {
> /* ... */
> }
>
> That seems like common usage -- not weird. It would be nice to handle
> that usage correctly. However, if we have never handled it in the
> past, I think it is not urgent to fix it now.
>
> Masatake YAMATO <jet@gyve.org> thinks that code is badly formatted.
> Masatake, why do you think so?
Just because I have never seen such a comment style. So to be exact
I should write it is not badly formatted, but it is rare comment style
in my experiences. Usually I have seen following comment style:
/*
Sort the "n" elements of vector "base" into increasing order
using the comparison function pointed to by "cmp".
The elements are of size "sz".
Shell sort (Knuth, Vol3, pg84)
*/
void ssort(void * base, size_t n, size_t sz, CFT cmp)
{
/* ... */
}
Masatake YAMATO
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: Some problems in `add-log-current-defun'
2006-12-28 12:41 ` Masatake YAMATO
@ 2006-12-29 15:44 ` Richard Stallman
2006-12-30 4:55 ` Masatake YAMATO
0 siblings, 1 reply; 19+ messages in thread
From: Richard Stallman @ 2006-12-29 15:44 UTC (permalink / raw)
Cc: herberteuler, emacs-devel
> void ssort(void * base, size_t n, size_t sz, CFT cmp)
> /*
> Sort the "n" elements of vector "base" into increasing order
> using the comparison function pointed to by "cmp".
> The elements are of size "sz".
>
> Shell sort (Knuth, Vol3, pg84)
> */
> {
> /* ... */
> }
>
> That seems like common usage -- not weird. It would be nice to handle
> that usage correctly. However, if we have never handled it in the
> past, I think it is not urgent to fix it now.
>
> Masatake YAMATO <jet@gyve.org> thinks that code is badly formatted.
> Masatake, why do you think so?
Just because I have never seen such a comment style.
The comment looks ordinary to me. It starts with /* at the start of a
line, and ends with */ on a separate line.
Do you mean the POSITION of the comment is unusual?
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: Some problems in `add-log-current-defun'
2006-12-29 15:44 ` Richard Stallman
@ 2006-12-30 4:55 ` Masatake YAMATO
2006-12-30 18:24 ` Richard Stallman
0 siblings, 1 reply; 19+ messages in thread
From: Masatake YAMATO @ 2006-12-30 4:55 UTC (permalink / raw)
Cc: herberteuler, emacs-devel
> > void ssort(void * base, size_t n, size_t sz, CFT cmp)
> > /*
> > Sort the "n" elements of vector "base" into increasing order
> > using the comparison function pointed to by "cmp".
> > The elements are of size "sz".
> >
> > Shell sort (Knuth, Vol3, pg84)
> > */
> > {
> > /* ... */
> > }
> >
> > That seems like common usage -- not weird. It would be nice to handle
> > that usage correctly. However, if we have never handled it in the
> > past, I think it is not urgent to fix it now.
> >
> > Masatake YAMATO <jet@gyve.org> thinks that code is badly formatted.
> > Masatake, why do you think so?
>
> Just because I have never seen such a comment style.
>
>
> The comment looks ordinary to me. It starts with /* at the start of a
> line, and ends with */ on a separate line.
>
> Do you mean the POSITION of the comment is unusual?
Yes. A comment put between a function signature( void ssort(void * base, size_t n, size_t sz, CFT cmp) )
and its body is unusual for me.
Masatake YAMATO
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Some problems in `add-log-current-defun'
2006-12-27 21:17 ` Richard Stallman
2006-12-28 12:41 ` Masatake YAMATO
@ 2006-12-28 12:47 ` Herbert Euler
2006-12-29 15:44 ` Richard Stallman
2006-12-29 3:10 ` Herbert Euler
2 siblings, 1 reply; 19+ messages in thread
From: Herbert Euler @ 2006-12-28 12:47 UTC (permalink / raw)
Cc: emacs-devel
I have solved I and IV, more or less. But because of the
earthquake in Taiwan my network status is poor so that
I'm not able to post the patch. Let me try it later...
Regards,
Guanpeng Xu
>From: Richard Stallman <rms@gnu.org>
>Reply-To: rms@gnu.org
>To: "Herbert Euler" <herberteuler@hotmail.com>
>CC: emacs-devel@gnu.org
>Subject: Re: Some problems in `add-log-current-defun'
>Date: Wed, 27 Dec 2006 16:17:18 -0500
>
> But this is not reliable. If someone forgets to put a newline after a
> function, `add-log-current-defun' will report wrong name. Please
> consider the following example:
>
> int
> f1 ()
> {
> /* If point is here `add-log-current-defun' gets wrong result. */
> }
> int
> f2 ()
> {
> /* ... */
> }
>
> When the point is inside the body of `f1', invoking
> `add-log-current-defun' will get `f2', rather than `f1'.
>
>That bug is worth fixing.
>
> In the past, CC mode does not consider the arguments
> of DEFUN as a defun, so `beginning-of-defun' will move point to the
> beginning of the function that appear before this DEFUN. With the
> forms in `add-log-current-defun', the result is correct. But I found
> in the newest CC mode considers the arguments (starting with
> ``("catch"'', ending with ``*/)'') as a defun,
>
>That change in CC mode would clearly be a change for the better, but
>it doesn't work for me.
>
>I updated my sources yesterday around noon EST.
>I find that `beginning-of-defun' when given inside the DEFUN args
>of Fcatch moves back to the { at the start of the body of Fmacroexpand.
>
>I also find that C-M-a is bound to `beginning-of-defun' in C mode.
>Wasn't it supposed to be `c-beginning-of-defun' nowadays?
>
>What is wrong here?
>
>In any case, it would be good to change `add-log-current-defun'
>to correspond to the new improved behavior of CC mode.
>
> void ssort(void * base, size_t n, size_t sz, CFT cmp)
> /*
> Sort the "n" elements of vector "base" into increasing order
> using the comparison function pointed to by "cmp".
> The elements are of size "sz".
>
> Shell sort (Knuth, Vol3, pg84)
> */
> {
> /* ... */
> }
>
>That seems like common usage -- not weird. It would be nice to handle
>that usage correctly. However, if we have never handled it in the
>past, I think it is not urgent to fix it now.
>
>Masatake YAMATO <jet@gyve.org> thinks that code is badly formatted.
>Masatake, why do you think so?
>
> This is not general too: C++ permits the nested name to be put in many
> lines. For example, the following name is valid:
>
> void
> class_1
> ::
> sub_class_2
> ::
> method_3 ()
> {
> /* ... */
> }
>
> The current implementation cannot handle this name correctly.
>
>I'd say don't bother with that. At least not now.
>
>
>_______________________________________________
>Emacs-devel mailing list
>Emacs-devel@gnu.org
>http://lists.gnu.org/mailman/listinfo/emacs-devel
_________________________________________________________________
Don't just search. Find. Check out the new MSN Search!
http://search.msn.click-url.com/go/onm00200636ave/direct/01/
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: Some problems in `add-log-current-defun'
2006-12-27 21:17 ` Richard Stallman
2006-12-28 12:41 ` Masatake YAMATO
2006-12-28 12:47 ` Herbert Euler
@ 2006-12-29 3:10 ` Herbert Euler
2 siblings, 0 replies; 19+ messages in thread
From: Herbert Euler @ 2006-12-29 3:10 UTC (permalink / raw)
Cc: emacs-devel
The simplest fix for I is
*** add-log.el Thu Dec 28 20:49:44 2006
--- add-log.el.1 Fri Dec 29 10:27:20 2006
*************** (defun add-log-current-defun ()
*** 816,822 ****
(beginning-of-line)
;; See if we are in the beginning part of a function,
;; before the open brace. If so, advance forward.
! (while (not (looking-at "{\\|\\(\\s *$\\)"))
(forward-line 1))
(or (eobp)
(forward-char 1))
--- 816,822 ----
(beginning-of-line)
;; See if we are in the beginning part of a function,
;; before the open brace. If so, advance forward.
! (while (not (looking-at "{\\|}\\|\\(\\s *$\\)"))
(forward-line 1))
(or (eobp)
(forward-char 1))
But I think this is still not very robust, and perhaps there is a more
reliable way of moving point, working for all of I-III. Let me test
whether this approach works...
And for IV, I think the following changes make a complete solution.
At least the changed version of `add-log-current-defun' passed all
cases I can think of, including the ``not reasonably formatted'' C++
name in the beginning of this thread:
void
class_1
::
sub_class_2
::
method_3 ()
{
/* ... */
}
The patch is:
*** add-log.el Thu Dec 28 20:49:44 2006
--- add-log.el.4 Fri Dec 29 10:23:42 2006
*************** (defun add-log-current-defun ()
*** 916,941 ****
;; Include certain keywords if they
;; precede the name.
(setq middle (point))
! ;; Single (forward-sexp -1) invocation is
! ;; not enough for C++ member function defined
! ;; as part of nested class and/or namespace
! ;; like:
! ;;
! ;; void
! ;; foo::bar::baz::bazz ()
! ;; { ...
! ;;
! ;; Here we have to move the point to
! ;; the beginning of foo, not bazz.
! (while (not (looking-back "\\(^\\|[ \t]\\)"))
(forward-sexp -1))
;; Is this C++ method?
(when (and (< 2 middle)
! (string= (buffer-substring (-
middle 2)
! middle)
! "::"))
;; Include "classname::".
! (setq middle (point)))
;; Ignore these subparts of a class decl
;; and move back to the class name itself.
(while (looking-at "public \\|private ")
--- 916,939 ----
;; Include certain keywords if they
;; precede the name.
(setq middle (point))
! ;; Move through C++ nested name
! (while (looking-back "::[ \t\n]*")
(forward-sexp -1))
+ (forward-sexp -1)
;; Is this C++ method?
(when (and (< 2 middle)
! (save-excursion
! (goto-char middle)
! (looking-back "::[ \t\n]*")))
;; Include "classname::".
! (save-excursion
! ;; The `forward-sexp' form after
! ;; the `while' form above moves
! ;; backward one more sexp, so we
! ;; move forward one.
! (forward-sexp 1)
! (re-search-forward "\\(\\s \\|\n\\)*")
! (setq middle (point))))
;; Ignore these subparts of a class decl
;; and move back to the class name itself.
(while (looking-at "public \\|private ")
*************** (defun add-log-current-defun ()
*** 953,960 ****
(forward-char -1)
(skip-chars-backward " \t")
(setq end (point)))
! (buffer-substring-no-properties
! middle end))))))))
((memq major-mode add-log-tex-like-modes)
(if (re-search-backward
"\\\\\\(sub\\)*\\(section\\|paragraph\\|chapter\\)"
--- 951,962 ----
(forward-char -1)
(skip-chars-backward " \t")
(setq end (point)))
! (let ((name (buffer-substring-no-properties
! middle end)))
! (setq name (replace-regexp-in-string
! "\n" "" name)
! name (replace-regexp-in-string
! "[ \t]*::[ \t]*" "::"
name))))))))))
((memq major-mode add-log-tex-like-modes)
(if (re-search-backward
"\\\\\\(sub\\)*\\(section\\|paragraph\\|chapter\\)"
Because of the network status I cannot update my source, could
somebody please tell me how the newest CC mode moves point with
`beginning-of-defun'? And I didn't quite understand Richard's comment
on II:
> In the past, CC mode does not consider the arguments
> of DEFUN as a defun, so `beginning-of-defun' will move point to the
> beginning of the function that appear before this DEFUN. With the
> forms in `add-log-current-defun', the result is correct. But I found
> in the newest CC mode considers the arguments (starting with
> ``("catch"'', ending with ``*/)'') as a defun,
>
>That change in CC mode would clearly be a change for the better, but
>it doesn't work for me.
>
>...
>
>I also find that C-M-a is bound to `beginning-of-defun' in C mode.
>Wasn't it supposed to be `c-beginning-of-defun' nowadays?
>
>What is wrong here?
>
>...
Some explain? Thanks.
(I may not able to reply shortly, also because of the poor network
status :-( )
Regards,
Guanpeng Xu
_________________________________________________________________
Express yourself instantly with MSN Messenger! Download today it's FREE!
http://messenger.msn.click-url.com/go/onm00200471ave/direct/01/
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2007-01-03 21:11 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-12-31 8:28 Some problems in `add-log-current-defun' Herbert Euler
2006-12-31 22:13 ` Richard Stallman
2007-01-01 1:27 ` Herbert Euler
2007-01-02 23:35 ` Stefan Monnier
2007-01-03 21:11 ` Richard Stallman
-- strict thread matches above, loose matches on Subject: below --
2006-12-27 10:32 Herbert Euler
2006-12-27 10:45 ` David Kastrup
2006-12-27 11:48 ` Masatake YAMATO
2006-12-27 12:22 ` David Kastrup
2006-12-27 11:55 ` Masatake YAMATO
2006-12-27 13:46 ` Masatake YAMATO
2006-12-27 21:17 ` Richard Stallman
2006-12-28 12:41 ` Masatake YAMATO
2006-12-29 15:44 ` Richard Stallman
2006-12-30 4:55 ` Masatake YAMATO
2006-12-30 18:24 ` Richard Stallman
2006-12-28 12:47 ` Herbert Euler
2006-12-29 15:44 ` Richard Stallman
2006-12-29 3:10 ` Herbert Euler
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.