On Jan 6, 2012, at 2:35 PM, d...@gnu.org wrote: > Is there a reason you have ignored Patrick's comments? >
They were incorporated into the version that was pushed to staging. > The issue is missing a description, so is any part of the code. > > It needs at least a regtest demonstrating the use of this feature, > otherwise nobody will be able to ever put this code to actual use (or > write user documentation for it) if you were to be hit by a bus. > Will do. > Of course it would be better if you would write the user documentation > yourself, but without even a regtest for bootstrapping, nobody else can > be expected to be able to do it. > > > http://codereview.appspot.com/5504106/diff/1/lily/grob.cc > File lily/grob.cc (right): > > http://codereview.appspot.com/5504106/diff/1/lily/grob.cc#newcode174 > lily/grob.cc:174: SCM DOM_id = get_property ("DOM-id"); > This probably deserves a comment to that effect that this must come > last, or the effect of the DOM-id on the generated XML (whatever that > may be) will not encompass the whole stencil. Will do. > > http://codereview.appspot.com/5504106/diff/1/scm/framework-svg.scm > File scm/framework-svg.scm (right): > > http://codereview.appspot.com/5504106/diff/1/scm/framework-svg.scm#newcode116 > scm/framework-svg.scm:116: (define (comment s) > What is this for? You changed the definition of the routine as well as > moving it, removing the space padding. Why would that be a good idea? > As a consequence, you had to change existing callers. > I also got rid of this in the patch that got reverted from staging. > http://codereview.appspot.com/5504106/diff/1/scm/output-svg.scm > File scm/output-svg.scm (right): > > http://codereview.appspot.com/5504106/diff/1/scm/output-svg.scm#newcode306 > scm/output-svg.scm:306: (comment " FIXME: how to select glyph by name, > altglyph is broken? ")) > What is the deal with adding the space here? > I also fixed this in the reverted patch. Unfortunately, I did not save the final pushed version of the patch - do you have a copy of it? If so, I won't need to make the changes again. Thanks for the comments! I'll post a new patch in the coming days. Cheers, MS _______________________________________________ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel