Re: Transposable guitar fret diagrams

[Han-Wen Nienhuys]

Looks good to me.  Just one concern:

  (define (get-chord-pitches chordmode-chord)

I suspect similar functionality is already present, since some
functions in msuic-functions-init.ly use it.

On Thu, Jul 31, 2008 at 10:33 PM, Carl D. Sorensen <address@hidden> wrote:
> Han-wen,
>
> I've moved to a void music function to avoid defchord = ...
>
> I've moved from a global variable for the table to one defined in
> default-fretboards-init.ly.
>
> I've changed from a boolean useDefaultDiagrams to a hash-table
> defaultDiagramTable.
>
> Does this meet the needs?
>
> Carl
>
>
>
>
> On 7/31/08 9:47 AM, "Han-Wen Nienhuys" <address@hidden> wrote:
>
>> A couple of comments:
>>
>>  - there is
>>
>>    defchord =...
>>
>> everywhere. Why?
>>
>> - You're storing the fretboards in a global variable, so changes to it
>> will leak from one file into the other.  Declare the table in the init
>> .ly files, like
>>
>> #(define default-frets ... )
>>
>> and have the music function add to that.   It should probably be a
>> list, so it is easy to modify.
>>
>> Then in the engraver init, you can assign
>>
>>   defaultFretDiagrams = #default-frets
>>
>> Then, you can get the default frets using (ly:context-property context
>>  'defaultFretDiagram) when you need it.
>> Once the fret_diagram_engraver comes alive, it can compute the hash
>> table from the list to have efficient lookups.
>>
>> I suspect that using a list will make it easier to create a sane
>> mechanism for users to extend the default fret diagrams.  With a bit
>> of luck, you could even do
>>
>>   \override defaultFretDiagrams #(make-fret-key ... ) = #...fret-diagram..
>>
>> Another option is to have 2 properties: the hash table (as a context
>> property) for the system defaults (efficiency!) and a list for user
>> overrides.
>>
>> Perhaps you can also drop the boolean var, and just use the list/table
>> context property.
>>
>> On Thu, Jul 31, 2008 at 11:43 AM, Carl D. Sorensen <address@hidden> wrote:
>>>
>>>
>>>
>>> On 7/31/08 8:18 AM, "Han-Wen Nienhuys" <address@hidden> wrote:
>>>
>>>> On Thu, Jul 31, 2008 at 1:28 AM, Carl D. Sorensen <address@hidden>
>>>> wrote:
>>>>> I've entered diagrams for 7 chords for each key (major, minor, augmented,
>>>>> diminished, dominant7, major7, and minor7.
>>>>>
>>>>> I've developed a user-accessible method for entering diagrams, so users 
>>>>> can
>>>>> enter whichever diagrams they want.  And they can have multiple diagrams
>>>>> for
>>>>> the same chord, by using different octaves in the input.
>>>>
>>>> Cool; can you show me the diff? I'm probably interested in pull origin
>>>> and then diff origin.
>>>
>>> I've attached the diff from git format-patch.  I can go ahead and push it if
>>> it's acceptable.
>>>
>>> There are four remaining issues:
>>>
>>> 1) I would like to have ly/default-fretboard-init.ly automatically loaded,
>>> so that users don't need to include it.  I remember seeing some discussion
>>> of how to do this on -devel, but I couldn't find it in the archives.  What
>>> do I need to do to get a file on the "automatically include me" list?  Or
>>> should this be left off the automatic include list, and expect users to
>>> include it if they want to use it?
>>>
>>> 2) I would like to add predefines to turn on and off the default fret
>>> diagrams:
>>>
>>> \defaultFretDiagramsOn and \defaultFretDiagramsOff.
>>>
>>> 3) I need to add documentation to fretted-strings.itely.
>>>
>>> 4) I need to add a regtest (which can be modified from fretboardtest.ly).
>>>
>>>>
>>>>> 2) Can we try to get this in before we release 2.12, or do we need to wait
>>>>> and put it in 2.13?
>>>>
>>>> If this does not destabilize other features (and I think it does not),
>>>> we can put it in whenever we want.
>>>>
>>>
>>> The only destabilization I can see is that the test file doesn't work
>>> properly with the vertical spacing algorithm.  For some reason (perhaps the
>>> fretboard space isn't included?), the default page-breaker forces the
>>> fretboards off the bottom of the page.  But I don't think that's the fault
>>> of the new code; it's just an existing bug that is demonstrated.
>>>
>>> I'll put this in once you approve.
>>>
>>> Thanks,
>>>
>>> Carl
>>>
>>>
>>
>>
>>
>> --
>> Han-Wen Nienhuys - address@hidden - http://www.xs4all.nl/~hanwen
>
>



-- 
Han-Wen Nienhuys - address@hidden - http://www.xs4all.nl/~hanwen