[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Allows user to set ChordName text (issue 6496085)
From: |
address@hidden |
Subject: |
Re: Allows user to set ChordName text (issue 6496085) |
Date: |
Wed, 12 Sep 2012 14:14:29 +0300 |
On 12 sept. 2012, at 14:05, address@hidden wrote:
>
> http://codereview.appspot.com/6496085/diff/1/lily/chord-name-engraver.cc
> File lily/chord-name-engraver.cc (right):
>
> 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. As for the present point, that is an interesting
>> conundrum...I'll drum up some logic for that. That may eliminate the
> need for
>> guarding the values above having to do with chord changes, in which
> point the if
>> else statement will be able to be simplified.
>
> Is there a reason you did none of the above, addressed only the very
> first point of my review, ignored all of the rest and pushed?
>
There are four points in your previous review and I addressed all of them in
the code before I pushed to staging. If it's unclear how any of them are
addressed, please make reference to a specific one and I can put a comment in
the code to clear things up.
Cheers,
MS
- Allows user to set ChordName text (issue 6496085), dak, 2012/09/06
- Re: Allows user to set ChordName text (issue 6496085), mtsolo, 2012/09/06
- Re: Allows user to set ChordName text (issue 6496085), dak, 2012/09/12
- Re: Allows user to set ChordName text (issue 6496085),
address@hidden <=
- Re: Allows user to set ChordName text (issue 6496085), dak, 2012/09/12
- Re: Allows user to set ChordName text (issue 6496085), dak, 2012/09/12
- Re: Allows user to set ChordName text (issue 6496085), dak, 2012/09/12