SCM/bool confusion in One_page_breaking::solve

SCM/bool confusion in One_page_breaking::solve

This replaces an accidentally always-true condition with the
originally intended one.

I have no idea about the consequences: it would be best if the
original author Paul Morris (in issue 4752) checked that this now
works as intended and also tried figuring out why we failed to notice
this problem.

[pwm, 2017-02-18 18:08:57]

https://codereview.appspot.com/314490043/diff/1/lily/one-page-breaking.cc
File lily/one-page-breaking.cc (right):

https://codereview.appspot.com/314490043/diff/1/lily/one-page-breaking.cc#new...
lily/one-page-breaking.cc:97: if (scm_is_true (scm_gr_p (this_pos,
lowest_line_pos)))
Thanks for catching this.  I'll try to take a closer look in the next day or so.

[dak, 2017-02-18 20:20:27]

On 2017/02/18 18:08:57, pwm wrote:
> https://codereview.appspot.com/314490043/diff/1/lily/one-page-breaking.cc
> File lily/one-page-breaking.cc (right):
> 
>
https://codereview.appspot.com/314490043/diff/1/lily/one-page-breaking.cc#new...
> lily/one-page-breaking.cc:97: if (scm_is_true (scm_gr_p (this_pos,
> lowest_line_pos)))
> Thanks for catching this.  I'll try to take a closer look in the next day or
so.

Actually I didn't.  I just put the finishing touches on a task mainly done by
Valentin that made it possible to enable a debugging option in Guile, and the
compiler then dug this one up in consequence as well as a few others.

[pwm, 2017-02-20 03:04:55]

On 2017/02/18 20:20:27, dak wrote:
> 
> Actually I didn't.  I just put the finishing touches on a task mainly done by
> Valentin that made it possible to enable a debugging option in Guile, and the
> compiler then dug this one up in consequence as well as a few others.

Ah, cool, nice to have these checks.  I gave it a try with the new change to the
code and it still works as intended.

I think the reason this did not show up as a problem before is that this part of
the code is a defensive measure against a rare edge case where the last musical
system (or top-level-markup) in the data structure does not produce the lowest
visual point of the 'music' on the page (excluding footers, etc.).  

I looked back at the test file I was using locally while working on this, and
found an example that seems to cover this edge case.  However, it appears to
work fine with or without the newest change to the code...  So it's possible
that this code is more defensive than it needs to be, but it's also possible
that removing the defensive measure might lead to bugs in other edge cases, and
it doesn't hurt to have it in there.  So I think it's better to leave it as is
and err on the defensive side.

I can upload a regtest for that edge case in another patch.