headers
Richard heck | 6 Oct 05:24
Favicon
Gravatar

[PATCH] Bug5316


The cause of the bug is that the counter-reading code overwrites an 
existing counter when it is read again, rather than updating it, as for 
other bits of layout information. This patch fixes the bug and, along 
the way, makes the counter-reading code more like the style-reading 
code. The bulk of it is moved to Counters.cpp.

The counter syntax is changed from:
    Counter
       Name cnt
to the more sensible:
    Counter cnt
This is more like styles, etc, and makes the code in TextClass::read() 
parallel to that for styles.

I've checked this and it seems to work. Comments welcome.

If it's committed, then we'll want to update all the files containing 
counters to the new format.

Richard


Index: src/Counters.h
===================================================================
--- src/Counters.h	(revision 26762)
+++ src/Counters.h	(working copy)
@@ -23,6 +23,8 @@
(Continue reading)

Permalink | Reply |
headers
Jean-Marc Lasgouttes | 6 Oct 09:05
Favicon

Re: [PATCH] Bug5316

> The cause of the bug is that the counter-reading code overwrites an  
> existing counter when it is read again, rather than updating it, as  
> for other bits of layout information. This patch fixes the bug and,  
> along the way, makes the counter-reading code more like the style- 
> reading code. The bulk of it is moved to Counters.cpp.

I am all for it if it is corectly tested (but of course, this looks  
moe like cleanup that bug fixing...).

JMarc
Permalink | Reply |
headers
rgheck | 6 Oct 14:05
Favicon
Gravatar

Re: [PATCH] Bug5316

Jean-Marc Lasgouttes wrote:
>> The cause of the bug is that the counter-reading code overwrites an 
>> existing counter when it is read again, rather than updating it, as 
>> for other bits of layout information. This patch fixes the bug and, 
>> along the way, makes the counter-reading code more like the 
>> style-reading code. The bulk of it is moved to Counters.cpp.
>
> I am all for it if it is corectly tested (but of course, this looks 
> more like cleanup than bug fixing...).
>
Some of it is cleanup, yes, but it's the best way to fix the bug, I 
think. It's messy if you don't make this kind of change, because we have 
to start reading the counter to get the Name, and that's not actually 
required to be the first entry in the Counter declaration. So we can't, 
say, get the old counter and update it instead of reading a new one.

But the real issue is that we have no way of accessing the original 
counter anyway. You can of course get to the CounterList, via 
TextClass::counters_, but there's no Counters::operator[] or anything of 
the sort. Of course, we could add that---but do we really want it? And 
there are other options, but they all involve messing with CounterList, too.

I've tested this some, but of course more would be welcome. The layouts 
still open properly, with layout2layout conversion. The counters still 
work, with no layout errors. Here's how to see the bug fix: Open the 
Introduction; change the Document Class to paper; the numbering is 
currently messed up (0.1, e.g.). With the patch, it should be ok.

rh
(Continue reading)

Permalink | Reply |
headers
rgheck | 6 Oct 14:29
Favicon
Gravatar

[PATCH-Update] Bug 5316

rgheck wrote:
> Jean-Marc Lasgouttes wrote:
>>> The cause of the bug is that the counter-reading code overwrites an 
>>> existing counter when it is read again, rather than updating it, as 
>>> for other bits of layout information. This patch fixes the bug and, 
>>> along the way, makes the counter-reading code more like the 
>>> style-reading code. The bulk of it is moved to Counters.cpp.
>>
>> I am all for it if it is corectly tested (but of course, this looks 
>> more like cleanup than bug fixing...).
>>
> Some of it is cleanup, yes, but it's the best way to fix the bug, I 
> think. It's messy if you don't make this kind of change, because we 
> have to start reading the counter to get the Name, and that's not 
> actually required to be the first entry in the Counter declaration. So 
> we can't, say, get the old counter and update it instead of reading a 
> new one.
>
> But the real issue is that we have no way of accessing the original 
> counter anyway. You can of course get to the CounterList, via 
> TextClass::counters_, but there's no Counters::operator[] or anything 
> of the sort. Of course, we could add that---but do we really want it? 
> And there are other options, but they all involve messing with 
> CounterList, too.
>
> I've tested this some, but of course more would be welcome. The 
> layouts still open properly, with layout2layout conversion. The 
> counters still work, with no layout errors. Here's how to see the bug 
> fix: Open the Introduction; change the Document Class to paper; the 
> numbering is currently messed up (0.1, e.g.). With the patch, it
(Continue reading)

Permalink | Reply |
headers
Jean-Marc Lasgouttes | 6 Oct 15:38
Favicon

Re: [PATCH] Bug5316

rgheck <rgheck@...> writes:
> Some of it is cleanup, yes, but it's the best way to fix the bug, I
> think. It's messy if you don't make this kind of change, because we
> have to start reading the counter to get the Name, and that's not
> actually required to be the first entry in the Counter declaration. So
> we can't, say, get the old counter and update it instead of reading a
> new one.

Thanks, this is what I wanted to know.

> I've tested this some, but of course more would be welcome. The
> layouts still open properly, with layout2layout conversion. The
> counters still work, with no layout errors. Here's how to see the bug
> fix: Open the Introduction; change the Document Class to paper; the
> numbering is currently messed up (0.1, e.g.). With the patch, it
> should be ok.

I think this patch is The Right Thing.

JMarc
Permalink | Reply |
headers
Abdelrazak Younes | 6 Oct 15:56
Favicon

Re: [PATCH] Bug5316

On 06/10/2008 15:38, Jean-Marc Lasgouttes wrote:
> rgheck<rgheck@...>  writes:
>    
>> Some of it is cleanup, yes, but it's the best way to fix the bug, I
>> think. It's messy if you don't make this kind of change, because we
>> have to start reading the counter to get the Name, and that's not
>> actually required to be the first entry in the Counter declaration. So
>> we can't, say, get the old counter and update it instead of reading a
>> new one.
>>      
>
> Thanks, this is what I wanted to know.
>
>    
>> I've tested this some, but of course more would be welcome. The
>> layouts still open properly, with layout2layout conversion. The
>> counters still work, with no layout errors. Here's how to see the bug
>> fix: Open the Introduction; change the Document Class to paper; the
>> numbering is currently messed up (0.1, e.g.). With the patch, it
>> should be ok.
>>      
>
> I think this patch is The Right Thing.
>    

I like it too.

Abdel.
(Continue reading)

Permalink | Reply |

Gmane