Fix 1336

Fix 1336

[Carl, 2010-12-11 17:40:20]

Here's a patch for issue 1336 that combine's Neil's idea of not creating the
paper columns with Graham's idea of erroring out when there are less than 2
columns.

Thanks,

Carl

[Graham Percival, 2010-12-11 19:46:05]

On Sat, Dec 11, 2010 at 05:40:20PM +0000, Carl.D.Sorensen@gmail.com wrote:
> Message:
> Here's a patch for issue 1336 that combine's Neil's idea of not creating
> the paper columns with Graham's idea of erroring out when there are less
> than 2 columns.

If nobody else wants to do a full regtest comparison and doc build
from scratch, I can do this on Monday.

Note that my idea of quitting with less than 2 columns is not
based on thinking about the code / spacing engine / lilypond
architecture; it was simply from adding a few printfs() and
looking at one file that compiled with no problem and a file that
had no problems.  I don't even know what a paper column is!

Cheers,
- Graham

[c_sorensen, 2010-12-11 20:10:18]

On 12/11/10 12:46 PM, "Graham Percival" <graham@percival-music.ca> wrote:

> On Sat, Dec 11, 2010 at 05:40:20PM +0000, Carl.D.Sorensen@gmail.com wrote:
>> Message:
>> Here's a patch for issue 1336 that combine's Neil's idea of not creating
>> the paper columns with Graham's idea of erroring out when there are less
>> than 2 columns.
> 
> If nobody else wants to do a full regtest comparison and doc build
> from scratch, I can do this on Monday.
> 
> Note that my idea of quitting with less than 2 columns is not
> based on thinking about the code / spacing engine / lilypond
> architecture; it was simply from adding a few printfs() and
> looking at one file that compiled with no problem and a file that
> had no problems.  I don't even know what a paper column is!

But it's clear from the code that the problem occurs when the number of
paper columns is less than 2.

Paper columns are things that hold music elements.

The segfault was in code that was trying to partition paper columns between
loose columns and non-loose columns.  We needed more than 2 for the code to
work.

Thanks,

Carl

[Valentin Villenave, 2010-12-11 20:28:18]

Hi Carl,

thanks for this patch! Hopefully we can close this skipTypesetting issue soon
:-)

I'm just not entirely sure about the error message:

http://codereview.appspot.com/3594041/diff/1/lily/simple-spacer.cc
File lily/simple-spacer.cc (right):

http://codereview.appspot.com/3594041/diff/1/lily/simple-spacer.cc#newcode397
lily/simple-spacer.cc:397: error (_ ("no music remaining to typeset"));
"remaining" implies that something has been skipped. Are we sure that this can
only happen with skipTypesetting?

[Carl, 2010-12-11 20:33:52]

On 2010/12/11 20:28:18, Valentin Villenave wrote:
> I'm just not entirely sure about the error message:
> 
> http://codereview.appspot.com/3594041/diff/1/lily/simple-spacer.cc
> File lily/simple-spacer.cc (right):
> 
> http://codereview.appspot.com/3594041/diff/1/lily/simple-spacer.cc#newcode397
> lily/simple-spacer.cc:397: error (_ ("no music remaining to typeset"));
> "remaining" implies that something has been skipped. Are we sure that this can
> only happen with skipTypesetting?

If we run an empty score, we get the following:

sorensen2:lilypond Carl$ more emptytest.ly
\score {
{}
}
sorensen2:lilypond Carl$ lilypond emptytest.ly
GNU LilyPond 2.13.43
Processing `emptytest.ly'
Parsing...
emptytes.ly:0: warning: no \version statement found, please add

\version "2.13.43"

for future compatibility
warning: no music found in score
success: Compilation successfully completed


I don't know of any other way to get an empty score.

[Graham Percival, 2010-12-11 20:38:28]

On Sat, Dec 11, 2010 at 08:33:52PM +0000, Carl.D.Sorensen@gmail.com wrote:
> >lily/simple-spacer.cc:397: error (_ ("no music remaining to
> typeset"));
> >"remaining" implies that something has been skipped. Are we sure that
> this can
> >only happen with skipTypesetting?
> 
> If we run an empty score, we get the following:
> 
> 
> I don't know of any other way to get an empty score.

What about:
\markup{foo}

?  how many paper columns does that create?

Cheers,
- Graham

[Neil Puttock, 2010-12-11 23:23:46]

On 11 December 2010 17:40,  <Carl.D.Sorensen@gmail.com> wrote:

> Here's a patch for issue 1336 that combine's Neil's idea of not creating
> the paper columns with Graham's idea of erroring out when there are less
> than 2 columns.

I think it would be better to create the missing column rather than aborting.

Though we can't avoid creating the first pair of columns (they're
created before translation starts, so skipTypesetting can only prevent
column creation from the first timestep), it's easy to check whether
any further columns have been created.  Once we have this information,
we can create another pair in Paper_column_engraver::finalize (),
which will ensure the root system has left and right bounds.

Cheers,
Neil

[Neil Puttock, 2010-12-11 23:28:20]

On 11 December 2010 20:38, Graham Percival <graham@percival-music.ca> wrote:

> What about:
> \markup{foo}
>
> ?  how many paper columns does that create?

None: toplevel markup doesn't create any grobs.

Cheers,
Neil

[Neil Puttock, 2010-12-11 23:53:38]

On 2010/12/11 23:23:46, Neil Puttock wrote:

> I think it would be better to create the missing column rather than aborting.

Like this: http://codereview.appspot.com/3598041

[hanwenn, 2010-12-12 13:20:21]

On Sat, Dec 11, 2010 at 9:53 PM,  <n.puttock@gmail.com> wrote:
> On 2010/12/11 23:23:46, Neil Puttock wrote:
>
>> I think it would be better to create the missing column rather than
>
> aborting.
>

Neil's approach looks better to me than Carl's, as it is closer to the
place where skipTypesetting is handled.

-- 
Han-Wen Nienhuys - hanwen@xs4all.nl - http://www.xs4all.nl/~hanwen

[c_sorensen, 2010-12-12 13:28:14]

On Dec 12, 2010, at 6:20 AM, "Han-Wen Nienhuys" <hanwenn@gmail.com>  
wrote:

> On Sat, Dec 11, 2010 at 9:53 PM,  <n.puttock@gmail.com> wrote:
>> On 2010/12/11 23:23:46, Neil Puttock wrote:
>>
>>> I think it would be better to create the missing column rather than
>>
>> aborting.
>>
>
> Neil's approach looks better to me than Carl's, as it is closer to the
> place where skipTypesetting is handled.

I completely agree

Carl