lilypond-devel
[Top][All Lists]
Advanced

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

Re: Implements DOM-id property for grobs. (issue 5504106)


From: dak
Subject: Re: Implements DOM-id property for grobs. (issue 5504106)
Date: Fri, 06 Jan 2012 13:35:04 +0000

Is there a reason you have ignored Patrick's comments?

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.

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.

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.

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?

http://codereview.appspot.com/5504106/



reply via email to

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