unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* [salve@debian.org: lisp/mail/supercite.el: sc-select-attribution logic is broken (patch included)]
@ 2005-05-11 16:25 Richard Stallman
  0 siblings, 0 replies; 6+ messages in thread
From: Richard Stallman @ 2005-05-11 16:25 UTC (permalink / raw)


Is there someone here who can evaluate this proposed patch?
I don't use supercite.

------- Start of forwarded message -------
To: emacs-pretest-bug@gnu.org
From: "Davide G. M. Salvetti" <salve@debian.org>
Organization: Quis ut Deus?
X-PGP-Fingerprint: D3B2 A3F5 E9EB BDE6 A245  AA4A 212B 306C 9396 865D
X-PGP-Affinity: Will accept encrypted messages for GNU Privacy Guard
X-Accept-Language: en, it, de, es
Date: Mon, 09 May 2005 13:58:58 +0200
X-ASICTP-MailScanner-Information: Please see
	http://www.ictp.trieste.it/antispam.html
X-ASICTP-MailScanner: Found to be clean
X-ASICTP-MailScanner-SpamCheck: not spam (whitelisted),
	SpamAssassin (score=-2.82, required 5, ALL_TRUSTED -2.82)
X-MailScanner-From: salve@hal.linux.it
Subject: lisp/mail/supercite.el: sc-select-attribution logic is broken
	(patch included)
Sender: emacs-pretest-bug-bounces+rms=gnu.org@gnu.org
X-Spam-Checker-Version: SpamAssassin 2.63 (2004-01-11) on monty-python
X-Spam-Level: 
X-Spam-Status: No, hits=0.0 required=5.0 tests=none autolearn=no version=2.63

This bug report will be sent to the Free Software Foundation,
not to your local site managers!
Please write in English if possible, because the Emacs maintainers
usually do not have translators to read other languages for them.

Your bug report will be posted to the emacs-pretest-bug@gnu.org mailing list.

Please describe exactly what actions triggered the bug
and the precise symptoms of the bug:

Premise: a very short patch is included.  I don't think you need signed
papers for it, but otherwise I'm willing to sign them (I did it already
for AUCTeX contributions).

I added "sc-consult" to sc-preferred-attribution-list and the suggested
form to sc-attrib-selection-list in order to have the BBDB consulted when
choosing an attribution to quote citations from a message being replied to
(as per bbdb.info, node: Supercite Prep):
========================================================================
(setq sc-preferred-attribution-list '("sc-lastchoice" "x-attribution"
				      "sc-consult" "initials" "firstname"
				      "lastname")
      sc-attrib-selection-list
      '(("sc-from-address"
	 ((".*" . (bbdb/sc-consult-attr
		       (sc-mail-field "sc-from-address")))))))
========================================================================

However, contrary to what I expected, SuperCite ended with the
"firstname" citation style whenever the BBDB had no suggestion, rather
than with "initials".  By inspecting the code (lisp/mail/supercite.el),
I found that the logic of the function sc-select-attribution is (I
believe) broken: it stops scanning sc-preferred-attribution-list even if
bbdb/sc-consult-attr returns nil.  Therefore, the backup attribution
selection code controlled by sc-use-only-preference-p is executed and
"firstname" get selected, even if I expected "initials" (look at my
sc-preferred-attribution-list).

The very simple patch included below checks to see if bbdb/sc-consult-attr
returns a not nil value and doesn't stop to scan
sc-preferred-attribution-list otherwise (I believe this should be the
intended behavior).  Please, apply it to lisp/mail/supercite.el.

========================================================================
- --- lisp/mail/supercite.el	2005-04-04 11:19:51.000000000 +0200
+++ lisp/mail/supercite-dgms.el	2005-05-09 13:30:56.000000000 +0200
@@ -1174,8 +1174,9 @@
 	      (setq attribution attrib
 		    attriblist nil))
 	     ((listp attrib)
- -	      (setq attribution (eval attrib)
- -		    attriblist nil))
+	      (setq attribution (eval attrib))
+	      (when attribution
+		(setq attriblist nil))
 	     (t (error "%s did not evaluate to a string or list!"
 		       "sc-attrib-selection-list"))
 	     )))
========================================================================

In GNU Emacs 22.0.50.1 (i386-pc-linux-gnu, GTK+ Version 2.6.4)
 of 2005-05-09 on rino, modified by Debian
Distributor `The X.Org Foundation', version 11.0.60801099
configured using `configure '--build' 'i386-linux-gnu' '--host' 'i386-linux-gnu' '--prefix=/usr' '--sharedstatedir=/var/lib' '--libexecdir=/usr/lib' '--localstatedir=/var' '--infodir=/usr/share/info' '--mandir=/usr/share/man' '--with-pop=yes' '--with-gif' '--with-x=yes' '--with-x-toolkit=gtk' 'CFLAGS=-DDEBIAN -g -O2' 'build_alias=i386-linux-gnu' 'host_alias=i386-linux-gnu''

Important settings:
  value of $LC_ALL: nil
  value of $LC_COLLATE: nil
  value of $LC_CTYPE: nil
  value of $LC_MESSAGES: nil
  value of $LC_MONETARY: nil
  value of $LC_NUMERIC: nil
  value of $LC_TIME: nil
  value of $LANG: nil
  locale-coding-system: nil
  default-enable-multibyte-characters: t

Major mode: Emacs-Lisp

Minor modes in effect:
  eldoc-mode: t
  checkdoc-minor-mode: t
  whitespace-global-mode: t
  auto-image-file-mode: t
  show-paren-mode: t
  mouse-wheel-mode: t
  tooltip-mode: t
  auto-compression-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  unify-8859-on-encoding-mode: t
  utf-translate-cjk-mode: t
  column-number-mode: t
  line-number-mode: t
  next-error-follow-minor-mode:  Fol

Recent input:
C-b C-b C-b u t i o n C-e C-j ( i f SPC a t t r i b 
u t i o n M-b M-b M-d w h e n C-e C-b C-j ( s e t q 
SPC a t t r i b l i s t SPC n i l C-f C-f C-f C-M-k 
C-k C-k C-x C-s M-x e b <backspace> m a c s - b u <tab> 
<backspace> <backspace> <tab> C-g M-x g <backspace> 
b u g <tab> <tab> C-a C-k r e p o <tab> t <tab> <backspace> 
r <tab> <return>

Recent messages:
Mark saved where search started
Undo! [3 times]
Auto-saving...
Saving file /home/salve/+wd/emacs/lisp/mail/supercite-dgms.el...
Whitespaces: [i] in /home/salve/+wd/emacs/lisp/mail/supercite-dgms.el
Wrote /home/salve/+wd/emacs/lisp/mail/supercite-dgms.el
Making completion list...
Quit
Making completion list...
Loading emacsbug...done


_______________________________________________
Emacs-pretest-bug mailing list
Emacs-pretest-bug@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-pretest-bug
------- End of forwarded message -------

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

* [salve@debian.org: lisp/mail/supercite.el: sc-select-attribution logic is broken (patch included)]
@ 2005-05-25 15:02 Richard Stallman
  2005-05-26 17:21 ` Glenn Morris
  2005-05-26 17:32 ` Glenn Morris
  0 siblings, 2 replies; 6+ messages in thread
From: Richard Stallman @ 2005-05-25 15:02 UTC (permalink / raw)


[I sent this message two weeks ago but did not get a response.
Would someone please look at this?]

Is there someone here who can evaluate this proposed patch?
I don't use supercite.

------- Start of forwarded message -------
To: emacs-pretest-bug@gnu.org
From: "Davide G. M. Salvetti" <salve@debian.org>
Organization: Quis ut Deus?
X-PGP-Fingerprint: D3B2 A3F5 E9EB BDE6 A245  AA4A 212B 306C 9396 865D
X-PGP-Affinity: Will accept encrypted messages for GNU Privacy Guard
X-Accept-Language: en, it, de, es
Date: Mon, 09 May 2005 13:58:58 +0200
X-ASICTP-MailScanner-Information: Please see
	http://www.ictp.trieste.it/antispam.html
X-ASICTP-MailScanner: Found to be clean
X-ASICTP-MailScanner-SpamCheck: not spam (whitelisted),
	SpamAssassin (score=-2.82, required 5, ALL_TRUSTED -2.82)
X-MailScanner-From: salve@hal.linux.it
Subject: lisp/mail/supercite.el: sc-select-attribution logic is broken
	(patch included)
Sender: emacs-pretest-bug-bounces+rms=gnu.org@gnu.org
X-Spam-Checker-Version: SpamAssassin 2.63 (2004-01-11) on monty-python
X-Spam-Level: 
X-Spam-Status: No, hits=0.0 required=5.0 tests=none autolearn=no version=2.63

This bug report will be sent to the Free Software Foundation,
not to your local site managers!
Please write in English if possible, because the Emacs maintainers
usually do not have translators to read other languages for them.

Your bug report will be posted to the emacs-pretest-bug@gnu.org mailing list.

Please describe exactly what actions triggered the bug
and the precise symptoms of the bug:

Premise: a very short patch is included.  I don't think you need signed
papers for it, but otherwise I'm willing to sign them (I did it already
for AUCTeX contributions).

I added "sc-consult" to sc-preferred-attribution-list and the suggested
form to sc-attrib-selection-list in order to have the BBDB consulted when
choosing an attribution to quote citations from a message being replied to
(as per bbdb.info, node: Supercite Prep):
========================================================================
(setq sc-preferred-attribution-list '("sc-lastchoice" "x-attribution"
				      "sc-consult" "initials" "firstname"
				      "lastname")
      sc-attrib-selection-list
      '(("sc-from-address"
	 ((".*" . (bbdb/sc-consult-attr
		       (sc-mail-field "sc-from-address")))))))
========================================================================

However, contrary to what I expected, SuperCite ended with the
"firstname" citation style whenever the BBDB had no suggestion, rather
than with "initials".  By inspecting the code (lisp/mail/supercite.el),
I found that the logic of the function sc-select-attribution is (I
believe) broken: it stops scanning sc-preferred-attribution-list even if
bbdb/sc-consult-attr returns nil.  Therefore, the backup attribution
selection code controlled by sc-use-only-preference-p is executed and
"firstname" get selected, even if I expected "initials" (look at my
sc-preferred-attribution-list).

The very simple patch included below checks to see if bbdb/sc-consult-attr
returns a not nil value and doesn't stop to scan
sc-preferred-attribution-list otherwise (I believe this should be the
intended behavior).  Please, apply it to lisp/mail/supercite.el.

========================================================================
- --- lisp/mail/supercite.el	2005-04-04 11:19:51.000000000 +0200
+++ lisp/mail/supercite-dgms.el	2005-05-09 13:30:56.000000000 +0200
@@ -1174,8 +1174,9 @@
 	      (setq attribution attrib
 		    attriblist nil))
 	     ((listp attrib)
- -	      (setq attribution (eval attrib)
- -		    attriblist nil))
+	      (setq attribution (eval attrib))
+	      (when attribution
+		(setq attriblist nil))
 	     (t (error "%s did not evaluate to a string or list!"
 		       "sc-attrib-selection-list"))
 	     )))
========================================================================

In GNU Emacs 22.0.50.1 (i386-pc-linux-gnu, GTK+ Version 2.6.4)
 of 2005-05-09 on rino, modified by Debian
Distributor `The X.Org Foundation', version 11.0.60801099
configured using `configure '--build' 'i386-linux-gnu' '--host' 'i386-linux-gnu' '--prefix=/usr' '--sharedstatedir=/var/lib' '--libexecdir=/usr/lib' '--localstatedir=/var' '--infodir=/usr/share/info' '--mandir=/usr/share/man' '--with-pop=yes' '--with-gif' '--with-x=yes' '--with-x-toolkit=gtk' 'CFLAGS=-DDEBIAN -g -O2' 'build_alias=i386-linux-gnu' 'host_alias=i386-linux-gnu''

Important settings:
  value of $LC_ALL: nil
  value of $LC_COLLATE: nil
  value of $LC_CTYPE: nil
  value of $LC_MESSAGES: nil
  value of $LC_MONETARY: nil
  value of $LC_NUMERIC: nil
  value of $LC_TIME: nil
  value of $LANG: nil
  locale-coding-system: nil
  default-enable-multibyte-characters: t

Major mode: Emacs-Lisp

Minor modes in effect:
  eldoc-mode: t
  checkdoc-minor-mode: t
  whitespace-global-mode: t
  auto-image-file-mode: t
  show-paren-mode: t
  mouse-wheel-mode: t
  tooltip-mode: t
  auto-compression-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  unify-8859-on-encoding-mode: t
  utf-translate-cjk-mode: t
  column-number-mode: t
  line-number-mode: t
  next-error-follow-minor-mode:  Fol

Recent input:
C-b C-b C-b u t i o n C-e C-j ( i f SPC a t t r i b 
u t i o n M-b M-b M-d w h e n C-e C-b C-j ( s e t q 
SPC a t t r i b l i s t SPC n i l C-f C-f C-f C-M-k 
C-k C-k C-x C-s M-x e b <backspace> m a c s - b u <tab> 
<backspace> <backspace> <tab> C-g M-x g <backspace> 
b u g <tab> <tab> C-a C-k r e p o <tab> t <tab> <backspace> 
r <tab> <return>

Recent messages:
Mark saved where search started
Undo! [3 times]
Auto-saving...
Saving file /home/salve/+wd/emacs/lisp/mail/supercite-dgms.el...
Whitespaces: [i] in /home/salve/+wd/emacs/lisp/mail/supercite-dgms.el
Wrote /home/salve/+wd/emacs/lisp/mail/supercite-dgms.el
Making completion list...
Quit
Making completion list...
Loading emacsbug...done


_______________________________________________
Emacs-pretest-bug mailing list
Emacs-pretest-bug@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-pretest-bug
------- End of forwarded message -------

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

* Re: [salve@debian.org: lisp/mail/supercite.el: sc-select-attribution logic is broken (patch included)]
  2005-05-25 15:02 [salve@debian.org: lisp/mail/supercite.el: sc-select-attribution logic is broken (patch included)] Richard Stallman
@ 2005-05-26 17:21 ` Glenn Morris
  2005-05-27 14:18   ` Richard Stallman
  2005-05-26 17:32 ` Glenn Morris
  1 sibling, 1 reply; 6+ messages in thread
From: Glenn Morris @ 2005-05-26 17:21 UTC (permalink / raw)
  Cc: emacs-devel

Richard Stallman wrote:

> Is there someone here who can evaluate this proposed patch?
> I don't use supercite.

I used to use supercite. I think the complaint is correct, but the
patch

> - --- lisp/mail/supercite.el	2005-04-04 11:19:51.000000000 +0200
> +++ lisp/mail/supercite-dgms.el	2005-05-09 13:30:56.000000000 +0200
> @@ -1174,8 +1174,9 @@
>  	      (setq attribution attrib
>  		    attriblist nil))
>  	     ((listp attrib)
> - -	      (setq attribution (eval attrib)
> - -		    attriblist nil))
> +	      (setq attribution (eval attrib))
> +	      (when attribution
> +		(setq attriblist nil))
>  	     (t (error "%s did not evaluate to a string or list!"
>  		       "sc-attrib-selection-list"))
>  	     )))

is wrong since it leads to an infinite loop when (eval attrib) returns
nil. I suggest the following:

diff -c -c -w -r1.44 supercite.el
*** supercite.el	14 May 2005 11:27:40 -0000	1.44
--- supercite.el	26 May 2005 17:15:39 -0000
***************
*** 1183,1189 ****
  		    attriblist nil))
  	     ((listp attrib)
  	      (setq attribution (eval attrib)
! 		    attriblist nil))
  	     (t (error "%s did not evaluate to a string or list!"
  		       "sc-attrib-selection-list"))
  	     )))
--- 1183,1190 ----
  		    attriblist nil))
  	     ((listp attrib)
  	      (setq attribution (eval attrib)
! 		    attriblist (if (stringp attribution) nil
!                                  (cdr attriblist))))
  	     (t (error "%s did not evaluate to a string or list!"
  		       "sc-attrib-selection-list"))
  	     )))

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

* Re: [salve@debian.org: lisp/mail/supercite.el: sc-select-attribution logic is broken (patch included)]
  2005-05-25 15:02 [salve@debian.org: lisp/mail/supercite.el: sc-select-attribution logic is broken (patch included)] Richard Stallman
  2005-05-26 17:21 ` Glenn Morris
@ 2005-05-26 17:32 ` Glenn Morris
  1 sibling, 0 replies; 6+ messages in thread
From: Glenn Morris @ 2005-05-26 17:32 UTC (permalink / raw)
  Cc: emacs-devel


Actually, the following patch is better than the first one I sent.


*** supercite.el	14 May 2005 11:27:40 -0000	1.44
--- supercite.el	26 May 2005 17:29:26 -0000
***************
*** 1182,1189 ****
  	      (setq attribution attrib
  		    attriblist nil))
  	     ((listp attrib)
! 	      (setq attribution (eval attrib)
! 		    attriblist nil))
  	     (t (error "%s did not evaluate to a string or list!"
  		       "sc-attrib-selection-list"))
  	     )))
--- 1182,1192 ----
  	      (setq attribution attrib
  		    attriblist nil))
  	     ((listp attrib)
! 	      (setq attribution (eval attrib))
!               (if (stringp attribution)
!                   (setq attriblist nil)
!                 (setq attribution nil
!                       attriblist (cdr attriblist))))
  	     (t (error "%s did not evaluate to a string or list!"
  		       "sc-attrib-selection-list"))
  	     )))

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

* Re: [salve@debian.org: lisp/mail/supercite.el: sc-select-attribution logic is broken (patch included)]
  2005-05-26 17:21 ` Glenn Morris
@ 2005-05-27 14:18   ` Richard Stallman
  2005-05-30 11:57     ` Glenn Morris
  0 siblings, 1 reply; 6+ messages in thread
From: Richard Stallman @ 2005-05-27 14:18 UTC (permalink / raw)
  Cc: rms, emacs-devel

If that's your idea of the best patch, would you please
install it?

supercite does not need a lot of maintenance, but once
in a while it does.  Could you possibly be its maintainer?

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

* Re: [salve@debian.org: lisp/mail/supercite.el: sc-select-attribution logic is broken (patch included)]
  2005-05-27 14:18   ` Richard Stallman
@ 2005-05-30 11:57     ` Glenn Morris
  0 siblings, 0 replies; 6+ messages in thread
From: Glenn Morris @ 2005-05-30 11:57 UTC (permalink / raw)
  Cc: emacs-devel

Richard Stallman wrote:

> If that's your idea of the best patch, would you please install it?

Done.

> supercite does not need a lot of maintenance, but once
> in a while it does.  Could you possibly be its maintainer?

Yes, I can and will do that (though obviously it would be better if
someone who actively uses it could do the job).

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

end of thread, other threads:[~2005-05-30 11:57 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2005-05-25 15:02 [salve@debian.org: lisp/mail/supercite.el: sc-select-attribution logic is broken (patch included)] Richard Stallman
2005-05-26 17:21 ` Glenn Morris
2005-05-27 14:18   ` Richard Stallman
2005-05-30 11:57     ` Glenn Morris
2005-05-26 17:32 ` Glenn Morris
  -- strict thread matches above, loose matches on Subject: below --
2005-05-11 16:25 Richard Stallman

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

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

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