[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Rewrite chordnames - disentangle data from formatting (issue 2234200

From: thomasmorley65
Subject: Re: Rewrite chordnames - disentangle data from formatting (issue 223420043 by address@hidden)
Date: Sun, 26 Apr 2015 20:24:08 +0000

On 2015/04/13 02:22:15, Carl wrote:
I think that this is a great start.  You're working in a really
complex area,
and trying to sort it out.  Good for you!

I've made some specific comments below.  I think the separation
between list
creation and markup creation needs to be made stronger and more
probably we need to change the property names.
File ly/ (right):
ly/ chordRootNamer = #note-name->list
Perhaps we should rename chordRootNamer to something like
chordAnalyzer.  That
would show the meaning much more clearly.

Or perhaps more importantly, we should have a chordAnalyzer that takes
a chord
and returns a list, and a chordNamer that takes a list an returns a
markup.  At
that point, we'd really have a clear separation of the two functions,
and if you
were happy with the Analyzer, you would only need to change the Namer.
File ly/ (right):
ly/ \set chordRootNamer =
#(chord-name->italian-list #t)
In scm/chord-generic-names.scm you are saying that a namer returns a
markup, not
a list.  I think that is probably a pretty good use.

Perhaps we should think of two different kinds of things:  "analyzers"
convert chord names to lists, and "namers" that convert lists to
markups.  And
we should hold strong to that convention.

I think it's a mistake to have a "namer" return a list, since the
thing that is
printed as a ChordName is a markup.  A namer should produce a markup,
in my
File scm/chord-generic-names.scm (right):
scm/chord-generic-names.scm:33: "Return a markup for @var{pitch}, with
name of
@var{pitch} and a (possible)
I think the name of this function should change to something like

scm/chord-generic-names.scm:35: (let* ((note-namer (note-name->list
pitch #f))
I don't like the name note-namer here; it sounds like a function,
rather than an

Perhaps note-alist  or note-name-alist   or structure-alist   or even

Or you could eliminate the alist part and call it

note-structure  or note-elements (although elements has a specific
meaning that probably makes this name undesirable) or note-parts.
File scm/chord-ignatzek-names.scm (right):
scm/chord-ignatzek-names.scm:288: ;; Build the list for the chord-data
'root-info, 'slash-chord-separato,
Isn't slash-chord-separator part of the display, rather than part of
the chord

It seems to me that this patch is mostly maintaining the mix of
parsing and
display; it's just putting the stuff into a list first.
scm/chord-ignatzek-names.scm:395: ;;;; Step 2: Define formatter for
chord-elements using this list
I'm not sure how this separation between step 1 and step 2 really
the stated goal of the patch.  Can you give an example of how this
makes it
easier to define a new display style for a chord?
scm/chord-ignatzek-names.scm:427: (make-hspace-markup (if (=
alteration FLAT)
0.57285385 0.5))
Magic numbers are of concern, both here and later.  At the least,
there should
be a TODO comment.
File scm/chord-name.scm (right):
scm/chord-name.scm:53: (define-safe-public ((chord-name->german-list
Shouldn't all these functions be combined into one

(define-safe-public ((chord-name->note-alteration-list language-name
... )

It seems like to really separate it out, we need to do more than just
return a
list instead of a markup.  We should create a real logical structure
that will
separate things out cleanly.

Hi Carl,

most of the above comments needs still to be adressed, especially the
ones about namings.
Though, with the recent patchset I try to do a much deeper
disentangeling of structure and output.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]