Re: [Lilypond-auto] Issue 4163 in lilypond: Patch: Woodwind Diagrams - Added recorder diagram

[lilypond]

Updates:
        Summary: Patch: Woodwind Diagrams - Added recorder diagram
        Labels: -Patch-push Patch-review

Comment #7 on issue 4163 by address@hidden: Patch: Woodwind Diagrams -  
Added recorder diagram
https://code.google.com/p/lilypond/issues/detail?id=4163

I'm not comfortable pushing this, there were some things on the original  
email

http://lists.gnu.org/archive/html/bug-lilypond/2014-10/msg00024.html

(after which I offered to help move a patch through the list) that I am  
simply not qualified to say if it is still OK to push or still needs some  
work (considering there are probably some other issues with Woodwind  
Diagrams in general that these may or may not relate too in the LilyPond  
Code).

From the original email by Erik:

--snip--

attached is a recorder diagram patch, would love for feedback and for
it to be incorporated.  hopefully it's ok it's not actually in patch
format, it just drops into display-woodwind-diagrams.scm (of course a
corresponding entry needs to be added to
woodwind-data-assembly-instructions in that file as well).

my biggest problems:

- 1h (half-covered) works for eg 'flute two', but on my recorder thumb
(T) it doesn't work -- it just shows fully covered.

- why are partial covers shown as shaded, then there is no distinction
w/trills (ie 1h and 1hT are identical)?

i don't know scheme, so i was mainly pattern-matching from existing
diagrams.  some issues i had while trying to figure this out:

- what is the purpose of the baked-in cc/lh/rh grouping?

- i can't find doc for draw-instructions rules -- seems to determine
whether keys are hidden unless specified -- i didn't want that
behavior, but was stuck unexpectedly getting it for a while.

- difference between identity and return-1 -- they sound identical to
me (when xy-scaling), but gave different results.

- the style used encourages a lot of duplicated code -- it needs to be
refactored so that keys are defined as simple declarative structures
(with properties like name, group, position, complexity, stencil,
textual-representation) and graphical/textual-commands derived
therefrom.

- similarly, key positions should be described in relative terms --
most stuff is absolute w/brittle hardcoding.

- explicit support for when there is no text-override (key name
instead of graphical stencil) available.  i tripped across a
previously reported bug that doesn't seem to have made it to the issue
list even though it's a crash:
(http://lists.gnu.org/archive/html/lilypond-user/2014-09/msg00405.html):

  c^
  \markup \override #'(graphical . #f) {
      \woodwind-diagram
        #'tin-whistle
        #'()
  }

C:/Program Files  
(x86)/LilyPond/usr/share/lilypond/current/scm/display-woodwind-
diagrams.scm:1977:20: In procedure = in expression (= 0 (assoc-get node  
draw-ins
tructions)):
C:/Program Files  
(x86)/LilyPond/usr/share/lilypond/current/scm/display-woodwind-
diagrams.scm:1977:20: Wrong type: #f

also broken for saxophone (a different error tho), but works for piccolo.
for tin-whistle, seems to be from use of CENTRAL-COLUMN-HOLE-H-LIST
instead of CENTRAL-COLUMN-HOLE-LIST.

- when using \override #'(graphical . #f) (is there a way to call this
"textual"?) with an empty keylist, should show all possible text
stencils (as it currently does for graphical).  also, how are partial
coverings/trills handled in this case?

- would be nice if i didn't have to edit display-woodwind-diagrams.scm
and instead could just \include my raw .scm file from a .ly file.

thanks!
-erik

--snip--


I am attaching the patch to the incident based on current master as of now,  
it probably at least needs adding to the various other files like the  
couple of snippets that exist showing them (trivial) but there are some  
other 'woodwind' files in the main code that may also need adjusting  
like 'define-woodwind-diagrams.scm' for instance.

I am setting this back to review. But if I get nothing back at all, I will  
set it back to 'Needs_work'



Attachments:
        recorder.ly  1.5 KB
        recorder-display.scm  7.6 KB
        recorder.png  23.7 KB
        0001-Added-Recorder-Diagram-to-Woodwind-diagrams.scm.patch  8.8 KB

--
You received this message because this project is configured to send all  
issue notifications to this address.
You may adjust your notification preferences at:
https://code.google.com/hosting/settings