I have listed some specific changes that must be made (move to the proper alphabetical order) and raised questions about how the code works with an apparently misspelled argument. I believe both of those need to be fixed before pushing the patch.
I would also like to see crop apply to at least the pdf backend in addition to svg, if it's not too much work. https://codereview.appspot.com/326960043/diff/1/Documentation/usage/running.itely File Documentation/usage/running.itely (right): https://codereview.appspot.com/326960043/diff/1/Documentation/usage/running.itely#newcode683 Documentation/usage/running.itely:683: @tab Match the size of the normal output to the typeset image for SVGs. This should be placed in alphabetical order (about line 532) I would prefer to see the description be something like Match the size of the normal output to the typeset image (currently only implemented for svg backend). Actually, I'd even more prefer to have the crop function work on the pdf backend as well as the svg backend. Have you investigated whether your code could work on PDF as well? Also, the placement of crop in this file causes reading problems, because it's placed in the middle of preview, and the text below says "This option is supported by all backends", which applies to preview, but not to crop. https://codereview.appspot.com/326960043/diff/1/scm/framework-svg.scm File scm/framework-svg.scm (right): https://codereview.appspot.com/326960043/diff/1/scm/framework-svg.scm#newcode173 scm/framework-svg.scm:173: (define (dump-page-crop paper filename page page-number page-count stensil) Hmm -- the argument here is spelled "stensil" https://codereview.appspot.com/326960043/diff/1/scm/framework-svg.scm#newcode178 scm/framework-svg.scm:178: (x-extent (ly:stencil-extent stencil X)) But here it is spelled "stencil" How does this work? Perhaps the stencil argument is unnecessary and not doing anything here? I'm confused. https://codereview.appspot.com/326960043/diff/1/scm/lily.scm File scm/lily.scm (right): https://codereview.appspot.com/326960043/diff/1/scm/lily.scm#newcode341 scm/lily.scm:341: (crop This should be put in the proper alphabetical order (up at line 231), rather than placed under preview https://codereview.appspot.com/326960043/ _______________________________________________ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel