Re: FIX

Re: FIX

[Christoph Conrad]

Hi Justus,

> The result is at http://www.montefiore.ulg.ac.be/~piater/test/nnir.el
> and works for me. Christoph, can you test it?

Yes, it works, tested on Windows XP with swish++ 5 in the company. This
evening i will also test it on GNU/Linux @ home with a newer swish++
(that one that comes with Ubuntu 7.10, probably 6.x).

> what is gnus.gnus-bug?

This is a mirror on news.gnus.org from the gnus bug mailing list. But i
see that it is by far not ideal to use it when writing to the group. I
will switch to the mailing list ASAP - it is bugs@gnus.org, like in the
bug reports, isn't it? Or should i also get ding@gnus.org?

With kind regards,
Christoph

nnir.el swish++ interface broken (was: FIX)

[Reiner Steib]

On Wed, Apr 16 2008, Christoph Conrad wrote:

>> The result is at http://www.montefiore.ulg.ac.be/~piater/test/nnir.el
>> and works for me. Christoph, can you test it?

This isn't based on the current CVS version.  Could you please provide
a diff against current CVS?

> Yes, it works, tested on Windows XP with swish++ 5 in the company. This
> evening i will also test it on GNU/Linux @ home with a newer swish++
> (that one that comes with Ubuntu 7.10, probably 6.x).
>
>> what is gnus.gnus-bug?
>
> This is a mirror on news.gnus.org from the gnus bug mailing list. But i
> see that it is by far not ideal to use it when writing to the group. I
> will switch to the mailing list ASAP - it is bugs@gnus.org, like in the
> bug reports, isn't it? Or should i also get ding@gnus.org?

I'd suggest to use ding@gnus.org aka gmane.emacs.gnus.general

Bye, Reiner.
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/

Re: nnir.el swish++ interface broken

[Justus-bulk]

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

Reiner Steib <reinersteib+gmane@imap.cc> wrote on Wed, 16 Apr 2008
20:22:21 +0200:

> On Wed, Apr 16 2008, Christoph Conrad wrote:
>
>>> The result is at http://www.montefiore.ulg.ac.be/~piater/test/nnir.el
>>> and works for me. Christoph, can you test it?
>
> This isn't based on the current CVS version.  Could you please provide
> a diff against current CVS?

Attached. I also updated the complete file on my Web server.

Justus



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: nnir.el.diff --]
[-- Type: text/x-diff, Size: 4401 bytes --]

Index: nnir.el
===================================================================
RCS file: /usr/local/cvsroot/gnus/contrib/nnir.el,v
retrieving revision 7.26
diff -c -r7.26 nnir.el
*** nnir.el	15 Apr 2008 21:16:28 -0000	7.26
--- nnir.el	17 Apr 2008 07:19:38 -0000
***************
*** 859,864 ****
--- 859,872 ----
  (nnoo-define-skeleton nnir)
  
  
+ (defmacro nnir-add-result (dirnam artno score prefix server artlist)
+   "Ask `nnir-compose-result' to construct a result vector, 
+ and if it is non-nil, add it to artlist."
+   `(let ((result (nnir-compose-result dirnam artno score prefix server)))
+      (when (not (null result))
+        (push result artlist))))
+ 
+ 
  ;; Helper function currently used by the Swish++ and Namazu backends;
  ;; perhaps useful for other backends as well
  (defun nnir-compose-result (dirnam article score prefix server)
***************
*** 869,889 ****
    (when (string-match (concat "^" prefix) dirnam)
      (setq dirnam (replace-match "" t t dirnam)))
  
!   ;; remove trailing slash and, for nnmaildir, cur/new/tmp
!   (setq dirnam (substring dirnam 0 (if (string= server "nnmaildir:") -5 -1)))
! 
!   ;; eliminate all ".", "/", "\" from beginning. Always matches.
!   (string-match "^[./\\]*\\(.*\\)$" dirnam)
!   (setq group (substitute ?. ?/ (match-string 1 dirnam))) ;; "/" -> "."
!   (setq group (substitute ?. ?\\ group)) ;; "\\" -> "."
! 
!   (vector (nnir-group-full-name group server)
! 	  (if (string= server "nnmaildir:")
! 	      (nnmaildir-base-name-to-article-number
! 	       (substring article 0 (string-match ":" article))
! 	       group nil)
! 	    (string-to-int article))
! 	  (string-to-int score)))
  
  
  ;;; Search Engine Interfaces:
--- 877,900 ----
    (when (string-match (concat "^" prefix) dirnam)
      (setq dirnam (replace-match "" t t dirnam)))
  
!   (if (not (file-readable-p (concat prefix dirnam article)))
!       nil
!     ;; remove trailing slash and, for nnmaildir, cur/new/tmp
!     (setq dirnam
! 	  (substring dirnam 0 (if (string= server "nnmaildir:") -5 -1)))
! 
!     ;; eliminate all ".", "/", "\" from beginning. Always matches.
!     (string-match "^[./\\]*\\(.*\\)$" dirnam)
!     (setq group (substitute ?. ?/ (match-string 1 dirnam))) ;; "/" -> "."
!     (setq group (substitute ?. ?\\ group)) ;; "\\" -> "."
! 
!     (vector (nnir-group-full-name group server)
! 	    (if (string= server "nnmaildir:")
! 		(nnmaildir-base-name-to-article-number
! 		 (substring article 0 (string-match ":" article))
! 		 group nil)
! 	      (string-to-int article))
! 	    (string-to-int score))))
  
  
  ;;; Search Engine Interfaces:
***************
*** 1223,1238 ****
                artno (file-name-nondirectory filenam)
                dirnam (file-name-directory filenam))
  
!         ;; don't match directories or inexistent/unreadable files
!         (when (and (string-match article-pattern artno)
! 		   (file-readable-p (concat prefix filenam)))
            (when (not (null dirnam))
  
  	    ;; maybe limit results to matching groups.
  	    (when (or (not groupspec)
  		      (string-match groupspec dirnam))
! 	      (push (nnir-compose-result dirnam artno score prefix server)
! 		    artlist)))))
  
        (message "Massaging swish++ output...done")
  
--- 1234,1247 ----
                artno (file-name-nondirectory filenam)
                dirnam (file-name-directory filenam))
  
!         ;; don't match directories
!         (when (string-match article-pattern artno)
            (when (not (null dirnam))
  
  	    ;; maybe limit results to matching groups.
  	    (when (or (not groupspec)
  		      (string-match groupspec dirnam))
! 	      (nnir-add-result dirnam artno score prefix server artlist)))))
  
        (message "Massaging swish++ output...done")
  
***************
*** 1480,1487 ****
          ;; make sure article and group is sane
          (when (and (string-match article-pattern article)
                     (not (null group)))
! 	  (push (nnir-compose-result group article score prefix server)
! 		artlist)))
  
        ;; sort artlist by score
        (apply 'vector
--- 1489,1495 ----
          ;; make sure article and group is sane
          (when (and (string-match article-pattern article)
                     (not (null group)))
! 	  (nnir-add-result group article score prefix server artlist)))
  
        ;; sort artlist by score
        (apply 'vector

Re: nnir.el swish++ interface broken

[Christoph Conrad]

Hi Justus,

> Attached. I also updated the complete file on my Web server.

Again: tested on Windows XP, swish++ 5.0, and works. This evening
another test on Ubuntu 7.10 with a newer swish++, will report.

With kind regards,
Christoph

Re: nnir.el swish++ interface broken

[Christoph Conrad]

> This evening another test on Ubuntu 7.10 with a newer swish++, will
> report.

Works also with Ubuntu 7.10, swish++ 6.1.4.

With kind regards,
Christoph

Re: nnir.el swish++ interface broken

[Reiner Steib]

On Thu, Apr 17 2008, Justus-bulk@Piater.name wrote:

> Reiner Steib <reinersteib+gmane@imap.cc> wrote:
>> This isn't based on the current CVS version.  Could you please provide
>> a diff against current CVS?
>
> Attached. 

Thanks, installed.  Next time, please also provide a ChangeLog entry.

BTW...

    (setq dirnam
	  (substring dirnam 0 (if (string= server "nnmaildir:") -5 -1)))

    ;; eliminate all ".", "/", "\" from beginning. Always matches.
    (string-match "^[./\\]*\\(.*\\)$" dirnam)
    (setq group (substitute ?. ?/ (match-string 1 dirnam))) ;; "/" -> "."
    (setq group (substitute ?. ?\\ group)) ;; "\\" -> "."

I'm not sure what values of dirnam and group need to be considered.
Maybe using `gnus-replace-in-string' could simplify this code?

Additionally, `substitute' is a function from the `cl' package.  While
it is okay to use cl macros at compile time[1], using cl at runtime.

> I also updated the complete file on my Web server.

I'd suggest not to distribute Gnus files on your Web server.  

Bye, Reiner.

[1] (eval-when-compile (require 'cl))
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/

nnir.el cleanup (was Re: nnir.el swish++ interface broken)

[Justus]

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

Reiner Steib <reinersteib+gmane@imap.cc> wrote on Sat, 19 Apr 2008
19:36:57 +0200:

>     ;; eliminate all ".", "/", "\" from beginning. Always matches.
>     (string-match "^[./\\]*\\(.*\\)$" dirnam)
>     (setq group (substitute ?. ?/ (match-string 1 dirnam))) ;; "/" -> "."
>     (setq group (substitute ?. ?\\ group)) ;; "\\" -> "."
>
> I'm not sure what values of dirnam and group need to be considered.
> Maybe using `gnus-replace-in-string' could simplify this code?
>
> Additionally, `substitute' is a function from the `cl' package.  While
> it is okay to use cl macros at compile time[1], using cl at runtime.

This is not my code; I just moved it around during restructuring. But
how about the attached patch and the following change-log entry?

(nnir-compose-result): use `gnus-replace-in-string' instead of
`string-match', `match-string' and `substitute'

Christoph, can you verify that it still works for you? The changed
lines are irrelevant to the nnmaildir backend. You'll have to apply
the patch though; no more easy downloading of the full file :-):

>> I also updated the complete file on my Web server.
>
> I'd suggest not to distribute Gnus files on your Web server.  

(Of course, this was never intended to be a "distribution", just to
lower the barrier for testers.)

Justus


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: nnir.el.diff --]
[-- Type: text/x-diff, Size: 977 bytes --]

Index: nnir.el
===================================================================
RCS file: /usr/local/cvsroot/gnus/contrib/nnir.el,v
retrieving revision 7.28
diff -u -r7.28 nnir.el
--- nnir.el	19 Apr 2008 17:19:12 -0000	7.28
+++ nnir.el	20 Apr 2008 07:35:24 -0000
@@ -880,10 +880,11 @@
     (setq dirnam
 	  (substring dirnam 0 (if (string= server "nnmaildir:") -5 -1)))
 
-    ;; eliminate all ".", "/", "\" from beginning. Always matches.
-    (string-match "^[./\\]*\\(.*\\)$" dirnam)
-    (setq group (substitute ?. ?/ (match-string 1 dirnam))) ;; "/" -> "."
-    (setq group (substitute ?. ?\\ group)) ;; "\\" -> "."
+    ;; Set group to dirnam without any leading dots or slashes,
+    ;; and with all subsequent slashes replaced by dots
+    (setq group (gnus-replace-in-string
+                 (gnus-replace-in-string dirnam "^[./\\]" "" t)
+                 "[/\\]" "." t))
 
     (vector (nnir-group-full-name group server)
 	    (if (string= server "nnmaildir:")

Re: nnir.el cleanup

[Reiner Steib]

On Sun, Apr 20 2008, Justus@piater.name wrote:

> Reiner Steib <reinersteib+gmane@imap.cc> wrote on Sat, 19 Apr 2008:
>> Additionally, `substitute' is a function from the `cl' package.  While
>> it is okay to use cl macros at compile time[1], using cl at runtime.
>
> This is not my code; I just moved it around during restructuring. 

Sorry, I didn't notice.

> But how about the attached patch and the following change-log entry?

Thanks.  Unless someone points out a problem, I'll install it.

>> I'd suggest not to distribute Gnus files on your Web server.  
>
> (Of course, this was never intended to be a "distribution", 

Sure.  But people tend to pick up files outdated files from random
places which often leads to problems and strange bug reports.

> just to lower the barrier for testers.)

It raises the barrier for committers, though, unless you provide a
diff as well.  ;-)

Could someone look into the compiler warnings, please?  I don't
understand what the warnings about the arguments are about.

,----[ M-x emacs-lisp-byte-compile RET ]
| Compiling file [...]/contrib/nnir.el at Sun Apr 20 12:04:10 2008
| 
| In nnir-request-group:
| nnir.el:730:9:Warning: function nnir-request-group used to take 0+ arguments,
|     now takes 1-3
| 
| In nnir-retrieve-headers:
| nnir.el:760:9:Warning: function nnir-retrieve-headers used to take 0+
|     arguments, now takes 1-4
| 
| In nnir-request-article:
| nnir.el:828:9:Warning: function nnir-request-article used to take 0+
|     arguments, now takes 1-4
| 
| In nnir-compose-result:
| nnir.el:886:35:Warning: assignment to free variable `group'
| nnir.el:890:18:Warning: reference to free variable `group'
| 
| In nnir-run-swish++:
| nnir.el:1231:45:Warning: assignment to free variable `filenam'
| nnir.el:1232:43:Warning: reference to free variable `filenam'
| 
| In nnir-run-namazu:
| nnir.el:1489:28:Warning: reference to free variable `dirnam'
| nnir.el:1489:28:Warning: reference to free variable `artno'
`----

(Line numbers may differ due to local modifications.)

Bye, Reiner
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/

Re: nnir.el cleanup (was Re: nnir.el swish++ interface broken)

[Christoph Conrad]

Hi Justus,

> This is not my code; I just moved it around during restructuring.

Originally it was by me, so it is also in the swish-e interface (i
didn't use for years).

> Christoph, can you verify that it still works for you?

Works for me, on GNU/Linux. Test on Windows follows tomorrow, please
wait til committing until then.

With kind regards,
Christoph

Re: nnir.el cleanup

[Christoph Conrad]

Hi Reiner,

> Could someone look into the compiler warnings, please? I don't
> understand what the warnings about the arguments are about.

With CVS Emacs i get mostly warnings, interestingly. Some mapcars are
only used for side-effect and can be replace by mapc. The 'reference to
free variable' are due moving code outside a context where the variables
where defined as parameter or in a `let'-binding. Simply make a new
`let' binding. I do not get any 'argument' warnings here.

With kind regards,
Christoph

Re: nnir.el cleanup

[Reiner Steib]

[ The following message is a courtesy copy of an article that has
  been posted to news:gmane.emacs.gnus.general as well. ]

On Sun, Apr 20 2008, Christoph Conrad wrote:

>> Could someone look into the compiler warnings, please? I don't
>> understand what the warnings about the arguments are about.
>
> With CVS Emacs i get mostly warnings, interestingly. Some mapcars are
> only used for side-effect and can be replace by mapc. 

As we want to move nnir.el to lisp/ in Gnus trunk, it will be synced
to Emacs 23 (trunk).  Thus, we should fix the warning that Emacs 23
produces. 

> The 'reference to free variable' are due moving code outside a
> context where the variables where defined as parameter or in a
> `let'-binding. Simply make a new `let' binding. I do not get any
> 'argument' warnings here.

I'm not sure that I understand what you have in mind.  Could you
provide a patch that fixes/silences the Emacs 23 compiler warnings?

Bye, Reiner.
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/

Re: nnir.el cleanup (was Re: nnir.el swish++ interface broken)

[Christoph Conrad]

Hi Justus,

> Christoph, can you verify that it still works for you?

Works also on Windows with swish++ 5.

Best regards,
Christoph

Re: nnir.el cleanup

[Christoph Conrad]

Hi Reiner,

> I'm not sure that I understand what you have in mind. Could you
> provide a patch that fixes/silences the Emacs 23 compiler warnings?

See below. One warning remains, i don't understand why.

In nnir-run-namazu:
nnir.el:1475:28:Warning: reference to free variable `dirnam'
nnir.el:1475:28:Warning: reference to free variable `artno'

Possibly it has to do with the usage of nnir-add-result, a macro, but i
don't understand why.

Changelog entry proposal:
Fixed byte compiler warnings, deleted unnecessary code.

I also deleted superfluous settings of variables to nil in a let binding.

I also see some suspicious and potentially dangerous code: the
nnir-run-* functions not supporting the &optional group all define a
local group variable in a let binding with default value nil which
occlude the variable of the parameter list. This is IMHO unnecessary
cause they throw an error at the beginning of the same function when the
group parameter is not nil.

With kind regards,
Christoph

===================================================================
RCS file: /usr/local/cvsroot/gnus/contrib/nnir.el,v
retrieving revision 7.30
diff -u -b -B -w -d -r7.30 nnir.el
--- nnir.el	21 Apr 2008 16:37:45 -0000	7.30
+++ nnir.el	22 Apr 2008 17:47:50 -0000
@@ -760,15 +760,8 @@
 (deffoo nnir-retrieve-headers (articles &optional group server fetch-old)
   (save-excursion
     (let ((artlist (copy-sequence articles))
-          (art nil)
-          (artitem nil)
-          (artgroup nil) (artno nil)
-          (artrsv nil)
-          (artfullgroup nil)
-          (novitem nil)
-          (novdata nil)
-          (foo nil)
-	  server)
+          art artitem artgroup artno artrsv artfullgroup
+          novitem novdata foo server)
       (while (not (null artlist))
         (setq art (car artlist))
         (or (numberp art)
@@ -822,7 +815,7 @@
         (setq artlist (cdr artlist)))
       (setq novdata (nreverse novdata))
       (set-buffer nntp-server-buffer) (erase-buffer)
-      (mapcar 'nnheader-insert-nov novdata)
+      (mapc 'nnheader-insert-nov novdata)
       'nov)))
 
 (deffoo nnir-request-article (article
@@ -879,9 +872,9 @@
 
     ;; Set group to dirnam without any leading dots or slashes,
     ;; and with all subsequent slashes replaced by dots
-    (setq group (gnus-replace-in-string
+    (let ((group (gnus-replace-in-string
                  (gnus-replace-in-string dirnam "^[./\\]" "" t)
-                 "[/\\]" "." t))
+                 "[/\\]" "." t)))
 
     (vector (nnir-group-full-name group server)
 	    (if (string= server "nnmaildir:")
@@ -889,7 +882,7 @@
 		 (substring article 0 (string-match ":" article))
 		 group nil)
 	      (string-to-number article))
-	    (string-to-number score))))
+              (string-to-number score)))))
 
 ;;; Search Engine Interfaces:
 
@@ -902,8 +895,7 @@
   (save-excursion
     (let ((qstring (cdr (assq 'query query)))
 	  (prefix (nnir-read-server-parm 'nnir-wais-remove-prefix server))
-          (artlist nil)
-          (score nil) (artno nil) (dirnam nil) (group nil))
+          artlist score artno dirnam group)
       (set-buffer (get-buffer-create nnir-tmp-buffer))
       (erase-buffer)
       (message "Doing WAIS query %s..." query)
@@ -971,7 +963,7 @@
             (let ((arts 0)
                   (mbx (gnus-group-real-name group)))
               (when (imap-mailbox-select mbx nil buf)
-                (mapcar
+                (mapc
                  (lambda (artnum)
                    (push (vector group artnum 1) artlist)
                    (setq arts (1+ arts)))
@@ -1176,7 +1168,7 @@
     (let ( (qstring (cdr (assq 'query query)))
 	   (groupspec (cdr (assq 'group query)))
 	   (prefix (nnir-read-server-parm 'nnir-swish++-remove-prefix server))
-           (artlist nil)
+           artlist
 	   ;; nnml-use-compressed-files might be any string, but probably this
 	   ;; is sufficient.  Note that we can't only use the value of
 	   ;; nnml-use-compressed-files because old articles might have been
@@ -1184,7 +1176,7 @@
 	   (article-pattern (if (string= server "nnmaildir:")
 				":[0-9]+"
 			      "^[0-9]+\\(\\.[a-z0-9]+\\)?$"))
-           (score nil) (artno nil) (dirnam nil) (group nil) )
+           score artno dirnam group filenam )
 
       (when (equal "" qstring)
         (error "swish++: You didn't enter anything."))
@@ -1266,8 +1258,7 @@
 	  (prefix
 	   (or (nnir-read-server-parm 'nnir-swish-e-remove-prefix server)
 	       (error "Missing parameter `nnir-swish-e-remove-prefix'")))
-	  (artlist nil)
-	  (score nil) (artno nil) (dirnam nil) (group nil) )
+          artlist score artno dirnam group )
 
       (when (equal "" qstring)
         (error "swish-e: You didn't enter anything."))
@@ -1431,18 +1422,13 @@
   (when group
     (error "The Namazu backend cannot search specific groups"))
   (save-excursion
-    (let (
-	  (article-pattern (if (string= server "nnmaildir:")
+    (let ((article-pattern (if (string= server "nnmaildir:")
 			       ":[0-9]+"
 			     "^[0-9]+$"))
-          (artlist nil)
-          (qstring (cdr (assq 'query query)))
+          artlist (qstring (cdr (assq 'query query)))
 	  (prefix (nnir-read-server-parm 'nnir-namazu-remove-prefix server))
-          (score nil)
-          (group nil)
-          (article nil)
-	  (process-environment (copy-sequence process-environment))
-          )
+          score group article
+          (process-environment (copy-sequence process-environment)))
       (setenv "LC_MESSAGES" "C")
       (set-buffer (get-buffer-create nnir-tmp-buffer))
       (erase-buffer)
@@ -1672,7 +1658,7 @@
     ;; from each artitem, extract group component
     (setq with-dups (mapcar 'nnir-artitem-group artlist))
     ;; remove duplicates from above
-    (mapcar (function (lambda (x) (add-to-list 'res x)))
+    (mapc (function (lambda (x) (add-to-list 'res x)))
             with-dups)
     res))
 

Re: nnir.el cleanup

[Reiner Steib]

On Tue, Apr 22 2008, Christoph Conrad wrote:

>> Could you provide a patch that fixes/silences the Emacs 23 compiler
>> warnings?
[...]
> Changelog entry proposal:
> Fixed byte compiler warnings, deleted unnecessary code.
>
> I also deleted superfluous settings of variables to nil in a let binding.

Thanks.  I've committed it with a more verbose ChangeLog entry.

> I also see some suspicious and potentially dangerous code: the
> nnir-run-* functions not supporting the &optional group all define a
> local group variable in a let binding with default value nil which
> occlude the variable of the parameter list. This is IMHO unnecessary
> cause they throw an error at the beginning of the same function when the
> group parameter is not nil.

I see that it is unnecessary, but could you elaborate why it could be
dangerous and/or provide a patch?

Or is this sufficient?

--8<---------------cut here---------------start------------->8---
--- nnir.el	24 Apr 2008 19:36:35 +0200	7.31
+++ nnir.el	24 Apr 2008 20:05:38 +0200	
@@ -895,7 +895,7 @@
   (save-excursion
     (let ((qstring (cdr (assq 'query query)))
 	  (prefix (nnir-read-server-parm 'nnir-wais-remove-prefix server))
-          artlist score artno dirnam group)
+          artlist score artno dirnam)
       (set-buffer (get-buffer-create nnir-tmp-buffer))
       (erase-buffer)
       (message "Doing WAIS query %s..." query)
@@ -1176,7 +1176,7 @@
 	   (article-pattern (if (string= server "nnmaildir:")
 				":[0-9]+"
 			      "^[0-9]+\\(\\.[a-z0-9]+\\)?$"))
-           score artno dirnam group filenam )
+           score artno dirnam filenam)
 
       (when (equal "" qstring)
         (error "swish++: You didn't enter anything."))
--8<---------------cut here---------------end--------------->8---

Bye, Reiner.
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/

Compiler warnings in nnir.el (was: nnir.el cleanup)

[Reiner Steib]

[ Cc-ing emacs-devel ]

On Tue, Apr 22 2008, Christoph Conrad wrote:

>> Could you provide a patch that fixes/silences the Emacs 23 compiler
>> warnings?
> [...] One warning remains, i don't understand why.
>
> In nnir-run-namazu:
> nnir.el:1475:28:Warning: reference to free variable `dirnam'
> nnir.el:1475:28:Warning: reference to free variable `artno'
>
> Possibly it has to do with the usage of nnir-add-result, a macro, but i
> don't understand why.

Here's a simplified test case of the problem in contrib/nnir.el[1].
Maybe someone on emacs-devel can explain whether we should silence the
compiler warnings, modify the code or if the byte-compiler should be
improved.

--8<---------------cut here---------------start------------->8---
(defmacro nnir-add-result-dummy (dirnam artno score prefix server artlist)
  "Ask `nnir-compose-result' to construct a result vector, 
and if it is non-nil, add it to artlist."
  `(let ((result (nnir-compose-result-dummy dirnam artno score prefix server)))
     (when (not (null result))
       (push result artlist))))

(defun nnir-compose-result-dummy (dirnam article score prefix server)
  t)

(defun nnir-run-namazu-dummy (query server &optional group)
  (let ((article-pattern "^[0-9]+$")
	artlist
	(qstring (cdr (assq 'query query)))
	(prefix "nnfoobar")
	score group article)
    (nnir-add-result-dummy group article score prefix server artlist)))
--8<---------------cut here---------------end--------------->8---

In Emacs 22.2.50, I get:

,----
| Compiling file nnir-warnings.el at Thu Apr 24 20:08:09 2008
| 
| In nnir-run-namazu-dummy:
| nnir-warnings.el:12:10:Warning: reference to free variable `dirnam'
| nnir-warnings.el:12:10:Warning: reference to free variable `artno'
`----

Bye, Reiner.

[1] From contrib/ in the Gnus repository.  nnir.el is not included in
    Emacs yet, since we wait for an assignment.
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/

Re: nnir.el cleanup

[Christoph Conrad]

Hi Reiner,

> Thanks. I've committed it with a more verbose ChangeLog entry.

Ok, now i understand what you mean by "verbose" :-) Next time my
changelog proposal will be better.

>> I also see some suspicious and potentially dangerous code: the
> I see that it is unnecessary, but could you elaborate why it could be
> dangerous and/or provide a patch?

I exaggerated a little bit. But generally it's not good style to do
that. If you overlook the local re-binding and make changes supporting
groups, they will have no effect. I am too lazy for supplying a patch.
If you are patching the next time, change it if you are in the mood for.

> Or is this sufficient?

It does not catch all places. Also some of the other nnir-run* have the
same shadowing of the group parameter by a local let binding, but emit
an error before the let binding when group is not nil.

With kind regards,
Christoph

Re: Compiler warnings in nnir.el

[Stefan Monnier]

> (defmacro nnir-add-result-dummy (dirnam artno score prefix server artlist)
>   "Ask `nnir-compose-result' to construct a result vector, 
> and if it is non-nil, add it to artlist."
>   `(let ((result (nnir-compose-result-dummy dirnam artno score prefix server)))
>      (when (not (null result))
>        (push result artlist))))

This looks very strange.  I guess there are missing commas in front of
dirnam, artno, score, prefix, and server in the call to
nnir-compose-result-dummy.


        Stefan