Re: Fix spacing for accidentals in tied chords

[joeneeman]

http://codereview.appspot.com/28134/diff/1/3
File lily/accidental-engraver.cc (right):

http://codereview.appspot.com/28134/diff/1/3#newcode431
Line 431: Grob *newa = 0;
Please use a more descriptive name. Also, add a comment regarding why
you need two copies of the accidental.

http://codereview.appspot.com/28134/diff/1/4
File lily/accidental-placement.cc (right):

http://codereview.appspot.com/28134/diff/1/4#newcode30
Line 30: Accidental_placement::add_accidental (Grob *me, Grob *a, Grob
*newa)
Please use more descriptive parameter names.

http://codereview.appspot.com/28134/diff/1/4#newcode71
Line 71: if (break_reminder)
slightly cleaner, perhaps, to do
SCM accidental_sym = ly_symbol2scm(break_reminder ?
"accidental-break-reminder-grobs" : "accidental_grobs");

and then
accs = me->get_object(accidental_sym);
...
me->set_object(accidental_sym, accs);

http://codereview.appspot.com/28134/diff/1/4#newcode97
Line 97: ret.push_back(a);
space before (

http://codereview.appspot.com/28134/diff/1/4#newcode433
Line 433: return;
Don't need the return here

http://codereview.appspot.com/28134/diff/1/4#newcode436
Line 436: void Accidental_placement::filter_tied_accidentals (Grob * me)
Rather than coping every accidental and then filtering out some, have
you considered delaying the copying until calc_positioning_done and only
copying the accidentals you care about? This also has the advantage of
hiding more stuff in accidental-placement instead of modifying
accidental-engraver.

http://codereview.appspot.com/28134/diff/1/4#newcode460
Line 460: "accidental-break-reminder-grobs "
Only properties go here, not objects.

http://codereview.appspot.com/28134/diff/1/5
File lily/accidental.cc (right):

http://codereview.appspot.com/28134/diff/1/5#newcode181
Line 181: }
I'm a little confused about this. Depending on whether a group of
accidentals is at the beginning of a line or not, you want either all of
the break-reminder accidentals or all of the non-break-reminder
accidentals to suicide. It's not clear to me that this is achieved by
the above code.

http://codereview.appspot.com/28134/diff/1/8
File lily/ledger-line-spanner.cc (right):

http://codereview.appspot.com/28134/diff/1/8#newcode237
Line 237: if (Grob *g = unsmob_grob (me->get_property
("accidental-grob")))
get_object (and subsequently, too)

http://codereview.appspot.com/28134/diff/1/13
File scm/define-grob-properties.scm (right):

http://codereview.appspot.com/28134/diff/1/13#newcode835
Line 835: @var{groblist})} entries. Includes accidentals that will be
printed if at the beginning of the line.")
"Consists of" is more accurate than "Includes", I think.

http://codereview.appspot.com/28134/diff/1/13#newcode841
Line 841: @var{groblist})} entries. Does not include tied accidentals
that won't be printed mid-line.")
Can you rephrase to avoid the double-negative?

http://codereview.appspot.com/28134