lilypond-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Nested book parts


From: joeneeman
Subject: Nested book parts
Date: Tue, 14 Oct 2008 18:06:06 -0700

Reviewers: nicolas.sceaux,

Message:
I'm really sorry I took so long to do this. I honestly thought it had
already been committed (probably because it has been merged into my
working copy for such a long time).


http://codereview.appspot.com/4848/diff/1/4
File lily/book.cc (right):

http://codereview.appspot.com/4848/diff/1/4#newcode139
Line 139: Book::add_bookpart ()
This method confused me for a while (because it has quite a different
purpose from add_bookpart(SCM)). I suggest renaming this method and
adding a comment along the lines of "all scores before an explicit
\bookpart are implicitly lumped into their own separate bookpart".

http://codereview.appspot.com/4848/diff/1/4#newcode190
Line 190: Book::process (Output_def *default_paper,
This method is becoming very long. Can it be split up into multiple
functions like:

if (there are bookparts)
  process_bookparts();
else
  for (s in scores)
    process_score(s);

http://codereview.appspot.com/4848/diff/1/5
File lily/include/book.hh (right):

http://codereview.appspot.com/4848/diff/1/5#newcode35
Line 35: void add_bookpart ();
Can this be private? Yeah, I know everything else here is public, but
that's no reason to perpetuate the problem.

http://codereview.appspot.com/4848/diff/1/5#newcode38
Line 38: bool error_found ();
also private?

http://codereview.appspot.com/4848/diff/1/8
File lily/minimal-page-breaking.cc (right):

http://codereview.appspot.com/4848/diff/1/8#newcode43
Line 43: vsize first_page_num = robust_scm2int
(book_->paper_->c_variable ("part-first-page-number"), 1);
Do we really need new variables for properties in book parts (as opposed
to using some cascading to allow bookparts to override properties in the
parent)? I could see this leading to an explosion of properties.

http://codereview.appspot.com/4848/diff/1/10
File lily/page-breaking.cc (right):

http://codereview.appspot.com/4848/diff/1/10#newcode266
Line 266: bool last_from_part = (i == lines_per_page.size () - 1);
I'm not sure this is the logic we want here. I think we want to treat
the last page in a bookpart the same as the last page in a book. This
would give the per-score ragged-last-bottom feature that some people
have been asking for.

http://codereview.appspot.com/4848/diff/1/13
File lily/paper-book.cc (right):

http://codereview.appspot.com/4848/diff/1/13#newcode112
Line 112: Paper_book::output_aux (SCM output_channel,
This gets simpler if you no longer care about is_first and is_last...

http://codereview.appspot.com/4848/diff/1/13#newcode549
Line 549: /* backwards compatibility for the old page breaker */
We can probably scrap this soon.

Description:
Book parts aim at splitting a book into several parts, in order to be
able to use eg. different page breaking functions, or to make the page
breaking problem less difficult and more likely to finish.

- Book and Paper_book instances respectively are nestable: children
  book or paper_book are added to the bookparts_ slot;

- the paper_ slot of a child Book (or Book_paper) is created empty,
  and has its parent set to the paper object of the parent Book (or
  Paper_book), so that default paper properties are got from the
  higher level paper object, and child objects only store part-wide
  overrides. This way, we ensure that fonts are loaded in the higher
  level paper object, so that the output framework can get all the
  loaded fonts from the top level book;

- a Paper_book::top_paper() method is added to access the higher level
  paper object, to access properties that are book-wide, for instance
  the table used to store labels and page numbers;

- in the parser, \bookpart blocks are introduced, which can be used at
  toplevel, or inside a \book block. It can contain the same things as
  \book blocks (except \bookpart blocks, though that would be
  possible). The associated handlers are added.

Please review this at http://codereview.appspot.com/4848

Affected files:
  A input/regression/bookparts.ly
  M lily/book-scheme.cc
  M lily/book.cc
  M lily/include/book.hh
  M lily/include/paper-book.hh
  M lily/lily-lexer.cc
  M lily/minimal-page-breaking.cc
  M lily/optimal-page-breaking.cc
  M lily/page-breaking.cc
  M lily/page-turn-page-breaking.cc
  M lily/paper-book-scheme.cc
  M lily/paper-book.cc
  M lily/parser.yy
  M ly/declarations-init.ly
  M ly/init.ly
  M scm/lily-library.scm
  M scm/midi.scm






reply via email to

[Prev in Thread] Current Thread [Next in Thread]