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/ _______________________________________________ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel