Re: lilypond-book: Group line-width settings together (issue 2222). (issue 5553056)

[reinhold . kainhofer]

LGTM in general. Some comments though.

Plus: The example in the comment above will now compile, but it will be
missing the additional padding...

Another thing about the hard-coded .ly extension: I would leave the
extension extraction in the file snippet class, so that the base class
sets a default of .ly, but for file snippets the extension will be
extracted from the file name.


http://codereview.appspot.com/5553056/diff/1/python/book_snippets.py
File python/book_snippets.py (left):

http://codereview.appspot.com/5553056/diff/1/python/book_snippets.py#oldcode184
python/book_snippets.py:184: line-width = #(- line-width (* mm
%(padding_mm)f) (* mm 1))
On 2012/01/21 21:18:00, janek wrote:
> Ah, so the problem was that sometimes line-width wasn't defined and
thus trying
> to adjust it failed?

Yes, the problem is that I don't know of any way to get the default
line-width, because there are cases where line-width is not explicitly
defined in the snippets. In those cases we also need to subtract the
padding...

http://codereview.appspot.com/5553056/diff/1/python/book_snippets.py#oldcode806
python/book_snippets.py:806: self.ext = os.path.splitext
(os.path.basename (self.filename))[1]
Why are you removing a generic statement and hardcoding .ly above? This
will break e.g. if you try to include a .ily file as a snippet!

http://codereview.appspot.com/5553056/diff/1/python/book_snippets.py#oldcode890
python/book_snippets.py:890: return result
On 2012/01/21 19:14:15, Julien Rioux wrote:
> ...this part.

Yeah, appending the original extension makes more sense (and also works
when you have a compressed .xml file where you explicitly specify
--compress).

http://codereview.appspot.com/5553056/diff/1/python/book_snippets.py
File python/book_snippets.py (right):

http://codereview.appspot.com/5553056/diff/1/python/book_snippets.py#newcode124
python/book_snippets.py:124: line-width = #(- line-width (* mm
%(padding_mm)f) (* mm 1))''',
On 2012/01/21 19:14:15, Julien Rioux wrote:
> This fixes the first problem by adjusting line-width directly where it
is
> defined.

Aren't there cases when neither LINE_WIDTH nor QUOTE is used? In that
case we don't subtract the padding...

http://codereview.appspot.com/5553056/