GNU bug report logs -
#38011
27.0.50; [PATCH] WIP on allowing Gnus backends to return header data directly
Previous Next
To reply to this bug, email your comments to 38011 AT debbugs.gnu.org.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#38011
; Package
emacs
.
(Thu, 31 Oct 2019 21:35:02 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Eric Abrahamsen <eric <at> ericabrahamsen.net>
:
New bug report received and forwarded. Copy sent to
bug-gnu-emacs <at> gnu.org
.
(Thu, 31 Oct 2019 21:35:02 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Right now, Gnus backends return article header data by writing it in a
parseable format into the `nntp-server-buffer', and returning one of the
symbols 'nov or 'headers, indicating how the data should be parsed.
This isn't great because it requires backends to first munge their data
into a format that looks like the NNTP format, which is then parsed
again, which is an extra layer of data transformation. It also makes use
of the `nntp-server-buffer', which is something I'd like to work on
reducing because it causes problems with threading and introduces
potential encoding bugs.
This patch provides the possibility for backends to return their own
headers (ie a list of vectors), though it doesn't actually change any of
the backends to do that -- that will be another patch.
I have one question at this stage: the 'nov or 'headers value gets
stored into the `gnus-headers-retrieved-by' variable. That variable is
later checked in a couple of places like so:
(when (and gnus-fetch-old-headers
(eq gnus-headers-retrieved-by 'nov))
(if (eq gnus-fetch-old-headers 'invisible)
(gnus-build-all-threads)
(gnus-build-old-threads)))
If the variable is 'headers, the `gnus-build-*-threads' functions don't
get called at all.
What's the difference between 'nov and 'headers, and why can we build
threads in one case and not the other? If backends were to return their
own headers, what value should they return? I'll also note that the nnir
version of this function returns 'nov regardless of what the "real"
backend function returned -- why is that?
I would love to use some other, more direct, heuristic to decide about
building threads or not, and get rid of the
'nov/'headers/gnus-headers-retrieved-by stuff altogether, but I don't
yet know how to do that.
Eric
[0001-WIP-on-allowing-Gnus-backends-to-return-headers-dire.patch (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#38011
; Package
emacs
.
(Fri, 01 Nov 2019 14:13:02 GMT)
Full text and
rfc822 format available.
Message #8 received at submit <at> debbugs.gnu.org (full text, mbox):
Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
> This patch provides the possibility for backends to return their own
> headers (ie a list of vectors), though it doesn't actually change any of
> the backends to do that -- that will be another patch.
Great!
> I have one question at this stage: the 'nov or 'headers value gets
> stored into the `gnus-headers-retrieved-by' variable. That variable is
> later checked in a couple of places like so:
>
> (when (and gnus-fetch-old-headers
> (eq gnus-headers-retrieved-by 'nov))
> (if (eq gnus-fetch-old-headers 'invisible)
> (gnus-build-all-threads)
> (gnus-build-old-threads)))
>
> If the variable is 'headers, the `gnus-build-*-threads' functions don't
> get called at all.
>
> What's the difference between 'nov and 'headers, and why can we build
> threads in one case and not the other? If backends were to return their
> own headers, what value should they return?
It's not about threading per se, but about displaying information about
already-read articles (or more precisely -- about articles not in the
set that was requested). Threads are build no matter how the backend
delivers the data.
If nn-*retrieve-headers supports NOV fetching, then certain Gnus
variables about filling in threads with "old" articles is switched on,
because fetching extra NOV headers is fast (you just say "fetch 100-150"
instead of "fetch 100-110,120-130,150"). With the backends that fetch
head by head, this is slow, so it's not done.
It's basically a distinction between NNTP and almost all the other
backends.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#38011
; Package
emacs
.
(Fri, 01 Nov 2019 18:42:01 GMT)
Full text and
rfc822 format available.
Message #11 received at 38011 <at> debbugs.gnu.org (full text, mbox):
Lars Ingebrigtsen <larsi <at> gnus.org> writes:
> Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
>
>> This patch provides the possibility for backends to return their own
>> headers (ie a list of vectors), though it doesn't actually change any of
>> the backends to do that -- that will be another patch.
>
> Great!
>
>> I have one question at this stage: the 'nov or 'headers value gets
>> stored into the `gnus-headers-retrieved-by' variable. That variable is
>> later checked in a couple of places like so:
>>
>> (when (and gnus-fetch-old-headers
>> (eq gnus-headers-retrieved-by 'nov))
>> (if (eq gnus-fetch-old-headers 'invisible)
>> (gnus-build-all-threads)
>> (gnus-build-old-threads)))
>>
>> If the variable is 'headers, the `gnus-build-*-threads' functions don't
>> get called at all.
>>
>> What's the difference between 'nov and 'headers, and why can we build
>> threads in one case and not the other? If backends were to return their
>> own headers, what value should they return?
>
> It's not about threading per se, but about displaying information about
> already-read articles (or more precisely -- about articles not in the
> set that was requested). Threads are build no matter how the backend
> delivers the data.
>
> If nn-*retrieve-headers supports NOV fetching, then certain Gnus
> variables about filling in threads with "old" articles is switched on,
> because fetching extra NOV headers is fast (you just say "fetch 100-150"
> instead of "fetch 100-110,120-130,150"). With the backends that fetch
> head by head, this is slow, so it's not done.
>
> It's basically a distinction between NNTP and almost all the other
> backends.
Okay, that makes sense. So it's basically a flag saying header retrieval
is cheap enough that we might as well pull in more old messages than we
otherwise would. For example, nnmaildir builds and stores nov data
(which drives people with large maildirs insane because it takes
enormous amounts of time and space), so it returns 'nov, but if we ever
got rid of that (or made it optional) it would return 'headers.
I'm going to assume that any backend capable of returning its own
headers is probably going to... return 'headers. At any rate, as I
implement this for various backends, I'll start with the ones that
return 'headers to begin with. At some point, though, I'd still like to
get rid of this flag and build the distinction into the backend
functions themselves.
Hmmm... nnmaildir needs some love.
Eric
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#38011
; Package
emacs
.
(Fri, 01 Nov 2019 20:53:01 GMT)
Full text and
rfc822 format available.
Message #14 received at 38011 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
> Lars Ingebrigtsen <larsi <at> gnus.org> writes:
>
>> Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
>>
>>> This patch provides the possibility for backends to return their own
>>> headers (ie a list of vectors), though it doesn't actually change any of
>>> the backends to do that -- that will be another patch.
>>
>> Great!
>>
>>> I have one question at this stage: the 'nov or 'headers value gets
>>> stored into the `gnus-headers-retrieved-by' variable. That variable is
>>> later checked in a couple of places like so:
Okay, here's the patch as it stands -- I'll do some testing first. Andy,
I'm copying you in because this touches nnir.el, and nnselect.el will
need to be edited accordingly.
[0001-WIP-on-allowing-Gnus-backends-to-return-headers-dire.patch (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#38011
; Package
emacs
.
(Sat, 02 Nov 2019 14:50:01 GMT)
Full text and
rfc822 format available.
Message #17 received at 38011 <at> debbugs.gnu.org (full text, mbox):
Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
> Okay, that makes sense. So it's basically a flag saying header retrieval
> is cheap enough that we might as well pull in more old messages than we
> otherwise would.
Yup.
> Hmmm... nnmaildir needs some love.
Indeed.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#38011
; Package
emacs
.
(Thu, 07 Nov 2019 23:22:01 GMT)
Full text and
rfc822 format available.
Message #20 received at 38011 <at> debbugs.gnu.org (full text, mbox):
I knew this seemed too simple...
Andy Cohen pointed out that `gnus-get-newsgroup-headers{,-xover}' were
doing more work (running hooks, and the alter function) that needed to
also be done if headers were returned directly. There's a bunch of other
overlap between those two functions (and between
`gnus-get-newsgroup-headers' and `nnheader-parse-naked-head') that Andy
is working on refactoring (I hope).
In the meantime there are several other places in the code that use
`gnus-retrieve-headers', and will need to be updated to handle the
possibility that headers have been returned directly. What I'd really
like to do, as much as possible, is to switch to `gnus-fetch-headers',
so we get our headers back and don't need to know how that happened.
This will also avoid ugliness like `nnvirtual-convert-headers'.
I think I can probably figure out how to do this in most spots.
gnus-agent and gnus-cache are particularly gnarly, though: what does
-braid-nov/-braid-heads actually do?
Eric
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#38011
; Package
emacs
.
(Fri, 08 Nov 2019 21:04:02 GMT)
Full text and
rfc822 format available.
Message #23 received at 38011 <at> debbugs.gnu.org (full text, mbox):
Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
> I think I can probably figure out how to do this in most spots.
> gnus-agent and gnus-cache are particularly gnarly, though: what does
> -braid-nov/-braid-heads actually do?
Take two sets of nov/head headers and combine them. Basically sort |
uniq, but they're already sorted.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#38011
; Package
emacs
.
(Fri, 08 Nov 2019 21:44:02 GMT)
Full text and
rfc822 format available.
Message #26 received at 38011 <at> debbugs.gnu.org (full text, mbox):
On 11/08/19 22:03 PM, Lars Ingebrigtsen wrote:
> Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
>
>> I think I can probably figure out how to do this in most spots.
>> gnus-agent and gnus-cache are particularly gnarly, though: what does
>> -braid-nov/-braid-heads actually do?
>
> Take two sets of nov/head headers and combine them. Basically sort |
> uniq, but they're already sorted.
Okay good -- that should be as easy (easier) to do with an actual list
of headers than with text in a buffer.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#38011
; Package
emacs
.
(Fri, 08 Nov 2019 21:59:01 GMT)
Full text and
rfc822 format available.
Message #29 received at 38011 <at> debbugs.gnu.org (full text, mbox):
Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
> Okay good -- that should be as easy (easier) to do with an actual list
> of headers than with text in a buffer.
Yup; much easier.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#38011
; Package
emacs
.
(Sun, 29 Mar 2020 19:51:01 GMT)
Full text and
rfc822 format available.
Message #32 received at 38011 <at> debbugs.gnu.org (full text, mbox):
Lars Ingebrigtsen <larsi <at> gnus.org> writes:
> Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
>
>> Okay good -- that should be as easy (easier) to do with an actual list
>> of headers than with text in a buffer.
>
> Yup; much easier.
Okay I've had a little more, uh, free time recently to work on Gnus
stuff, and am nearly ready with a patch for this. It got gnarly in
gnus-agent.el, but I think I've got a handle on it.
I have one question right now, about `nnvirtual-update-xref-header',
which needs to be rewritten from altering header text in a buffer, to
altering the `mail-header-xref' value of an already-parsed header.
First of all, it unconditionally adds an xref header to our group and
article, even if there wasn't one before. Then it does the following.
I'm not sure how to describe this in plain English. I would say it is
replacing all the server names with our "prefix", but that doesn't
explain the deletion of the group name *and* the article number in the
second "(when (re-search-forward" below.
Can someone explain what exactly this function is supposed to do?
(save-restriction
(narrow-to-region (point)
(or (search-forward "\t" (point-at-eol) t)
(point-at-eol)))
(goto-char (point-min))
(when (re-search-forward "Xref: *[^\n:0-9 ]+ *" nil t)
(replace-match "" t t))
(goto-char (point-min))
(when (re-search-forward
(concat (regexp-quote (gnus-group-real-name group)) ":[0-9]+")
nil t)
(replace-match "" t t))
(unless (eobp)
(insert " ")
(when (not (string= "" prefix))
(while (re-search-forward "[^ ]+:[0-9]+" nil t)
(save-excursion
(goto-char (match-beginning 0))
(insert prefix))))))
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#38011
; Package
emacs
.
(Thu, 30 Apr 2020 04:52:02 GMT)
Full text and
rfc822 format available.
Message #35 received at 38011 <at> debbugs.gnu.org (full text, mbox):
Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
> Can someone explain what exactly this function is supposed to do?
Somebody should have written more comments when they wrote that... and,
like, made it correct.
It transforms this:
64007 Formatting labels used for URL buttons in Gnus articles Narendra Joshi <narendraj9 <at> gmail.com> Sun, 19 Apr 2020 00:31:13 +0200 <mailman.633.1587249088.3066.help-gnu-emacs <at> gnu.org> <87eesk2zpq.fsf <at> gmail.com> 3614 14 Xref: reader01.eternal-september.org gnu.emacs.help:57603
Into this:
64007 Formatting labels used for URL buttons in Gnus articles Narendra Joshi <narendraj9 <at> gmail.com> Sun, 19 Apr 2020 00:31:13 +0200 <mailman.633.1587249088.3066.help-gnu-emacs <at> gnu.org> <87eesk2zpq.fsf <at> gmail.com> 3614 14 Xref: marnie gnu.emacs.help:57603 01.eternal-september.org
Which is wrong, of course -- the "01.eternal-september.org" thing
shouldn't be there.
Anyway, what it's supposed to do it rewrite
Xref: reader01.eternal-september.org foo.bar:2523 gnu.emacs.help:57603 zot.bar:3242
to
Xref: whatever gnu.emacs.help:57603 foo.bar:2523 zot.bar:3242
That is, put the group/article we're really selecting first in the Xref
header, but leave the remaining as they were. This is because we need
those to mark the article as read in those other groups, but we
primarily need to know where this article really came from (the first
entry).
Feel free to adapt this to comments in the code. :-) And rewrite to be
correct. I don't understand why it's doing all the regexp stuff
(wrongly) in the first place -- it should just split the data into a
list and then do its work...
Perhaps it was marginally faster to do it this way?
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#38011
; Package
emacs
.
(Sun, 27 Sep 2020 04:14:01 GMT)
Full text and
rfc822 format available.
Message #38 received at 38011 <at> debbugs.gnu.org (full text, mbox):
Lars Ingebrigtsen <larsi <at> gnus.org> writes:
> Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
>
>> Can someone explain what exactly this function is supposed to do?
>
> Somebody should have written more comments when they wrote that... and,
> like, made it correct.
>
> It transforms this:
>
> 64007 Formatting labels used for URL buttons in Gnus articles Narendra
> Joshi <narendraj9 <at> gmail.com> Sun, 19 Apr 2020 00:31:13 +0200
> <mailman.633.1587249088.3066.help-gnu-emacs <at> gnu.org>
> <87eesk2zpq.fsf <at> gmail.com> 3614 14 Xref:
> reader01.eternal-september.org gnu.emacs.help:57603
>
> Into this:
>
> 64007 Formatting labels used for URL buttons in Gnus articles Narendra
> Joshi <narendraj9 <at> gmail.com> Sun, 19 Apr 2020 00:31:13 +0200
> <mailman.633.1587249088.3066.help-gnu-emacs <at> gnu.org>
> <87eesk2zpq.fsf <at> gmail.com> 3614 14 Xref: marnie gnu.emacs.help:57603
> 01.eternal-september.org
>
> Which is wrong, of course -- the "01.eternal-september.org" thing
> shouldn't be there.
>
> Anyway, what it's supposed to do it rewrite
>
> Xref: reader01.eternal-september.org foo.bar:2523 gnu.emacs.help:57603 zot.bar:3242
>
> to
>
> Xref: whatever gnu.emacs.help:57603 foo.bar:2523 zot.bar:3242
>
> That is, put the group/article we're really selecting first in the Xref
> header, but leave the remaining as they were. This is because we need
> those to mark the article as read in those other groups, but we
> primarily need to know where this article really came from (the first
> entry).
Slowly, slowly, I'm getting this done. I'm still a bit confused here,
though. The xref elements look like they're not supposed to have spaces
in them, but the existing code does this:
(insert "Xref: " sysname " " group ":")
(princ article (current-buffer))
Which leaves a space between sysname and group.
You say the existing xrefs should be left as they are, but the code adds
"prefix" to them. Should this be added unconditionally?
Here's the new version of the function, operating on a header struct.
Does this look right to you?
Thanks,
Eric
(defun nnvirtual-update-xref-header (header group prefix sysname)
"Add xref to component GROUP to HEADER.
Also add a server PREFIX any existing xref lines."
(let ((bits (split-string (mail-header-xref header)
nil t "[[:blank:]]"))
(art-no (mail-header-number header)))
(setq bits
(mapcar (lambda (bit)
(concat prefix bit))
bits))
(setf (mail-header-xref header)
(mapconcat #'identity
(cons (format "%s %s:%d"
sysname group art-no)
bits)
" "))))
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#38011
; Package
emacs
.
(Sun, 27 Sep 2020 12:17:01 GMT)
Full text and
rfc822 format available.
Message #41 received at 38011 <at> debbugs.gnu.org (full text, mbox):
Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
>> Xref: reader01.eternal-september.org foo.bar:2523 gnu.emacs.help:57603 zot.bar:3242
[...]
> Slowly, slowly, I'm getting this done. I'm still a bit confused here,
> though. The xref elements look like they're not supposed to have spaces
> in them, but the existing code does this:
>
> (insert "Xref: " sysname " " group ":")
> (princ article (current-buffer))
>
> Which leaves a space between sysname and group.
I'm not quite sure I understand the question? The sysname is just a
part of the syntax of the Xref header and isn't used for anything by
Gnus, as far as I know. So there has to be a space? It's certainly not
part of the group name.
> You say the existing xrefs should be left as they are, but the code adds
> "prefix" to them. Should this be added unconditionally?
Uhm... I think so? But I'm not sure.
> Here's the new version of the function, operating on a header struct.
> Does this look right to you?
>
> Thanks,
> Eric
>
> (defun nnvirtual-update-xref-header (header group prefix sysname)
> "Add xref to component GROUP to HEADER.
> Also add a server PREFIX any existing xref lines."
> (let ((bits (split-string (mail-header-xref header)
> nil t "[[:blank:]]"))
> (art-no (mail-header-number header)))
> (setq bits
> (mapcar (lambda (bit)
> (concat prefix bit))
> bits))
> (setf (mail-header-xref header)
> (mapconcat #'identity
> (cons (format "%s %s:%d"
> sysname group art-no)
> bits)
> " "))))
I think so. The body of the let form is perhaps more easily expressed
as
(setf (mail-header-xref header)
(concat (format "%s %s:%d " sysname group art-no)
(mapconcat (lambda (bit)
(concat prefix bit))
bits " ")))
?
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#38011
; Package
emacs
.
(Sun, 27 Sep 2020 23:43:02 GMT)
Full text and
rfc822 format available.
Message #44 received at 38011 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Lars Ingebrigtsen <larsi <at> gnus.org> writes:
> Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
>
>>> Xref: reader01.eternal-september.org foo.bar:2523 gnu.emacs.help:57603 zot.bar:3242
>
> [...]
>
>> Slowly, slowly, I'm getting this done. I'm still a bit confused here,
>> though. The xref elements look like they're not supposed to have spaces
>> in them, but the existing code does this:
>>
>> (insert "Xref: " sysname " " group ":")
>> (princ article (current-buffer))
>>
>> Which leaves a space between sysname and group.
>
> I'm not quite sure I understand the question? The sysname is just a
> part of the syntax of the Xref header and isn't used for anything by
> Gnus, as far as I know. So there has to be a space? It's certainly not
> part of the group name.
TBH I only just went and read the RFC for this -- something I'd been
trying to avoid!
>> You say the existing xrefs should be left as they are, but the code adds
>> "prefix" to them. Should this be added unconditionally?
>
> Uhm... I think so? But I'm not sure.
Looking over the code again, I think it's best to only add if the prefix
isn't already there.
>> Here's the new version of the function, operating on a header struct.
>> Does this look right to you?
>>
>> Thanks,
>> Eric
>>
>> (defun nnvirtual-update-xref-header (header group prefix sysname)
>> "Add xref to component GROUP to HEADER.
>> Also add a server PREFIX any existing xref lines."
>> (let ((bits (split-string (mail-header-xref header)
>> nil t "[[:blank:]]"))
>> (art-no (mail-header-number header)))
>> (setq bits
>> (mapcar (lambda (bit)
>> (concat prefix bit))
>> bits))
>> (setf (mail-header-xref header)
>> (mapconcat #'identity
>> (cons (format "%s %s:%d"
>> sysname group art-no)
>> bits)
>> " "))))
>
> I think so. The body of the let form is perhaps more easily expressed
> as
>
> (setf (mail-header-xref header)
> (concat (format "%s %s:%d " sysname group art-no)
> (mapconcat (lambda (bit)
> (concat prefix bit))
> bits " ")))
>
> ?
Sure, this was just my halfway-there muddle.
I've cleaned this branch, squashed it, and am preparing to test for a
while. I'm attaching the full diff in case anyone wants to read it :)
A net removal of 562 lines with, I hope, no change in behavior.
[0001-Allow-gnus-retrieve-headers-to-return-headers-direct.patch (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#38011
; Package
emacs
.
(Sat, 02 Jan 2021 03:19:01 GMT)
Full text and
rfc822 format available.
Message #47 received at 38011 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
> Lars Ingebrigtsen <larsi <at> gnus.org> writes:
>
>> Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
>>
>>>> Xref: reader01.eternal-september.org foo.bar:2523 gnu.emacs.help:57603 zot.bar:3242
>>
>> [...]
>>
>>> Slowly, slowly, I'm getting this done. I'm still a bit confused here,
>>> though. The xref elements look like they're not supposed to have spaces
>>> in them, but the existing code does this:
>>>
>>> (insert "Xref: " sysname " " group ":")
>>> (princ article (current-buffer))
>>>
>>> Which leaves a space between sysname and group.
>>
>> I'm not quite sure I understand the question? The sysname is just a
>> part of the syntax of the Xref header and isn't used for anything by
>> Gnus, as far as I know. So there has to be a space? It's certainly not
>> part of the group name.
>
> TBH I only just went and read the RFC for this -- something I'd been
> trying to avoid!
>
>>> You say the existing xrefs should be left as they are, but the code adds
>>> "prefix" to them. Should this be added unconditionally?
>>
>> Uhm... I think so? But I'm not sure.
>
> Looking over the code again, I think it's best to only add if the prefix
> isn't already there.
>
>>> Here's the new version of the function, operating on a header struct.
>>> Does this look right to you?
>>>
>>> Thanks,
>>> Eric
>>>
>>> (defun nnvirtual-update-xref-header (header group prefix sysname)
>>> "Add xref to component GROUP to HEADER.
>>> Also add a server PREFIX any existing xref lines."
>>> (let ((bits (split-string (mail-header-xref header)
>>> nil t "[[:blank:]]"))
>>> (art-no (mail-header-number header)))
>>> (setq bits
>>> (mapcar (lambda (bit)
>>> (concat prefix bit))
>>> bits))
>>> (setf (mail-header-xref header)
>>> (mapconcat #'identity
>>> (cons (format "%s %s:%d"
>>> sysname group art-no)
>>> bits)
>>> " "))))
>>
>> I think so. The body of the let form is perhaps more easily expressed
>> as
>>
>> (setf (mail-header-xref header)
>> (concat (format "%s %s:%d " sysname group art-no)
>> (mapconcat (lambda (bit)
>> (concat prefix bit))
>> bits " ")))
>>
>> ?
>
> Sure, this was just my halfway-there muddle.
>
> I've cleaned this branch, squashed it, and am preparing to test for a
> while. I'm attaching the full diff in case anyone wants to read it :)
>
> A net removal of 562 lines with, I hope, no change in behavior.
I revisit this every few months, and have to completely relearn all the
code each time. With any luck that means that I've looked over these
diffs sufficiently to have caught more bugs.
At any rate, I think this is finally ready to go.
Eric
[0001-Allow-gnus-retrieve-headers-to-return-headers-direct.patch (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#38011
; Package
emacs
.
(Sat, 02 Jan 2021 06:00:02 GMT)
Full text and
rfc822 format available.
Message #50 received at 38011 <at> debbugs.gnu.org (full text, mbox):
Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
> I revisit this every few months, and have to completely relearn all the
> code each time. With any luck that means that I've looked over these
> diffs sufficiently to have caught more bugs.
>
> At any rate, I think this is finally ready to go.
Congratulations!
That's a big patch, and skimming it, I'm not quite sure I understand it
all. Could you put this on a branch so that we can get a bit of testing
before merging it?
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#38011
; Package
emacs
.
(Sat, 02 Jan 2021 20:50:02 GMT)
Full text and
rfc822 format available.
Message #53 received at 38011 <at> debbugs.gnu.org (full text, mbox):
Lars Ingebrigtsen <larsi <at> gnus.org> writes:
> Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
>
>> I revisit this every few months, and have to completely relearn all the
>> code each time. With any luck that means that I've looked over these
>> diffs sufficiently to have caught more bugs.
>>
>> At any rate, I think this is finally ready to go.
>
> Congratulations!
>
> That's a big patch, and skimming it, I'm not quite sure I understand it
> all. Could you put this on a branch so that we can get a bit of testing
> before merging it?
Sure thing. It's in girzel/gnus-headers now. I made a few more sneaky
last minute changes, so yes... testing is in order.
The basic principle is simple: it gives backends the option of parsing
their own article headers, rather than writing text into the
nntp-server-buffer to get parsed later. In this sense, the diff on
`gnus-fetch-headers' is all there is to it. None of the backends
actually do this right now.
It gets complicated because the cache and the agent need to mix their
saved headers into whatever newly-fetched headers we get from the
server. So instead of having them call `gnus-retrieve-headers' and
mixing their cached text into the nntp-server-buffer, they now call
`gnus-fetch-headers' on the server, which actually returns real headers.
That means they also need to be responsible for extracting real headers
from their own cache files (rather than letting that happen further down
the line). In this patch both do that with
`gnus-get-newsgroup-headers-xover' (which efficiently parses only a
subset of [potentially very many] cached headers), then merge/sort those
headers with what came back from the server.
A few points of contention:
1. I'm not sure there's a real difference between
`gnus-agent-fetch-headers' and `gnus-agent-retrieve-headers' anymore.
Both return actual headers. It would take a quiet afternoon of
staring at the code to know for sure.
2. The agent and cache are now using `gnus-get-newsgroup-headers-xover'
to parse their cached headers, which does its own dependency
building. This means that `gnus-fetch-headers' has to be careful not
to double-register headers in the dependency table. It also means
that the agent and cache have to reach waaaaaay back to find a
reference to the `gnus-dependency-table', which they're both doing
with a call to `buffer-local-value', which feels gross and fragile.
In general I would much prefer to build the dependency table in one
place, preferably after all the headers have been retrieved,
preferably in `gnus-select-newsgroup'. Another option would be to not
use the higher-level `gnus-get-newsgroup-headers-xover', but instead
to scan the cache files for article numbers and use the lower-level
`nnheader-parse-nov', which isn't concerned with dependencies.
3. In general it took many extra brain cycles (of which I do not have a
surplus) to follow the code flow. I would love it if
`gnus-retrieve-headers' -- instead of calling one of
`gnus-(cache|agent)-retrieve-headers' and expecting them to re-call
`gnus-retrieve-headers' multiple times with various global variables
set -- instead called everything consecutively, dumping the article
list into each function by turn -- cache, agent, server -- and
filtering the list according to which headers we get back. But that's
another patch for another time.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#38011
; Package
emacs
.
(Sun, 03 Jan 2021 07:46:01 GMT)
Full text and
rfc822 format available.
Message #56 received at 38011 <at> debbugs.gnu.org (full text, mbox):
Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
> Sure thing. It's in girzel/gnus-headers now. I made a few more sneaky
> last minute changes, so yes... testing is in order.
I'm now running this branch, and good news: No breakages so far. :-)
> It gets complicated because the cache and the agent need to mix their
> saved headers into whatever newly-fetched headers we get from the
> server. So instead of having them call `gnus-retrieve-headers' and
> mixing their cached text into the nntp-server-buffer, they now call
> `gnus-fetch-headers' on the server, which actually returns real headers.
Yes, that was the main bit I was unsure of. The braiding stuff tries to
be efficient and avoid parsing things twice (or more) -- you usually get
a bunch of headers from the NNTP server, and then you have even more
headers in the agent/cache, and it stitches them all together as text,
and parses the resulting mess. (If I remember correctly; it's been at
least a decade since I looked at that code.)
Are headers parsed more than once now and then merged?
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#38011
; Package
emacs
.
(Sun, 03 Jan 2021 19:54:01 GMT)
Full text and
rfc822 format available.
Message #59 received at 38011 <at> debbugs.gnu.org (full text, mbox):
On 01/03/21 08:45 AM, Lars Ingebrigtsen wrote:
> Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
>
>> Sure thing. It's in girzel/gnus-headers now. I made a few more sneaky
>> last minute changes, so yes... testing is in order.
>
> I'm now running this branch, and good news: No breakages so far. :-)
Great!
>> It gets complicated because the cache and the agent need to mix their
>> saved headers into whatever newly-fetched headers we get from the
>> server. So instead of having them call `gnus-retrieve-headers' and
>> mixing their cached text into the nntp-server-buffer, they now call
>> `gnus-fetch-headers' on the server, which actually returns real headers.
>
> Yes, that was the main bit I was unsure of. The braiding stuff tries to
> be efficient and avoid parsing things twice (or more) -- you usually get
> a bunch of headers from the NNTP server, and then you have even more
> headers in the agent/cache, and it stitches them all together as text,
> and parses the resulting mess. (If I remember correctly; it's been at
> least a decade since I looked at that code.)
>
> Are headers parsed more than once now and then merged?
I certainly hope they're not parsed more than once, but it would
definitely be good to have more eyes on that part of the code. For
instance, `gnus-cache-retrieve-headers' finds cached articles in the
group, then uses (gnus-sorted-difference articles cached) to find
uncached articles. The uncached articles are sent to
`gnus-fetch-headers', and the cached articles are parsed from the cache
file using:
(gnus-get-newsgroup-headers-xover
(gnus-sorted-difference
cached uncached-articles))
Then the two sets of headers are merged and sorted. The agent does
something similar. I don't think the parsing should do any duplicate
work, though it's possible that adding the newly-received headers back
into the cache file is not optimal: it appends the new headers at the end
of the buffer, then re-sorts the whole buffer.
Crud, I just realized that the agent will re-sort its cache file using gnus-agent-check-overview-buffer
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#38011
; Package
emacs
.
(Sun, 03 Jan 2021 19:55:02 GMT)
Full text and
rfc822 format available.
Message #62 received at 38011 <at> debbugs.gnu.org (full text, mbox):
On 01/03/21 08:45 AM, Lars Ingebrigtsen wrote:
> Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
>
>> Sure thing. It's in girzel/gnus-headers now. I made a few more sneaky
>> last minute changes, so yes... testing is in order.
>
> I'm now running this branch, and good news: No breakages so far. :-)
Great!
>> It gets complicated because the cache and the agent need to mix their
>> saved headers into whatever newly-fetched headers we get from the
>> server. So instead of having them call `gnus-retrieve-headers' and
>> mixing their cached text into the nntp-server-buffer, they now call
>> `gnus-fetch-headers' on the server, which actually returns real headers.
>
> Yes, that was the main bit I was unsure of. The braiding stuff tries to
> be efficient and avoid parsing things twice (or more) -- you usually get
> a bunch of headers from the NNTP server, and then you have even more
> headers in the agent/cache, and it stitches them all together as text,
> and parses the resulting mess. (If I remember correctly; it's been at
> least a decade since I looked at that code.)
>
> Are headers parsed more than once now and then merged?
I certainly hope they're not parsed more than once, but it would
definitely be good to have more eyes on that part of the code. For
instance, `gnus-cache-retrieve-headers' finds cached articles in the
group, then uses (gnus-sorted-difference articles cached) to find
uncached articles. The uncached articles are sent to
`gnus-fetch-headers', and the cached articles are parsed from the cache
file using:
(gnus-get-newsgroup-headers-xover
(gnus-sorted-difference
cached uncached-articles))
Then the two sets of headers are merged and sorted. The agent does
something similar. I don't think the parsing should do any duplicate
work, though it's possible that adding the newly-received headers back
into the cache file is not optimal: it appends the new headers at the end
of the buffer, then re-sorts the whole buffer.
Crud, I just realized that the agent will re-sort its cache file using
`gnus-agent-check-overview-buffer', but gnus-cache doesn't do anything
similar. So the write process will have to insert the new headers into
the buffer in sorted order.
Eric
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#38011
; Package
emacs
.
(Sun, 03 Jan 2021 21:39:02 GMT)
Full text and
rfc822 format available.
Message #65 received at 38011 <at> debbugs.gnu.org (full text, mbox):
Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
> On 01/03/21 08:45 AM, Lars Ingebrigtsen wrote:
>> Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
>>
>>> Sure thing. It's in girzel/gnus-headers now. I made a few more sneaky
>>> last minute changes, so yes... testing is in order.
>>
>> I'm now running this branch, and good news: No breakages so far. :-)
>
> Great!
>
>>> It gets complicated because the cache and the agent need to mix their
>>> saved headers into whatever newly-fetched headers we get from the
>>> server. So instead of having them call `gnus-retrieve-headers' and
>>> mixing their cached text into the nntp-server-buffer, they now call
>>> `gnus-fetch-headers' on the server, which actually returns real headers.
>>
>> Yes, that was the main bit I was unsure of. The braiding stuff tries to
>> be efficient and avoid parsing things twice (or more) -- you usually get
>> a bunch of headers from the NNTP server, and then you have even more
>> headers in the agent/cache, and it stitches them all together as text,
>> and parses the resulting mess. (If I remember correctly; it's been at
>> least a decade since I looked at that code.)
>>
>> Are headers parsed more than once now and then merged?
>
> I certainly hope they're not parsed more than once, but it would
> definitely be good to have more eyes on that part of the code. For
> instance, `gnus-cache-retrieve-headers' finds cached articles in the
> group, then uses (gnus-sorted-difference articles cached) to find
> uncached articles. The uncached articles are sent to
> `gnus-fetch-headers', and the cached articles are parsed from the cache
> file using:
>
> (gnus-get-newsgroup-headers-xover
> (gnus-sorted-difference
> cached uncached-articles))
>
> Then the two sets of headers are merged and sorted. The agent does
> something similar. I don't think the parsing should do any duplicate
> work, though it's possible that adding the newly-received headers back
> into the cache file is not optimal: it appends the new headers at the end
> of the buffer, then re-sorts the whole buffer.
>
> Crud, I just realized that the agent will re-sort its cache file using
> `gnus-agent-check-overview-buffer', but gnus-cache doesn't do anything
> similar. So the write process will have to insert the new headers into
> the buffer in sorted order.
Oh hang on, we're not supposed to be adding any new headers to the
gnus-cache cache files here at all. So that chunk of code can just be
removed. Just when you thought you had a handle on what was happening...
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#38011
; Package
emacs
.
(Mon, 04 Jan 2021 09:06:02 GMT)
Full text and
rfc822 format available.
Message #68 received at 38011 <at> debbugs.gnu.org (full text, mbox):
Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
> I certainly hope they're not parsed more than once, but it would
> definitely be good to have more eyes on that part of the code. For
> instance, `gnus-cache-retrieve-headers' finds cached articles in the
> group, then uses (gnus-sorted-difference articles cached) to find
> uncached articles. The uncached articles are sent to
> `gnus-fetch-headers', and the cached articles are parsed from the cache
> file using:
So that sounds OK... is it the same with the Agent?
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#38011
; Package
emacs
.
(Mon, 04 Jan 2021 18:11:01 GMT)
Full text and
rfc822 format available.
Message #71 received at 38011 <at> debbugs.gnu.org (full text, mbox):
On 01/04/21 10:05 AM, Lars Ingebrigtsen wrote:
> Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
>
>> I certainly hope they're not parsed more than once, but it would
>> definitely be good to have more eyes on that part of the code. For
>> instance, `gnus-cache-retrieve-headers' finds cached articles in the
>> group, then uses (gnus-sorted-difference articles cached) to find
>> uncached articles. The uncached articles are sent to
>> `gnus-fetch-headers', and the cached articles are parsed from the cache
>> file using:
>
> So that sounds OK... is it the same with the Agent?
Morally the same: the agent has its own `gnus-agent-uncached-articles'
function, which consults the .agentview file, so it uses that instead of
`gnus-sorted-difference'. But the same idea.
(I saw the other bug report about nnmaildir and nov entries, once this
patch is in I'm planning on working on nnmaildir next.)
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#38011
; Package
emacs
.
(Tue, 05 Jan 2021 08:48:02 GMT)
Full text and
rfc822 format available.
Message #74 received at 38011 <at> debbugs.gnu.org (full text, mbox):
Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
> Morally the same: the agent has its own `gnus-agent-uncached-articles'
> function, which consults the .agentview file, so it uses that instead of
> `gnus-sorted-difference'. But the same idea.
Right.
> (I saw the other bug report about nnmaildir and nov entries, once this
> patch is in I'm planning on working on nnmaildir next.)
Great; nnmaildir needs some tender love and care. Or... a complete
rewrite. :-)
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#38011
; Package
emacs
.
(Tue, 05 Jan 2021 17:03:01 GMT)
Full text and
rfc822 format available.
Message #77 received at 38011 <at> debbugs.gnu.org (full text, mbox):
On 01/05/21 09:47 AM, Lars Ingebrigtsen wrote:
> Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
>
>> Morally the same: the agent has its own `gnus-agent-uncached-articles'
>> function, which consults the .agentview file, so it uses that instead of
>> `gnus-sorted-difference'. But the same idea.
>
> Right.
>
>> (I saw the other bug report about nnmaildir and nov entries, once this
>> patch is in I'm planning on working on nnmaildir next.)
>
> Great; nnmaildir needs some tender love and care. Or... a complete
> rewrite. :-)
Well that doesn't sound like very much fun! I was planning on doing the
header return thing, and then providing more options for incrementally
building the nov cache files, as the main user complaint seems to be
that adopting large collections of messages can result in hours and
hours of caching. Then adapting nnmaildir to actually use Gnus' OO
stuff, whether that's deffoo or generic functions.
Over the years I've gotten the impression that edits purely for coding
style are discouraged, but nnmaildir's weird semi-imperative style is
*so* hard to parse I think it would be worth just trying to clear some
of that out, as well.
Anyway, I'll keep looking at this patch for a bit, and see if I can find
any more problems with it.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#38011
; Package
emacs
.
(Sun, 17 Jan 2021 05:01:01 GMT)
Full text and
rfc822 format available.
Message #80 received at 38011 <at> debbugs.gnu.org (full text, mbox):
Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
> On 01/05/21 09:47 AM, Lars Ingebrigtsen wrote:
>> Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
>>
>>> Morally the same: the agent has its own `gnus-agent-uncached-articles'
>>> function, which consults the .agentview file, so it uses that instead of
>>> `gnus-sorted-difference'. But the same idea.
>>
>> Right.
[...]
> Anyway, I'll keep looking at this patch for a bit, and see if I can find
> any more problems with it.
I found a couple more, in agent file writing, and have pushed a commit
fixing that. So far as I can tell this now works as intended. I've run
side-by-side comparisons of master and this feature branch, messing with
the cache and the agent, and feel fairly confident that behavior is the
same in terms of header fetching, and files written to disk. I think I'd
feel okay merging this, but would also be happy to let it mellow longer.
Eric
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#38011
; Package
emacs
.
(Mon, 18 Jan 2021 10:49:02 GMT)
Full text and
rfc822 format available.
Message #83 received at 38011 <at> debbugs.gnu.org (full text, mbox):
>>>>> On Sat, 16 Jan 2021 21:00:36 -0800, Eric Abrahamsen <eric <at> ericabrahamsen.net> said:
Eric> Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
>> On 01/05/21 09:47 AM, Lars Ingebrigtsen wrote:
>>> Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
>>>
>>>> Morally the same: the agent has its own `gnus-agent-uncached-articles'
>>>> function, which consults the .agentview file, so it uses that instead of
>>>> `gnus-sorted-difference'. But the same idea.
>>>
>>> Right.
Eric> [...]
>> Anyway, I'll keep looking at this patch for a bit, and see if I can find
>> any more problems with it.
Eric> I found a couple more, in agent file writing, and have pushed a commit
Eric> fixing that. So far as I can tell this now works as intended. I've run
Eric> side-by-side comparisons of master and this feature branch, messing with
Eric> the cache and the agent, and feel fairly confident that behavior is the
Eric> same in terms of header fetching, and files written to disk. I think I'd
Eric> feel okay merging this, but would also be happy to let it mellow longer.
I think previous experience shows that the only way to find bugs in
this kind of code is to foist it on us :-)
Robert
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#38011
; Package
emacs
.
(Mon, 18 Jan 2021 16:39:01 GMT)
Full text and
rfc822 format available.
Message #86 received at 38011 <at> debbugs.gnu.org (full text, mbox):
Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
> I found a couple more, in agent file writing, and have pushed a commit
> fixing that. So far as I can tell this now works as intended. I've run
> side-by-side comparisons of master and this feature branch, messing with
> the cache and the agent, and feel fairly confident that behavior is the
> same in terms of header fetching, and files written to disk. I think I'd
> feel okay merging this, but would also be happy to let it mellow longer.
Sure, go ahead.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
Reply sent
to
Eric Abrahamsen <eric <at> ericabrahamsen.net>
:
You have taken responsibility.
(Mon, 18 Jan 2021 21:13:01 GMT)
Full text and
rfc822 format available.
Notification sent
to
Eric Abrahamsen <eric <at> ericabrahamsen.net>
:
bug acknowledged by developer.
(Mon, 18 Jan 2021 21:13:01 GMT)
Full text and
rfc822 format available.
Message #91 received at 38011-done <at> debbugs.gnu.org (full text, mbox):
Robert Pluim <rpluim <at> gmail.com> writes:
>>>>>> On Sat, 16 Jan 2021 21:00:36 -0800, Eric Abrahamsen <eric <at> ericabrahamsen.net> said:
>
> Eric> Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
> >> On 01/05/21 09:47 AM, Lars Ingebrigtsen wrote:
> >>> Eric Abrahamsen <eric <at> ericabrahamsen.net> writes:
> >>>
> >>>> Morally the same: the agent has its own `gnus-agent-uncached-articles'
> >>>> function, which consults the .agentview file, so it uses that instead of
> >>>> `gnus-sorted-difference'. But the same idea.
> >>>
> >>> Right.
>
> Eric> [...]
>
> >> Anyway, I'll keep looking at this patch for a bit, and see if I can find
> >> any more problems with it.
>
> Eric> I found a couple more, in agent file writing, and have pushed a commit
> Eric> fixing that. So far as I can tell this now works as intended. I've run
> Eric> side-by-side comparisons of master and this feature branch, messing with
> Eric> the cache and the agent, and feel fairly confident that behavior is the
> Eric> same in terms of header fetching, and files written to disk. I think I'd
> Eric> feel okay merging this, but would also be happy to let it mellow longer.
>
> I think previous experience shows that the only way to find bugs in
> this kind of code is to foist it on us :-)
Bombs away, then!
Did not alter fixed versions and reopened.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org
.
(Tue, 26 Jan 2021 16:53:01 GMT)
Full text and
rfc822 format available.
Removed tag(s) patch.
Request was from
Eric Abrahamsen <eric <at> ericabrahamsen.net>
to
control <at> debbugs.gnu.org
.
(Tue, 26 Jan 2021 16:53:01 GMT)
Full text and
rfc822 format available.
This bug report was last modified 4 years and 143 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.