[sword-devel] breakage in verse management at -r2785?

[Troy A. Griffitts]

Hi Greg,

In your test, on like 19, you turn off auto normalization.  This means 
SWORD won't mess with the values you set for the components of a verse.  
If you setText("Gen.1.1"); setChapter(999); setVerse(9999); and print, 
you should get "Genesis 999:9999".  If you want normalization, then you 
need to remove the line in your test which says key->setAutoNormalize(0);

There is a good encapsulation of how I expect normalization to work in 
the test suite.  The specific test is:

http://crosswire.org/svn/sword/trunk/tests/versekeytest.cpp

And the expected output is:

http://crosswire.org/svn/sword/trunk/tests/testsuite/versekeytest.good

(the labels in that test still use the old terminology of 'headings', 
but this is really 'intros')



On 03/22/2013 02:02 AM, Greg Hellings wrote:
> Test code: https://gist.github.com/greg-hellings/5218094
>
> Output with your current patch, Troy, is $ g++ `pkg-config --cflags 
> sword` `pkg-config --libs sword` test.cpp -o test && ./test
> intro:  0
> bk (1): 1
> ch (1): 1
> vs (1): 1
> -----------------
> intro:  0
> bk (1): 0
> ch (1): 0
> vs (1): 0
> -----------------
> intro:  1
> bk (0): 0
> ch (0): 0
> vs (0): 0
> -----------------
>
> The value in parenthsis represents what I thought the value "should" 
> be. If I comment out line 19, I get this result $ g++ `pkg-config 
> --cflags sword` `pkg-config --libs sword` test.cpp -o test && ./test
> intro:  0
> bk (1): 1
> ch (1): 1
> vs (1): 1
> -----------------
> intro:  0
> bk (1): 1
> ch (1): 1
> vs (1): 1
> -----------------
> intro:  1
> bk (0): 0
> ch (0): 0
> vs (0): 0
> -----------------
>
> which matches the expected results. So Xiphos is getting around 
> calling setIntros(1) by instead disabling auto-normalizing. It's odd 
> to me that you can set a key to a value in the intros while setIntros 
> is false. That doesn't seem like normalizing to me, that seems more 
> like bounds checking, but that's not necessarily a bug in the API, 
> possibly it's a bug in my understanding of intended behavior.
>
> Troy's fix along with shuffling the location of setTestament (a change 
> I made earlier to Xiphos' Subversion) fixes the known bugs I've seen 
> in Xiphos regarding chapters not rendering and Genesis 1:1 causing a 
> SegFault.
>
> --Greg
>
>
>
> On Thu, Mar 21, 2013 at 6:51 PM, Greg Hellings 
> <greg.hellings at gmail.com <mailto:greg.hellings at gmail.com>> wrote:
>
>
>
>
>     On Thu, Mar 21, 2013 at 6:26 PM, Troy A. Griffitts
>     <scribe at crosswire.org <mailto:scribe at crosswire.org>> wrote:
>
>         Thanks Karl,
>
>         Yes, each snippet was helpful. Nic's was a quick test which
>         caused the bug and was easy to use for testing. Greg's snippet
>         wasn't as helpful as all his comments and stack traces leading
>         up to his patch. He is preventing book from getting to 0 which
>         does alleviate the problem but also stops a book from becoming
>         an intro. Hope that explains.
>
>
>     If the conditional in my code is changed to
>
>     if (book > (intros?0:1) ) {
>
>     Then my code corrects the problem that -- was moving a VerseKey
>     from Genesis 1:1 to [Testament 1 Intro] when intros is false.
>     While not a SegFault producing bug, this is still a bug. Similar
>     checks should be made through the same run of code to ensure
>     moving to verse 0 and chapter 0 are not possible when intros is
>     false. Both your fix and an adapted form of mine should be
>     introduced. Additionally, the issue with OSISFootnotes should
>     probably be fixed.
>
>     I will write up a simple test case for that, if it helps, once I
>     get back to my development machine.
>
>     --Greg
>
>
>         Troy
>
>         Karl Kleinpaste <karl at kleinpaste.org
>         <mailto:karl at kleinpaste.org>> wrote:
>
>             You didn't also need this snippet from Greg a couple days
>             ago? --- src/keys/versekey.cpp (revision 2792) +++
>             src/keys/versekey.cpp (working copy) @@ -1347,7 +1347,9 @@
>             } if (verse < (intros?0:1)) { if (--chapter <
>             (intros?0:1)) { - --book; + if (book > 1) { + --book; + }
>             chapter += (getChapterMax() + (intros?1:0)); } verse +=
>             (getVerseMax() + (intros?1:0));
>             ------------------------------------------------------------------------
>             sword-devel mailing list: sword-devel at crosswire.org
>             <mailto:sword-devel at crosswire.org>
>             http://www.crosswire.org/mailman/listinfo/sword-devel
>             Instructions to unsubscribe/change your settings at above
>             page
>
>
>         -- 
>         Sent from my Android phone with K-9 Mail. Please excuse my
>         brevity.
>
>         _______________________________________________
>
>         sword-devel mailing list: sword-devel at crosswire.org
>         <mailto:sword-devel at crosswire.org>
>         http://www.crosswire.org/mailman/listinfo/sword-devel
>         Instructions to unsubscribe/change your settings at above page
>
>
>
>
>
> _______________________________________________
> sword-devel mailing list: sword-devel at crosswire.org
> http://www.crosswire.org/mailman/listinfo/sword-devel
> Instructions to unsubscribe/change your settings at above page

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.crosswire.org/pipermail/sword-devel/attachments/20130322/7dc36994/attachment-0001.html>