bad fixes to a higher level problem

bad fixes to a higher level problem

[Wes Hardaker]

So, I've had odd problems that aren't reproducible when entering a
group.  Random nil values being passed around where they shouldn't be.
I have a sneaking suspicion it's related to my use of the
gnus-summary-display-while-building variable (which I admittedly
wrote).  I'm not sure other people are using it, and the only reason I
think it might be related is because it was about the same time that I
think the problem started happening (a loooong time ago).

Upon entering a summary buffer (nnimap backend), it would occasionally
crash.  Killing the incomplete summary buffer and re-entering
immediately again and it won't crash.  That's what's really wierd.

I finally debugged the crashes enough to create a few patches to
gnus-sum.el which have merely moved the crashes to different places
every time i still see them.  Thus, I suspect something higher up is
going wrong somewhere and stuff is getting handed downward in the
stack that shouldn't.  Thoughts?

[line numbers may be off because there are other font-related patches
of mine in the same file]

@@ -10725,6 +10725,7 @@
     ;; Go forward until either the buffer ends or the subthread
     ;; ends.
     (when (and (not (eobp))
+              article
               (or (zerop (gnus-summary-next-thread 1 t))
                   (goto-char (point-max))))
       (prog1
@@ -10740,12 +10741,14 @@
   "Go to the same level (or less) next thread.
 If PREVIOUS is non-nil, go to previous thread instead.
 Return the article number moved to, or nil if moving was impossible."
-  (let ((level (gnus-summary-thread-level))
+  (let ((level (or (gnus-summary-thread-level) 0))
        (way (if previous -1 1))
-       (beg (point)))
+       (beg (point))
+       thlevel)
     (forward-line way)
     (while (and (not (eobp))
-               (< level (gnus-summary-thread-level)))
+               (setq thlevel (gnus-summary-thread-level))
+               (< level thlevel))
       (forward-line way))
     (if (eobp)
        (progn


-- 
"In the bathtub of history the truth is harder to hold than the soap,
 and much more difficult to find."  -- Terry Pratchett

Re: bad fixes to a higher level problem

[drkm]

Wes Hardaker <wes@hardakers.net> writes:

> @@ -10740,12 +10741,14 @@
>    "Go to the same level (or less) next thread.
>  If PREVIOUS is non-nil, go to previous thread instead.
>  Return the article number moved to, or nil if moving was impossible."
> -  (let ((level (gnus-summary-thread-level))
> +  (let ((level (or (gnus-summary-thread-level) 0))
>         (way (if previous -1 1))
> -       (beg (point)))
> +       (beg (point))
> +       thlevel)
>      (forward-line way)
>      (while (and (not (eobp))
> -               (< level (gnus-summary-thread-level)))
> +               (setq thlevel (gnus-summary-thread-level))
> +               (< level thlevel))
>        (forward-line way))
>      (if (eobp)
>         (progn

  Why introduce `thlevel' if you don't use it ?  Are there other
reference to this variable, not shown in the patch ?

--drkm

Re: bad fixes to a higher level problem

[Wes Hardaker]

>>>>> On Thu, 30 Dec 2004 22:43:12 +0100, drkm <darkman_spam@yahoo.fr> said:

>> (while (and (not (eobp))
>> -               (< level (gnus-summary-thread-level)))
>> +               (setq thlevel (gnus-summary-thread-level))
>> +               (< level thlevel))

drkm> Why introduce `thlevel' if you don't use it ?  Are there other
drkm> reference to this variable, not shown in the patch ?

Welcome to confusing elisp 101.  Elisp is naturally obfuscated,
fortunately, so I don't have to work hard to make it that way.

The (setq thlevel) exists within the and statement, which means the
results of the setq is evaluated before the < operation is called.  It
was the < operation that was causing problems with a nil argument.  I
had 2 choices: use a temporary variable to store the results (thlevel)
or call (gnus-summary-thread-level) twice, once in the and to check
that it was !nil and once in the <.  I suppose, actually, it would
have been better to do something like:

  (< level (or (gnus-summary-thread-level) 0))

That would have dropped the need for the temp var.

-- 
"In the bathtub of history the truth is harder to hold than the soap,
 and much more difficult to find."  -- Terry Pratchett

Re: bad fixes to a higher level problem

[drkm]

Wes Hardaker <wes@hardakers.net> writes:

> The (setq thlevel) exists within the and statement

  Sure!  In the patch context, I didn't pay attention to that.  Tanks.

--drkm