all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* add-change-log-entry
@ 2007-07-16 19:09 Paul Pogonyshev
  2007-07-17 15:05 ` add-change-log-entry Richard Stallman
  0 siblings, 1 reply; 23+ messages in thread
From: Paul Pogonyshev @ 2007-07-16 19:09 UTC (permalink / raw)
  To: emacs-devel

Hi,

Currently, when I add an entry to the changelog, I get this:

	* some-very-long-file-name (SomeLongClassName.some_even_longer_method_name): -*- point -*-

When I then press M-q, I get:

	*
	some-very-long-file-name (SomeLongClassName.some_even_longer_method_name):
-*- point -*-

Would it be possible that in case of such long identifiers,
`add-change-log-entry' started with

	* some-very-long-file-name
	(SomeLongClassName.some_even_longer_method_name): -*- point -*-

in the buffer?

Paul

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

* Re: add-change-log-entry
  2007-07-16 19:09 add-change-log-entry Paul Pogonyshev
@ 2007-07-17 15:05 ` Richard Stallman
  2007-07-18  9:02   ` add-change-log-entry martin rudalics
  0 siblings, 1 reply; 23+ messages in thread
From: Richard Stallman @ 2007-07-17 15:05 UTC (permalink / raw)
  To: Paul Pogonyshev; +Cc: emacs-devel

    Currently, when I add an entry to the changelog, I get this:

	    * some-very-long-file-name (SomeLongClassName.some_even_longer_method_name): -*- point -*-

    When I then press M-q, I get:

	    *
	    some-very-long-file-name (SomeLongClassName.some_even_longer_method_name):
    -*- point -*-

That is a bug -- whatever else we change, we should fix that.  In
Change Log mode, it should be impossible to break the line after that *.

Can someone please fix this (in 22 and the trunk), then ack?

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

* Re: add-change-log-entry
  2007-07-17 15:05 ` add-change-log-entry Richard Stallman
@ 2007-07-18  9:02   ` martin rudalics
  2007-07-18 13:54     ` add-change-log-entry Sam Steingold
  0 siblings, 1 reply; 23+ messages in thread
From: martin rudalics @ 2007-07-18  9:02 UTC (permalink / raw)
  To: rms; +Cc: emacs-devel, Paul Pogonyshev

>     Currently, when I add an entry to the changelog, I get this:
> 
> 	    * some-very-long-file-name (SomeLongClassName.some_even_longer_method_name): -*- point -*-
> 
>     When I then press M-q, I get:
> 
> 	    *
> 	    some-very-long-file-name (SomeLongClassName.some_even_longer_method_name):
>     -*- point -*-
> 
> That is a bug -- whatever else we change, we should fix that.  In
> Change Log mode, it should be impossible to break the line after that *.
> 
> Can someone please fix this (in 22 and the trunk), then ack?

I checked in a fix into the trunk which should solve the M-q problem.

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

* Re: add-change-log-entry
  2007-07-18  9:02   ` add-change-log-entry martin rudalics
@ 2007-07-18 13:54     ` Sam Steingold
  2007-07-18 14:32       ` add-change-log-entry martin rudalics
  0 siblings, 1 reply; 23+ messages in thread
From: Sam Steingold @ 2007-07-18 13:54 UTC (permalink / raw)
  To: martin rudalics; +Cc: Paul Pogonyshev, rms, emacs-devel

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

martin rudalics wrote:
>>     Currently, when I add an entry to the changelog, I get this:
>>
>>         * some-very-long-file-name
>> (SomeLongClassName.some_even_longer_method_name): -*- point -*-
>>
>>     When I then press M-q, I get:
>>
>>         *
>>         some-very-long-file-name
>> (SomeLongClassName.some_even_longer_method_name):
>>     -*- point -*-
>>
>> That is a bug -- whatever else we change, we should fix that.  In
>> Change Log mode, it should be impossible to break the line after that *.
>>
>> Can someone please fix this (in 22 and the trunk), then ack?
> 
> I checked in a fix into the trunk which should solve the M-q problem.

you forgot the to disable removing newline before "^T*":


	fix bug #[ 1607416 ]: back_trace is not initialized when it is a register
	* spvw.d (back_trace): do not init to NULL at the declaration, instead...
	(init_memory): init back_trace to NULL here

M-q ===>

	fix bug #[ 1607416 ]: back_trace is not initialized when it is a
	register * spvw.d (back_trace): do not init to NULL at the
	declaration, instead...
	(init_memory): init back_trace to NULL here

(note "* spvw.d" in the middle of a line).

also, when the function (or file) list is too long, M-q should preserve
the standard ChangeLog formatting:

	* foo (bar, baz, zot): quux

===>

	* foo (bar, baz)
	(zot); quux

instead of

	* foo (bar, baz,
	zot); quux

(for the sake of font-lock which can fontify the former but not the latter)


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFGnhuqPp1Qsf2qnMcRAmsrAJ4g5N7Evz+LsGqHaC3GbdqKF7Ua1gCfefAs
UsTGa7o7pwq5PUaZivknBjo=
=Ffgq
-----END PGP SIGNATURE-----

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

* Re: add-change-log-entry
  2007-07-18 13:54     ` add-change-log-entry Sam Steingold
@ 2007-07-18 14:32       ` martin rudalics
  2007-07-18 18:58         ` add-change-log-entry Stefan Monnier
  2007-07-19 12:23         ` add-change-log-entry Richard Stallman
  0 siblings, 2 replies; 23+ messages in thread
From: martin rudalics @ 2007-07-18 14:32 UTC (permalink / raw)
  To: Sam Steingold; +Cc: rms, emacs-devel

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

 > you forgot the to disable removing newline before "^T*":

I didn't even care.

 > 	fix bug #[ 1607416 ]: back_trace is not initialized when it is a register
 > 	* spvw.d (back_trace): do not init to NULL at the declaration, instead...
 > 	(init_memory): init back_trace to NULL here
 >
 > M-q ===>
 >
 > 	fix bug #[ 1607416 ]: back_trace is not initialized when it is a
 > 	register * spvw.d (back_trace): do not init to NULL at the
 > 	declaration, instead...
 > 	(init_memory): init back_trace to NULL here
 >
 > (note "* spvw.d" in the middle of a line).

This is a different issue.  Please look at the attached patch and tell
me whether it works and/or breaks anything else.

 > also, when the function (or file) list is too long, M-q should preserve
 > the standard ChangeLog formatting:
 >
 > 	* foo (bar, baz, zot): quux
 >
 > ===>
 >
 > 	* foo (bar, baz)
 > 	(zot); quux
 >
 > instead of
 >
 > 	* foo (bar, baz,
 > 	zot); quux
 >
 > (for the sake of font-lock which can fontify the former but not the latter)

I could make ", " non-breakable but that wouldn't help much.  Has this
or a similar problem already been resolved somewhere else?  We'd need a
similar solution for `change-log-indent'.

[-- Attachment #2: add-log.patch --]
[-- Type: text/plain, Size: 680 bytes --]

*** add-log.el	Wed Jul 18 10:49:50 2007
--- add-log.el	Wed Jul 18 16:18:00 2007
***************
*** 732,738 ****
    (interactive "P")
    (let ((end (progn (forward-paragraph) (point)))
  	(beg (progn (backward-paragraph) (point)))
! 	(paragraph-start (concat paragraph-start "\\|\\s *\\s(")))
      (fill-region beg end justify)
      t))
  \f
--- 732,739 ----
    (interactive "P")
    (let ((end (progn (forward-paragraph) (point)))
  	(beg (progn (backward-paragraph) (point)))
! 	(paragraph-start (concat paragraph-start "\\|\\s *\\s("))
! 	(paragraph-separate (concat paragraph-separate "\\|\\s *\\*")))
      (fill-region beg end justify)
      t))
  \f

[-- 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] 23+ messages in thread

* Re: add-change-log-entry
  2007-07-18 14:32       ` add-change-log-entry martin rudalics
@ 2007-07-18 18:58         ` Stefan Monnier
  2007-07-18 21:32           ` add-change-log-entry martin rudalics
  2007-07-19 12:23         ` add-change-log-entry Richard Stallman
  1 sibling, 1 reply; 23+ messages in thread
From: Stefan Monnier @ 2007-07-18 18:58 UTC (permalink / raw)
  To: martin rudalics; +Cc: Sam Steingold, rms, emacs-devel

> I could make ", " non-breakable but that wouldn't help much.  Has this
> or a similar problem already been resolved somewhere else?  We'd need a
> similar solution for `change-log-indent'.

auto-fill-mode uses comment-line-break-function which might be used to do just
that.  Currently fill-paragraph does not offer any comparable hook, but that
could be changed if necessary.


        Stefan

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

* Re: add-change-log-entry
  2007-07-18 18:58         ` add-change-log-entry Stefan Monnier
@ 2007-07-18 21:32           ` martin rudalics
  2007-07-19 21:20             ` add-change-log-entry Richard Stallman
  0 siblings, 1 reply; 23+ messages in thread
From: martin rudalics @ 2007-07-18 21:32 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Sam Steingold, rms, emacs-devel

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

> auto-fill-mode uses comment-line-break-function which might be used to do just
> that.  Currently fill-paragraph does not offer any comparable hook, but that
> could be changed if necessary.

I can do with an after-change-hook as well, look at the attached patch.  The
code would get a bit more complicated if I had to do this without font-lock.

[-- Attachment #2: add-log.patch --]
[-- Type: text/plain, Size: 1984 bytes --]

*** add-log.el	Wed Jul 18 20:19:46 2007
--- add-log.el	Wed Jul 18 20:12:48 2007
***************
*** 681,686 ****
--- 681,717 ----
  	 (pos (save-excursion (indent-line-to indent) (point))))
      (if (> pos (point)) (goto-char pos))))
  
+ (defun change-log-after-change (start end old-length)
+   "Split parenthesized lists when breaking lines.
+ START and END are beginning and end of the changed text.  OLD-LENGTH is
+ the length of the text removed by the change."
+   (when (and font-lock-mode ; Works in font-lock-mode only.
+ 	     (not undo-in-progress) ; Avoid confusing undo.
+ 	     (zerop old-length) ; The change was an insertion.
+ 	     ;; The first inserted character must be a newline.
+ 	     (equal (char-after start) ?\n)
+ 	     (> start (1+ (point-min)))
+ 	     ;; The last char before the insertion must be a comma.
+ 	     (equal (char-before start) ?,)
+ 	     (progn
+ 	       (unless (get-text-property (1- start) 'fontified)
+ 		 ;; Fontify the line, we have to check the face of the
+ 		 ;; character before the comma.  I don't know why the
+ 		 ;; save-excursion is needed here.
+ 		 (save-excursion
+ 		   (font-lock-fontify-region (- start 2) start)))
+ 	       (eq (get-text-property (- start 2) 'face)
+ 		   'change-log-list)))
+     (save-excursion
+       (goto-char start)
+       ;; Delete the comma.
+       (delete-char -1)
+       ;; Close this list.
+       (insert ")")
+       (skip-chars-forward " \t\n")
+       ;; Start next list.
+       (insert-before-markers "("))))
+ 
  
  (defvar smerge-resolve-function)
  
***************
*** 704,709 ****
--- 735,741 ----
  	    '(lambda ()
  	       (looking-back "^\\s *\\*\\s *" (line-beginning-position))) 
  	    nil t)
+   (add-hook 'after-change-functions 'change-log-after-change nil t)
    (set (make-local-variable 'indent-line-function) 'change-log-indent)
    (set (make-local-variable 'tab-always-indent) nil)
    ;; We really do want "^" in paragraph-start below: it is only the

[-- 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] 23+ messages in thread

* Re: add-change-log-entry
  2007-07-18 14:32       ` add-change-log-entry martin rudalics
  2007-07-18 18:58         ` add-change-log-entry Stefan Monnier
@ 2007-07-19 12:23         ` Richard Stallman
  1 sibling, 0 replies; 23+ messages in thread
From: Richard Stallman @ 2007-07-19 12:23 UTC (permalink / raw)
  To: martin rudalics; +Cc: sds, emacs-devel

     > instead of
     >
     > 	* foo (bar, baz,
     > 	zot); quux
     >
     > (for the sake of font-lock which can fontify the former but not the latter)

    I could make ", " non-breakable but that wouldn't help much.  Has this
    or a similar problem already been resolved somewhere else?  We'd need a
    similar solution for `change-log-indent'.

I think that would require really special-case code.

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

* Re: add-change-log-entry
  2007-07-18 21:32           ` add-change-log-entry martin rudalics
@ 2007-07-19 21:20             ` Richard Stallman
  2007-07-20  8:27               ` add-change-log-entry martin rudalics
  0 siblings, 1 reply; 23+ messages in thread
From: Richard Stallman @ 2007-07-19 21:20 UTC (permalink / raw)
  To: martin rudalics; +Cc: sds, monnier, emacs-devel

It is not clean to make this work in font-lock mode only.

It is worth a bit more complexity to make the code work in
all cases.  And it is also more modular not to depend on
what font-lock does for this mode.

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

* Re: add-change-log-entry
  2007-07-19 21:20             ` add-change-log-entry Richard Stallman
@ 2007-07-20  8:27               ` martin rudalics
  2007-07-20 16:07                 ` add-change-log-entry Stefan Monnier
  0 siblings, 1 reply; 23+ messages in thread
From: martin rudalics @ 2007-07-20  8:27 UTC (permalink / raw)
  To: rms; +Cc: sds, monnier, emacs-devel

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

  > It is not clean to make this work in font-lock mode only.

Agreed.

  > It is worth a bit more complexity to make the code work in
  > all cases.  And it is also more modular not to depend on
  > what font-lock does for this mode.

I moved the stuff away from `after-change-functions' _and_ font-lock
mode.  Could people please try whether it works as intended.


[-- Attachment #2: add-log.patch --]
[-- Type: text/plain, Size: 2590 bytes --]

*** add-log.el	Wed Jul 18 20:19:46 2007
--- add-log.el	Fri Jul 20 10:26:14 2007
***************
*** 665,670 ****
--- 665,713 ----
  (defvar change-log-indent-text 0)
  
  (defun change-log-indent ()
+   ;; The following form is an attempt to split parenthesized lists
+   ;; correctly with (auto-)filling.  For example, an overlong line
+   ;;
+   ;; * file-name.ext (very-long-foo, very-long-bar, very-long-foobar):
+   ;;
+   ;; should be now broken as
+   ;;
+   ;; * file-name.ext (very-long-foo, very-long-bar)
+   ;; (very-long-foobar):
+   ;;
+   ;; instead of
+   ;; 
+   ;; * file-name.ext (very-long-foo, very-long-bar,
+   ;; very-long-foobar):
+   ;;
+   ;; because fontification doesn't like the latter.
+   (save-excursion
+     (end-of-line 0)
+     (skip-chars-backward " \t")
+     (when (and (equal (char-before) ?\,)
+ 	       (> (point) (1+ (point-min))))
+       (condition-case nil
+ 	  (when (save-excursion
+ 		  (and (prog2
+ 			   (up-list -1)
+ 			   (equal (char-after) ?\()
+ 			 (skip-chars-backward " \t"))
+ 		       (or (bolp)
+ 			   ;; Skip everything but a whitespace or asterisk.
+ 			   (and (not (zerop (skip-chars-backward "^ \t\n*")))
+ 				(skip-chars-backward " \t")
+ 				;; We want one asterisk here.
+ 				(= (skip-chars-backward "*") -1)
+ 				(skip-chars-backward " \t")
+ 				(bolp)))))
+ 	    ;; Delete the comma.
+ 	    (delete-char -1)
+ 	    ;; Close list on previous line.
+ 	    (insert ")")
+ 	    (skip-chars-forward " \t\n")
+ 	    ;; Start list on new line.
+ 	    (insert-before-markers "("))
+ 	(error nil))))
    (let* ((indent
  	  (save-excursion
  	    (beginning-of-line)
***************
*** 681,687 ****
  	 (pos (save-excursion (indent-line-to indent) (point))))
      (if (> pos (point)) (goto-char pos))))
  
- 
  (defvar smerge-resolve-function)
  
  ;;;###autoload
--- 724,729 ----
***************
*** 732,738 ****
    (interactive "P")
    (let ((end (progn (forward-paragraph) (point)))
  	(beg (progn (backward-paragraph) (point)))
! 	(paragraph-start (concat paragraph-start "\\|\\s *\\s(")))
      (fill-region beg end justify)
      t))
  \f
--- 774,783 ----
    (interactive "P")
    (let ((end (progn (forward-paragraph) (point)))
  	(beg (progn (backward-paragraph) (point)))
! 	;; Added lines starting with asterisk.
! 	(paragraph-start (concat paragraph-start "\\|\\s *\\(?:\\s(\\|\\*\\)"))
! 	;; Make sure we call `change-log-indent'.
! 	(fill-indent-according-to-mode t))
      (fill-region beg end justify)
      t))
  \f

[-- 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] 23+ messages in thread

* Re: add-change-log-entry
  2007-07-20  8:27               ` add-change-log-entry martin rudalics
@ 2007-07-20 16:07                 ` Stefan Monnier
  2007-07-21  9:09                   ` add-change-log-entry martin rudalics
  0 siblings, 1 reply; 23+ messages in thread
From: Stefan Monnier @ 2007-07-20 16:07 UTC (permalink / raw)
  To: martin rudalics; +Cc: sds, rms, emacs-devel

>   (defun change-log-indent ()
> +   ;; The following form is an attempt to split parenthesized lists
[...]
> + 	(error nil))))

Please move it into its own function and then call it from there.
That function is big enough already and your code has already been called
from some other place in a previous life, so there's indication that it
might be useful to call it from elsewhere.


        Stefan

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

* Re: add-change-log-entry
  2007-07-20 16:07                 ` add-change-log-entry Stefan Monnier
@ 2007-07-21  9:09                   ` martin rudalics
  2007-07-22  1:49                     ` add-change-log-entry Richard Stallman
  0 siblings, 1 reply; 23+ messages in thread
From: martin rudalics @ 2007-07-21  9:09 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: rms, emacs-devel

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

Stefan Monnier wrote:
 > Please move it into its own function and then call it from there.

Look at the attached patch (maybe you wanted to suggest a better name).
BTW, may I defvar c-beginning-of-defun and c-end-of-defun here, they
are driving me mad.

Richard Stallman wrote:
 > This is a better way to write the big comment:

Applied.

[-- Attachment #2: add-log.patch --]
[-- Type: text/plain, Size: 2365 bytes --]

*** add-log.el	Sat Jul 21 10:16:58 2007
--- add-log.el	Sat Jul 21 10:45:10 2007
***************
*** 659,667 ****
--- 659,703 ----
    (add-change-log-entry whoami file-name t))
  ;;;###autoload (define-key ctl-x-4-map "a" 'add-change-log-entry-other-window)
  
+ 
  (defvar change-log-indent-text 0)
  
+ (defun change-log-fill-parenthesized-list ()
+   ;; Fill parenthesized lists of names according to GNU standards.
+   ;; * file-name.ext (very-long-foo, very-long-bar, very-long-foobar):
+   ;; should be filled as
+   ;; * file-name.ext (very-long-foo, very-long-bar)
+   ;; (very-long-foobar):
+   (save-excursion
+     (end-of-line 0)
+     (skip-chars-backward " \t")
+     (when (and (equal (char-before) ?\,)
+ 	       (> (point) (1+ (point-min))))
+       (condition-case nil
+ 	  (when (save-excursion
+ 		  (and (prog2
+ 			   (up-list -1)
+ 			   (equal (char-after) ?\()
+ 			 (skip-chars-backward " \t"))
+ 		       (or (bolp)
+ 			   ;; Skip everything but a whitespace or asterisk.
+ 			   (and (not (zerop (skip-chars-backward "^ \t\n*")))
+ 				(skip-chars-backward " \t")
+ 				;; We want one asterisk here.
+ 				(= (skip-chars-backward "*") -1)
+ 				(skip-chars-backward " \t")
+ 				(bolp)))))
+ 	    ;; Delete the comma.
+ 	    (delete-char -1)
+ 	    ;; Close list on previous line.
+ 	    (insert ")")
+ 	    (skip-chars-forward " \t\n")
+ 	    ;; Start list on new line.
+ 	    (insert-before-markers "("))
+ 	(error nil)))))
+ 
  (defun change-log-indent ()
+   (change-log-fill-parenthesized-list)
    (let* ((indent
  	  (save-excursion
  	    (beginning-of-line)
***************
*** 729,735 ****
    (interactive "P")
    (let ((end (progn (forward-paragraph) (point)))
  	(beg (progn (backward-paragraph) (point)))
! 	(paragraph-start (concat paragraph-start "\\|\\s *\\s(")))
      (fill-region beg end justify)
      t))
  \f
--- 765,775 ----
    (interactive "P")
    (let ((end (progn (forward-paragraph) (point)))
  	(beg (progn (backward-paragraph) (point)))
! 	;; Add lines starting with whitespace followed by a left paren or an
! 	;; asterisk.
! 	(paragraph-start (concat paragraph-start "\\|\\s *\\(?:\\s(\\|\\*\\)"))
! 	;; Make sure we call `change-log-indent'.
! 	(fill-indent-according-to-mode t))
      (fill-region beg end justify)
      t))
  \f

[-- 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] 23+ messages in thread

* Re: add-change-log-entry
  2007-07-21  9:09                   ` add-change-log-entry martin rudalics
@ 2007-07-22  1:49                     ` Richard Stallman
  2007-07-22  8:44                       ` add-change-log-entry martin rudalics
  0 siblings, 1 reply; 23+ messages in thread
From: Richard Stallman @ 2007-07-22  1:49 UTC (permalink / raw)
  To: martin rudalics; +Cc: monnier, emacs-devel

    BTW, may I defvar c-beginning-of-defun and c-end-of-defun here, they
    are driving me mad.

You can add defvars for them without values or doc strings
whenever needed to prevent warnings about code that is correct.

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

* Re: add-change-log-entry
  2007-07-22  1:49                     ` add-change-log-entry Richard Stallman
@ 2007-07-22  8:44                       ` martin rudalics
  2007-07-22  9:02                         ` add-change-log-entry Andreas Schwab
  2007-07-22 13:24                         ` add-change-log-entry Alan Mackenzie
  0 siblings, 2 replies; 23+ messages in thread
From: martin rudalics @ 2007-07-22  8:44 UTC (permalink / raw)
  To: rms; +Cc: emacs-devel

 >     BTW, may I defvar c-beginning-of-defun and c-end-of-defun here, they
 >     are driving me mad.
 >
 > You can add defvars for them without values or doc strings
 > whenever needed to prevent warnings about code that is correct.

My question was non-sensical.  I should have asked instead: What is the
canonical way to avoid byte-compiler messages for the undefined

(c-beginning-of-defun)

in add-log.el?  Do I have to use

(if (fboundp 'c-beginning-of-defun)
     (c-beginning-of-defun))

or would it suffice to write

(funcall 'c-beginning-of-defun)

instead?

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

* Re: add-change-log-entry
  2007-07-22  8:44                       ` add-change-log-entry martin rudalics
@ 2007-07-22  9:02                         ` Andreas Schwab
  2007-07-22  9:31                           ` add-change-log-entry martin rudalics
  2007-07-22 13:24                         ` add-change-log-entry Alan Mackenzie
  1 sibling, 1 reply; 23+ messages in thread
From: Andreas Schwab @ 2007-07-22  9:02 UTC (permalink / raw)
  To: martin rudalics; +Cc: rms, emacs-devel

martin rudalics <rudalics@gmx.at> writes:

> My question was non-sensical.  I should have asked instead: What is the
> canonical way to avoid byte-compiler messages for the undefined
>
> (c-beginning-of-defun)
>
> in add-log.el?

(eval-when-compile (require 'cc-cmds))

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

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

* Re: add-change-log-entry
  2007-07-22  9:02                         ` add-change-log-entry Andreas Schwab
@ 2007-07-22  9:31                           ` martin rudalics
  0 siblings, 0 replies; 23+ messages in thread
From: martin rudalics @ 2007-07-22  9:31 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: emacs-devel

> (eval-when-compile (require 'cc-cmds))

Thank you.  Should I generally put this at top-level?

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

* Re: add-change-log-entry
  2007-07-22  8:44                       ` add-change-log-entry martin rudalics
  2007-07-22  9:02                         ` add-change-log-entry Andreas Schwab
@ 2007-07-22 13:24                         ` Alan Mackenzie
  2007-07-22 21:00                           ` add-change-log-entry martin rudalics
  2007-07-23  4:28                           ` add-change-log-entry Richard Stallman
  1 sibling, 2 replies; 23+ messages in thread
From: Alan Mackenzie @ 2007-07-22 13:24 UTC (permalink / raw)
  To: martin rudalics; +Cc: rms, emacs-devel

Schönen guten Sontag, Martin!

On Sun, Jul 22, 2007 at 10:44:41AM +0200, martin rudalics wrote:
> >     BTW, may I defvar c-beginning-of-defun and c-end-of-defun here,
> >     they are driving me mad.

No, you may not!  The only place they are defined, and as defuns, not
variables, is in cc-cmds.el.

> > You can add defvars for them without values or doc strings whenever
> > needed to prevent warnings about code that is correct.

> My question was non-sensical.  I should have asked instead: What is the
> canonical way to avoid byte-compiler messages for the undefined

Er, I have a suspicion that this is a bug in the byte compiler that I
wrote a fix for and forgot to commit.  [Apologies to Richard, who asked
me to commit it.]  Would you please try this patch out and tell me if it
fixes anything:

[ Date: Tue, 23 May 2006 10:15:06 +0000 (GMT)
From: Alan Mackenzie <acm@muc.de>
To: emacs-devel@gnu.org
Subject: BUG + FIX. Spurious "might not be defined at runtime" from
        byte-compile-filei]
#########################################################################
Hi, Emacs!

Evaluate the following:

(defun foo ()
  (bar))
(byte-compile 'foo)
(defun bar ())
(byte-compile 'bar)

Get a cup of coffee, go for a twenty mile run, come back and do some
furious hacking.

Then do M-x byte-compile-file jrandom.el.  You get the spurious warning:

  ** The function `bar' might not be defined at runtime.

This happens because the (byte-compile 'foo) form (correctly) flags 'bar
as an unknown function, recording it in
byte-compile-unresolved-functions.  The subsequent byte-compile-file
initialises this variable AFTER doing the compilation rather than before
- hence the silly warning.

It seems to me this state of affairs was probably caused by the sloppy
doc-string for the variable - it doesn't give any context for when its
contents are gathered and used, hence hackers were (subconsciously)
scared to initialise it properly, fearing they might discard important
info.  So I've amended the doc-string too.

Should I commit this fix?



2006-05-23  Alan Mackenzie  <acm@muc.de>

	* emacs-lisp/bytecomp.el (byte-compile-from-buffer): initialize
	byte-compile-unresolved-functions before doing a compilation
	rather than afterwards.  Amend the variable's doc-string.


Index: bytecomp.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/emacs-lisp/bytecomp.el,v
retrieving revision 2.183
diff -c -r2.183 bytecomp.el
*** bytecomp.el	11 Apr 2006 17:58:49 -0000	2.183
--- bytecomp.el	23 May 2006 09:56:18 -0000
***************
*** 453,459 ****
  
  (defvar byte-compile-unresolved-functions nil
    "Alist of undefined functions to which calls have been compiled.
! Used for warnings when the function is not known to be defined or is later
  defined with incorrect args.")
  
  (defvar byte-compile-noruntime-functions nil
--- 453,460 ----
  
  (defvar byte-compile-unresolved-functions nil
    "Alist of undefined functions to which calls have been compiled.
! This variable is only significant whilst compiling an entire buffer.
! Used for warnings when a function is not known to be defined or is later
  defined with incorrect args.")
  
  (defvar byte-compile-noruntime-functions nil
***************
*** 1821,1826 ****
--- 1822,1832 ----
        (save-excursion
  	(set-buffer inbuffer)
  	(goto-char 1)
+ 	;; Should we always do this?  When calling multiple files, it
+ 	;; would be useful to delay this warning until all have been
+ 	;; compiled.  A: Yes!  b-c-u-f might contain dross from a
+ 	;; previous byte-compile.
+ 	(setq byte-compile-unresolved-functions nil)
  
  	;; Compile the forms from the input buffer.
  	(while (progn
***************
*** 1837,1847 ****
  	;; Make warnings about unresolved functions
  	;; give the end of the file as their position.
  	(setq byte-compile-last-position (point-max))
! 	(byte-compile-warn-about-unresolved-functions)
! 	;; Should we always do this?  When calling multiple files, it
! 	;; would be useful to delay this warning until all have
! 	;; been compiled.
! 	(setq byte-compile-unresolved-functions nil))
        ;; Fix up the header at the front of the output
        ;; if the buffer contains multibyte characters.
        (and filename (byte-compile-fix-header filename inbuffer outbuffer))))
--- 1843,1849 ----
  	;; Make warnings about unresolved functions
  	;; give the end of the file as their position.
  	(setq byte-compile-last-position (point-max))
! 	(byte-compile-warn-about-unresolved-functions))
        ;; Fix up the header at the front of the output
        ;; if the buffer contains multibyte characters.
        (and filename (byte-compile-fix-header filename inbuffer outbuffer))))

#########################################################################

-- 
Alan.




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


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

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

* Re: add-change-log-entry
  2007-07-22 13:24                         ` add-change-log-entry Alan Mackenzie
@ 2007-07-22 21:00                           ` martin rudalics
  2007-07-23  3:22                             ` add-change-log-entry Stefan Monnier
  2007-07-23  4:28                           ` add-change-log-entry Richard Stallman
  1 sibling, 1 reply; 23+ messages in thread
From: martin rudalics @ 2007-07-22 21:00 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: rms, emacs-devel

Good evening, Alan.

 >>My question was non-sensical.  I should have asked instead: What is the
 >>canonical way to avoid byte-compiler messages for the undefined
 >
 >
 > Er, I have a suspicion that this is a bug in the byte compiler that I
 > wrote a fix for and forgot to commit.  [Apologies to Richard, who asked
 > me to commit it.]  Would you please try this patch out and tell me if it
 > fixes anything:

No, it's a different issue.  I want to byte-compile the file add-log.el
without loading c-mode before, in order to simulate bootstrapping
behavior.  Hence `c-beginning-of-defun' and `c-end-of-defun' are
undefined and the byte-compiler issues its due warnings.  This is a
problem I have to resolve within add-log.el - after all people might
want to write ChangeLogs without being aware that a language like C
exists at all.

I could defeat the byte-compiler by writing either

(funcall 'c-beginning-of-defun)

or

(if (fboundp 'c-beginning-of-defun) (c-beginning-of-defun))

instead of

(c-beginning-of-defun)

Both should act just like defvars work for undefined variables.

Andreas says that I have to require cc-cmds instead.  I think he's right
but this means that I have to know where `c-beginning-of-defun' is
defined.  If you eventually decided to move `c-beginning-of-defun' to
cc-movement-cmds.el, someone would have to restart the game of finding
the definer of this.  In practice, dependency analysis is left to those
who more or less accidentally watch the warnings of the byte-compiler.

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

* Re: add-change-log-entry
  2007-07-22 21:00                           ` add-change-log-entry martin rudalics
@ 2007-07-23  3:22                             ` Stefan Monnier
  0 siblings, 0 replies; 23+ messages in thread
From: Stefan Monnier @ 2007-07-23  3:22 UTC (permalink / raw)
  To: martin rudalics; +Cc: Alan Mackenzie, rms, emacs-devel

> Andreas says that I have to require cc-cmds instead.  I think he's right
> but this means that I have to know where `c-beginning-of-defun' is
> defined.  If you eventually decided to move `c-beginning-of-defun' to
> cc-movement-cmds.el, someone would have to restart the game of finding
> the definer of this.  In practice, dependency analysis is left to those
> who more or less accidentally watch the warnings of the byte-compiler.

Of course, The Right Thing to do is to move the add-log code that deals with
C and variants from add-log.el to cc-*.el (using
add-log-current-defun-function).


        Stefan

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

* Re: add-change-log-entry
  2007-07-22 13:24                         ` add-change-log-entry Alan Mackenzie
  2007-07-22 21:00                           ` add-change-log-entry martin rudalics
@ 2007-07-23  4:28                           ` Richard Stallman
  2007-07-23 15:38                             ` add-change-log-entry Alan Mackenzie
  1 sibling, 1 reply; 23+ messages in thread
From: Richard Stallman @ 2007-07-23  4:28 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: emacs-devel

What does the byte-compiler bug consist of?
What does your patch intend to do?

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

* Re: add-change-log-entry
  2007-07-23  4:28                           ` add-change-log-entry Richard Stallman
@ 2007-07-23 15:38                             ` Alan Mackenzie
  2007-07-23 22:31                               ` add-change-log-entry Richard Stallman
  0 siblings, 1 reply; 23+ messages in thread
From: Alan Mackenzie @ 2007-07-23 15:38 UTC (permalink / raw)
  To: Richard Stallman; +Cc: emacs-devel

On Mon, Jul 23, 2007 at 12:28:09AM -0400, Richard Stallman wrote:
> What does the byte-compiler bug consist of?

emacs -Q

Evaluate the following:

(defun foo ()
  (bar))
(byte-compile 'foo)
(defun bar ())
(byte-compile 'bar)

Now do M-: byte-compile-file <CR> .../emacs/lisp/emacs-lisp/bytecomp.el
(or any other file, for that matter).

This gives the following message:
In end of data:
bytecomp.el:4256:1:Warning: the function `bar' might not be defined at
    runtime.

> What does your patch intend to do?

The bug happens because the variable byte-compile-unresolved-functions
is cleared at the END of a compilation rather than being initialised at
the beginning of it.  In the recipe above, it's value is ((bar 0)) at
the start of the compilation.

My patch initialises the variable correctly at the start of
byte-compile-file, and also makes the variable's doc string less vague,
this vagueness probably being what made the original hacker scared to
initialise it in the first place.

The following patch is against the Emacs 22 branch:



2007-07-23  Alan Mackenzie  <acm@muc.de>

	* emacs-lisp/bytecomp.el (byte-compile-from-buffer): initialise
	byte-compile-unresolved-functions before rather than after a
	compilation.
	(byte-compile-unresolved-functions): Amplify doc string.


Index: bytecomp.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/emacs-lisp/bytecomp.el,v
retrieving revision 2.198
diff -c -r2.198 bytecomp.el
*** bytecomp.el	11 Apr 2007 03:59:20 -0000	2.198
--- bytecomp.el	23 Jul 2007 15:26:07 -0000
***************
*** 476,482 ****
  
  (defvar byte-compile-unresolved-functions nil
    "Alist of undefined functions to which calls have been compiled.
! Used for warnings when the function is not known to be defined or is later
  defined with incorrect args.")
  
  (defvar byte-compile-noruntime-functions nil
--- 476,483 ----
  
  (defvar byte-compile-unresolved-functions nil
    "Alist of undefined functions to which calls have been compiled.
! This variable is only significant whilst compiling an entire buffer.
! Used for warnings when a function is not known to be defined or is later
  defined with incorrect args.")
  
  (defvar byte-compile-noruntime-functions nil
***************
*** 1848,1853 ****
--- 1849,1859 ----
        (save-excursion
  	(set-buffer inbuffer)
  	(goto-char 1)
+ 	;; Should we always do this?  When calling multiple files, it
+ 	;; would be useful to delay this warning until all have been
+ 	;; compiled.  A: Yes!  b-c-u-f might contain dross from a
+ 	;; previous byte-compile.
+ 	(setq byte-compile-unresolved-functions nil)
  
  	;; Compile the forms from the input buffer.
  	(while (progn
***************
*** 1864,1874 ****
  	;; Make warnings about unresolved functions
  	;; give the end of the file as their position.
  	(setq byte-compile-last-position (point-max))
! 	(byte-compile-warn-about-unresolved-functions)
! 	;; Should we always do this?  When calling multiple files, it
! 	;; would be useful to delay this warning until all have
! 	;; been compiled.
! 	(setq byte-compile-unresolved-functions nil))
        ;; Fix up the header at the front of the output
        ;; if the buffer contains multibyte characters.
        (and filename (byte-compile-fix-header filename inbuffer outbuffer))))
--- 1870,1876 ----
  	;; Make warnings about unresolved functions
  	;; give the end of the file as their position.
  	(setq byte-compile-last-position (point-max))
! 	(byte-compile-warn-about-unresolved-functions))
        ;; Fix up the header at the front of the output
        ;; if the buffer contains multibyte characters.
        (and filename (byte-compile-fix-header filename inbuffer outbuffer))))


-- 
Alan Mackenzie (Ittersbach, Germany).

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

* Re: add-change-log-entry
  2007-07-23 15:38                             ` add-change-log-entry Alan Mackenzie
@ 2007-07-23 22:31                               ` Richard Stallman
  2007-07-24  8:19                                 ` add-change-log-entry Alan Mackenzie
  0 siblings, 1 reply; 23+ messages in thread
From: Richard Stallman @ 2007-07-23 22:31 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: emacs-devel

Please install your byte-compiler patch in Emacs 22 and the trunk.

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

* Re: add-change-log-entry
  2007-07-23 22:31                               ` add-change-log-entry Richard Stallman
@ 2007-07-24  8:19                                 ` Alan Mackenzie
  0 siblings, 0 replies; 23+ messages in thread
From: Alan Mackenzie @ 2007-07-24  8:19 UTC (permalink / raw)
  To: Richard Stallman; +Cc: emacs-devel

On Mon, Jul 23, 2007 at 06:31:10PM -0400, Richard Stallman wrote:
> Please install your byte-compiler patch in Emacs 22 and the trunk.

DONE.

-- 
Alan.

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

end of thread, other threads:[~2007-07-24  8:19 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-07-16 19:09 add-change-log-entry Paul Pogonyshev
2007-07-17 15:05 ` add-change-log-entry Richard Stallman
2007-07-18  9:02   ` add-change-log-entry martin rudalics
2007-07-18 13:54     ` add-change-log-entry Sam Steingold
2007-07-18 14:32       ` add-change-log-entry martin rudalics
2007-07-18 18:58         ` add-change-log-entry Stefan Monnier
2007-07-18 21:32           ` add-change-log-entry martin rudalics
2007-07-19 21:20             ` add-change-log-entry Richard Stallman
2007-07-20  8:27               ` add-change-log-entry martin rudalics
2007-07-20 16:07                 ` add-change-log-entry Stefan Monnier
2007-07-21  9:09                   ` add-change-log-entry martin rudalics
2007-07-22  1:49                     ` add-change-log-entry Richard Stallman
2007-07-22  8:44                       ` add-change-log-entry martin rudalics
2007-07-22  9:02                         ` add-change-log-entry Andreas Schwab
2007-07-22  9:31                           ` add-change-log-entry martin rudalics
2007-07-22 13:24                         ` add-change-log-entry Alan Mackenzie
2007-07-22 21:00                           ` add-change-log-entry martin rudalics
2007-07-23  3:22                             ` add-change-log-entry Stefan Monnier
2007-07-23  4:28                           ` add-change-log-entry Richard Stallman
2007-07-23 15:38                             ` add-change-log-entry Alan Mackenzie
2007-07-23 22:31                               ` add-change-log-entry Richard Stallman
2007-07-24  8:19                                 ` add-change-log-entry Alan Mackenzie
2007-07-19 12:23         ` add-change-log-entry Richard Stallman

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.