Hi Mike, i apologize for the delay; i focused on other things that seemed more urgent to me.
On 2012/01/11 12:27:10, mike_apollinemike.com wrote:
[explanation of the patch]
I'm not sure how/where to include this info in the source: if you can
think of a
good way to phrase it that would make sense to other people, I'd be
happy to
include it in the patch.
I'm thinking about it (as a matter of fact, the new docstring looks nice!), but i need to understand your code better. Below are my questions. http://codereview.appspot.com/5504106/diff/11013/lily/grob.cc File lily/grob.cc (right): http://codereview.appspot.com/5504106/diff/11013/lily/grob.cc#newcode128 lily/grob.cc:128: retval = *m; so retval is the current stencil, but only when it's not empty? http://codereview.appspot.com/5504106/diff/11013/lily/grob.cc#newcode175 lily/grob.cc:175: if (scm_is_string (id)) I understand that we're doing something if the grob already has an id set. http://codereview.appspot.com/5504106/diff/11013/lily/grob.cc#newcode177 lily/grob.cc:177: SCM expr = scm_list_3 (ly_symbol2scm ("id"), isn't 'id' a scheme-thingy already? I mean, in line 174, it is declared as SCM, so why convert it with ly_symbol2scm? http://codereview.appspot.com/5504106/diff/11013/lily/grob.cc#newcode178 lily/grob.cc:178: id, why store id two times? http://codereview.appspot.com/5504106/diff/11013/lily/grob.cc#newcode179 lily/grob.cc:179: retval.expr ()); do i understand correcly that retval stands for "return value" and is some kind of an object? http://codereview.appspot.com/5504106/diff/11013/lily/grob.cc#newcode181 lily/grob.cc:181: retval = Stencil (retval.extent_box (), expr); Do we overwrite the retval that was set earlier (in other if's)? Why? http://codereview.appspot.com/5504106/diff/11013/lily/stencil-interpret.cc File lily/stencil-interpret.cc (right): http://codereview.appspot.com/5504106/diff/11013/lily/stencil-interpret.cc#newcode81 lily/stencil-interpret.cc:81: (*func) (func_arg, scm_list_2 (ly_symbol2scm ("start-enclosing-id-node"), id)); this line is longer than 80 chars, do we care about it? http://codereview.appspot.com/5504106/diff/11013/lily/stencil-interpret.cc#newcode82 lily/stencil-interpret.cc:82: interpret_stencil_expression (scm_caddr (expr), func, func_arg, o); i understand that we extract actual stencil here and interpret it again, but what these func's do? http://codereview.appspot.com/5504106/ _______________________________________________ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
