I'm not a programmer, but accustomed to doing code review as a systems engineer.
Change stringTunings entries from semitones to pitches This lays the foundation for creating a TabKey grob
Presumably the idea is to store the correct spelling of the note in the future TabKey, should anyone tune a string to des or cis. Tell us if there is a less-obvious reason. I *thought* that this would make \transpose, when applied to the whole score, shift the tuning as well. When I tried your patch, though, only the music was transposed and not the tuning. I think that is fine; I'm just jiggling our brains looking for side-effects. http://codereview.appspot.com/3842041/diff/6001/Documentation/notation/fretted-strings.itely File Documentation/notation/fretted-strings.itely (right): http://codereview.appspot.com/3842041/diff/6001/Documentation/notation/fretted-strings.itely#newcode477 Documentation/notation/fretted-strings.itely:477: A string tuning is a Scheme list of string pitches, Your \makeStringTuning allows us to proceed without knowing this. If you move the "Any desired" and "As a convenience" paragraphs up here, with a bit of rewording and with the example, then readers are spared the trouble of reading and then partially /un-learning/ these paragraphs. http://codereview.appspot.com/3842041/diff/6001/ly/declarations-init.ly File ly/declarations-init.ly (right): http://codereview.appspot.com/3842041/diff/6001/ly/declarations-init.ly#newcode120 ly/declarations-init.ly:120: \include "property-init.ly" the re-ordering looks nice, but any other reason? http://codereview.appspot.com/3842041/diff/6001/ly/string-tunings-init.ly File ly/string-tunings-init.ly (right): http://codereview.appspot.com/3842041/diff/6001/ly/string-tunings-init.ly#newcode28 ly/string-tunings-init.ly:28: (ly:parser-define! parser tuning-symbol (reverse pitches)))) ordering. If we reverse here, the two methods of entering a tuning require opposite orders of entry. An arbitrary decision was made in 2002 to enter the tuning and number strings in the top-to-bottom vertical order of the printed tablature, and no inputs to that decision have changed. http://codereview.appspot.com/3842041/diff/6001/ly/string-tunings-init.ly#newcode43 ly/string-tunings-init.ly:43: (make-music 'SequentialMusic 'void #t))) Is it possible to return a PropertySet event instead? If so, we could call it \setStringTuning and define the tuning where it is \set : \new TabStaff { \setStringTuning TabStaff.stringTunings <c' g' d'' a''> http://codereview.appspot.com/3842041/diff/6001/ly/string-tunings-init.ly#newcode96 ly/string-tunings-init.ly:96: ;; orchestral strings Why bother to add more default tunings? With the semitone method, people needed help to get them right, but your patch makes it easier to enter a tuning than to look up the name. http://codereview.appspot.com/3842041/ _______________________________________________ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
