Why wait with calling setResolver on Evaluate and PageNumberReference?

[bertfrees]

Why wait with calling setResolver on Evaluate and PageNumberReference? This makes it impossible to "peek" a Evaluate or PageNumberReference segment. Instead of calling setResolver when the segments are layed out, I simply call the method when SegmentProcessor is initialized.

I must have done something wrong though because I made a test fail. Do you know what's wrong?

org.daisy.dotify.formatter.test.RowByRowTest > testCurrentPage_02 FAILED
    java.lang.AssertionError at RowByRowTest.java:18

[kalaspuffar]

Hi @bertfrees

I've looked into this issue and it seems that the test actually catches the problem with page numbering before layout. We can't know the actual page numbers before layout as the data can have hard linebreaks that could create new pages.

<evaluate expression="(format {0} $page)"/><br/><evaluate expression="(format {0} $page)"/>

Maybe this is not an issue that the page numbers aren't 100% correct, and we could find workarounds.

I'm still looking at the code but I thought I report my initial findings.

Best regards Daniel

[bertfrees]

@kalaspuffar. Thanks for looking into the issue. Yes, we can't know the actual page numbers, but we can get a prediction. That is what the "peek" method is for (to get the actual value you need "resolve"). What I (thought I) did in this change is just setting the "resolver", which provides the values for both "peek" and "resolve", right from the start. It doesn't set the value from the start, it just set the function to get the value later. There didn't seem any harm in that, but I must have missed something. Or maybe there is a bug in another part of the code?

[bertfrees]

I think the reason might be that the setResolver method in PageNumberReference has the side effect that it also "unfreezes" the value, i.e. that it cancels any previous calls to "resolve". I suspect this might be a bug because it could break the [contract of "resolve"](http://brailleapps.github.io/dotify.api/5.0.0/javadoc/org/daisy/dotify/api/translator/ResolvableText.html#resolve()) if setResolver() is called at the wrong moment. And this could mask another problem somewhere when resolve() is called when it shouldn't be.

[bertfrees]

I checked that it is indeed the case that even when setResolver() is called within initFields, the resolved value of Evaluate is sometimes not null within layoutEvaluate. So either initFields is not called when you think it would be called, or resolve() is called when it shouldn't.

[bertfrees]

@kalaspuffar I think I found the problem. In BlockContentManager, line 105, we're creating a new SegmentProcessor to check if there is a next row. However the new object is not a proper clone, i.e. we also mutate the original when we call getNext() on it. This is because we're not making a deep copy of the segments lists.

[bertfrees]

Hmm, seems not to be the (only?) issue after all... Looking further...

[bertfrees]

@kalaspuffar I have made a change. The tests should pass now. I spent some time yesterday but couldn't find out how to solve this the way I wanted. In the end decided to leave it more or less the way it is now, with just a small change to make it possible to "peek" to Evaluate and PageNumberReference segments, and I added some comments that explain why the code does what it does and how it could be improved.

[kalaspuffar]

Hi @PaulRambags and @bertfrees

I've now run the regression tests and there were no issues. So when Paul has reviewed the code, and we don't find any issues, we are ready for merge.

Best regards Daniel