Re: Issue 4154: Compact Chord Symbols Patch (issue 153160043 by d...@gnu.org)
On 2014/10/09 12:02:16, richard_rshann.plus.com wrote: Well, that depends what I meant by the existing code - the specific file I was modifying calls chordRootNamer which is initialized to note-name-markup which is in chord-names.scm:62, and that is where the quoted construct exists in the current lilypond code, in fact I see it is repeated several times in that file. Well, pulling code from a different file and functionality in parts into a different file where they are basically integrated as flat code into some parts of the code while other parts still call the other file -- that's a total maintenance and reading nightmare. If the duplicated code needs changes or maintenance, it is quite improbable that someone working on it will find and change *both* copies as needed. It would be best to extend the code to be able to do both tasks. If that is not reasonably possible, it would be good to factor out common elements but keep the code doing the non-common parts in the same file. https://codereview.appspot.com/153160043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Issue 4154: Compact Chord Symbols Patch (issue 153160043 by d...@gnu.org)
On Sat, 2014-10-18 at 17:19 +, d...@gnu.org wrote: On 2014/10/09 12:02:16, richard_rshann.plus.com wrote: Well, that depends what I meant by the existing code - the specific file I was modifying calls chordRootNamer which is initialized to note-name-markup which is in chord-names.scm:62, and that is where the quoted construct exists in the current lilypond code, in fact I see it is repeated several times in that file. Well, pulling code from a different file and functionality in parts into a different file where they are basically integrated as flat code into some parts of the code while other parts still call the other file -- that's a total maintenance and reading nightmare. If the duplicated code needs changes or maintenance, it is quite improbable that someone working on it will find and change *both* copies as needed. as I say, it is not a question of both, the construct is quite short and is repeated several times. But, of course, it makes it worse to repeat it in a different file. And more to the point, a different (if more direct) mechanism is used for compact symbols than for the other cases. It would be best to extend the code to be able to do both tasks. If that is not reasonably possible, it would be good to factor out common elements but keep the code doing the non-common parts in the same file. https://codereview.appspot.com/153160043/ The problem with doing the compact symbols in the same file as the others is that there seems to be no access to the context property that gives the scaling needed. I asked about this on the mailing list, http://lists.gnu.org/archive/html/lilypond-user/2014-09/msg00525.html but it seemed this couldn't be done. It is completely unclear to me why the creation of markup for a chord symbol is spread over several files in the first place - it seems to result in the loss of the context information at the point where the root name markup is being created. Is there a way in which chordRootNamer can take an extra scale argument, or is there, indeed a way to access this value as the context property chordCompactScale ? If so, I could easily move that code back in to chord-names.scm Richard ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Issue 4154: Compact Chord Symbols Patch (issue 153160043 by d...@gnu.org)
On Wed, 2014-10-08 at 17:41 +, d...@gnu.org wrote: Reviewers: , https://codereview.appspot.com/153160043/diff/1/scm/chord-ignatzek-names.scm File scm/chord-ignatzek-names.scm (right): https://codereview.appspot.com/153160043/diff/1/scm/chord-ignatzek-names.scm#newcode98 scm/chord-ignatzek-names.scm:98: (vector-ref #(C D E F G A B) (ly:pitch-notename This looks like a bad idea. It does not obey the various chord name languages. It does not use the same callbacks. It is a large duplication of code not connected with the other code and not using the same options, functionality and interfaces. When I saw this (on the lilypond-devel mailing list) I thought this was a comment about the existing code. The code quoted is the existing code, which I haven't changed. My code does not generate any chord names, so it can't obey any chord name languages, it just typesets the elements of the chord name in a more compact fashion. Sorry for not replying earlier, but I did think that my patch had sparked a debate about the quality of the original file, not my patch to it, which is purely concerned with how the markup is created for the chord names as generated by the existing code. Richard Can't you try to integrate this kind of code better with the existing code, both regarding the code that is called as well as the naming conventions and functionality that are available? Description: Issue 4154: Compact Chord Symbols Patch Please review this at https://codereview.appspot.com/153160043/ Affected files (+100, -26 lines): M scm/chord-ignatzek-names.scm M scm/define-context-properties.scm ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Issue 4154: Compact Chord Symbols Patch (issue 153160043 by d...@gnu.org)
On 2014/10/09 11:08:16, richard_rshann.plus.com wrote: On Wed, 2014-10-08 at 17:41 +, mailto:d...@gnu.org wrote: Reviewers: , https://codereview.appspot.com/153160043/diff/1/scm/chord-ignatzek-names.scm File scm/chord-ignatzek-names.scm (right): https://codereview.appspot.com/153160043/diff/1/scm/chord-ignatzek-names.scm#newcode98 scm/chord-ignatzek-names.scm:98: (vector-ref #(C D E F G A B) (ly:pitch-notename This looks like a bad idea. It does not obey the various chord name languages. It does not use the same callbacks. It is a large duplication of code not connected with the other code and not using the same options, functionality and interfaces. When I saw this (on the lilypond-devel mailing list) I thought this was a comment about the existing code. The code quoted is the existing code, which I haven't changed. It most certainly isn't the existing code. It is not even present anywhere in the same file. You might have copied parts of it into the file from some other file, but even git blame -w -C -C -C does not point to any purported other file as a potential source. https://codereview.appspot.com/153160043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Issue 4154: Compact Chord Symbols Patch (issue 153160043 by d...@gnu.org)
On Thu, 2014-10-09 at 11:36 +, d...@gnu.org wrote: On 2014/10/09 11:08:16, richard_rshann.plus.com wrote: On Wed, 2014-10-08 at 17:41 +, mailto:d...@gnu.org wrote: Reviewers: , https://codereview.appspot.com/153160043/diff/1/scm/chord-ignatzek-names.scm File scm/chord-ignatzek-names.scm (right): https://codereview.appspot.com/153160043/diff/1/scm/chord-ignatzek-names.scm#newcode98 scm/chord-ignatzek-names.scm:98: (vector-ref #(C D E F G A B) (ly:pitch-notename This looks like a bad idea. It does not obey the various chord name languages. It does not use the same callbacks. It is a large duplication of code not connected with the other code and not using the same options, functionality and interfaces. When I saw this (on the lilypond-devel mailing list) I thought this was a comment about the existing code. The code quoted is the existing code, which I haven't changed. It most certainly isn't the existing code. Well, that depends what I meant by the existing code - the specific file I was modifying calls chordRootNamer which is initialized to note-name-markup which is in chord-names.scm:62, and that is where the quoted construct exists in the current lilypond code, in fact I see it is repeated several times in that file. Richard ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Issue 4154: Compact Chord Symbols Patch (issue 153160043 by d...@gnu.org)
Reviewers: , https://codereview.appspot.com/153160043/diff/1/scm/chord-ignatzek-names.scm File scm/chord-ignatzek-names.scm (right): https://codereview.appspot.com/153160043/diff/1/scm/chord-ignatzek-names.scm#newcode98 scm/chord-ignatzek-names.scm:98: (vector-ref #(C D E F G A B) (ly:pitch-notename This looks like a bad idea. It does not obey the various chord name languages. It does not use the same callbacks. It is a large duplication of code not connected with the other code and not using the same options, functionality and interfaces. Can't you try to integrate this kind of code better with the existing code, both regarding the code that is called as well as the naming conventions and functionality that are available? Description: Issue 4154: Compact Chord Symbols Patch Please review this at https://codereview.appspot.com/153160043/ Affected files (+100, -26 lines): M scm/chord-ignatzek-names.scm M scm/define-context-properties.scm ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Compact Chord Symbols Patch
On Mon, 2014-10-06 at 20:06 +0100, James wrote: Richard, [..] Richard I have created http://code.google.com/p/lilypond/issues/detail?id=4154 I'll help shepherd this patch through via the standard review process for this. Thank you very much. I thought I would follow the instructions for contributing to LilyPond starting from the top at http://lilypond.org as I know how useful it is for a fresh pair of eyes to look over stuff like this. This lead me to the step after creating a patch where it suggested if you have a mentor email the patch, but didn't say what one was. So I improvised at this point and emailed the mailing list. I would have liked to run the regression tests at that stage, but I think I ran out of step-by-step instructions. Also See: http://lilypond.org/doc/v2.19/Documentation/contributor-big-page#commits-and-patches if you intend (or think you might want) to make further patches for other parts of LilyPond in the future Well, it is about 10 years since I last had to tweak the actual LilyPond code (at the time you had to type bass figures in the reverse order), which far exceeds my memory span for how-to-do-it instructions. I just manage to keep up with the processes for developing the Denemo LilyPond GUI. I would have found this development much easier if I could have avoided the use of a virtual machine for the actual git part of it (that is, if I had permission to create remote branches such as dev/compact-chords in the lilypond repository). Then I would only have needed the virtual box to compile and run the new version. As it is, I had to copy and paste from my virtual box out to the real machine, merge my changes and copy and paste them back, a process fraught with danger. (I think the LilyDev must have some way of sharing file systems but I didn't look into that). If I could have created a remote branch, modified and pushed back and then switched to LilyDev to pull the remote branch, compile and test that would have been perfect. Reading over the documentation you quote it seems that you do have contributors with the limited permission to create branches, but I didn't immediately see how to register for that... Richard ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Compact Chord Symbols Patch
On 07/10/14 09:08, Richard Shann wrote: On Mon, 2014-10-06 at 20:06 +0100, James wrote: Richard, [..] Richard I have created http://code.google.com/p/lilypond/issues/detail?id=4154 I'll help shepherd this patch through via the standard review process for this. Thank you very much. I thought I would follow the instructions for contributing to LilyPond starting from the top at http://lilypond.org as I know how useful it is for a fresh pair of eyes to look over stuff like this. This lead me to the step after creating a patch where it suggested if you have a mentor email the patch, but didn't say what one was. So I improvised at this point and emailed the mailing list. I would have liked to run the regression tests at that stage, but I think I ran out of step-by-step instructions. Also See: http://lilypond.org/doc/v2.19/Documentation/contributor-big-page#commits-and-patches if you intend (or think you might want) to make further patches for other parts of LilyPond in the future Well, it is about 10 years since I last had to tweak the actual LilyPond code (at the time you had to type bass figures in the reverse order), which far exceeds my memory span for how-to-do-it instructions. I just manage to keep up with the processes for developing the Denemo LilyPond GUI. I would have found this development much easier if I could have avoided the use of a virtual machine for the actual git part of it (that is, if I had permission to create remote branches such as dev/compact-chords in the lilypond repository). Then I would only have needed the virtual box to compile and run the new version. As it is, I had to copy and paste from my virtual box out to the real machine, merge my changes and copy and paste them back, a process fraught with danger. (I think the LilyDev must have some way of sharing file systems but I didn't look into that). If I could have created a remote branch, modified and pushed back and then switched to LilyDev to pull the remote branch, compile and test that would have been perfect. Reading over the documentation you quote it seems that you do have contributors with the limited permission to create branches, but I didn't immediately see how to register for that... Probably here http://lilypond.org/doc/v2.19/Documentation/contributor-big-page.html#commit-access I think we could improve the notes in the contributor's Guide generally. Having had to help three or four people over the last few months with a patch or two, I get how the instructions are probably rather confusing if not intimidating. I'll take a look and see what I can come up with. The structure of the Contributor's Guide is much 'looser' (shall we say) than the manuals on how to use LilyPond. That's not to say it is inaccurate, but that perhaps we could condense some of it down or organize more logically certain aspects. Anyway, I haven't yet done anything with the Patch simply because I was waiting on if it was really a snippet than something that we want to add into the core code. I am not a 'developer' as such (I don't write code - just Doc and manage patches) so don't have any real knowledge of these things. James ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Compact Chord Symbols Patch
On Tue, 2014-10-07 at 12:53 +0100, James wrote: On 07/10/14 09:08, Richard Shann wrote: On Mon, 2014-10-06 at 20:06 +0100, James wrote: Richard, [..] Richard I have created http://code.google.com/p/lilypond/issues/detail?id=4154 I'll help shepherd this patch through via the standard review process for this. Thank you very much. I thought I would follow the instructions for contributing to LilyPond starting from the top at http://lilypond.org as I know how useful it is for a fresh pair of eyes to look over stuff like this. This lead me to the step after creating a patch where it suggested if you have a mentor email the patch, but didn't say what one was. So I improvised at this point and emailed the mailing list. I would have liked to run the regression tests at that stage, but I think I ran out of step-by-step instructions. [...] Reading over the documentation you quote it seems that you do have contributors with the limited permission to create branches, but I didn't immediately see how to register for that... Probably here http://lilypond.org/doc/v2.19/Documentation/contributor-big-page.html#commit-access Well this seems to be a rather general access - it warns the user not to push to master but to staging - perhaps I was wrong thinking you had another tier of access... I think we could improve the notes in the contributor's Guide generally. I hasten to say that what you have created is very good, I was able to get to the submitting a patch stage without any serious problems. Having had to help three or four people over the last few months with a patch or two, I get how the instructions are probably rather confusing if not intimidating. I'll take a look and see what I can come up with. The structure of the Contributor's Guide is much 'looser' (shall we say) than the manuals on how to use LilyPond. That's not to say it is inaccurate, but that perhaps we could condense some of it down or organize more logically certain aspects. Anyway, I haven't yet done anything with the Patch simply because I was waiting on if it was really a snippet Well, I certainly tried to do this without attacking the core code itself, but I'd love it if there were some point of entry (a scheme engraver?) that would allow chord symbols to be drawn by a user generated routine (this is everything, including bass inversions). than something that we want to add into the core code. I am not a 'developer' as such (I don't write code - just Doc and manage patches) so don't have any real knowledge of these things. Thank you very much Richard ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Compact Chord Symbols Patch
On Tue, Oct 07, 2014 at 12:53:53PM +0100, James wrote: I think we could improve the notes in the contributor's Guide generally. Having had to help three or four people over the last few months with a patch or two, I get how the instructions are probably rather confusing if not intimidating. I think the most important point would be to have a single checklist for what should be done. Two years ago, the CG had at least three such lists, and I was aware of mistakes in two of them. I don't know what's changed sinc then, though. Having a single list has two advantages: it's an obvious thing to refer to, and since it focuses attention from both readers and CG-writers, mistakes should be more obvious (and thus easier to fix). Cheers, - Graham ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Compact Chord Symbols Patch
Richard Shann rich...@rshann.plus.com writes: Well, I certainly tried to do this without attacking the core code itself, but I'd love it if there were some point of entry (a scheme engraver?) that would allow chord symbols to be drawn by a user generated routine (this is everything, including bass inversions). The core code, both for identifying chord names as well as for formatting them, sucks. I am not overly enthused about the conversion from chord mode entry to actual chords, but meddling with that rather than later stages would make it hard to use the same input for, say, keyboard, guitar, and accordion chords. At any rate, the various chord name and chord naming and identification functions are designed with some generality and replaceability in mind, but the mechanisms available for that do not work well in practice except possibly per-document, and that would defeat producing, say, keyboard and accordion chords from the same input. -- David Kastrup ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Compact Chord Symbols Patch
I have created a patch for LilyPond (following the guidelines in the documentation). I enclose a file that creates a chord chart from this version, though I realize that it is too verbose for use in documenting the new functionality. I'll be happy to create something smaller if it looks like the patch will be welcomed. (This one gives an idea of the output). Richard From 8a3569e75d1fdff5318497051bb840e330162e47 Mon Sep 17 00:00:00 2001 From: Richard Shann rich...@rshann.plus.com Date: Mon, 6 Oct 2014 18:09:51 +0100 Subject: [PATCH] Allow creation of compact chord symbols Compact chord symbols have the elements of the chord (the root name, quality and bass-inversion) packed tightly together so as to allow the creation of fakebooks. Nowadays these will often be stored on hand held devices. This patch allows the default chord symbols to be drawn in such a manner, by defining the context property chordCompactScale. Where this is not defined, the default behavior is maintained. --- scm/chord-ignatzek-names.scm | 125 ++ scm/define-context-properties.scm | 1 + 2 files changed, 100 insertions(+), 26 deletions(-) diff --git a/scm/chord-ignatzek-names.scm b/scm/chord-ignatzek-names.scm index 22f54fe..c7671cb 100644 --- a/scm/chord-ignatzek-names.scm +++ b/scm/chord-ignatzek-names.scm @@ -88,6 +88,46 @@ (ly:context-property context 'chordRootNamer) ;; name-root nn))) + (define (compact-name-root pitch scale) + (let* ((alt (ly:pitch-alteration pitch))) + (make-line-markup +(list + (make-bold-markup +(make-scale-markup '(0.5 . 1) +(make-simple-markup +(vector-ref #(C D E F G A B) (ly:pitch-notename +pitch) + (if (= alt 0) +(make-hspace-markup 0.1) +(make-line-markup +(list + (make-hspace-markup 0.1) + (make-fontsize-markup -7 (make-raise-markup 1.2 ;(* 1 scale) + (alteration-text-accidental-markup alt))) + (make-hspace-markup -0.5 + (define (name-inversion pitch scale) +(let* ((alt (ly:pitch-alteration pitch))) + (make-line-markup +(list + (make-raise-markup 1 +(make-scale-markup '(0.75 . 0.5) +(make-bold-markup (make-simple-markup / + (make-bold-markup +(make-scale-markup '(0.5 . 0.75) +(make-simple-markup +(vector-ref #(C D E F G A B) (ly:pitch-notename +pitch) + (if (= alt NATURAL) +(make-hspace-markup 2) +(make-line-markup +(list + (make-hspace-markup 0.1) + (make-fontsize-markup -7 + (if (= alt SHARP) +(make-raise-markup 0.1 +(alteration-text-accidental-markup alt)) +(make-raise-markup 0.2 +(alteration-text-accidental-markup alt))) (define (is-natural-alteration? p) (= (natural-chord-alteration p) (ly:pitch-alteration p))) @@ -169,9 +209,42 @@ work than classifying the pitches. (make-line-markup total))) +(define (markup-formatting sep root-markup prefixes to-be-raised-stuff bass-pitch) +(define bass-inv #f) +(define slashsep (ly:context-property context 'slashChordSeparator)) +(define scale (ly:context-property context 'chordCompactScale)) +(if (pair? scale) +(begin +(set! bass-inv (if (ly:pitch? bass-pitch) + (name-inversion bass-pitch scale) +empty-markup)) + (make-scale-markup scale (make-combine-markup +(make-line-markup (list root-markup +(make-scale-markup '(0.4 . 0.6) +(make-bold-markup (conditional-kern-before (markup-join prefixes sep) +(and (not (null? prefixes)) + (= (ly:pitch-alteration root) NATURAL)) +(ly:context-property context 'chordPrefixSpacer +(make-scale-markup '(0.4 . 0.6) (make-bold-markup to-be-raised-stuff +(make-raise-markup -2 bass-inv +(begin +(set! bass-inv +(if (ly:pitch? bass-pitch) + (list slashsep (name-note bass-pitch #f)) + '())) +(make-line-markup +
Re: Compact Chord Symbols Patch
Richard, On 06/10/14 18:30, Richard Shann wrote: I have created a patch for LilyPond (following the guidelines in the documentation). I enclose a file that creates a chord chart from this version, though I realize that it is too verbose for use in documenting the new functionality. I'll be happy to create something smaller if it looks like the patch will be welcomed. (This one gives an idea of the output). Richard I have created http://code.google.com/p/lilypond/issues/detail?id=4154 I'll help shepherd this patch through via the standard review process for this. Also See: http://lilypond.org/doc/v2.19/Documentation/contributor-big-page#commits-and-patches if you intend (or think you might want) to make further patches for other parts of LilyPond in the future, as this documents the group's method of uploading a patch for proper review and test; and saves having to involve a third party (like me) and/or guaranteeing some kind of proper review and so that your work is not ignored. James ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Compact Chord Symbols Patch
Developers, On 06/10/14 18:30, Richard Shann wrote: I have created a patch for LilyPond (following the guidelines in the documentation). I enclose a file that creates a chord chart from this version, though I realize that it is too verbose for use in documenting the new functionality. I'll be happy to create something smaller if it looks like the patch will be welcomed. (This one gives an idea of the output). Richard Although I have already created a tracker for this; http://code.google.com/p/lilypond/issues/detail?id=4154 is this actually one of those 'Scheme engraver' type-things that Urs was talking about a week or so back or is this simply best put in the LilyPond Snippet Repository (LSR)? Otherwise I'll feed it in via the normal patch review process. I just wanted clarification before I (or Richard) spend any time on this. James ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
