First separator for subassignments must be '.' (issue 341490043 by d...@gnu.org)

2018-10-03 Thread thomasmorley65
Even without your patch one can do: part.1,1 = "foo" part.1,2 = "bar" part.1,3 = "buzz" part.2,1 = "xx" part.2,2 = "yy" part.2,3 = "zz" part.3 = "whatever" #(format #t "\n\"part\" contains:\n~y\n" part) #(format #t "Example call in guile: \"1\" of \"part\":\n~a" (assoc-get 1 part)) #(format

Escape nul, cr, newline in PDF metadata (issue 345090043 by d...@gnu.org)

2018-10-03 Thread thomasmorley65
LGTM https://codereview.appspot.com/345090043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel

Prob::equal_p: discard "origin" property (issue 363740043 by d...@gnu.org)

2018-07-27 Thread thomasmorley65
Can't review C++. But from a little testing: http://lilypond.1069038.n5.nabble.com/Iterating-music-for-voiceOne-Checking-with-equal-td215135.html#a215158 it works as wished. A question not directly related to your patch: https://codereview.appspot.com/363740043/diff/1/lily/prob.cc File

Change highmidtom to himidtom in notation-appendices.itely (issue 355740043 by thomasmorle...@gmail.com)

2018-07-05 Thread thomasmorley65
Reviewers: , Description: Change highmidtom to himidtom in notation-appendices.itely Please review this at https://codereview.appspot.com/355740043/ Affected files (+4, -4 lines): M Documentation/es/notation/notation-appendices.itely M Documentation/fr/notation/notation-appendices.itely

Let general-column deal reliable with empty args (issue 346030043 by thomasmorle...@gmail.com)

2018-05-26 Thread thomasmorley65
Reviewers: , Description: Let general-column deal reliable with empty args Previously an args-list of empty-stencils issued a programming error: Improbable offset for stencil: -inf staff space This patch removes empty stencils from the args-list. If args-list is empty, an empty stencil is

Improve make-bracket-bar-line (issue 339670043 by thomasmorle...@gmail.com)

2018-05-24 Thread thomasmorley65
Reviewers: , Description: Improve make-bracket-bar-line The previously used ly:stencil-scale -1 1 resulted in wrongly positioned SpanBars in certain cases. Replaces ly:stencil-scale by flip-stencil. Please review this at https://codereview.appspot.com/339670043/ Affected files (+1, -1 lines):

Allow Scheme/identifiers for duration multipliers (issue 346810043 by d...@gnu.org)

2018-05-22 Thread thomasmorley65
From description and regtest: very nice. Will it work for below as well? #(define frac (inexact->exact (/ 3.0 4.0))) { r1*/frac } https://codereview.appspot.com/346810043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org

Re: Place barres on fret diagrams if they can be inferred (issue 294570043 by carl.d.soren...@gmail.com)

2018-05-06 Thread thomasmorley65
On 2018/05/05 23:07:45, Carl wrote: Remove debug printing Very nice! LGTM ofcourse Thanks! https://codereview.appspot.com/294570043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Place barres on fret diagrams if they can be inferred (issue 294570043 by carl.d.soren...@gmail.com)

2018-05-04 Thread thomasmorley65
Hi Carl, apart from one nit (see below) LGTM Probably not related to this patch: Clicking "Delta from patch set" gives strange results. No clue what Rietveld does... https://codereview.appspot.com/294570043/diff/20001/scm/translation-functions.scm File scm/translation-functions.scm (right):

Place barres on fret diagrams if they can be inferred (issue 294570043 by carl.d.soren...@gmail.com)

2018-05-03 Thread thomasmorley65
Hi Carl, nice work. A general thought: As far as I understand the code will work automatically. Though, I foresee some users wanting to switch it off (ofcourse not the majority, but there is always somebody with different wishes). Any chance to create an option for it? Some inline remarks

Re: Clarify notation for slurs and beams (issue 343060043 by carl.d.soren...@gmail.com)

2018-05-01 Thread thomasmorley65
On 2018/05/01 02:47:20, Carl wrote: According to parser.yy: In line 3259, a post_event is either: 1) post_event_nofinger, or 2) '-' plus a fingering In line 3200, a post_event_nofinger is either 1) direction_less_event 2) script_dir plus a music_function 3,4 ) Lyric hyphen or lyric

Re: Clarify notation for slurs and beams (issue 343060043 by carl.d.soren...@gmail.com)

2018-04-30 Thread thomasmorley65
On 2018/04/30 22:55:35, thomasmorley651 wrote: On 2018/04/30 22:19:12, Carl wrote: > I don't think I agree that things are attached with "-" signs. For example, > \staccato, \mordent, \turn, \fermata, \prall, (, [. None of these are attached > with "-" signs, although they can have a

Re: Clarify notation for slurs and beams (issue 343060043 by carl.d.soren...@gmail.com)

2018-04-30 Thread thomasmorley65
On 2018/04/30 22:19:12, Carl wrote: https://codereview.appspot.com/343060043/diff/40001/Documentation/learning/fundamental.itely File Documentation/learning/fundamental.itely (right): https://codereview.appspot.com/343060043/diff/40001/Documentation/learning/fundamental.itely#newcode465

Re: Clarify notation for slurs and beams (issue 343060043 by carl.d.soren...@gmail.com)

2018-04-30 Thread thomasmorley65
Hi Carl, many thanks for your work. Some remarks: https://codereview.appspot.com/343060043/diff/40001/Documentation/learning/fundamental.itely File Documentation/learning/fundamental.itely (right):

Let \tweak on event chords manipulate its elements (issue 344770043 by d...@gnu.org)

2018-04-12 Thread thomasmorley65
LGTM https://codereview.appspot.com/344770043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Issue 5301: Refactor mark events ... (issue 340650043 by nine.fierce.ball...@gmail.com)

2018-04-08 Thread thomasmorley65
Hi Dan, some remarks/questions: https://codereview.appspot.com/340650043/diff/1/ly/music-functions-init.ly File ly/music-functions-init.ly (right): https://codereview.appspot.com/340650043/diff/1/ly/music-functions-init.ly#newcode833 ly/music-functions-init.ly:833: #(define-music-function

Re: Allow event functions from partial functions or partial markups (issue 336670043 by d...@gnu.org)

2018-03-20 Thread thomasmorley65
On 2018/03/20 12:56:37, dak wrote: On 2018/03/16 19:19:15, thomasmorley651 wrote: > Can't review this one. > Though, from description, I'd like to suggest to put up an entry in `changes'. Ugh. Documentation for \etc is sort-of sparse, and the main problem for "Changes" is "which Changes"?

Regtest for event functions from incomplete text events (issue 339390043 by d...@gnu.org)

2018-03-20 Thread thomasmorley65
LGTM A nit below https://codereview.appspot.com/339390043/diff/1/input/regression/textetc.ly File input/regression/textetc.ly (right): https://codereview.appspot.com/339390043/diff/1/input/regression/textetc.ly#newcode5 input/regression/textetc.ly:5: event functions for \\samp{TextScript}

Allow event functions from partial functions or partial markups (issue 336670043 by d...@gnu.org)

2018-03-16 Thread thomasmorley65
Can't review this one. Though, from description, I'd like to suggest to put up an entry in `changes'. https://codereview.appspot.com/336670043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org

add glyphs for 256th, 512th and 1024th flags and rests (issue 336590043 by lilyp...@maltemeyn.de)

2018-02-21 Thread thomasmorley65
I suggest to update regtests ‘markup-rest.ly’ and ‘markup-rest-styles.ly’ as well. If no glyph for a certain style is found the default is taken anyway, but it would be nice if the new glyphs would be demonstrated via markup as well. Also, it deserves an entry in changes, imho.

Re: Let parser use define-markup-command-internal (issue 334460043 by d...@gnu.org)

2018-02-06 Thread thomasmorley65
Out of my own testings as well as running a full `make doc' with guilev2: LGTM https://codereview.appspot.com/334460043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Let parser use define-markup-command-internal (issue 334460043 by d...@gnu.org)

2018-02-04 Thread thomasmorley65
On 2018/02/04 21:36:17, thomasmorley651 wrote: Works with my guilev2 setup and compiling input/regression/markup-partial.ly directly. make doc (with guilev2) fails at different place now, while using -j5. I'll redo with single thread and report afterwards. make doc (guilev2) fails for the

Re: Let parser use define-markup-command-internal (issue 334460043 by d...@gnu.org)

2018-02-04 Thread thomasmorley65
Works with my guilev2 setup and compiling input/regression/markup-partial.ly directly. make doc (with guilev2) fails at different place now, while using -j5. I'll redo with single thread and report afterwards. https://codereview.appspot.com/334460043/

Re: Allow left-handed fret-markups (issue 339270043 by thomasmorle...@gmail.com)

2018-01-23 Thread thomasmorley65
On 2018/01/23 09:55:04, dak wrote: mailto:thomasmorle...@gmail.com writes: > Yep, the difference between left- and right-handed are certain negative > instead of a positive values. > I'd love to use a multiplier retrieved from left-handed. > Though, how to code properly and user-friendly. >

Re: Allow left-handed fret-markups (issue 339270043 by thomasmorle...@gmail.com)

2018-01-23 Thread thomasmorley65
On 2018/01/22 23:52:29, Carl wrote: Looks good to me, but I have one suggestion. Thanks, Carl https://codereview.appspot.com/339270043/diff/1/scm/fret-diagrams.scm File scm/fret-diagrams.scm (right): https://codereview.appspot.com/339270043/diff/1/scm/fret-diagrams.scm#newcode365

Allow left-handed fret-markups (issue 339270043 by thomasmorle...@gmail.com)

2018-01-22 Thread thomasmorley65
Reviewers: , Message: please review Description: Allow left-handed fret-markups By adding a new boolean subproperty: left-handed. - Extends Documentation/notation/fretted-strings.itely - Extends Documentation/changes.tely - Extenss scm/define-grob-properties.scm - Adds regtest

Re: Issue 5264: fret diagram nut alignment (issue 335430043 by lilyp...@maltemeyn.de)

2018-01-18 Thread thomasmorley65
The proper approach is to introduce a property that indicates the diagram should be left-handed, and to make the stencil-coordinates function in scm/fret-diagrams.scm respect that property. That's exactly what I'm trying to do with my patch on the tracker

Re: Issue 5264: fret diagram nut alignment (issue 335430043 by lilyp...@maltemeyn.de)

2018-01-18 Thread thomasmorley65
I don't think this is the right approach to make left-handed fret-diagrams work. Ofcourse it cures a single problem while using negative string-distance, but other problems are shining up as already mentioned. Instead I think we should disallow negative string-distance, or rather (because

Callbacks setting properties are programming errors (issue 334970043 by d...@gnu.org)

2017-11-12 Thread thomasmorley65
I've downloaded the patch and after playing around with it: Very nice. Thanks. LGTM https://codereview.appspot.com/334970043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: typos and conistency (issue 332110043 by thomasmorle...@gmail.com)

2017-11-11 Thread thomasmorley65
Reviewers: Jean-Charles, Message: On 2017/11/11 10:11:39, Jean-Charles wrote: Didn't you forget to amend the English version of changes.tely?… I changed every changes.tely containing addLyrics in _master_ The english version is empty apart from defaults, because it was moved to the stable

Fix regression introduced with 5122 "Fix not scaling stem ..." (issue 337880043 by thomasmorle...@gmail.com)

2017-11-05 Thread thomasmorley65
Reviewers: , Message: Please review Description: Fix regression introduced with 5122 "Fix not scaling stem ..." commit eee677c480c78d58a5215e246575aa94ba2d1897 only took text-font-size from current layout into account. Now text-font-size from $defaultpaper is respected as well. As a result

Extend regtest rest-dot-position.ly up to four voices (issue 331150043 by thomasmorle...@gmail.com)

2017-11-01 Thread thomasmorley65
Reviewers: , Message: Please review. Description: Extend regtest rest-dot-position.ly up to four voices Please review this at https://codereview.appspot.com/331150043/ Affected files (+22, -22 lines): M input/regression/rest-dot-position.ly Index: input/regression/rest-dot-position.ly

Issue 5220/3: Derive Repeat_tie_engraver from Laissez_vibrer_engraver (issue 332990043 by d...@gnu.org)

2017-10-24 Thread thomasmorley65
From testing it: LGTM I think my own patch is obsolete now. So I'll remove myself as owner from the tracker-issue and close https://codereview.appspot.com/335910043/ https://codereview.appspot.com/332990043/ ___ lilypond-devel mailing list

Re: Let repeatTie work inside of event-chord (issue 335910043 by thomasmorle...@gmail.com)

2017-10-24 Thread thomasmorley65
I tried to make both engravers as equal as I could, to make it easy for a follow up to create a common base class for both. My own naive attempts to introduce something like 'semi-tie-event' failed, though. https://codereview.appspot.com/335910043/

Re: Let repeatTie work inside of event-chord (issue 335910043 by thomasmorle...@gmail.com)

2017-10-22 Thread thomasmorley65
On 2017/10/22 20:01:03, dak wrote: On 2017/10/22 19:26:12, benko.pal wrote: > if it's really the same, can't it be used without copying? or if separate > classes are absolutely needed, can't the common part be gathered in a common > base class? You mean, like with Slur/PhrasingSlur?

Let repeatTie work inside of event-chord (issue 335910043 by thomasmorle...@gmail.com)

2017-10-21 Thread thomasmorley65
Reviewers: , Message: please review Description: Let repeatTie work inside of event-chord The previous coding of repeat-tie-engraver.cc is replaced by the renamed coding of the laissez-vibrer-engraver. Please review this at https://codereview.appspot.com/335910043/ Affected files (+37, -21

Re: make metronomeMarkFormatter more flexible (issue 327620043 by lilyp...@maltemeyn.de)

2017-10-15 Thread thomasmorley65
On 2017/10/15 13:09:30, kieren_macmillan_sympatico.ca wrote: Hi David, > I think it's rather obvious that we won't get something that caters for > every format without actually giving the user the equivalent of > providing a formatting function (in the form of a markup command): even > a

Re: make metronomeMarkFormatter more flexible (issue 327620043 by lilyp...@maltemeyn.de)

2017-10-15 Thread thomasmorley65
On 2017/10/10 18:47:14, Malte Meyn wrote: \tempo "" 4 = 120→ (텟 = 120) [misaligned] \tempo "" 4 = 120-132→ (텟 = 120 – 132) [misaligned] Why misaligned? Could you give an example? https://codereview.appspot.com/327620043/

Re: make metronomeMarkFormatter more flexible (issue 327620043 by lilyp...@maltemeyn.de)

2017-10-12 Thread thomasmorley65
On 2017/10/12 08:53:31, dak wrote: On 2017/10/12 08:27:51, Malte Meyn wrote: > How would that reflect that parentheses are shown if and only if text is > present? And how could you then change this behaviour? Pass. For more extensive changes, we'd need a markup function. Basically, where

Re: Multiple properties for \override \override-lines markup (issue 331860043 by d...@gnu.org)

2017-10-11 Thread thomasmorley65
On 2017/10/11 10:19:25, dak wrote: https://codereview.appspot.com/331860043/diff/20001/scm/define-markup-commands.scm#newcode4026 scm/define-markup-commands.scm:4026: \\override-lines #'(multi-measure-rest . #t) On 2017/10/08 15:19:44, thomasmorley651 wrote: > Why override_-lines_ here.

Re: Allow quoted strings as Scheme arguments to markup commands (issue 331820043 by d...@gnu.org)

2017-10-06 Thread thomasmorley65
On 2017/10/06 22:00:07, dak wrote: https://codereview.appspot.com/331820043/diff/20001/python/convertrules.py File python/convertrules.py (right): https://codereview.appspot.com/331820043/diff/20001/python/convertrules.py#newcode3963 python/convertrules.py:3963: str = re.sub

Re: Merge_rests_engraver: fix vertical rest positions (issue 334740043 by lilyp...@maltemeyn.de)

2017-10-05 Thread thomasmorley65
Hi Malte, your patch fixes the "magnifyStaff"-problem. Though, this functionality needs testing, imho. I'd extend the reg-test merge-rests-engraver.ly or add a new one. Some other thoughts: https://codereview.appspot.com/334740043/diff/1/scm/scheme-engravers.scm File scm/scheme-engravers.scm

Remove apostroph in backend-options of german running.itely (issue 326620043 by thomasmorle...@gmail.com)

2017-09-29 Thread thomasmorley65
Reviewers: , Message: Please review. This came up in the german forum https://lilypondforum.de/index.php/topic,152.msg1019.html#msg1019 As far as I know the german translation is currently not maintained... Description: Remove apostroph in backend-options of german running.itely This was

Remove white-space from storePredefinedDiagram input-string (issue 330340043 by thomasmorle...@gmail.com)

2017-09-28 Thread thomasmorley65
Reviewers: , Message: please review Description: Remove white-space from storePredefinedDiagram input-string This extends issue 4575 Please review this at https://codereview.appspot.com/330340043/ Affected files (+2, -1 lines): M ly/predefined-fretboards-init.ly Index:

Re: Add regtest for issue 5181 (issue 327470043 by d...@gnu.org)

2017-09-25 Thread thomasmorley65
On 2017/09/25 21:53:06, dak wrote: On 2017/09/25 21:28:50, thomasmorley651 wrote: > Very nice, i.e LGTM > > Not sure whether this should be reflected by a reg-test, but currently there is > no case in it triggering the warning like: > > warns = { > \override NoteHead.color = #red > c'1 d'

Re: Add regtest for issue 5181 (issue 327470043 by d...@gnu.org)

2017-09-25 Thread thomasmorley65
Very nice, i.e LGTM Not sure whether this should be reflected by a reg-test, but currently there is no case in it triggering the warning like: warns = { \override NoteHead.color = #red c'1 d' \revert NoteHead.color } { \warns -1 e' } And there are cases which will probably never work,

display-lily-tests.ly: Remove unused lily-string->markup (issue 324490043 by d...@gnu.org)

2017-09-23 Thread thomasmorley65
LGTM https://codereview.appspot.com/324490043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel

Improve warning for unattached post-event(s) (issue 328630043 by d...@gnu.org)

2017-09-16 Thread thomasmorley65
I'm not capable to review parser-code. But from description: LGTM https://codereview.appspot.com/328630043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Let Merge_rests_engraver deal with dotted rests (issue 324310043 by thomasmorle...@gmail.com)

2017-08-29 Thread thomasmorley65
On 2017/08/29 07:55:06, dak wrote: https://codereview.appspot.com/324310043/diff/40001/Documentation/notation/simultaneous.itely File Documentation/notation/simultaneous.itely (right): https://codereview.appspot.com/324310043/diff/40001/Documentation/notation/simultaneous.itely#newcode10

Re: Let Merge_rests_engraver deal with dotted rests (issue 324310043 by thomasmorle...@gmail.com)

2017-08-29 Thread thomasmorley65
https://codereview.appspot.com/324310043/diff/40001/Documentation/notation/simultaneous.itely File Documentation/notation/simultaneous.itely (right): https://codereview.appspot.com/324310043/diff/40001/Documentation/notation/simultaneous.itely#newcode10

Re: Let Merge_rests_engraver deal with dotted rests (issue 324310043 by thomasmorle...@gmail.com)

2017-08-28 Thread thomasmorley65
On 2017/08/28 15:18:06, dak wrote: https://codereview.appspot.com/324310043/diff/1/scm/scheme-engravers.scm File scm/scheme-engravers.scm (right): https://codereview.appspot.com/324310043/diff/1/scm/scheme-engravers.scm#newcode121 scm/scheme-engravers.scm:121: (define-public

Let Merge_rests_engraver deal with dotted rests (issue 324310043 by thomasmorle...@gmail.com)

2017-08-28 Thread thomasmorley65
Reviewers: , Message: Please review. Does this engraver needs an entry via ly:register-translator like the Measure_counter_engraver in the same file (scheme-engravers.scm)? Description: Let Merge_rests_engraver deal with dotted rests Compare simple rests by their duration-length, duration-log

Re: Output better message for wrong argument of dash-definiton (issue 321400043 by thomasmorle...@gmail.com)

2017-08-02 Thread thomasmorley65
On 2017/08/02 20:53:46, dak wrote: On 2017/08/02 20:44:07, thomasmorley651 wrote: > adds list-of-lists? in lilypond-scheme-predicates Frankly, I find that sort-of pointless: this just replaces one insufficient predicate with a slightly less insufficient one. It's nice that this triggers

Output better message for wrong argument of dash-definiton (issue 321400043 by thomasmorle...@gmail.com)

2017-08-01 Thread thomasmorley65
Reviewers: , Message: Please review Description: Output better message for wrong argument of dash-definiton Previously an argument like '(0 1.0 0.5 0.5) passed the pair?-typecheck, segfaulting later. Thus a new predicate, list-of-lists? is introduced. Please review this at

Re: Let arpeggio-bracket/slur depend on grob-property thickness (issue 326970043 by thomasmorle...@gmail.com)

2017-08-01 Thread thomasmorley65
On 2017/08/01 12:53:25, Carl wrote: After reviewing the slur code, I remove my objections to using grob.line-thickness in this patch. ok, so I'll go for patch set 3 https://codereview.appspot.com/326970043/ ___ lilypond-devel mailing list

Re: Allow defining markup commands in LilyPond syntax (issue 324140043 by d...@gnu.org)

2017-07-30 Thread thomasmorley65
If I understand correctly the main advantage for the user will be the possibility to define custom markup-commands with LilyPond-syntax. Collecting a subset of preexisting commands to substitute tedious coding/typing. P.e. \markup my-name = \markup \with-color #red \rotate #90 \italic \fontsize

Re: Let arpeggio-bracket/slur depend on grob-property thickness (issue 326970043 by thomasmorle...@gmail.com)

2017-07-29 Thread thomasmorley65
https://codereview.appspot.com/326970043/diff/1/lily/arpeggio.cc File lily/arpeggio.cc (right): https://codereview.appspot.com/326970043/diff/1/lily/arpeggio.cc#newcode192 lily/arpeggio.cc:192: Real lt = robust_scm2double (me->get_property ("thickness"), 0.1); On 2017/07/28 23:12:37, Dan Eble

Let arpeggio-bracket/slur depend on grob-property thickness (issue 326970043 by thomasmorle...@gmail.com)

2017-07-28 Thread thomasmorley65
Reviewers: , Message: Please review. (It's my first patch changing operative c++-code on my own) Description: Let arpeggio-bracket/slur depend on grob-property thickness Before thickness based on line-thickness from layout Please review this at https://codereview.appspot.com/326970043/

Re: Change \note markup command to get a duration (issue 328050043 by d...@gnu.org)

2017-07-25 Thread thomasmorley65
LGTM Though, wouldn't the rest-markup-command needed to be changed accordingly? https://codereview.appspot.com/328050043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Centering bass figures on whole notes and longer (issue 325070043 by pkx1...@gmail.com)

2017-07-15 Thread thomasmorley65
On 2017/07/15 09:13:08, thomasmorley651 wrote: If you try \override BassFigure.X-offset = #ly:self-alignment-interface::centered-on-x-parent you'll see improvements in some cases (others are worse). Therefore I think ly:self-alignment-interface::centered-on-x-parent is not the correct tool,

Re: Centering bass figures on whole notes and longer (issue 325070043 by pkx1...@gmail.com)

2017-07-15 Thread thomasmorley65
On 2017/07/15 09:03:30, richard_rshann.plus.com wrote: On Sat, 2017-07-15 at 01:50 -0700, mailto:thomasmorle...@gmail.com wrote: > On 2017/07/15 08:06:41, http://richard_rshann.plus.com wrote: > > > So I think you could validly object that you didn't like them being > > treated differently.

Re: Centering bass figures on whole notes and longer (issue 325070043 by pkx1...@gmail.com)

2017-07-15 Thread thomasmorley65
On 2017/07/15 08:55:43, richard_rshann.plus.com wrote: On Sat, 2017-07-15 at 08:51 +0100, Richard Shann wrote: > On Sat, 2017-07-15 at 00:40 -0700, mailto:thomasmorle...@gmail.com wrote: > > On 2017/07/15 07:25:37, http://richard_rshann.plus.com wrote: > > > On Sat, 2017-07-15 at 00:09 -0700,

Re: Centering bass figures on whole notes and longer (issue 325070043 by pkx1...@gmail.com)

2017-07-15 Thread thomasmorley65
On 2017/07/15 08:06:41, richard_rshann.plus.com wrote: So I think you could validly object that you didn't like them being treated differently. If you look at the following, namely the colored BassFigures cbf = \once \override BassFigure.color = #red << \relative c' { c1 c8 d e f g f e d

Re: Centering bass figures on whole notes and longer (issue 325070043 by pkx1...@gmail.com)

2017-07-15 Thread thomasmorley65
On 2017/07/15 07:40:01, thomasmorley651 wrote: On 2017/07/15 07:25:37, http://richard_rshann.plus.com wrote: > On Sat, 2017-07-15 at 00:09 -0700, mailto:thomasmorle...@gmail.com wrote: > > I'm afraid this patch does not fix the problem as wished. > You give an example of multiple bass

Re: Centering bass figures on whole notes and longer (issue 325070043 by pkx1...@gmail.com)

2017-07-15 Thread thomasmorley65
On 2017/07/15 07:25:37, richard_rshann.plus.com wrote: On Sat, 2017-07-15 at 00:09 -0700, mailto:thomasmorle...@gmail.com wrote: > I'm afraid this patch does not fix the problem as wished. You give an example of multiple bass figures on a note, I'm not sure what you would wish to see for

Centering bass figures on whole notes and longer (issue 325070043 by pkx1...@gmail.com)

2017-07-15 Thread thomasmorley65
I'm afraid this patch does not fix the problem as wished. See my comment on the tracker: https://sourceforge.net/p/testlilyissues/issues/5154/#1933 https://codereview.appspot.com/325070043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org

Various chain-assoc-get -> #:properties (issue 323940043 by d...@gnu.org)

2017-06-16 Thread thomasmorley65
LGTM one nit: https://codereview.appspot.com/323940043/diff/1/Documentation/snippets/three-sided-box.ly File Documentation/snippets/three-sided-box.ly (right): https://codereview.appspot.com/323940043/diff/1/Documentation/snippets/three-sided-box.ly#newcode45

Re: Create engravers for merging rests (issue 321930043 by horndud...@gmail.com)

2017-06-12 Thread thomasmorley65
One nit. See below. No need for a new patch-set, imho. You could change it right before pushing. Otherwise LGTM https://codereview.appspot.com/321930043/diff/160001/Documentation/notation/simultaneous.itely File Documentation/notation/simultaneous.itely (right):

Re: Create engravers for merging rests (issue 321930043 by horndud...@gmail.com)

2017-05-29 Thread thomasmorley65
I think this one warrants an entry in changes. https://codereview.appspot.com/321930043/diff/140001/scm/scheme-engravers.scm File scm/scheme-engravers.scm (right): https://codereview.appspot.com/321930043/diff/140001/scm/scheme-engravers.scm#newcode152 scm/scheme-engravers.scm:152: (define

Re: Create engravers for merging rests (issue 321930043 by horndud...@gmail.com)

2017-05-21 Thread thomasmorley65
I'd like to mention another point: What to do with pitched rests and rests with user-set staff-position, merge them automatically to the zero-position? I'd say using suspendRestMerging-property is sufficient to cover this case, but this is only me. Other opinions?

Re: Create engravers for merging rests (issue 321930043 by horndud...@gmail.com)

2017-05-21 Thread thomasmorley65
On 2017/05/21 04:27:33, horndude77 wrote: https://codereview.appspot.com/321930043/diff/11/scm/scheme-engravers.scm File scm/scheme-engravers.scm (right): https://codereview.appspot.com/321930043/diff/11/scm/scheme-engravers.scm#newcode151 scm/scheme-engravers.scm:151: (define

Re: Create engravers for merging rests (issue 321930043 by horndud...@gmail.com)

2017-05-20 Thread thomasmorley65
Much better now, though: https://codereview.appspot.com/321930043/diff/11/scm/scheme-engravers.scm File scm/scheme-engravers.scm (right): https://codereview.appspot.com/321930043/diff/11/scm/scheme-engravers.scm#newcode151 scm/scheme-engravers.scm:151: (define (rest-eqv rest-len-prop)

Re: Create engravers for merging rests (issue 321930043 by horndud...@gmail.com)

2017-05-18 Thread thomasmorley65
https://codereview.appspot.com/321930043/diff/60001/scm/merge-rests-engraver.scm File scm/merge-rests-engraver.scm (right): https://codereview.appspot.com/321930043/diff/60001/scm/merge-rests-engraver.scm#newcode10 scm/merge-rests-engraver.scm:10: (define (rest-length rest) This definition is

Avoid possible segfault in stencil-with-color (issue 320900043 by thomasmorle...@gmail.com)

2017-05-02 Thread thomasmorley65
Reviewers: , Message: Please review. Description: Avoid possible segfault in stencil-with-color Check whether the argument is of type color at all Please review this at https://codereview.appspot.com/320900043/ Affected files (+6, -4 lines): M scm/stencil.scm Index: scm/stencil.scm diff

Re: Fix the encoding of the PDF metadata when using guile-2.0 (issue 322930043 by thomasmorle...@gmail.com)

2017-05-01 Thread thomasmorley65
Thanks for review. https://codereview.appspot.com/322930043/diff/1/scm/framework-ps.scm File scm/framework-ps.scm (right): https://codereview.appspot.com/322930043/diff/1/scm/framework-ps.scm#newcode604 scm/framework-ps.scm:604: ;; could be replaced with the followng code: On 2017/05/01

Fix the encoding of the PDF metadata when using guile-2.0 (issue 322930043 by thomasmorle...@gmail.com)

2017-05-01 Thread thomasmorley65
Reviewers: , Message: This is one of Antonio's set of patches to make LilyPond work with guile-2.x. And one (of two) which should be compatible with guile-1. In case the cc-part will need changes during review I'll be pretty much at a loss. So Antonio is cc'ed. Please review. Description: Fix

Re: Implement extra-offset for system positioning (issue 324810043 by david.nales...@gmail.com)

2017-04-28 Thread thomasmorley65
On 2017/04/28 22:27:12, david.nalesnik wrote: On 2017/04/28 22:24:57, david.nalesnik wrote: > On 2017/04/28 16:15:42, david.nalesnik wrote: > > On 2017/04/28 15:50:52, dak wrote: > > > Does an extra-offset in X direction even make sense when the systems are > > spaced > > > according to their

Re: Implement extra-offset for system positioning (issue 324810043 by david.nales...@gmail.com)

2017-04-28 Thread thomasmorley65
Some nits, otherwise LGTM Worth an entry in changes? https://codereview.appspot.com/324810043/diff/1/input/regression/page-layout-extra-offset.ly File input/regression/page-layout-extra-offset.ly (right):

Re: Improve elbowed-hairpin (issue 324800043 by thomasmorle...@gmail.com)

2017-04-25 Thread thomasmorley65
https://codereview.appspot.com/324800043/diff/20001/scm/output-lib.scm File scm/output-lib.scm (right): https://codereview.appspot.com/324800043/diff/20001/scm/output-lib.scm#newcode1170 scm/output-lib.scm:1170: Returns a line connecting @var{pts}, using @code{ly:line-interface::line}, gets On

Re: Slashed Half-flat and add. scales for Arabic music (issue 317550043 by amir...@hotmail.de)

2017-04-25 Thread thomasmorley65
https://codereview.appspot.com/317550043/diff/20001/ly/arabic.ly File ly/arabic.ly (right): https://codereview.appspot.com/317550043/diff/20001/ly/arabic.ly#newcode19 ly/arabic.ly:19: \version "2.18.2" On 2017/04/25 20:39:10, dak wrote: On 2017/04/25 20:32:19, thomasmorley651 wrote: > The

Re: Slashed Half-flat and add. scales for Arabic music (issue 317550043 by amir...@hotmail.de)

2017-04-25 Thread thomasmorley65
Hi Amir, thanks a lot for your patch. I don't know anything about arabic music. So most comments are naggings about formating/indenting issues... Thanks, Harm https://codereview.appspot.com/317550043/diff/20001/ly/arabic.ly File ly/arabic.ly (right):

Re: Improve elbowed-hairpin (issue 324800043 by thomasmorle...@gmail.com)

2017-04-24 Thread thomasmorley65
On 2017/04/24 14:42:18, david.nalesnik wrote: LGTM. I've pointed out two minor issues, but I don't believe they should hold up the review process. By the way, applying \override Hairpin.style = #'dashed-line to the regtest input/regression/ferneyhough-hairpins.ly looks great!

Improve elbowed-hairpin (issue 324800043 by thomasmorle...@gmail.com)

2017-04-22 Thread thomasmorley65
Reviewers: , Message: Please review. Description: Improve elbowed-hairpin Let the lines be printed by the new make-connected-line-procedure, using ly:line-interface::line. The new stencil now reacts on overrides for style and dash-period/fraction. Not closing Hairpins created by

Fix not scaling stem in note-by-number-markup (issue 324780043 by thomasmorle...@gmail.com)

2017-04-16 Thread thomasmorley65
Reviewers: , Message: Please review. Description: Fix not scaling stem in note-by-number-markup Done by calling text-font-size from layout, which defaults to 11. Then scaling stem-thickness and stem-length by division of text-font-size through 11. This ensures a stem-appearance according to

Re: Add a \voicify command (issue 320820043 by d...@gnu.org)

2017-04-10 Thread thomasmorley65
On 2017/04/09 20:30:45, dak wrote: Fix the error behavior, rename \voicify to \voices Error handling is nice now. { \voices 1 << e''2 \\ a \\ c'' \\ a' >> \voices 1,2 << e'' \\ a \\ c'' \\ a' >> \voices 1,2,3 << e'' \\ a \\ c'' \\ a' >> } returns appropriate warnings. The visual

Re: Add a \voicify command (issue 320820043 by d...@gnu.org)

2017-04-09 Thread thomasmorley65
On 2017/04/09 15:13:32, dak wrote: mailto:thomasmorle...@gmail.com writes: > Great! > > Two thoughts: > > (1) > it's a common request on the user list how to continue a tie into the << > \\ >> - construct. > I'm aware we explain it at the lines of "don't use the > double-backslash". > Also,

Re: Add a \voicify command (issue 320820043 by d...@gnu.org)

2017-04-09 Thread thomasmorley65
On 2017/04/09 15:15:47, dak wrote: mailto:thomasmorle...@gmail.com writes: > https://codereview.appspot.com/320820043/diff/40001/scm/music-functions.scm > File scm/music-functions.scm (right): > > https://codereview.appspot.com/320820043/diff/40001/scm/music-functions.scm#newcode968 >

Re: Add a \voicify command (issue 320820043 by d...@gnu.org)

2017-04-08 Thread thomasmorley65
https://codereview.appspot.com/320820043/diff/40001/scm/music-functions.scm File scm/music-functions.scm (right): https://codereview.appspot.com/320820043/diff/40001/scm/music-functions.scm#newcode968 scm/music-functions.scm:968: On 2017/04/08 12:02:45, thomasmorley651 wrote: Why not throw an

Re: Add a \voicify command (issue 320820043 by d...@gnu.org)

2017-04-08 Thread thomasmorley65
Great! Two thoughts: (1) it's a common request on the user list how to continue a tie into the << \\ >> - construct. I'm aware we explain it at the lines of "don't use the double-backslash". Also, how to use named voices is explained, iirc. Though, I'd like to see an explicit example like

Add guile-config-1.8 etc. searching (issue 320830043 by truer...@gmail.com)

2017-04-05 Thread thomasmorley65
LGTM https://codereview.appspot.com/320830043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Let configure find all needed files with guile-2.2 (issue 320800043 by thomasmorle...@gmail.com)

2017-03-28 Thread thomasmorley65
https://codereview.appspot.com/320800043/diff/40001/aclocal.m4 File aclocal.m4 (right): https://codereview.appspot.com/320800043/diff/40001/aclocal.m4#newcode666 aclocal.m4:666: do On 2017/03/28 13:35:51, lemzwerg wrote: Just curious: What's the reason to have 1 - 1.9 - 1.8 - 1 - 1.9 - 1.8 as

Re: Let configure find all needed files with guile-2.2 (issue 320800043 by thomasmorle...@gmail.com)

2017-03-28 Thread thomasmorley65
https://codereview.appspot.com/320800043/diff/20001/aclocal.m4 File aclocal.m4 (right): https://codereview.appspot.com/320800043/diff/20001/aclocal.m4#newcode661 aclocal.m4:661: for r in $GUILE_CONFIG $target_guile_config $host_guile_config $build_guile_config guile-config guile-2.2-config

Let configure find all needed files with guile-2.2 (issue 320800043 by thomasmorle...@gmail.com)

2017-03-28 Thread thomasmorley65
Reviewers: , Message: This is needed to compile LilyPond with guile-2.2.x Please review. Description: Let configure find all needed files with guile-2.2 in configure.ac Adds support for STEPMAKE_GUILE Update guile-versions for STEPMAKE_GUILE and STEPMAKE_GUILE_DEVEL in aclocal.m4 update the

Re: Additions in event-listener.ly (issue 152600043 by pkx1...@gmail.com)

2017-03-25 Thread thomasmorley65
On 2017/03/25 10:34:02, pkx166h wrote: Thanks to David (and Graham). I understand David's comments but don't have the knowledge (or the time) to make the edits that are suggested here - at least from as I understand it there would need to be some kind of 'check/test' (if/then)

Let scheme-sandbox use system-repl with guile-2.x (issue 319520043 by thomasmorle...@gmail.com)

2017-03-21 Thread thomasmorley65
Reviewers: , Message: Please review Description: Let scheme-sandbox use system-repl with guile-2.x This fixes the warning returned by guile-2.x `scm-style-repl' in the default environment is deprecated. Find it in the `(ice-9 scm-style-repl)' module instead, or better

Fix some warnings about string-delete and string-filter argument order (issue 318680043 by thomasmorle...@gmail.com)

2017-03-19 Thread thomasmorley65
Reviewers: , Message: This replaces 333eaf6cee3f1717f91a1a0f70592021daeaad38 from branch remotes/origin/dev/guile-v2-work Please review. Description: Fix some warnings about string-delete and string-filter argument order This makes guile2 stop printing some warning messages: Guile used to

Fix regtest scheme-engraver.ly wrt to guile-2.2 (issue 322720043 by thomasmorle...@gmail.com)

2017-03-15 Thread thomasmorley65
Reviewers: , Message: Please review. Instead of simply replacing 1 by (current-error-port) a variable for the port could have been used probably. Opinions? Description: Fix regtest scheme-engraver.ly wrt to guile-2.2 Passing a number to format as the port is deprecated in guile-2.2 Thus using

Fix some programming errors "no interface for property" (issue 321730043 by thomasmorle...@gmail.com)

2017-03-15 Thread thomasmorley65
Reviewers: , Message: Please review Description: Fix some programming errors "no interface for property" Adds glissando-skip to the properties in note-column.cc Adds line-thickness to the properties in semi-tie.cc Please review this at https://codereview.appspot.com/321730043/ Affected files

Re: Add lilypond version predicates/operators (issue 317270043 by g...@ursliska.de)

2017-02-14 Thread thomasmorley65
On 2017/02/14 21:08:48, thomasmorley651 wrote: > The issue I can imagine: probably more expensive, especially with guilev2 Please disregard the time values from my previous posts, they are adulterated by the problems of _LilyPond_ with guile2 Doing all in pure guile-2.1.6 gives:

Re: Add lilypond version predicates/operators (issue 317270043 by g...@ursliska.de)

2017-02-14 Thread thomasmorley65
On 2017/02/14 21:03:19, thomasmorley651 wrote: To throw in my own 2cts. Why not compare strings, looks more straight forward to me. (define (calculate-version-harm ref-version) (cond ((string? ref-version) ref-version) ((number-list? ref-version) (string-concatenate

<    1   2   3   4   5   >