Re: Allows user to set ChordName text (issue 6496085)

[address@hidden]

> http://codereview.appspot.com/6496085/diff/1/lily/chord-name-engraver.cc#newcode87
> lily/chord-name-engraver.cc:87: {
> On 2012/09/06 08:50:40, dak wrote:
>> What kind of contorted logic and guessing game is that?
>> if (make_markup)
>> {
>>    [old code ending in setting "text"]
>> }
> 
>> Please don't obfuscate code just to save reindentation.
> 
> This comment has not been addressed.
> 

I think I misunderstood your comment - could you show what you mean by 
proposing an alternative (in pseudo-code if you'd like) to the code that's in 
there?

> http://codereview.appspot.com/6496085/diff/1/lily/chord-name-engraver.cc#newcode136
> lily/chord-name-engraver.cc:136: markup = scm_call_4 (name_proc,
> pitches, bass, inversion,
> On 2012/09/06 08:50:40, dak wrote:
>> You do all the calculation and then throw it away?  Where is the
> point?
> 
> This comment has not been addressed.  If neither rest_event nor
> make_markup are set, you calculate pitches, bass, inversion and throw
> them away.

I'm not sure what you mean - every time these are calculated, these are used on 
line 139.  Unless I am reading the code incorrectly.

> 
> http://codereview.appspot.com/6496085/diff/1/lily/chord-name-engraver.cc#newcode149
> lily/chord-name-engraver.cc:149: && ly_is_equal (chord_as_scm,
> last_chord_))
> On 2012/09/06 09:59:09, MikeSol wrote:
>> On 2012/09/06 08:50:40, dak wrote:
>> > If one is doing the chord calculation manually, you can't make the
> decision of
>> > whether a chord changed based on the automatic calculation.  For
> better or
>> > worse, you need to compare the computed chord versions/text.
> 
>> To respond to your points above, I don't throw away the values above
> because
>> they're used here.
> 
> I don't see where the current code version would use them.  Can you
> point that out to me?
> 

They're no longer used as per your suggestion to compare text instead of these 
values.

> http://codereview.appspot.com/6496085/