Simplify coord-rotate (issue 263690043 by d...@gnu.org)

2015-10-10 Thread thomasmorley65
Great simplification! Though, I'd like to add two features 1) Making it possible to rotate at a different point other than (0 . 0) 2) Getting the needed exactness as discussed starting: http://lists.gnu.org/archive/html/lilypond-user/2015-10/msg00320.html Below my attempt following your advice.

Create \= command for setting spanner-id (issue 268050045 by d...@gnu.org)

2015-10-03 Thread thomasmorley65
LGTM Some nitpicks: https://codereview.appspot.com/268050045/diff/1/Documentation/notation/expressive.itely File Documentation/notation/expressive.itely (right): https://codereview.appspot.com/268050045/diff/1/Documentation/notation/expressive.itely#newcode724

Let note-by-number and rest-by-number be robust against overriding font-name (issue 260690043 by thomasmorle...@gmail.com)

2015-09-14 Thread thomasmorley65
Reviewers: , Message: Please review Is (export format-metronome-markup) still needed? Description: Let note-by-number and rest-by-number be robust against overriding font-name This is done by setting font-name #f as it is done for musicglyph already. Reverts some additions made by issue 3096,

Re: Issue 4591: Fix font-name overriding (issue 262330043 by truer...@gmail.com)

2015-09-07 Thread thomasmorley65
Some nitpick, see below. Otherwise LGTM https://codereview.appspot.com/262330043/diff/20001/Documentation/snippets/new/unfretted-headword.ly File Documentation/snippets/new/unfretted-headword.ly (right):

Re: Issue 4591: Fix font-name overriding (issue 262330043 by truer...@gmail.com)

2015-09-07 Thread thomasmorley65
https://codereview.appspot.com/262330043/diff/20001/Documentation/snippets/new/unfretted-headword.ly File Documentation/snippets/new/unfretted-headword.ly (right): https://codereview.appspot.com/262330043/diff/20001/Documentation/snippets/new/unfretted-headword.ly#newcode119

Re: Implement new markup-command combine-list (issue 264960043 by thomasmorle...@gmail.com)

2015-09-06 Thread thomasmorley65
On 2015/09/06 10:31:16, thomasmorley651 wrote: change name to overlay After it's been pushed I'll change said LSR-snippet accordingly. For now: PATch_NEW https://codereview.appspot.com/264960043/ ___ lilypond-devel mailing list

Re: Implement new markup-command combine-list (issue 264960043 by thomasmorle...@gmail.com)

2015-09-05 Thread thomasmorley65
On 2015/08/31 10:27:04, dak wrote: Ok, I've taken a look. Markups are so undelimited in their nature that there is not much of an option other than convert-ly actually knowing all markup commands with their signatures. That's doable but rather onerous. In addition, musicxml2ly uses the

Implement new markup-command combine-list (issue 264960043 by thomasmorle...@gmail.com)

2015-08-30 Thread thomasmorley65
Reviewers: , Message: Please review PATCH_NEW Description: Implement new markup-command combine-list Allows for \markup \combine-list { a list } Please review this at https://codereview.appspot.com/264960043/ Affected files (+23, -5 lines): M scm/define-markup-commands.scm Index:

Re: Implement new markup-command combine-list (issue 264960043 by thomasmorle...@gmail.com)

2015-08-30 Thread thomasmorley65
On 2015/08/30 11:44:52, dak wrote: https://codereview.appspot.com/264960043/diff/1/scm/define-markup-commands.scm File scm/define-markup-commands.scm (right): https://codereview.appspot.com/264960043/diff/1/scm/define-markup-commands.scm#newcode1758 scm/define-markup-commands.scm:1758:

Re: Clear fret-diagram- and harp-pedal-input-strings from whitespace (issue 257580043 by thomasmorle...@gmail.com)

2015-08-29 Thread thomasmorley65
On 2015/08/23 15:11:08, dak wrote: https://codereview.appspot.com/257580043/diff/1/scm/fret-diagrams.scm File scm/fret-diagrams.scm (right): https://codereview.appspot.com/257580043/diff/1/scm/fret-diagrams.scm#newcode45 scm/fret-diagrams.scm:45: (define (not-number-error input output) input

Re: Clear fret-diagram- and harp-pedal-input-strings from whitespace (issue 257580043 by thomasmorle...@gmail.com)

2015-08-29 Thread thomasmorley65
On 2015/08/26 08:55:14, dak wrote: https://codereview.appspot.com/257580043/diff/1/scm/lily-library.scm File scm/lily-library.scm (right): https://codereview.appspot.com/257580043/diff/1/scm/lily-library.scm#newcode793 scm/lily-library.scm:793: char-set:whitespace)) A mostly theoretic

Re: Clear fret-diagram- and harp-pedal-input-strings from whitespace (issue 257580043 by thomasmorle...@gmail.com)

2015-08-29 Thread thomasmorley65
On 2015/08/29 18:13:13, dak wrote: On 2015/08/29 17:28:33, thomasmorley651 wrote: On 2015/08/26 08:55:14, dak wrote: https://codereview.appspot.com/257580043/diff/1/scm/lily-library.scm File scm/lily-library.scm (right):

Re: Clear fret-diagram- and harp-pedal-input-strings from whitespace (issue 257580043 by thomasmorle...@gmail.com)

2015-08-26 Thread thomasmorley65
On 2015/08/26 08:55:14, dak wrote: https://codereview.appspot.com/257580043/diff/1/scm/lily-library.scm File scm/lily-library.scm (right): https://codereview.appspot.com/257580043/diff/1/scm/lily-library.scm#newcode793 scm/lily-library.scm:793: char-set:whitespace)) A mostly theoretic

Clear fret-diagram- and harp-pedal-input-strings from whitespace (issue 257580043 by thomasmorle...@gmail.com)

2015-08-23 Thread thomasmorley65
Reviewers: , Message: please review Description: Clear fret-diagram- and harp-pedal-input-strings from whitespace Whitespace-characters are deleted before further processing. Allows for \markup \fret-diagram #s:2;h:5; 6-3;5-5;4-5;3-4;2-3;1-x; Also adding errors for typos in fret-diagram with

input/regression/bookparts.ly fails at PDF conversion stage (issue 260790043 by thomasmorle...@gmail.com)

2015-08-13 Thread thomasmorley65
Reviewers: , Message: please review Description: input/regression/bookparts.ly fails at PDF conversion stage issue 4554 fixes \with-link let it look at the first page number of the entire book Please review this at https://codereview.appspot.com/260790043/ Affected files (+12, -9 lines):

Re: Let \addlyrics accept an optional context mod (issue 255610043 by d...@gnu.org)

2015-08-03 Thread thomasmorley65
Can't review parser-code. Otherwise: Doing \layout { \context { \Lyrics ... } } will work for the Lyrics created by \addlyrics as well. Thus it's more than consistent to allow \with { ... } for \addlyrics. I'd say it was a badly missing feature. My point: Looks very good to me! About

Let fret-diagram scale markups to fit into dots (issue 258070043 by thomasmorle...@gmail.com)

2015-07-31 Thread thomasmorley65
Reviewers: , Message: Please review Description: Let fret-diagram scale markups to fit into dots Issue 4531 Follow up to issue 4120. Also adding - documentation - a new regtest Please review this at https://codereview.appspot.com/258070043/ Affected files (+84, -6 lines): M

Fix ugly output from make-parenthesis-stencil for increased thickness (issue 257140044 by thomasmorle...@gmail.com)

2015-07-31 Thread thomasmorley65
Reviewers: , Message: please review Description: Fix ugly output from make-parenthesis-stencil for increased thickness issue 4532 This was caused by the fix for issue 3930. Small and very small object were focused by 3930 and it did the job. Though, obviously increasing the thickness for

Re: Let \autochange accept optional arguments for the turning-point and clefs (issue 259080043 by thomasmorle...@gmail.com)

2015-07-31 Thread thomasmorley65
On 2015/07/31 10:57:07, dak wrote: https://codereview.appspot.com/259080043/diff/20001/scm/autochange.scm File scm/autochange.scm (right): https://codereview.appspot.com/259080043/diff/20001/scm/autochange.scm#newcode36 scm/autochange.scm:36: (cond ((ly:pitch? ref-pitch pitch) On 2015/07/31

Re: Let \autochange accept optional arguments for the turning-point and clefs (issue 259080043 by thomasmorle...@gmail.com)

2015-07-31 Thread thomasmorley65
Thanks Dan and David for review https://codereview.appspot.com/259080043/diff/20001/Documentation/notation/keyboards.itely File Documentation/notation/keyboards.itely (right): https://codereview.appspot.com/259080043/diff/20001/Documentation/notation/keyboards.itely#newcode294

Re: Let \autochange accept optional arguments for the turning-point and clefs (issue 259080043 by thomasmorle...@gmail.com)

2015-07-30 Thread thomasmorley65
Please review. It's a pity that obviously putting something like #{ c' #} or #{ \clef bass #} in music-functions-init.ly is currently not possible. Trying so returns: error: unrecognized string, not in text script or \lyricmode c' or error: unknown escaped string: `\clef' \clef treble It

Re: Table Of Contents crash with negative first-page-number (issue 258740043 by thomasmorle...@gmail.com)

2015-07-12 Thread thomasmorley65
On 2015/07/12 20:58:41, thomasmorley651 wrote: On 2015/07/12 18:09:03, dak wrote: It may be worth trying if this makes the rather awkward fix for issue 3388 unnecessary, but I would guess that the problem is likely sufficiently different that that fix is still needed. As far as I

Re: Table Of Contents crash with negative first-page-number (issue 258740043 by thomasmorle...@gmail.com)

2015-07-12 Thread thomasmorley65
On 2015/07/12 18:09:03, dak wrote: On 2015/07/12 17:49:44, thomasmorley651 wrote: Do the links actually work when first-page-number is, say, 10? yes If so, did they not work before? No, it was broken. Well, then we probably have to thank Ghostscript for bringing this

Re: Table Of Contents crash with negative first-page-number (issue 258740043 by thomasmorle...@gmail.com)

2015-07-12 Thread thomasmorley65
On 2015/07/12 23:39:52, dak wrote: On 2015/07/12 17:49:44, thomasmorley651 wrote: On 2015/07/12 17:01:06, dak wrote: https://codereview.appspot.com/258740043/diff/1/scm/define-markup-commands.scm File scm/define-markup-commands.scm (right):

Table Of Contents crash with negative first-page-number (issue 258740043 by thomasmorle...@gmail.com)

2015-07-12 Thread thomasmorley65
Reviewers: , Description: Table Of Contents crash with negative first-page-number Issue 4494 Let \with-link point to the physical page-number Please review this at https://codereview.appspot.com/258740043/ Affected files (+20, -11 lines): M scm/define-markup-commands.scm Index:

Re: Table Of Contents crash with negative first-page-number (issue 258740043 by thomasmorle...@gmail.com)

2015-07-12 Thread thomasmorley65
On 2015/07/12 17:01:06, dak wrote: https://codereview.appspot.com/258740043/diff/1/scm/define-markup-commands.scm File scm/define-markup-commands.scm (right): https://codereview.appspot.com/258740043/diff/1/scm/define-markup-commands.scm#newcode542 scm/define-markup-commands.scm:542: (+

Re: Implement partial function calls. (issue 249670043 by d...@gnu.org)

2015-07-11 Thread thomasmorley65
Will this allow trans-cis = \transpose c cis \etc ? Then I'd say it's a great improvement. Far better than having to do: my-trans = #(define-scheme-function (parser location from to) (ly:pitch? ly:pitch?) (define-music-function (parser location music) (ly:music?) #{ \transpose $from $to

Re: Auto_change_iterator: move some state from C++ to Scheme (issue 248470043 by nine.fierce.ball...@gmail.com)

2015-07-03 Thread thomasmorley65
On 2015/07/03 00:54:27, Dan Eble wrote: On 2015/07/02 12:36:04, Dan Eble wrote: I haven't yet reached an understanding of what should be done about make-non-relative-music. After some testing, I understand that make-non-relative-music should have been used within \autochange. I've posted

Re: Avoid treating x as relative in \relative { \autochange {x} } (issue 250170043 by nine.fierce.ball...@gmail.com)

2015-07-03 Thread thomasmorley65
Looks like it's now consistent with the NR saying: A \relative section that is outside of \autochange has no effect on the pitches of the music, so if necessary, put \relative inside \autochange. LGTM https://codereview.appspot.com/250170043/ ___

Re: Auto_change_iterator no longer creates staff contexts itself. They are created in scheme. (issue 249970043 by nine.fierce.ball...@gmail.com)

2015-07-02 Thread thomasmorley65
On 2015/07/02 12:15:37, Dan Eble wrote: On 2015/07/02 10:45:49, dak wrote: https://codereview.appspot.com/249970043/diff/1/ly/music-functions-init.ly#newcode189 ly/music-functions-init.ly:189: \context Staff = down \with { Why not \with { \clef bass } instead? Should not squeeze this

Re: Auto_change_iterator no longer creates staff contexts itself. They are created in scheme. (issue 249970043 by nine.fierce.ball...@gmail.com)

2015-06-29 Thread thomasmorley65
On 2015/06/28 12:30:21, Dan Eble wrote: On 2015/06/28 11:17:58, thomasmorley651 wrote: Let me clearify, I don't insist in keeping those properties, though I'd like to keep the possibility to simply write \autochange { ... } _and_ to have the possibility to set the clefs in both staves.

Re: Auto_change_iterator: move some state from C++ to Scheme (issue 248470043 by nine.fierce.ball...@gmail.com)

2015-06-29 Thread thomasmorley65
Some remarks. Not related to your changes, though, why not fix them, when you're already on it? https://codereview.appspot.com/248470043/diff/1/scm/autochange.scm File scm/autochange.scm (right): https://codereview.appspot.com/248470043/diff/1/scm/autochange.scm#newcode2 scm/autochange.scm:2:

Re: Auto_change_iterator no longer creates staff contexts itself. They are created in scheme. (issue 249970043 by nine.fierce.ball...@gmail.com)

2015-06-28 Thread thomasmorley65
On 2015/06/28 00:17:58, Dan Eble wrote: On 2015/06/27 19:45:12, thomasmorley651 wrote: bassStaffProperties and trebleStaffProperties are not really documented, though I'd call it more a documentation issue. ... I vote for keeping them, although I think it's a very cumbersome method to

Re: Auto_change_iterator no longer creates staff contexts itself. They are created in scheme. (issue 249970043 by nine.fierce.ball...@gmail.com)

2015-06-27 Thread thomasmorley65
https://codereview.appspot.com/249970043/diff/1/scm/define-context-properties.scm File scm/define-context-properties.scm (left): https://codereview.appspot.com/249970043/diff/1/scm/define-context-properties.scm#oldcode179 scm/define-context-properties.scm:179: (bassStaffProperties ,list? An

Re: fret-diagram with barre returns weird output (issue 247750043 by d...@gnu.org)

2015-06-11 Thread thomasmorley65
On 2015/06/11 11:53:53, dak wrote: I just hate it when I find that a Draft of mine was Unpublished. This is really something that happens all the time when I am using Rietveld. Thanks for uploading the patch and the review!

Fretboards: markup strings in dot positions (issue 238180043 by thomasmorle...@gmail.com)

2015-05-11 Thread thomasmorley65
Reviewers: , Description: Fretboards: markup strings in dot positions Issue 4120 Fixes disappeared strings, restores the behaviour before the fix for Issue 2752 Does not scale those strings, though, in order to fix the regression first and do other feature requests for issue 4120 in a follow

Re: Add stencil-flip function (issue 235090043 by paulwmor...@gmail.com)

2015-05-06 Thread thomasmorley65
Some nitpicks. Otherwise LGTM https://codereview.appspot.com/235090043/diff/40001/scm/stencil.scm File scm/stencil.scm (right): https://codereview.appspot.com/235090043/diff/40001/scm/stencil.scm#newcode665 scm/stencil.scm:665: (define-public (stencil-flip axis stil) I'd call it

Re: Rewrite chordnames - disentangle data from formatting (issue 223420043 by thomasmorle...@gmail.com)

2015-04-26 Thread thomasmorley65
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

Re: Rewrite chordnames - disentangle data from formatting (issue 223420043 by thomasmorle...@gmail.com)

2015-04-26 Thread thomasmorley65
Although, this is not the final patch for this topic, James, may I ask you to run patchy against it? https://codereview.appspot.com/223420043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Rewrite chordnames - disentangle data from formatting (issue 223420043 by thomasmorle...@gmail.com)

2015-04-11 Thread thomasmorley65
Please review. Intended is a pure maintaining change. Maybe a first step for more flexibility, easier customizing etc. If I sorted it correctly, no change in visible output should occur. Apart from the Banter-style (see commit-message) no bug-fixing happens, although I noticed one (see FIXME

Re: make pretty-print available in ly files (issue 222810043 by david.nales...@gmail.com)

2015-04-08 Thread thomasmorley65
On 2015/04/08 10:21:55, dak wrote: value-lily-string is probably the most useful reasonably generic printing facility right now. But nobody really knows it. At least, I wasn't aware of it, because of (in a ly-file) #(display value-lily-string) - Unbound variable: value-lily-string

Re: make pretty-print available in ly files (issue 222810043 by david.nales...@gmail.com)

2015-04-08 Thread thomasmorley65
On 2015/04/08 08:11:35, dak wrote: On 2015/04/07 23:25:09, thomasmorley651 wrote: https://codereview.appspot.com/222810043/diff/1/ly/init.ly File ly/init.ly (right): https://codereview.appspot.com/222810043/diff/1/ly/init.ly#newcode38 ly/init.ly:38: #(use-modules (ice-9 pretty-print)) On

Re: make pretty-print available in ly files (issue 222810043 by david.nales...@gmail.com)

2015-04-07 Thread thomasmorley65
https://codereview.appspot.com/222810043/diff/1/ly/init.ly File ly/init.ly (right): https://codereview.appspot.com/222810043/diff/1/ly/init.ly#newcode38 ly/init.ly:38: #(use-modules (ice-9 pretty-print)) On 2015/04/07 17:43:07, david.nalesnik wrote: On 2015/04/07 16:13:44, dak wrote: Uh,

No midi with ChordNames (issue 217450043 by thomasmorle...@gmail.com)

2015-03-29 Thread thomasmorley65
Reviewers: , Message: please review Description: No midi with ChordNames Issue 4330 Restore the behaviour for ChordNames and ChordNameVoice in performer-init.ly which was changed by the patch for issue 4281 Please review this at https://codereview.appspot.com/217450043/ Affected files (+7,

Re: Add means to display objects accessible from a grob (issue 217260043 by david.nales...@gmail.com)

2015-03-26 Thread thomasmorley65
https://codereview.appspot.com/217260043/diff/1/scm/output-lib.scm File scm/output-lib.scm (right): https://codereview.appspot.com/217260043/diff/1/scm/output-lib.scm#newcode165 scm/output-lib.scm:165: (use-modules (ice-9 pretty-print)) On 2015/03/25 13:59:20, david.nalesnik wrote: On

Re: toward-stem-shift-in-column should only consider 'script-interface (issue 207620043 by david.nales...@gmail.com)

2015-03-16 Thread thomasmorley65
LGTM https://codereview.appspot.com/207620043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: toward-stem-shift-in-column should only consider 'script-interface (issue 207620043 by david.nales...@gmail.com)

2015-03-11 Thread thomasmorley65
https://codereview.appspot.com/207620043/diff/20001/scm/output-lib.scm File scm/output-lib.scm (right): https://codereview.appspot.com/207620043/diff/20001/scm/output-lib.scm#newcode1190 scm/output-lib.scm:1190: (shift (if (and (ly:grob? script-column) minor nit: Is it possible to stick to the

Re: toward-stem-shift-in-column should only consider 'script-interface (issue 207620043 by david.nales...@gmail.com)

2015-03-11 Thread thomasmorley65
https://codereview.appspot.com/207620043/diff/20001/scm/output-lib.scm File scm/output-lib.scm (right): https://codereview.appspot.com/207620043/diff/20001/scm/output-lib.scm#newcode1196 scm/output-lib.scm:1196: (grob::has-interface s 'script-interface) On 2015/03/11 23:49:44, thomasmorley651

making measure-counter-stencil public (issue 203130043 by thomasmorle...@gmail.com)

2015-02-17 Thread thomasmorley65
Reviewers: , Message: please review Description: making measure-counter-stencil public Issue 4292 In order to work with 'make-stencil-boxer' etc Please review this at https://codereview.appspot.com/203130043/ Affected files (+1, -1 lines): M scm/output-lib.scm Index: scm/output-lib.scm

Re: Making flat flags available (issue 14303044)

2015-02-15 Thread thomasmorley65
'using-alternative-flag-styles.ly' is not in Documentation/snippets/new/ any more. I changed it directly in LSR. The changed snippet will show up after next LSR-import. Looks I can't post to the tracker-issue for 3591 any more, because it's closed. https://codereview.appspot.com/14303044/

Re: Clean up inconsistencies in engraver-init.ly and performer-init.ly (issue 199460043 by thomasmorle...@gmail.com)

2015-02-09 Thread thomasmorley65
On 2015/02/09 06:09:34, lemzwerg wrote: LGTM. It would be nice if David's checker script could be added, too. More, I'd consider it a good idea to use the checker on the current patch again. There was such a mess that I'm not sure I corrected all.

Re: Allow independent adjustment of minimum length for spanner siblings (issue 201140043 by david.nales...@gmail.com)

2015-02-09 Thread thomasmorley65
LGTM https://codereview.appspot.com/201140043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Clean up inconsistencies in engraver-init.ly and performer-init.ly (issue 199460043 by thomasmorle...@gmail.com)

2015-02-08 Thread thomasmorley65
please review https://codereview.appspot.com/199460043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Allow independent adjustment of minimum length for spanner siblings (issue 201140043 by david.nales...@gmail.com)

2015-02-05 Thread thomasmorley65
Can't review the C++ part. Some nitpicks in the regtest. Will it be possible to tweak the first part of the Tie etc at line-end as well? With a combi of setting 'minimum-length-after-break' and 'minimum-length? If yes, I'd show an example in the regtest. At least it should be

Re: Allow independent adjustment of minimum length for spanner siblings (issue 201140043 by david.nales...@gmail.com)

2015-02-05 Thread thomasmorley65
https://codereview.appspot.com/201140043/diff/1/input/regression/minimum-length-after-break.ly File input/regression/minimum-length-after-break.ly (right): https://codereview.appspot.com/201140043/diff/1/input/regression/minimum-length-after-break.ly#newcode15

Re: Doc: Clarified how to use clip-systems option (issue 186640043 by pkx1...@gmail.com)

2015-01-04 Thread thomasmorley65
LGTM https://codereview.appspot.com/186640043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: ly:line-interface::print should read ly:line-spanner::print in NR (issue 186650043 by thomasmorle...@gmail.com)

2015-01-04 Thread thomasmorley65
To get used to the workflow again, a two-liner. Please review https://codereview.appspot.com/186650043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel

Doc: Clarified how to use clip-systems option (issue 186640043 by pkx1...@gmail.com)

2015-01-02 Thread thomasmorley65
One nitpick, otherwise LGTM https://codereview.appspot.com/186640043/diff/1/Documentation/notation/input.itely File Documentation/notation/input.itely (right): https://codereview.appspot.com/186640043/diff/1/Documentation/notation/input.itely#newcode2541

typo/oversight in align-interface.cc and page-layout-problem.cc (issue 115770043 by thomasmorle...@gmail.com)

2014-07-13 Thread thomasmorley65
Reviewers: , Message: please review Description: typo/oversight in align-interface.cc and page-layout-problem.cc issue 4008 fix oversight in doc-string of align-interface.cc fix typo in comment of page-layout-problem.cc Please review this at https://codereview.appspot.com/115770043/

Re: Remove thin-kern property. (issue 105560044 by markpole...@gmail.com)

2014-07-07 Thread thomasmorley65
On 2014/07/07 07:25:15, marc wrote: On 2014/07/07 01:31:01, Mark Polesky wrote: On 2014/07/06 11:52:56, thomasmorley651 wrote: I disagree! 'kern and 'thin-kern _are_ used differently. But *do* we need two different variables which then are used with exactly the same value? Do we need

Re: Issue 3997: \magnifyMusic: don't modify nested properties; reformat code. (issue 110840044 by markpole...@gmail.com)

2014-07-07 Thread thomasmorley65
On 2014/07/07 20:48:17, Mark Polesky wrote: [...] this raises a new question: How do I tweak the distance between the half-note's two stems in tablature? \new TabStaff { \tabFullNotation c2 } You can't. It's hardcoded with tabvoice::draw-double-stem-for-half-notes in

Re: Remove thin-kern property. (issue 105560044 by markpole...@gmail.com)

2014-07-06 Thread thomasmorley65
On 2014/07/06 06:46:53, Mark Polesky wrote: Hi, I'm proposing to get rid of the BarLine.thin-kern property with this patch, so if you're opposed, let me know. You can see my comments/rationale here: http://code.google.com/p/lilypond/issues/detail?id=3995 Thanks, Mark I disagree! 'kern

Re: Remove thin-kern property. (issue 105560044 by markpole...@gmail.com)

2014-07-06 Thread thomasmorley65
On 2014/07/06 12:53:55, Mark Polesky wrote: On 2014/07/06 11:52:56, thomasmorley651 wrote: I disagree! 'kern and 'thin-kern _are_ used differently. Look at the output from: Harm, what am I missing? To my eye (compiling this with the latest snapshot), your example shows that thin-kern

Re: Color and/or parenthesize single dots in fret-diagrams (issue 102450043 by thomasmorle...@gmail.com)

2014-06-15 Thread thomasmorley65
On 2014/06/14 18:48:14, J_lowe wrote: https://codereview.appspot.com/102450043/diff/1/Documentation/notation/fretted-strings.itely File Documentation/notation/fretted-strings.itely (right):

Re: Color and/or parenthesize single dots in fret-diagrams (issue 102450043 by thomasmorle...@gmail.com)

2014-06-15 Thread thomasmorley65
On 2014/06/15 07:46:22, marc wrote: Hi Harm, great work, mostly LGTM, just two small nitpicks. https://codereview.appspot.com/102450043/diff/1/scm/fret-diagrams.scm File scm/fret-diagrams.scm (right): https://codereview.appspot.com/102450043/diff/1/scm/fret-diagrams.scm#newcode119

Color and/or parenthesize single dots in fret-diagrams (issue 102450043 by thomasmorle...@gmail.com)

2014-06-14 Thread thomasmorley65
Reviewers: , Message: Please review Description: Color and/or parenthesize single dots in fret-diagrams Issue 2752 Makes it possible to color and/or parenthesize single dots in fret-diagram-verbose Introducing two properties for use in fret-diagram-details - fret-label-horizontal-offset

Re: Clearify use of break-align-orders in NR A.17 Layout properties (issue 96650050)

2014-06-03 Thread thomasmorley65
On 2014/06/02 23:23:23, Carl wrote: On 2014/06/02 21:47:59, thomasmorley651 wrote: Better referencing I think that this is a wrong approach. The list of properties in the .scm file is not the appropriate place to teach the use of the properties. Furthermore, in my opinion there should

Clearify use of break-align-orders in NR A.17 Layout properties (issue 96650050)

2014-06-01 Thread thomasmorley65
Reviewers: , Message: Please review Description: Clearify use of break-align-orders in NR A.17 Layout properties Issue 3939 It should be sufficient to refer to 'Separating key cancellations from key signature changes' from Documentation/snippets to show how to get different settings for

Re: Make music functions and identifiers and expressions take articulations (issue 78690043)

2014-03-21 Thread thomasmorley65
Can't review parser code. From description: great https://codereview.appspot.com/78690043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel

XY-extent: more accurate docstring (issue 35440044)

2013-11-30 Thread thomasmorley65
LGTM https://codereview.appspot.com/35440044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel

format-metronome-mark and metronome-markup don't support styles other than default (issue 14807043)

2013-10-17 Thread thomasmorley65
Reviewers: , Message: Please review. Description: format-metronome-mark and metronome-markup don't support styles other than default issue 3096 - introducing 'styled-metronome-markup' with the possibility to set different styles for note-head and flag. Also, and currently quite theoretical,

Re: format-metronome-mark and metronome-markup don't support styles other than default (issue 14807043)

2013-10-17 Thread thomasmorley65
On 2013/10/17 21:29:04, dak wrote: The interface looks cumbersome to use. Why don't you just take the styles that are used in the Score? Well, short answer: I don't know how. It might mean adding the respective properties to the font-interface (MetronomeMark has both font-interface and

Re: Move New_dynamic_engraver over the unused Dynamic_engraver (issue 14460043)

2013-10-06 Thread thomasmorley65
Although, I can't review C++, I've applied the patch for testing (hopefully without mistake) Testing this code { c''1_\mf^\ \break d''_\mp^\! } I've got: programming error: Spanner `Hairpin' is not fully contained in parent spanner. Ignoring orphaned part { c''1_\mf ^\ \break

Re: Move New_dynamic_engraver over the unused Dynamic_engraver (issue 14460043)

2013-10-06 Thread thomasmorley65
On 2013/10/06 23:10:16, dak wrote: On 2013/10/06 23:00:25, thomasmorley651 wrote: [...] Or am I completely wrong and this patch has nothing to do with the problem above? In this case, you are completely wrong. [...] Anyway, thanks for clarifying.

Making flat flags available (issue 14303044)

2013-10-02 Thread thomasmorley65
Reviewers: , Message: Please review. Description: Making flat flags available The markup-command 'note-ny-number' and the relvant regression- tests are extended, too. The sippet 'using-alternative-flag-styles.ly' from Documentation/snippets/new/ isn't changed for now, will be tackled in a

Make Adding extra fingering with scheme snippet do something useful (issue 13826047)

2013-09-27 Thread thomasmorley65
LGTM Question: considering LSR-update, I recently added an earlier version of it to the LSR http://lsr.dsi.unimi.it/LSR/Item?id=887 Should I replace it with this one? https://codereview.appspot.com/13826047/ ___ lilypond-devel mailing list

Let make-music accept existing music or alists as source (issue 13904043)

2013-09-25 Thread thomasmorley65
LGTM https://codereview.appspot.com/13904043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: makelsr (issue 13300048)

2013-09-10 Thread thomasmorley65
typo LGTM https://codereview.appspot.com/13300048/diff/1/Documentation/changes.tely File Documentation/changes.tely (right): https://codereview.appspot.com/13300048/diff/1/Documentation/changes.tely#newcode66 Documentation/changes.tely:66: the allowance of horizontal sapce for tempo and

Re: Make user-generated tab-clefs possible (issue 13612043)

2013-09-07 Thread thomasmorley65
Hi, please review. https://codereview.appspot.com/13612043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel

Some clean up about grace-settings in music-functions.scm (issue 13111045)

2013-08-20 Thread thomasmorley65
Reviewers: , Message: Hi, please review, though, I don't expect any visible changes in reg-tests or docs. Description: Some clean up about grace-settings in music-functions.scm - Stores a list,general-grace-settings, used by make-voice-props-set and make-voice-props-override. - Settings to be

Re: Issue 3498: fatal error with toplevel-markup-identifier (issue 12945043)

2013-08-18 Thread thomasmorley65
On 2013/08/18 10:20:07, janek wrote: LGTM (but i'm not a parser expert) I've no clue about the parser, from description: LGTM Sorry for a possibly ignorant question: will this patch also remove the error in case of mus = { a } \mus ? No. If not, would there be an easy way to fix that?

Let add-grace-property and remove-grace-property only work in current context (issue 13088043)

2013-08-17 Thread thomasmorley65
LGTM https://codereview.appspot.com/13088043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Issue 3491: Add \displayMarkup command. (issue 12732043)

2013-08-15 Thread thomasmorley65
https://codereview.appspot.com/12732043/diff/7001/Documentation/extending/programming-interface.itely File Documentation/extending/programming-interface.itely (right): https://codereview.appspot.com/12732043/diff/7001/Documentation/extending/programming-interface.itely#newcode277

Re: Issue 3491: Add \displayMarkup command. (issue 12732043)

2013-08-15 Thread thomasmorley65
On 2013/08/15 10:36:06, dak wrote: Also, the only thing that makes this \displayMarkup rather than \displayScheme is the restriction of the argument type to markup?. Perhaps it would make sense to call the whole function \displayScheme instead and allow arguments of type scheme? here? +1

Re: Issue 3491: Add \displayMarkup command. (issue 12732043)

2013-08-14 Thread thomasmorley65
LGTM https://codereview.appspot.com/12732043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Issue 3491: Add \displayMarkup command. (issue 12732043)

2013-08-12 Thread thomasmorley65
This disturbed me to, though, not to a point that I started to work on it my own. ;) One thought: https://codereview.appspot.com/12732043/diff/1/ly/music-functions-init.ly File ly/music-functions-init.ly (right):

adding markup-commands \oval and \ellipse (issue 11427044)

2013-07-17 Thread thomasmorley65
Reviewers: , Message: Please review. (git-cl seems to work - currently) Description: adding markup-commands \oval and \ellipse \oval and \ellipse are drawing an oval respectively an ellipse around their argument. Also adding a reg-test for markup-commands framing their argument. Please

Fix some oversights of \layout in \book (issue 10794044)

2013-07-10 Thread thomasmorley65
Reviewers: , Message: Please review. Description: Fix some oversights of \layout in \book follow-up for issue 3151 Please review this at https://codereview.appspot.com/10794044/ Affected files: M Documentation/notation/input.itely Index: Documentation/notation/input.itely diff --git

More details about limitation of \addlyrics in Documentation/notation/vocal.itely (issue 11062043)

2013-07-09 Thread thomasmorley65
Reviewers: , Message: Please review. Description: More details about limitation of \addlyrics in Documentation/notation/vocal.itely Add a hint, that \addlyrics does not work with TabStaff Fix for issue 3450 Please review this at https://codereview.appspot.com/11062043/ Affected files: M

Re: issue 3441: banjo example should use Staff + TabStaff (issue 10935046)

2013-07-07 Thread thomasmorley65
Some thoughts, otherwise LGTM. https://codereview.appspot.com/10935046/diff/1/Documentation/notation/fretted-strings.itely File Documentation/notation/fretted-strings.itely (right): https://codereview.appspot.com/10935046/diff/1/Documentation/notation/fretted-strings.itely#newcode1864

Re: Create a two-argument form of define-event-class (issue 10965043)

2013-07-06 Thread thomasmorley65
On 2013/07/06 00:05:35, dak wrote: https://codereview.appspot.com/10965043/diff/1/scm/define-event-classes.scm File scm/define-event-classes.scm (right): https://codereview.appspot.com/10965043/diff/1/scm/define-event-classes.scm#newcode87 scm/define-event-classes.scm:87: (define

Create a two-argument form of define-event-class (issue 10965043)

2013-07-05 Thread thomasmorley65
I didn't apply the patch and I can't review C++, though, after a first quick glance, some nitpicks: https://codereview.appspot.com/10965043/diff/1/scm/define-event-classes.scm File scm/define-event-classes.scm (right):

Re: Updates to NR chapter 2 (issue 10543044)

2013-06-25 Thread thomasmorley65
One nitpick. Otherwise LGTM https://codereview.appspot.com/10543044/diff/1/Documentation/notation/vocal.itely File Documentation/notation/vocal.itely (right): https://codereview.appspot.com/10543044/diff/1/Documentation/notation/vocal.itely#newcode629 Documentation/notation/vocal.itely:629:

Add @code{} to middleCOffset (issue 10566043)

2013-06-25 Thread thomasmorley65
Reviewers: , Message: Please review. Description: Add @code{} to middleCOffset Wraps middleCOffset from doc-string of ly:set-middle-C! (pitch-scheme.cc) into @code{} Fix issue 3424 Please review this at https://codereview.appspot.com/10566043/ Affected files: M lily/pitch-scheme.cc

Let horizontal-line be a straight-cut line rather than having rounded edges (issue 8685043)

2013-04-11 Thread thomasmorley65
Can't review C++ -code, though, from description, I do not understand the reason for it. Well, ofcourse it will merge better with barlines, but can you give an example where the problem isn't solvable with at most medium effort? Ah, wait, I just found http://lsr.dsi.unimi.it/LSR/Search?q=color

Re: Let horizontal-line be a straight-cut line rather than having rounded edges (issue 8685043)

2013-04-11 Thread thomasmorley65
On 2013/04/11 23:44:03, thomasmorley65 wrote: Ah, wait, I just found http://lsr.dsi.unimi.it/LSR/Search?q=color Regarding it with very high zoom shows a greater disadvantage of draw-line. Nevertheless, I think it's more a problem of said snippet, worth fixing. Sorry, wrong link. Should

Re: Removes '-signs in vectors - follow-up (issue 7834043)

2013-04-04 Thread thomasmorley65
On 2013/04/03 08:31:26, dak wrote: Perhaps double-check that the % end verbatim occurs only where expected in master, Done. and then close as fixed, possibly with the version number where the fix traveled in via makelsr. Done. https://codereview.appspot.com/7834043/

Re: Removes '-signs in vectors - follow-up (issue 7834043)

2013-04-02 Thread thomasmorley65
On 2013/04/02 21:35:23, janek wrote: Is this patch still valid? I don't see any ' signs being removed, Meanwhile I removed the '-signs from the relevant snippets in LSR as well. I wasn't aware that a new release would contain them. So it is indeed invalid. git grep #'# only returns some

Re: Add Changes entries for \temporary, \omit, \hide, \single, multiple tags (issue 8187044)

2013-03-30 Thread thomasmorley65
Some nitpicks. Otherwise LGTM https://codereview.appspot.com/8187044/diff/1/Documentation/changes.tely File Documentation/changes.tely (right): https://codereview.appspot.com/8187044/diff/1/Documentation/changes.tely#newcode73 Documentation/changes.tely:73: Two ways of letting graphical

Re: Add Changes entries for \temporary, \omit, \hide, \single, multiple tags (issue 8187044)

2013-03-30 Thread thomasmorley65
On 2013/03/30 19:38:09, dak wrote: Address Thomas' comments The description doesn't explain why \relative c'' { a e' \single \omit NoteHead ais1 } returns a programming error. Though, this is definitely not a topic of \single, \omit or Changes in general. LGTM

<    1   2   3   4   5   >