which-page (issue 6352049)

2012-06-28 Thread thomasmorley65

Reviewers: ,

Message:
It's possible to add a markup to a footer/header of a specified page,
using #(which-page page-number).
Multiple settings are possible.

Please review as

http://codereview.appspot.com/6352049

-Harm


Description:
which-page

It's possible to add a markup to a footer/header of a specified page,
using (which-page page-number).
Multiple settings are possible.

Please review this at http://codereview.appspot.com/6352049/

Affected files:
  M ly/titling-init.ly


Index: ly/titling-init.ly
diff --git a/ly/titling-init.ly b/ly/titling-init.ly
index  
f6daacddb399cf6bac89b3bcccbb291bab242f82..9f3c644a9cbbdfb170498fdfbf3432bd5a601d43  
100644

--- a/ly/titling-init.ly
+++ b/ly/titling-init.ly
@@ -101,6 +101,11 @@ book last one.
   (interpret-markup layout props arg)
   empty-stencil))

+#(define ((which-page nmbr) layout props arg)
+ (if (= (chain-assoc-get 'page:page-number props 0) nmbr)
+   (interpret-markup layout props arg)
+   empty-stencil))
+
 %% Bookpart first page and last page predicates
 #(define (part-first-page layout props arg)
   (if (= (chain-assoc-get 'page:page-number props -1)



___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: which-page (issue 6352049)

2012-06-30 Thread thomasmorley65

On 2012/06/29 10:13:42, t.daniels_treda.co.uk wrote:

Thomas Morley wrote Friday, June 29, 2012 10:32 AM



 Of course all features (old and new) should be documented.
 But, AFAICT, non of the possibilities offered in /ly/titling-init.ly
 (i.e. first-page, last-page, not-first-page, part-last-page) is
 documented anywhere in LM or NR.
 Well, they should be documented, but I propose to open a new issue

to do so.


Eluze already opened an issue for this - 2579 - and I'm
part-way through a fix to the docs.  I'll upload it for review
shortly, and I'd appreciate your comments and help.


Will do.


 I found only one multi-page-file in the regression-tests containing

a

 feature of titling-init.ly (i.e. `not-first-page'):
 input/regression/page-breaks.ly
 But I think this file tests something else.
 So I'd suggest to open a new issue for that, too.



Fine - please do.


Regtest added now.

-Harm



http://codereview.appspot.com/6352049/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: Doc: document \on-the-fly (2579) (issue 6347062)

2012-07-05 Thread thomasmorley65


http://codereview.appspot.com/6347062/diff/1/Documentation/notation/input.itely
File Documentation/notation/input.itely (right):

http://codereview.appspot.com/6347062/diff/1/Documentation/notation/input.itely#newcode1018
Documentation/notation/input.itely:1018: @item not-first-page
@tab  not first page in the book?
Please add the newly implemented on-page-feature. Perhaps:

@item (on-page nmbr)  @tab print on page specified by nmbr?

Feel free to improve my bad english.


-Harm

http://codereview.appspot.com/6347062/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: bar-line interface part 2/2 (issue 6498052)

2012-08-29 Thread thomasmorley65

Great work so far.
Some minor suggestions:




http://codereview.appspot.com/6498052/diff/5001/scm/bar-line.scm
File scm/bar-line.scm (right):

http://codereview.appspot.com/6498052/diff/5001/scm/bar-line.scm#newcode203
scm/bar-line.scm:203:
If a user defines some curious bar-lines like:
#(define-bar-line |SS...SS| |SS.. ..SS| |==...==|)
LilyPond will create some weird output without any hint whats wrong, I'd
suggest adding a warning like:

(map (lambda (x) (if (not (assoc-get x bar-glyph-alist))
  (ly:warning (_ bar-glyph ~a has to be defined before, in 
separate
definition) x)))
(list eol-glyph bol-glyph))

and to change the order of the predefined bar-lines at the bottom of the
file.

http://codereview.appspot.com/6498052/diff/5001/scm/bar-line.scm#newcode212
scm/bar-line.scm:212:
If a user defines his own bar-line and chooses a glyph with
string-length greater than 1, compiling will fail without a useful
log-message.
I'd suggest:

(define-public (add-bar-glyph-print-procedure glyph proc)
  Specify the single glyph @var{glyph} that calls print procedure
@var{proc}.
The procedure @var{proc} has to be defined in the form
@code{(make-...-bar-line grob extent)} even if the @var{extent}
is not used within the routine.
  (if (or (not (string? glyph))
  ( (string-length glyph) 1))
  (ly:warning (_ glyph ~a is not of string-length 1) glyph)
  (set! bar-glyph-print-procedures
(acons glyph proc bar-glyph-print-procedures

or sth like this

http://codereview.appspot.com/6498052/diff/5001/scm/bar-line.scm#newcode878
scm/bar-line.scm:878: (define-bar-line :.|.: :|. .|:  .|.)
The following to lines should be moved to the position directly after
;;repeats
(see my comment above)

http://codereview.appspot.com/6498052/diff/5001/scm/bar-line.scm#newcode882
scm/bar-line.scm:882: (define-bar-line :|] | :|]  |)
I think it should be:
(define-bar-line :|] :|]|)

http://codereview.appspot.com/6498052/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: bar-line interface part 2/2 (issue 6498052)

2012-08-30 Thread thomasmorley65


http://codereview.appspot.com/6498052/diff/10001/Documentation/snippets/printing-a-repeat-sign-at-the-beginning-of-a-piece.ly
File
Documentation/snippets/printing-a-repeat-sign-at-the-beginning-of-a-piece.ly
(right):

http://codereview.appspot.com/6498052/diff/10001/Documentation/snippets/printing-a-repeat-sign-at-the-beginning-of-a-piece.ly#newcode13
Documentation/snippets/printing-a-repeat-sign-at-the-beginning-of-a-piece.ly:13:
A @code{|:} bar line can be printed at the beginning of a piece, by
Should be:
@code{.|:}

http://codereview.appspot.com/6498052/diff/10001/input/regression/span-bar-break.ly
File input/regression/span-bar-break.ly (right):

http://codereview.appspot.com/6498052/diff/10001/input/regression/span-bar-break.ly#newcode5
input/regression/span-bar-break.ly:5: texidoc = At the beginning of a
system, the @code{|:} repeat
Should be:
texidoc = At the beginning of a system, the @code{.|:} repeat
barline is drawn between the staves, but the @code{:|.} is not.

http://codereview.appspot.com/6498052/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: bar-line interface part 2/2 (issue 6498052)

2012-09-02 Thread thomasmorley65

More nit-picking:


http://codereview.appspot.com/6498052/diff/7003/python/convertrules.py
File python/convertrules.py (right):

http://codereview.appspot.com/6498052/diff/7003/python/convertrules.py#newcode3395
python/convertrules.py:3395: @rule ((2, 17, 2), rNew bar line
interface)

There's no converting rule for the old |s
(Note the small `s')

I'd suggest to do:
|s - |-s
and to add
#(define-bar-line |-s  | |)
to /scm/bar-line.scm

http://codereview.appspot.com/6498052/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: Doc: Improve documentation of \glissando. (issue 6529043)

2012-09-20 Thread thomasmorley65

On 2012/09/20 18:08:15, Graham Percival wrote:

We normally do not include \override in most sections of the Notation

manual.

Instead, we ask users to submit LSR snippets showing the \override,

then we

include those snippets in the docs.  This allows us to improve the

documentation

with minimal effort on the part of developers.



If there's a special reason to include \override directly, then we can

-- we do

this when discussing automatic beaming, for example, because there's

no sense in

mentioning this without having overrides.  But I'm not certain if this

applies

in this case?


glissando-skip was introduced somewhere in 2.15.x, so no snippet using
it can make into LSR.
Thinking of an LSR-update, David recently suggested to wait a bit:
http://lists.gnu.org/archive/html/lilypond-devel/2012-09/msg00715.html




http://codereview.appspot.com/6529043/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: bar-line interface part 2/2: New bar line definition standard (issue 6498052)

2012-09-27 Thread thomasmorley65

One tiny addition:




http://codereview.appspot.com/6498052/diff/24001/scm/bar-line.scm
File scm/bar-line.scm (right):

http://codereview.appspot.com/6498052/diff/24001/scm/bar-line.scm#newcode1053
scm/bar-line.scm:1053: (define-bar-line :|] :|] #f  |)
When we provide a bracket-repeat-sign for left and right repeats, we
should provide a double-repeat-sign, too.
I suggest:
(define-bar-line :|][|: :|] [|:  |  |)

http://codereview.appspot.com/6498052/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: Provide define-session and define-session-public commands (issue 6588056)

2012-10-03 Thread thomasmorley65

After testing the patch's functionality, only:
LGTM

http://codereview.appspot.com/6588056/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: Allow (closed) scheme function calls as text scripts. (issue 6812088)

2012-11-05 Thread thomasmorley65

On 2012/11/05 22:40:36, lemzwerg wrote:

Very nice!


Can't review the code.
But from description:
GREAT

http://codereview.appspot.com/6812088/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


markup-commands rest-by-number and rest (issue 6850073)

2012-11-18 Thread thomasmorley65

Reviewers: ,

Message:
Please review

Description:
markup-commands rest-by-number and rest

Introduces two new markup-commands:
rest-by-number and rest
similiar to the existing note-by-number and note.
Two regression-tests for them are added.

Please review this at http://codereview.appspot.com/6850073/

Affected files:
  A input/regression/markup-rest-styles.ly
  A input/regression/markup-rest.ly
  M scm/define-markup-commands.scm



___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: markup-commands rest-by-number and rest (issue 6850073)

2012-11-19 Thread thomasmorley65

Hi Werner,

thanks for reviewing.

New patch set uploded.

http://codereview.appspot.com/6850073/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: markup-commands rest-by-number and rest (issue 6850073)

2012-11-19 Thread thomasmorley65

On 2012/11/19 05:11:38, lemzwerg wrote:

http://codereview.appspot.com/6850073/diff/1/scm/define-markup-commands.scm

File scm/define-markup-commands.scm (right):



http://codereview.appspot.com/6850073/diff/1/scm/define-markup-commands.scm#newcode3341

scm/define-markup-commands.scm:3341: breve, longa and maxima are valid
input-strings
This should be rather



   @code{breve}, @code{longa} and @code{maxima} ...



Ditto for the next lines, which are missing proper texinfo markup.

And please

make sure that at least the documentation stays within the 80

character line

length limit.


Done.
(At least I hope so)

http://codereview.appspot.com/6850073/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: Doc: Clarify footnotes as articulations (2971) (issue 6845078)

2012-11-22 Thread thomasmorley65

On 2012/11/22 11:04:00, Trevor Daniels wrote:
[...]

   I might chop the explanations - users in
general don't need this level of understanding.


Well, in the past I sometimes was beaten by the not explained why? and
how? in the docs.
So I'd always vote for deeper explanations in an additional paragraph.
Might be @warning{...} or sth else.




http://codereview.appspot.com/6845078/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: markup-commands rest-by-number and rest (issue 6850073)

2012-11-25 Thread thomasmorley65

On 2012/11/21 08:05:09, benko.pal wrote:
[...]

there are no separate glyphs for rests and multi measure rests: the M

in glyph

names stands not for MultiMeasure, but for Minus.


Didn't know that.


that makes for rewriting the
cond below, I'm afraid, but it will be simpler.


Well, I want the markup-commands to produce different output depending
on the used style. I can't see an other way, than to write detailed
conditions.
OTOH, I tried to consider your hint and to write them concise.

Thanks for reviewing!





http://codereview.appspot.com/6850073/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: Issue 2916: Document \hide and \omit (issue 6851102)

2012-11-25 Thread thomasmorley65

On 2012/11/25 23:06:58, Trevor Daniels wrote:

LGTM



Trevor


+1

Harm

http://codereview.appspot.com/6851102/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: Doc: Clarify footnotes as articulations (2971) (issue 6845078)

2012-11-25 Thread thomasmorley65

On 2012/11/22 12:42:04, Trevor Daniels wrote:
[...]

The Extending Manual is the place for explanations useful to
developers.  Granted this is rather embryonic at present, due
mainly to a dearth of knowledgeable people willing to contribute
to it.  It would benefit greatly from further input ;)


With other words: patches are welcome.
:)

-Harm



http://codereview.appspot.com/6845078/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: markup-commands rest-by-number and rest (issue 6850073)

2012-11-26 Thread thomasmorley65

On 2012/11/26 15:09:58, benko.pal wrote:

 So I'd suggest not treating the '-' at all (to make things not more
 confusing than they already are) and letting the font backend sort
 things out.



I concur.


Ok, I'll try.


http://codereview.appspot.com/6850073/diff/6004/scm/define-markup-commands.scm

File scm/define-markup-commands.scm (right):



http://codereview.appspot.com/6850073/diff/6004/scm/define-markup-commands.scm#newcode3251

scm/define-markup-commands.scm:3251: (or multi-measure-rest ( log

1)))

there are no Petrucci rest glyphs at all; use mensural without any

further

condition.  or don't even allow petrucci as style for these markups.


Well, writing this conditions I tried to orientate myself on
`select-head-glyph´ from output-lib.scm
`select-head-glyph´ returns note-head-glyphs for some styles without
defined note-head-glyphs as well. Mostly if ( log 1)
So I tried to transfer this behaviour to rests.

Did you had a look on the compiled output of the new reg-tests?
This output is what I tried to achieve.
You'll see that for some styles mensural- or neomensural-glyphes are
taken, others are defaulting.

The main question is:
If there are no defined glyphs for a specific style,
should I try to select others (currently tried)
or
should I return default-glyphs (I suspect this would be much easier)?

Opinions?


http://codereview.appspot.com/6850073/diff/6004/scm/define-markup-commands.scm#newcode3299

scm/define-markup-commands.scm:3299: ;; If there is a ledger, move the

dots in

X-direction to avoid collision.
is this comment true?  I can't see how the condition below checks for

ledger.

You're right, the comment is misleading.
I selected ledgered glyphs for whole and half rests for all styles,
apart from the listed ones.
Depending on the decision which glyphs are taken (see above), I'll
change the comment and/or the code.


http://codereview.appspot.com/6850073/diff/6004/scm/define-markup-commands.scm#newcode3305

scm/define-markup-commands.scm:3305: (and (= log 1) (equal? style

'petrucci

this check looks fishy, as there are no Petrucci-style rest glyphs.


Same problem as above:
Which glyph to take, if there's none for the specified style.



http://codereview.appspot.com/6850073/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: markup-commands rest-by-number and rest (issue 6850073)

2012-11-28 Thread thomasmorley65

On 2012/11/27 20:03:06, benko.pal wrote:


 Did you had a look on the compiled output of the new reg-tests?



no.  could you push to a dev/ branch?


Sorry. I'm still a newbie in devel tasks.
Especially with the tools needed for it.
Imagine that I tried to upload my new patch this evening. It lasted over
4 hours before I managed to _upload_ it.
And it was wrong. I had to do it again.
A hundred times (wasn't, but it feels like) I had to start again. Most
of the  mistakes were typos in git, which I couldn't correct. Two times
I corrupted my build-directory, so I had to start from scratch.
Or I produced a [PATCH 1/2] and I have no clue why, or why not the next
time I tried.
etc., etc

I remember trying to checkout a dev/ branch. It resulted in a messed up
git.
So no, I don't know how to push a dev/branch and currently I feel not
motivated to learn it.

But in case this will change, do you have a hint where to look to learn
not alone about dev/branch but to deal with git? Something like git for
dummies would be suitable.


 The main question is:
 If there are no defined glyphs for a specific style,
 should I try to select others (currently tried)
 or
 should I return default-glyphs (I suspect this would be much

easier)?


 Opinions?



I don't exactly know what is style and how is it set.  I thought it's

a local

property of your new command and must be set explicitly by the user;

in this

case you can either declare that petrucci is an invalid style or make

sure that

setting petrucci is read as setting mensural.  in other words,

translate

petrucci to mensural once and for all in the beginning.


style _is_ a local property, but I tried to make it consistent with the
style-property from \note-by-number, \note and the usage in \override
NoteHead #'style ... and \override Rest #'style ...

petrucci is now translated to mensural.


I don't know whether it's relevant or not, but (in the default style)

there's a

ledgered variant for breve rest too (though not for longa).


Yep.
I decided not to use it, _because_ there are no ledgered variants for
longa and maxima.
If wanted, this could be changed quite easily.



http://codereview.appspot.com/6850073/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: markup-commands rest-by-number and rest (issue 6850073)

2012-11-29 Thread thomasmorley65

On 2012/11/29 20:42:21, benko.pal wrote:

On 2012/11/29 00:41:14, thomasmorley65 wrote:
 On 2012/11/27 20:03:06, benko.pal wrote:

   Did you had a look on the compiled output of the new reg-tests?



I did now,


Thanks for doing this.


it's OK with me.



https://codereview.appspot.com/6850073/diff/7/input/regression/markup-rest-styles.ly

File input/regression/markup-rest-styles.ly (right):



https://codereview.appspot.com/6850073/diff/7/input/regression/markup-rest-styles.ly#newcode25

input/regression/markup-rest-styles.ly:25: '(-3 -2 -1 0 1 2 3 4 5 6

7

please fix indentation, this and the next list are at quite different

levels.

similarly in the other .ly file.


Couldn't find a guideline to do correct indentation here.
I ask on -devel


https://codereview.appspot.com/6850073/diff/7/input/regression/markup-rest.ly

File input/regression/markup-rest.ly (right):



https://codereview.appspot.com/6850073/diff/7/input/regression/markup-rest.ly#newcode27

input/regression/markup-rest.ly:27: (if (= duration 0) dots )
this if looks superfluous


Yep.
Will delete it.



https://codereview.appspot.com/6850073/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: markup-commands rest-by-number and rest (issue 6850073)

2012-12-03 Thread thomasmorley65

On 2012/11/30 01:33:08, thomasmorley65 wrote:

On 2012/11/29 20:42:21, benko.pal wrote:


https://codereview.appspot.com/6850073/diff/7/input/regression/markup-rest-styles.ly

 File input/regression/markup-rest-styles.ly (right):




https://codereview.appspot.com/6850073/diff/7/input/regression/markup-rest-styles.ly#newcode25

 input/regression/markup-rest-styles.ly:25: '(-3 -2 -1 0 1 2 3 4 5 6

7

 please fix indentation, this and the next list are at quite

different levels.

 similarly in the other .ly file.



Couldn't find a guideline to do correct indentation here.
I ask on -devel


Done.
(Hope so)




https://codereview.appspot.com/6850073/diff/7/input/regression/markup-rest.ly

 File input/regression/markup-rest.ly (right):




https://codereview.appspot.com/6850073/diff/7/input/regression/markup-rest.ly#newcode27

 input/regression/markup-rest.ly:27: (if (= duration 0) dots )
 this if looks superfluous



Yep.
Will delete it.


Done.



https://codereview.appspot.com/6850073/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: markup-commands rest-by-number and rest (issue 6850073)

2012-12-03 Thread thomasmorley65

On 2012/12/02 15:41:01, dak wrote:

mailto:thomasmorle...@googlemail.com writes:



 On 2012/11/27 20:03:06, benko.pal wrote:

  Did you had a look on the compiled output of the new reg-tests?

 no.  could you push to a dev/ branch?

 Sorry. I'm still a newbie in devel tasks.



If you have a branch where you checked in your changes, you can push

the

current HEAD with



git push origin HEAD:refs/heads/dev/thomas



or similar.  Don't ever use a fourth argument without explicit source
and target separated via colon: git's idea of putting defaults in

there

is rather absurd and can cause major headaches.



To delete a branch again, do



git push origin :refs/heads/dev/thomas



namely nothing before the colon.  That's all.


Thanks for the hints.



 of the mistakes were typos in git, which I couldn't correct. Two

times

 I corrupted my build-directory, so I had to start from scratch.  Or

I

 produced a [PATCH 1/2] and I have no clue why, or why not the next
 time I tried.  etc., etc



Huh.  The CG gives no useful information here?


The CG is quite nice to give the information you need, _if you
understand them (the CG is never translated) and if you don't make a
mistake while following them.
But there is no help-chapter if things are going wrong.



 But in case this will change, do you have a hint where to look to
 learn not alone about dev/branch but to deal with git? Something

like

 git for dummies would be suitable.



man git has a number of references at the bottom, both other manual
pages (under SEE ALSO) as well as further links.


Thanks again.
Both, Pál and Marc pointed me, privately, to a publication explaining
git.
Be sure, I will study it.



https://codereview.appspot.com/6850073/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: markup-commands rest-by-number and rest (issue 6850073)

2012-12-04 Thread thomasmorley65

On 2012/12/04 19:49:00, benko.pal wrote:

LGTM



https://codereview.appspot.com/6850073/diff/17001/input/regression/markup-rest-styles.ly

File input/regression/markup-rest-styles.ly (right):



https://codereview.appspot.com/6850073/diff/17001/input/regression/markup-rest-styles.ly#newcode18

input/regression/markup-rest-styles.ly:18: (symbol-string style))
sorry for nitpicking, but please don't use tabs.


Thought I had eliminated them.
Done now.
Also, in scm/define-markup-commands.scm


if nothing else is to be done,
I'll be happy to do the formatting.


Thanks for the offer. But if you do it for me, I'll never learn how to
do correct.



https://codereview.appspot.com/6850073/diff/17001/input/regression/markup-rest.ly

File input/regression/markup-rest.ly (right):



https://codereview.appspot.com/6850073/diff/17001/input/regression/markup-rest.ly#newcode30

input/regression/markup-rest.ly:30: (number-string (expt 2 duration))
more formatting nitpicking: do we have a line length limit?


I feel the guide-lines for indentation are contradictory.
How to do a good, readable indentation with leveled expressions on a
limited range?
Sometimes there mhas to be a reasonable compromise.

Hope I did it better this time.


https://codereview.appspot.com/6850073/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Removes '-signs from vectors (issue 6923043)

2012-12-10 Thread thomasmorley65

Reviewers: ,

Message:
Please review.


I didn't touch the translations. Correct?

git grep #'#
returns one file I don't know what to do with:
Binary file Documentation/pictures/baer-sarabande-original.jpg matches


Description:
Removes '-signs from vectors

Removes '-signs in vectors from the docs.

Please review this at http://codereview.appspot.com/6923043/

Affected files:
  M Documentation/ly-examples/cary-layout.ily
  M Documentation/notation/changing-defaults.itely
  M Documentation/notation/rhythms.itely
  M  
Documentation/snippets/applying-note-head-styles-depending-on-the-step-of-the-scale.ly
  M  
Documentation/snippets/preventing-final-mark-from-removing-final-tuplet.ly
  M  
Documentation/snippets/separating-key-cancellations-from-key-signature-changes.ly

  M input/regression/bar-number-volta-repeat.ly
  M input/regression/collision-head-solfa-fa.ly
  M input/regression/compound-time-signatures.ly
  M input/regression/easy-notation.ly
  M input/regression/note-head-solfa.ly
  M ly/property-init.ly


Index: Documentation/ly-examples/cary-layout.ily
diff --git a/Documentation/ly-examples/cary-layout.ily  
b/Documentation/ly-examples/cary-layout.ily
index  
fb30bb04db754bdc3d1e60e631d307bd6c8e898e..85db19169c697f4a3520df4c75af5d9db16ecdfa  
100644

--- a/Documentation/ly-examples/cary-layout.ily
+++ b/Documentation/ly-examples/cary-layout.ily
@@ -77,28 +77,28 @@ fraction = #(define-music-function (parser location  
music) (ly:music?)

#{ \tweak text #tuplet-number::calc-fraction-text #music #})

 triangle = #(define-music-function (parser location music) (ly:music?)
-   #{ \once \set shapeNoteStyles = #'#(do do do do do do do) #music #})
+   #{ \once \set shapeNoteStyles = ##(do do do do do do do) #music #})

 semicircle = #(define-music-function (parser location music) (ly:music?)
-   #{ \once \set shapeNoteStyles = #'#(re re re re re re re) #music #})
+   #{ \once \set shapeNoteStyles = ##(re re re re re re re) #music #})

 blackdiamond = #(define-music-function (parser location music) (ly:music?)
-   #{ \once \set shapeNoteStyles = #'#(mi mi mi mi mi mi mi) #music #})
+   #{ \once \set shapeNoteStyles = ##(mi mi mi mi mi mi mi) #music #})

 tiltedtriangle = #(define-music-function (parser location music)  
(ly:music?)

-   #{ \once \set shapeNoteStyles = #'#(fa fa fa fa fa fa fa) #music #})
+   #{ \once \set shapeNoteStyles = ##(fa fa fa fa fa fa fa) #music #})

 square = #(define-music-function (parser location music) (ly:music?)
-   #{ \once \set shapeNoteStyles = #'#(la la la la la la la) #music #})
+   #{ \once \set shapeNoteStyles = ##(la la la la la la la) #music #})

 wedge = #(define-music-function (parser location music) (ly:music?)
-   #{ \once \set shapeNoteStyles = #'#(ti ti ti ti ti ti ti) #music #})
+   #{ \once \set shapeNoteStyles = ##(ti ti ti ti ti ti ti) #music #})

 harmonic = #(define-music-function (parser location music) (ly:music?)
-	#{ \once \set shapeNoteStyles = #'#(harmonic harmonic harmonic harmonic  
harmonic harmonic harmonic) #music #})
+	#{ \once \set shapeNoteStyles = ##(harmonic harmonic harmonic harmonic  
harmonic harmonic harmonic) #music #})


 cross = #(define-music-function (parser location music) (ly:music?)
-	#{ \once \set shapeNoteStyles = #'#(cross cross cross cross cross cross  
cross) #music #})
+	#{ \once \set shapeNoteStyles = ##(cross cross cross cross cross cross  
cross) #music #})


 white = #(define-music-function (parser location music) (ly:music?)
#{ \once \override NoteHead.duration-log = #1 #music #})
Index: Documentation/notation/changing-defaults.itely
diff --git a/Documentation/notation/changing-defaults.itely  
b/Documentation/notation/changing-defaults.itely
index  
f473176eec8d9bc262e0cabb77a0d1a7cd9e06a3..42a3f40d7b32d77a557ab40b515498f529dc7d58  
100644

--- a/Documentation/notation/changing-defaults.itely
+++ b/Documentation/notation/changing-defaults.itely
@@ -3197,7 +3197,7 @@ visibility of bar lines:
 f4 g a b
 f4 g a b
 % Remove bar line at the end of the current line
-\once \override Score.BarLine.break-visibility = #'#(#f #t #t)
+\once \override Score.BarLine.break-visibility = ##(#f #t #t)
 \break
 f4 g a b
 f4 g a b
Index: Documentation/notation/rhythms.itely
diff --git a/Documentation/notation/rhythms.itely  
b/Documentation/notation/rhythms.itely
index  
dbfc038cd389be05a7cda40812604d592facae9b..50eb9f7ad1f00d37645cadc93f03c60755b95c31  
100644

--- a/Documentation/notation/rhythms.itely
+++ b/Documentation/notation/rhythms.itely
@@ -2959,7 +2959,7 @@ line visible}, @code{beginning of line visible}.  In  
the following

 example bar numbers are printed at all possible places:

 @lilypond[verbatim,quote,relative=1]
-\override Score.BarNumber.break-visibility = #'#(#t #t #t)
+\override Score.BarNumber.break-visibility = ##(#t #t #t)
 \set Score.currentBarNumber = #11
 % Permit first bar number to be printed
 \bar 
Index:  

Changes lilypond-example for rest-by-number (issue 6924053)

2012-12-12 Thread thomasmorley65

Reviewers: ,

Message:
Please review

Description:
Changes lilypond-example for rest-by-number

Please review this at https://codereview.appspot.com/6924053/

Affected files:
  M scm/define-markup-commands.scm


Index: scm/define-markup-commands.scm
diff --git a/scm/define-markup-commands.scm b/scm/define-markup-commands.scm
index  
2c6ca98aa031801d4f9ebefe4947e3c10640eade..da77f01ff228030e7098f1b376f622b56b9d0890  
100755

--- a/scm/define-markup-commands.scm
+++ b/scm/define-markup-commands.scm
@@ -3236,8 +3236,10 @@ A rest or multi-measure-rest symbol.
 \\markup {
   \\rest-by-number #3 #2
   \\hspace #2
+  \\rest-by-number #0 #1
+  \\hspace #2
   \\override #'(multi-measure-rest . #t)
-  \\rest-by-number #3 #0
+  \\rest-by-number #0 #0
 }
 @end lilypond




___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Flag an error for isolated post-events occuring in music lists (issue 6929046)

2012-12-12 Thread thomasmorley65

Can't review parser-code, but from description:
LGTM

For the sake of easier understanding I'd have given these
examples,though, to show that printing yes/no is more a matter of
accident:

% fingering is printed
%\displayMusic
{ \tweak #'color #red -3 c }

% fingering is not printed  
%\displayMusic
{ c \tweak #'color #red -3 }



https://codereview.appspot.com/6929046/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: Doc: CG Clarifying about Examples with overrides (issue 7013043)

2012-12-26 Thread thomasmorley65

On 2012/12/26 13:15:05, J_lowe wrote:


Is there any case where a snippet would not have the docs tag?



James


Some statistic from the last LSR-update:
The 2.12.3-LSR contained 645 snippets.
291 were tagged docs.

There are many LSR-snippets showing nice code/features, but not all of
them are worth to be integrated in /Documentation/snippets for different
reasons.
OTOH, some snippets from the docs should also be removed, imho.

I think someone should review the tags of each single snippet.
Perhaps during the next upgrade.



https://codereview.appspot.com/7013043/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: Simplify several library functions. (issue 7020044)

2012-12-28 Thread thomasmorley65

LGTM

Two questions:
a) Where is `identity´ defined? I can't find it, neither in LilyPond nor
in the guile-manual.
b) Why are the definitions in lily-library.scm not all (or nearly all)
defined public?
Some examples:
  split-list
  other-axis
  index-cell
  sign

I see no reason _not_ to make them public.

https://codereview.appspot.com/7020044/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Introducing two new markup-commands: draw-dashed-line and draw-dotted-line. (issue 7029045)

2012-12-30 Thread thomasmorley65

Reviewers: ,

Message:
Please review

Description:
Introducing two new markup-commands: draw-dashed-line and
draw-dotted-line.

Adding a reg-test for them.

Please review this at https://codereview.appspot.com/7029045/

Affected files:
  A input/regression/markup-line-styles.ly
  M scm/define-markup-commands.scm



___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: Introducing two new markup-commands: draw-dashed-line and draw-dotted-line. (issue 7029045)

2012-12-30 Thread thomasmorley65

On 2012/12/30 16:42:56, dak wrote:

On 2012/12/30 16:30:45, J_lowe wrote:
 Excuse my ignorance but is the \xxx-xxx-xxx ok in terms of

consistency of name

 of command and/or is 'draw' redundant?



\line arranges a text line, \draw-line draws an actual line, so the

name choice

seems ok.  Another possibility would be to use properties to specify
dotting/dashing and just use \draw-line (or \draw-circle and so on).


I thought about extending \draw-line with a 'style-property.
But in extreme cases you would have to write:

\markup {
  \override #'(style . dashed)
  \override #'(on . 0.5)
  \override #'(off . 0.2)
  \override #'(thickness . 2)
  \draw-line #'(4 . 1)
}

This seems a bit overlaoded to me. Therefore I decided to define
separate markup-commands.
Ok, this will reduce the needed \overrides only by one, but I think any
\override less would be nice.

And I'm thinking about writing additional commands for zigzag- and
trill-style lines...


After all,
lines are not the only things that might be desirable to do with a

different

line style.


Do you have anything particular in mind?


https://codereview.appspot.com/7029045/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: Introducing two new markup-commands: draw-dashed-line and draw-dotted-line. (issue 7029045)

2012-12-31 Thread thomasmorley65

On 2012/12/31 14:59:45, david.nalesnik wrote:

Harm--



This looks great!  Thank you for the 'full-length option.  I can't be

alone in

hating lines ending with incomplete dashes.


Hi David,

thanks for reviewing!


All I have is a suggestion or two, and some quibbles.



Besides what I've pointed out inline, I should mention that I got a

number of

whitespace errors when I applied your patch.  There are a number of

spots where

you should trim the ends of lines (mostly in the .scm file).


Done. (Hope so)


https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm

File scm/define-markup-commands.scm (right):



https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode155

scm/define-markup-commands.scm:155: If @code{full-length} is set

@code{#t}

(default) the dashed-line extends to the
set to #t


Done


https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode156

scm/define-markup-commands.scm:156: whole length given by @var{dest},

without

white space at begin/end.
beginning or end.


Done


https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode169

scm/define-markup-commands.scm:169: (let* (;; Get the line-thickness.
I think your variable name is sufficient--no comment needed.


Ok. Deleted.


https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode181

scm/define-markup-commands.scm:181: (begin
Branches of if expression should be aligned with test (here and

elsewhere).

Done


https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode195

scm/define-markup-commands.scm:195: (new-off (/ (- line-length corr (*

(+ 1

guess) on)) guess))
Why not use (1+ guess)


Done


https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode231

scm/define-markup-commands.scm:231: #f)
Could omit the boolean--value will be unspecified.


Done


https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode233

scm/define-markup-commands.scm:233: ;; If `on´ or `off´ is negative,

or both,

`on´ and `off´ equals zero a
Possibly ...or the sum of `on' and `off' equals [is] zero...


Changed


https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode244

scm/define-markup-commands.scm:244: #f)
Here again, you could omit the alternate.


Done



https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode248

scm/define-markup-commands.scm:248: (interval-widen (ordered-cons 0 x)
half-thick)
You do such a thorough job of commenting everything, but you don't

explain why

you need `half-thick' here.


Comment added.


https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode264

scm/define-markup-commands.scm:264: Manual settings for @code{off} is

possible

to get larger or smaller space
are possible


Done.



https://codereview.appspot.com/7029045/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


typo in time-signature-settings.scm (issue 7030052)

2013-01-01 Thread thomasmorley65

Reviewers: ,

Message:
Quite trivial.
Nevertheless, please review

Description:
typo in time-signature-settings.scm

Fixing missing space in commented example

issue 3074

Please review this at https://codereview.appspot.com/7030052/

Affected files:
  M scm/time-signature-settings.scm


Index: scm/time-signature-settings.scm
diff --git a/scm/time-signature-settings.scm  
b/scm/time-signature-settings.scm
index  
af852575dcedeb521c56b44be4672d29e1b21210..6beae72e6032173cd0c09555cb922522ed09c82b  
100644

--- a/scm/time-signature-settings.scm
+++ b/scm/time-signature-settings.scm
@@ -48,7 +48,7 @@
 ;;; ((1 . 8) . (3 3 2))
 ;;; will cause all primary beams to be broken at 3/8, 6/8, and 8/8.
 ;;;
-;;; ((1. 32) . (16 8 4 4))
+;;; ((1 . 32) . (16 8 4 4))
 ;;; will cause all 1/32, 1/64, and 1/128 beams to be broken at 1/2,  
3/4,

 ;;; 7/8, and 8/8.
 ;;;



___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: typo in time-signature-settings.scm (issue 7030052)

2013-01-07 Thread thomasmorley65

On 2013/01/07 15:35:37, J_lowe wrote:


I didn't think Thomas had push privileges (based on older commits last

year).

I asked Marc to push. But you were faster. :)


Please close Rietveld issue, thank you.



James


Closed.

https://codereview.appspot.com/7030052/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


\note-by-number supports flag-styles (issue 7231072)

2013-01-31 Thread thomasmorley65

Reviewers: ,

Message:
Please review.

The regtests markup-note.ly and markup-note-styles.ly are modified.
Changes are expected.

Description:
\note-by-number supports flag-styles

Issue 3104

For mensural and neomensural the flag is changed due to the
note-head-style.
Straight flags are possible using the new introduced flag-style-property

Please review this at https://codereview.appspot.com/7231072/

Affected files:
  M input/regression/markup-note-styles.ly
  M input/regression/markup-note.ly
  M scm/define-markup-commands.scm



___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


oversight in repeats.itely (issue 7232068)

2013-01-31 Thread thomasmorley65

Reviewers: ,

Message:
Please review

Description:
oversight in repeats.itely

Old bar-line-glyphs corrected

Fix for issue 3150

Please review this at https://codereview.appspot.com/7232068/

Affected files:
  M Documentation/notation/repeats.itely


Index: Documentation/notation/repeats.itely
diff --git a/Documentation/notation/repeats.itely  
b/Documentation/notation/repeats.itely
index  
63c2b7a9bdb4628bdbdef2b306468c4ae0855782..437088c936b6d5e87141509358b043f06b64e99a  
100644

--- a/Documentation/notation/repeats.itely
+++ b/Documentation/notation/repeats.itely
@@ -391,7 +391,7 @@ layout of repeats.  Its value is a Scheme list of  
repeat commands.


 @table @code
 @item start-repeat
-Print a @code{|:} bar line.
+Print a @code{.|:} bar line.

 @lilypond[verbatim,quote,relative=2]
 c1
@@ -404,7 +404,7 @@ As per standard engraving practice, repeat signs are  
not printed

 at the beginning of a piece.

 @item end-repeat
-Print a @code{:|} bar line:
+Print a @code{:|.} bar line:

 @lilypond[verbatim,quote,relative=2]
 c1



___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: \note-by-number supports flag-styles (issue 7231072)

2013-02-03 Thread thomasmorley65

Don't know why two identical patch-sets were uploaded

On 2013/02/02 16:11:19, dak wrote:

https://codereview.appspot.com/7231072/diff/1/scm/define-markup-commands.scm

File scm/define-markup-commands.scm (right):



https://codereview.appspot.com/7231072/diff/1/scm/define-markup-commands.scm#newcode3366

scm/define-markup-commands.scm:3366: (string-append flags.
I'd rather use
   (format #f (if ancient-flags? flags.mensural~a2~a
 flags.~a~a)
  (if ( dir 0) u d) log)
here.  It is somewhat more readable.


Changed.



https://codereview.appspot.com/7231072/diff/1/scm/define-markup-commands.scm#newcode3387

scm/define-markup-commands.scm:3387: (or (eq? flag-style 'default)

(list?

flag-style))
Why (list? flag-style)?  Is that a tricky obfuscation of (null?

flag-style)?

Rather bad thinking.
Changed.



https://codereview.appspot.com/7231072/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: Issue 3174: Implement \accidental markup and \pitched command for transposable accidental markups (issue 7323060)

2013-02-13 Thread thomasmorley65

On 2013/02/13 22:56:30, Trevor Daniels wrote:

I definitely prefer the alternative you suggest in para 3.
You would not then need the warning, which is only necessary due to

the

complicated 2-stage interface.


I'd second.

Regarding:

melody =
\relative c' {
%\override Script #'script-priority = #-100
%\override TextScript #'script-priority = #100
\key c\major
c2^\trill^\pitched des
  -\markup
 \accidental
d!
c2^\prallprall^\pitched des
  -\markup  % \fontsize #-6 \translate #'(0.6 . 0)
 \accidental
d!
}
\new Staff { \melody }
\new Staff { \transpose c' e' \melody }

Without the commented commands, TextScript and Script are switching
their positions and aren't well aligned (worse while using \prallprall).

Rather a matter of documentation/example.

https://codereview.appspot.com/7323060/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: Issue 3174: Implement \accidental markup and \pitched command for transposable accidental markups (issue 7323060)

2013-02-14 Thread thomasmorley65

On 2013/02/14 09:21:07, dak wrote:


So the message is we can do, but we still need to figure out _what_
we can do.  Proposals?


Well, currently I don't have a good idea.

Though, some time ago Arnold posted his approach on -user:
http://lists.gnu.org/archive/html/lilypond-user/2012-06/msg00278.html
direct link to his code:
http://old.nabble.com/file/p33991423/pitchedArticulations.ly

Perhaps this might be useful, I hadn't a closer look, though.

https://codereview.appspot.com/7323060/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: Make test-output-distance regtest contain span bars (issue 7400057)

2013-02-27 Thread thomasmorley65

LGTM

One suggestion:


https://codereview.appspot.com/7400057/diff/1/input/regression/test-output-distance.ly
File input/regression/test-output-distance.ly (right):

https://codereview.appspot.com/7400057/diff/1/input/regression/test-output-distance.ly#newcode20
input/regression/test-output-distance.ly:20:
How about setting \bar |.
Might be a stronger flag.

https://codereview.appspot.com/7400057/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: Adds Ferneyhough hairpins to LilyPond. (issue 7615043)

2013-03-08 Thread thomasmorley65

Hi Mike,

one thought.

The image Trevor Bača posted
http://lists.gnu.org/archive/html/lilypond-user/2013-03/pngIGBdggySyh.png
shows that the left ends of a decrecsendo-hairpin are vertical aligned
even if the hairpin isn't extended horizontal but ascending.
Currently the vertical alignment is gone if the hairpin is rotated, and,
afaik, this is the only way an ascending hairpin is available (apart
from constructing a new one from scratch).

(Similiar for cresc-hairpins.)

Is it possible to extend your code to achieve this?
Might be an interesting feature for common hairpins as well.

Since I don't know of I explained myself well enough, here some
markup-code which mimics it:

#(define drag-hairpin 5) % try different values

\markup
  \postscript
  #(string-append
  
0 1.5 moveto
0.5 -0.75 rlineto
15

(number-string (+ -0.75 drag-hairpin))

rlineto

0 -1.5 moveto
0.5 0.75 rlineto
15

(number-string (+ 0.75 drag-hairpin))

rlineto
stroke

% a red line to show vertical alignment:

gsave
0 3 moveto
1 0 0 setrgbcolor
0 -6 rlineto
stroke
grestore
   )






https://codereview.appspot.com/7615043/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: Adds Ferneyhough hairpins to LilyPond. (issue 7615043)

2013-03-09 Thread thomasmorley65

Hi Mike,

decrescendo doesn't work!!

My idea about drag-hairpin seems not too hard to implement, ofcourse
there would be need to do a clean definition of the property, currently
LilyPond returns a unsurprisingly warning:


#(define-public ((elbowed-hairpin coords mirrored?) grob)

 (define (normalize-coords cords x y)
(map
  (lambda (coord)
 (list (* x (car coord)) (* y (cdr coord
  coords))

 (define (my-c-p-s points)
 ;; !!
   (let ((drag-hairpin (ly:grob-property grob 'drag-hairpin 0)))
   (set! points
 (map (lambda (c) (list (car c) (+ drag-hairpin (cadr c
points))
 ;; !!
(make-connected-path-stencil
  points
  0.1
  1.0
  1.0
  #f
  #f)))

 (let* ((sten (ly:hairpin::print grob))
(xex (ly:stencil-extent sten X))
(lenx (interval-length xex))
(yex (ly:stencil-extent sten Y))
(leny (interval-length yex))
(uplist (normalize-coords coords lenx (/ leny 2)))
(downlist (normalize-coords coords lenx (/ leny -2

 (ly:stencil-translate
(ly:stencil-add
   (my-c-p-s uplist)
   (if mirrored? (my-c-p-s downlist) empty-stencil))
(cons (car xex) (car yex)

#(define-public ferneyhough-hairpin
 (elbowed-hairpin '((0.95 . 0.4) (1.0 . 1.0)) #t))

\relative c'' {
 %% !
 \override Hairpin #'drag-hairpin = #-2.2
 \override Hairpin #'stencil = #ferneyhough-hairpin
 a4\ c e a\f
}


https://codereview.appspot.com/7615043/diff/6001/input/regression/ferneyhough-hairpins.ly
File input/regression/ferneyhough-hairpins.ly (right):

https://codereview.appspot.com/7615043/diff/6001/input/regression/ferneyhough-hairpins.ly#newcode10
input/regression/ferneyhough-hairpins.ly:10: a4\ a a a\f
Please test decrescendo, too.

https://codereview.appspot.com/7615043/diff/6001/scm/output-lib.scm
File scm/output-lib.scm (right):

https://codereview.appspot.com/7615043/diff/6001/scm/output-lib.scm#newcode1032
scm/output-lib.scm:1032: #(define simple-hairpin
It's not clear to me why you give _this_ example.

https://codereview.appspot.com/7615043/diff/6001/scm/output-lib.scm#newcode1067
scm/output-lib.scm:1067: (define-public ferneyhough-hairpin
This defines a crescendo-hairpin for _all_ cases, even for
{ c\ d\! }
!!

https://codereview.appspot.com/7615043/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: Adds Ferneyhough hairpins to LilyPond. (issue 7615043)

2013-03-11 Thread thomasmorley65

Hi Mike,

sorry to have some more nit-picks.


https://codereview.appspot.com/7615043/diff/15001/scm/output-lib.scm
File scm/output-lib.scm (right):

https://codereview.appspot.com/7615043/diff/15001/scm/output-lib.scm#newcode1051
scm/output-lib.scm:1051: 0.1
Hard-coded thickness.
Why not multiply 'thickness-property from Hairpin and 'line-thickness as
usual?

https://codereview.appspot.com/7615043/diff/15001/scm/output-lib.scm#newcode1052
scm/output-lib.scm:1052: 1.0
I'd do the scaling here.
ly:stencil-scale would be superfluous than.

https://codereview.appspot.com/7615043/diff/15001/scm/output-lib.scm#newcode1077
scm/output-lib.scm:1077: (cons xtrans ytrans)))
I'm not sure ytrans is needed.
Setting it 0 seems to make no difference.
Delete and use ly:stencil-translate-axis?

https://codereview.appspot.com/7615043/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: Adds Ferneyhough hairpins to LilyPond. (issue 7615043)

2013-03-11 Thread thomasmorley65

On 2013/03/11 23:38:17, thomasmorley65 wrote:


https://codereview.appspot.com/7615043/diff/15001/scm/output-lib.scm#newcode1052

scm/output-lib.scm:1052: 1.0
I'd do the scaling here.
ly:stencil-scale would be superfluous than.


Either scaling-method will change the direction of constante-hairpin.
Is this intended?

https://codereview.appspot.com/7615043/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: Adds Ferneyhough hairpins to LilyPond. (issue 7615043)

2013-03-13 Thread thomasmorley65

Hi Mike,

sorry to be such an inch pincher.


https://codereview.appspot.com/7615043/diff/23001/scm/output-lib.scm
File scm/output-lib.scm (right):

https://codereview.appspot.com/7615043/diff/23001/scm/output-lib.scm#newcode1061
scm/output-lib.scm:1061: (thick (* thick (layout-line-thickness grob)))
At first sight I was surprised about `layout-line-thickness´. Than I
remembered, it is defined in bar-line.scm
How about making it public, moving it to lily-library.scm?
It could be used then in local custom-definitions.
Same with `layout-blot-diameter´.
I know, it's another topic, though, what do you think about it?

https://codereview.appspot.com/7615043/diff/23001/scm/output-lib.scm#newcode1078
scm/output-lib.scm:1078:
What do you think about predefining sth like
ferneyhoughHairpinOn/Off
constanteHairpinOn/Off
in /lyproperty-init.ly?
i.e.
ferneyhoughHairpinOn =
\override Hairpin #'stencil = #ferneyhough-hairpin
...

https://codereview.appspot.com/7615043/diff/23001/scm/output-lib.scm#newcode1082
scm/output-lib.scm:1082: (define-public constante-hairpin
I'm not happy with constante-hairpin.

Look at the output from:

dimMolto =
#(make-dynamic-script (markup #:normal-text #:italic dim. molto))

\relative c'' {
  \dynamicUp
  \override Hairpin #'stencil = #constante-hairpin
  a1 a4\dimMolto\ a a a\!
}

The hook is placed at the left pointing up.

Wouldn't it be better to have the hook _always_ at the right?
Perhaps by adding a boolean variable to `my-c-p-s´. Doing scaling only
while (and scalable? decresc?).

Is it possible to make the hook-direction depending on the position of
the Hairpin i.e below or above the staff?

https://codereview.appspot.com/7615043/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


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

2013-03-14 Thread thomasmorley65

Please review this patch as follow-up fix for issue 3010.

It was the first time I run scripts/auxiliar/makelsr.py
Hope I did it right.

https://codereview.appspot.com/7834043/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: Adds Ferneyhough hairpins to LilyPond. (issue 7615043)

2013-03-17 Thread thomasmorley65

On 2013/03/17 07:27:37, MikeSol wrote:

On 2013/03/13 21:38:59, thomasmorley65 wrote:



 https://codereview.appspot.com/7615043/diff/23001/scm/output-lib.scm
 File scm/output-lib.scm (right):




https://codereview.appspot.com/7615043/diff/23001/scm/output-lib.scm#newcode1061

 scm/output-lib.scm:1061: (thick (* thick (layout-line-thickness

grob)))

 At first sight I was surprised about `layout-line-thickness´. Than I
remembered,
 it is defined in bar-line.scm
 How about making it public, moving it to lily-library.scm?
 It could be used then in local custom-definitions.
 Same with `layout-blot-diameter´.
 I know, it's another topic, though, what do you think about it?



Great idea!


I'd have expected _this_ change in another patch.
At least it should be mentioned in the commit-message.





https://codereview.appspot.com/7615043/diff/23001/scm/output-lib.scm#newcode1078

 scm/output-lib.scm:1078:
 What do you think about predefining sth like
 ferneyhoughHairpinOn/Off
 constanteHairpinOn/Off
 in /lyproperty-init.ly?
 i.e.
 ferneyhoughHairpinOn =
 \override Hairpin #'stencil = #ferneyhough-hairpin



I'm not against this idea at all - I'd like to see it proposed in a

different

patch set, if possible.  I am teerible at judging UI issues

because I hack

so much.  It is true that shortcuts help, but there are many, many

overrides

that are shortcut-worthy.  I'm not sure what standard determines which

overrides

should get shortcuts and which shouldn't, so I leave that to a future

(and

important!) conversation.


ok





https://codereview.appspot.com/7615043/diff/23001/scm/output-lib.scm#newcode1082

 scm/output-lib.scm:1082: (define-public constante-hairpin
 I'm not happy with constante-hairpin.

[...]


The constante indication means that the dynamic should not change at

all, so the

decision to use a hairpin that grows in a given direction is

admittedly a hack.

The idea of dimMolto (or crescMolto) with constante is a contradiction

(get very

quiet while not changing dynamic).


Yep.
And this contradiction _is_ disturbing.


Insofar as a hairpin represents a change in
dynamics, constante should, in theory, be a different grob.  My

solution is not

ideal, but I think it is an OK middle-ground short of the creation of

a separate

grob.  If people think it is too hackish, I will propose a new patch

with a

Constante grob (or make the hairpin grow-direction = #CENTER default

to

constante, which'd be difficult but doable).


For now I'd tend to let it as it stands.
Nevertheless, it _is_ hackish and I think it should be replaced by one
of your proposals later.


Though, how about:


Is it possible to make the hook-direction depending on the position of

the

Hairpin i.e below or above the staff?

?



https://codereview.appspot.com/7615043/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


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

2013-03-17 Thread thomasmorley65

On 2013/03/17 16:24:16, janek wrote:

This patch needs a review from someone more knowledgeable in

docs/snippets than

me.


+1

All I did was:
I copied the files from /Documentation/snippets into
/Documentation/snippets/new.
Removed the '-signs, deleted the comments above the old
version-statement, changed the version and ran
scripts/auxiliar/makelsr.py

Concerning the version: The whole point of the issue is that '-signs in
vectors are superfluous.
Also, they were superfluous with former versions, perhaps always.
So I choosed 2.17.15 somewhat arbitrary, though, I see no problem here.

I didn't touch anything else. All other changes were made by
scripts/auxiliar/makelsr.py

https://codereview.appspot.com/7834043/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: Adds Ferneyhough hairpins to LilyPond. (issue 7615043)

2013-03-17 Thread thomasmorley65

On 2013/03/17 12:42:02, mike7 wrote:


You'd have to have a wrapper function with a call to ly:scale-stencil

with a -1

to the Y axis based on the direction of the grob.


Sure.
Or in
(define (my-c-p-s ...
as it is done for `decresc?´


This is doable, but I'd wait to see from someone who knows how these

things work

if this is actually how they are printed.
I want to say that they are always printed with the line going up

irrespective

of the side of the staff, but I could be wrong.


The only Ferneyhough-score I've at home prints the dynamics always below
the system.
(Cassandra's Dream Song for Solo Flute, Edition Peters, 1975)
Perhaps ask on the user-list.


Thanks for the review!


You're welcome.
Most of your patches are work with C++, which I don't know.
Since the current one provides some very nice feature using scheme, I
had a closer look.
Perhaps you noticed that I already used this approach to answer an
user-question.

https://codereview.appspot.com/7615043/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: Adds Ferneyhough hairpins to LilyPond. (issue 7615043)

2013-03-17 Thread thomasmorley65

On 2013/03/17 16:25:55, Joseph Rushton Wakeling wrote:

On 2013/03/17 14:37:52, thomasmorley65 wrote:



  I want to say that they are always printed with the line going up
irrespective
  of the side of the staff, but I could be wrong.



There are Ferneyhough scores with dynamics above the staff -- some of

the piano

works (e.g. Lemma-Icon-Epigram), and some of the vocal works.  See

e.g. the

second movement of his 4th String Quartet which has a part for

soprano:

http://www.youtube.com/watch?v=hVaDOz7bWU0



In this you see flared hairpins above the staff, and also dynamic

brackets above

the staff (e.g. at about 1:33 where there's a great long  bracket

above the

soprano part).


Thanks!
And while above the staff dynamic brackets have the hook down.

https://codereview.appspot.com/7615043/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


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 objects not
appear in the PDF are
What about the others: png, eps, svg?

https://codereview.appspot.com/8187044/diff/1/Documentation/changes.tely#newcode78
Documentation/changes.tely:78: respectively.  They can be given a music
expression to tweak, or
From description and example a user might expect that the following
snippets are working:

\relative c'' { a e' \hide Accidental ais1 }
\relative c'' { a e' \omit ais1 }
\relative c'' { a e' \omit Accidental ais1 }

Perhaps inserting a link to LM or NR or wherever they are explained,
would be sufficient.

https://codereview.appspot.com/8187044/diff/1/Documentation/changes.tely#newcode94
Documentation/changes.tely:94: \override NoteHead.color #red c4
Seems you missed some =

https://codereview.appspot.com/8187044/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


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

https://codereview.appspot.com/8187044/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


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 occurrences in doc-translations.

What to do? Close the issue?


only some
semi-random side-effects here and there.


What do you mean here?




https://codereview.appspot.com/7834043/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


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/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


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
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.

In general, I'd prefer rounded boxes, but with a little blot-diameter.
Though, I might change my mind once I understand the reason behind the
patch.

https://codereview.appspot.com/8685043/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


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 be:
http://lsr.dsi.unimi.it/LSR/Item?id=700



https://codereview.appspot.com/8685043/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


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: sung on one syllable; this is
called melisa, see
typo, should be
melisma

https://codereview.appspot.com/10543044/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


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


Index: lily/pitch-scheme.cc
diff --git a/lily/pitch-scheme.cc b/lily/pitch-scheme.cc
index  
0433325523d9c9dbf4e20c9993fed55ee6308043..238d235e72652853f0dda3850f05c37edef062a6  
100644

--- a/lily/pitch-scheme.cc
+++ b/lily/pitch-scheme.cc
@@ -172,7 +172,7 @@ LY_DEFINE (ly_set_middle_C_x, ly:set-middle-C!,
1, 0, 0, (SCM context),
Set the @code{middleCPosition} variable in @var{context}
 based on the variables @code{middleCClefPosition} and
-middleCOffset.)
+@code{middleCOffset}.)
 {
   LY_ASSERT_SMOB (Context, context, 1);




___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


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):

https://codereview.appspot.com/10965043/diff/1/scm/define-event-classes.scm#newcode87
scm/define-event-classes.scm:87: (define ancestor-lookup
(make-hash-table (length all-event-classes)))
I'm not convinced about the naming.

'ancestor' is defined in titling-init.ly, ofcourse with different
functionality.
'ancestor-lookup' is a _very_ generic naming, and it's usage _is_
generic in a certain way.
Though, I'd prefer something like 'ancestor-all-event-classes-lookup'.
No, that's to long.
But you get the point.

https://codereview.appspot.com/10965043/diff/1/scm/define-grobs.scm
File scm/define-grobs.scm (right):

https://codereview.appspot.com/10965043/diff/1/scm/define-grobs.scm#newcode28
scm/define-grobs.scm:28: (define-session-public all-grob-descriptions
Very nice!

https://codereview.appspot.com/10965043/diff/1/scm/document-music.scm
File scm/document-music.scm (right):

https://codereview.appspot.com/10965043/diff/1/scm/document-music.scm#newcode33
scm/document-music.scm:33: (let* ((class (ly:camel-case-lisp-identifier
(car entry)))
Apart from using tabs I'm not able to see any difference to the old
file.

No offense, just curious about it:
I was told not to use tabs, what's the reason for using them here?

https://codereview.appspot.com/10965043/diff/1/scm/document-music.scm#newcode89
scm/document-music.scm:89: (props (cdr obj))
Same here

https://codereview.appspot.com/10965043/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


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 ancestor-lookup

(make-hash-table

(length all-event-classes)))
AOn 2013/07/05 23:20:48, thomasmorley651 wrote:
 I'm not convinced about the naming.

 'ancestor' is defined in titling-init.ly, ofcourse with different
functionality.
 'ancestor-lookup' is a _very_ generic naming, and it's usage _is_

generic in a

 certain way.
 Though, I'd prefer something like

'ancestor-all-event-classes-lookup'.

 No, that's to long.
 But you get the point.



Actually, I don't get the point.  This is not a public variable, so

there is not

much chance for collision.  It's local to the lily module, and come

Guilev2, it

will be local to this file.


True. Ofcourse.
It's more that I'd prefer namings which mirrors more exactly what the
defined stuff will do.
(Though, that would mean to rename a the definition in titling-init.ly,
too. And probably a lot of others throughout our source-code.)


https://codereview.appspot.com/10965043/diff/1/scm/document-music.scm
File scm/document-music.scm (right):



https://codereview.appspot.com/10965043/diff/1/scm/document-music.scm#newcode33

scm/document-music.scm:33: (let* ((class

(ly:camel-case-lisp-identifier (car

entry)))
On 2013/07/05 23:20:48, thomasmorley651 wrote:
 Apart from using tabs I'm not able to see any difference to the old

file.


The call to ly:make-event-class takes one argument less, and

doc-context is no

longer defined.


Ahh, I've overlooked it.


 No offense, just curious about it:
 I was told not to use tabs, what's the reason for using them here?



The first commit in the series is a revert.  It brought the tabs back

from the

dead: apparently I untabified as part of the commit in question.  I

can probably

rerun the Scheme indenter.  The revert commit is not a clean revert

anyway.

Thanks for your explication.


https://codereview.appspot.com/10965043/diff/1/scm/document-music.scm#newcode91

scm/document-music.scm:91: (classes (ly:make-event-class class))
Again: this line has changed non-trivially, and the context around it

is then a

large merge conflict because of having gotten untabified in the

original commit

that has now been reverted.




https://codereview.appspot.com/10965043/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


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
Documentation/notation/fretted-strings.itely:1864: \override
Staff.StringNumber.stencil = ##f

I'm not sure about the \override
It surely looks better.
Though, our policy seems to be to put no \override into the NR, apart
from situations where it's necassary.
One might argue it's not.

https://codereview.appspot.com/10935046/diff/1/Documentation/notation/fretted-strings.itely#newcode1870
Documentation/notation/fretted-strings.itely:1870: \new StaffGroup 

Currently we have several examples with and without \new StaffGroup.
That's not consistent.
IMHO, we should group every \Staff with additional \TabStaff without
SystemStartBracket and SpanBar, i.e. omitting \new StaffGroup.

https://codereview.appspot.com/10935046/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


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 Documentation/notation/vocal.itely


Index: Documentation/notation/vocal.itely
diff --git a/Documentation/notation/vocal.itely  
b/Documentation/notation/vocal.itely
index  
f1317f169a45b6401abe674664ff7973b4e0b847..81155c956cccbac63412d92e3518e323e9879a4a  
100644

--- a/Documentation/notation/vocal.itely
+++ b/Documentation/notation/vocal.itely
@@ -448,6 +448,7 @@ More stanzas can be added by adding more
 @end lilypond

 The command @code{\addlyrics} cannot handle polyphonic settings.
+Also, it cannot be used to associate lyrics to a @code{TabVoice}.
 For these cases one should use @code{\lyricsto}.

 @subheading Using associatedVoice



___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


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 a/Documentation/notation/input.itely  
b/Documentation/notation/input.itely
index  
4381f68af39cbcddee5ecb05fc4d7199b96c1abb..0929c21801df0d3caf744113b534036309fcec74  
100644

--- a/Documentation/notation/input.itely
+++ b/Documentation/notation/input.itely
@@ -243,15 +243,15 @@ name which may clash, so
 @example
 \book @{
   \score @{ @dots{} @}
-  \layout @{ @dots{} @}
+  \paper @{ @dots{} @}
 @}
 \book @{
   \score @{ @dots{} @}
-  \layout @{ @dots{} @}
+  \paper @{ @dots{} @}
 @}
 \book @{
   \score @{ @dots{} @}
-  \layout @{ @dots{} @}
+  \paper @{ @dots{} @}
 @}
 @end example



___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


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 review this at https://codereview.appspot.com/11427044/

Affected files:
  A input/regression/markup-frame-text.ly
  M scm/define-markup-commands.scm


Index: input/regression/markup-frame-text.ly
diff --git a/input/regression/markup-frame-text.ly  
b/input/regression/markup-frame-text.ly

new file mode 100644
index  
..b7960807dc56c0d6f5a11ca43439db8608ded04e

--- /dev/null
+++ b/input/regression/markup-frame-text.ly
@@ -0,0 +1,15 @@
+\version 2.17.23
+
+\header {
+
+  texidoc = Text is framed properly with @code{\\box},
+  @code{\\circle}, @code{\\oval} and @code{\\ellipse}
+
+}
+
+\markup \column {
+   \line { \box { text in boxes 1 12 123 } }
+   \line { \circle { text in circles 1 12 123 } }
+   \line { \oval { text in ovals 1 12 123 } }
+   \line { \ellipse { text in ellipses 1 12 123 } }
+}
Index: scm/define-markup-commands.scm
diff --git a/scm/define-markup-commands.scm b/scm/define-markup-commands.scm
index  
ed1ab40420943d12fa0d4d83ce0e7dbc54d3c43f..f23644e1b55605dba04aefb40f6ac89612f5c69d  
100644

--- a/scm/define-markup-commands.scm
+++ b/scm/define-markup-commands.scm
@@ -380,6 +380,58 @@ thickness and padding around the markup.
 (m (interpret-markup layout props arg)))
 (circle-stencil m th pad)))

+(define-markup-command (ellipse layout props arg)
+  (markup?)
+  #:category graphic
+  #:properties ((thickness 1)
+(font-size 0)
+(ellipse-padding 0.2))
+ 
+@cindex drawing ellipse around text
+
+Draw an ellipse around @var{arg}.  Use @code{thickness},
+@code{ellipse-padding} and @code{font-size} properties to determine line
+thickness and padding around the markup.
+
+@lilypond[verbatim,quote]
+\\markup {
+  \\ellipse {
+Hi
+  }
+}
+@end lilypond
+  (let ((th (* (ly:output-def-lookup layout 'line-thickness)
+   thickness))
+(pad (* (magstep font-size) ellipse-padding))
+(m (interpret-markup layout props arg)))
+(ellipse-stencil m th pad pad)))
+
+(define-markup-command (oval layout props arg)
+  (markup?)
+  #:category graphic
+  #:properties ((thickness 1)
+(font-size 0)
+(oval-padding 0.75))
+ 
+@cindex drawing oval around text
+
+Draw a oval around @var{arg}.  Use @code{thickness},
+@code{oval-padding} and @code{font-size} properties to determine line
+thickness and padding around the markup.
+
+@lilypond[verbatim,quote]
+\\markup {
+  \\oval {
+Hi
+  }
+}
+@end lilypond
+  (let ((th (* (ly:output-def-lookup layout 'line-thickness)
+   thickness))
+(pad (* (magstep font-size) oval-padding))
+(m (interpret-markup layout props arg)))
+(oval-stencil m th pad pad)))
+
 (define-markup-command (with-url layout props url arg)
   (string? markup?)
   #:category graphic



___
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):

https://codereview.appspot.com/12732043/diff/1/ly/music-functions-init.ly#newcode346
ly/music-functions-init.ly:346: #(define-void-function (parser location
markup) (markup?)
I'd use define-scheme-function and return the markup in the end.

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-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-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
Documentation/extending/programming-interface.itely:277: If you want to
evaluate an expression only for its side-effect and
Here \void is declared.
Why repeat it later?
I think a reference should be sufficient.

https://codereview.appspot.com/12732043/diff/7001/Documentation/extending/programming-interface.itely#newcode651
Documentation/extending/programming-interface.itely:651: @w{@samp{\void
\displayMarkup @var{markup}}}.  Also, as with the
See above.

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-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
Though, that would mean very little difference between \displayMusic and
\displayScheme.
Any chance to join them completely?

https://codereview.appspot.com/12732043/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


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 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?


Well, we could delete all functionality of \addlyrics etc
(Ok, was a joke)



https://codereview.appspot.com/12945043/

___
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 used by the Score-context are appended to
general-grace-settings and stored in score-grace-settings, used in
engraver-init.ly

Please review this at https://codereview.appspot.com/13111045/

Affected files:
  M ly/engraver-init.ly
  M scm/music-functions.scm


Index: ly/engraver-init.ly
diff --git a/ly/engraver-init.ly b/ly/engraver-init.ly
index  
99647c75d03182f2ab159a44d4a86c9264d50149..0d1fe3d08056428cac0fb53b37fb442f6b743072  
100644

--- a/ly/engraver-init.ly
+++ b/ly/engraver-init.ly
@@ -704,26 +704,8 @@ automatically when an output definition (a  
@code{\\score} or

   figuredBassFormatter = #format-bass-figure
   metronomeMarkFormatter = #format-metronome-markup

-
   %% See also make-voice-props-set
-  graceSettings = #`(
-(Voice Stem direction ,UP)
-(Voice Stem font-size -3)
-(Voice Flag font-size -3)
-(Voice NoteHead font-size -3)
-(Voice TabNoteHead font-size -4)
-(Voice Dots font-size -3)
-(Voice Stem length-fraction 0.8)
-(Voice Stem no-stem-extend #t)
-(Voice Beam beam-thickness 0.384)
-(Voice Beam length-fraction 0.8)
-(Voice Accidental font-size -4)
-(Voice AccidentalCautionary font-size -4)
-(Voice Slur direction ,DOWN)
-(Voice Script font-size -3)
-(Voice Fingering font-size -8)
-(Voice StringNumber font-size -8)
-  )
+  graceSettings = #score-grace-settings

   keepAliveInterfaces = #'(
 bass-figure-interface
Index: scm/music-functions.scm
diff --git a/scm/music-functions.scm b/scm/music-functions.scm
index  
ff2bcb7ec72ea5b79e311f1566247af73b4940a5..bb5b6e8731c572d9b504d3f8945d5eaeb55c3753  
100644

--- a/scm/music-functions.scm
+++ b/scm/music-functions.scm
@@ -503,6 +503,28 @@ in @var{grob}.
 TupletBracket
 TrillSpanner))

+(define general-grace-settings
+  `((Voice Stem font-size -3)
+(Voice Flag font-size -3)
+(Voice NoteHead font-size -3)
+(Voice TabNoteHead font-size -4)
+(Voice Dots font-size -3)
+(Voice Stem length-fraction 0.8)
+(Voice Stem no-stem-extend #t)
+(Voice Beam beam-thickness 0.384)
+(Voice Beam length-fraction 0.8)
+(Voice Accidental font-size -4)
+(Voice AccidentalCautionary font-size -4)
+(Voice Script font-size -3)
+(Voice Fingering font-size -8)
+(Voice StringNumber font-size -8)))
+
+(define-public score-grace-settings
+  (append
+`((Voice Stem direction ,UP)
+  (Voice Slur direction ,DOWN))
+general-grace-settings))
+
 (define-safe-public (make-voice-props-set n)
   (make-sequential-music
(append
@@ -510,26 +532,9 @@ in @var{grob}.
  (if (odd? n) -1 1)))
  direction-polyphonic-grobs)
 (list
- (make-property-set 'graceSettings
-;; TODO: take this from voicedGraceSettings or  
similar.

-'((Voice Stem font-size -3)
-  (Voice Flag font-size -3)
-  (Voice NoteHead font-size -3)
-  (Voice TabNoteHead font-size -4)
-  (Voice Dots font-size -3)
-  (Voice Stem length-fraction 0.8)
-  (Voice Stem no-stem-extend #t)
-  (Voice Beam beam-thickness 0.384)
-  (Voice Beam length-fraction 0.8)
-  (Voice Accidental font-size -4)
-  (Voice AccidentalCautionary font-size -4)
-  (Voice Script font-size -3)
-  (Voice Fingering font-size -8)
-  (Voice StringNumber font-size -8)))
-
+ (make-property-set 'graceSettings general-grace-settings)
  (make-grob-property-set 'NoteColumn 'horizontal-shift (quotient n  
2))


-
 (define-safe-public (make-voice-props-override n)
   (make-sequential-music
(append
@@ -537,23 +542,7 @@ in @var{grob}.
   (if (odd? n) -1 1)))
  direction-polyphonic-grobs)
 (list
- (make-property-set 'graceSettings
-;; TODO: take this from voicedGraceSettings or  
similar.

-'((Voice Stem font-size -3)
-  (Voice Flag font-size -3)
-  (Voice NoteHead font-size -3)
-  (Voice TabNoteHead font-size -4)
-  (Voice Dots font-size -3)
-  (Voice Stem length-fraction 0.8)
-  (Voice Stem no-stem-extend #t)
-  (Voice Beam beam-thickness 0.384)
-  (Voice Beam length-fraction 0.8)
-  (Voice 

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


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 rehearsal marks.
space

https://codereview.appspot.com/13300048/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


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


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
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


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 follow up.

Please review this at https://codereview.appspot.com/14303044/

Affected files (+41, -12 lines):
  M input/regression/flags-straight.ly
  M input/regression/markup-note-styles.ly
  M input/regression/markup-note.ly
  M scm/define-markup-commands.scm
  M scm/flag-styles.scm


Index: input/regression/flags-straight.ly
diff --git a/input/regression/flags-straight.ly  
b/input/regression/flags-straight.ly
index  
80eb28c7f63de3586aa10e22dc0da8eb295ffd18..2d0c5280475d65245d5f5768437c9074d11dc711  
100644

--- a/input/regression/flags-straight.ly
+++ b/input/regression/flags-straight.ly
@@ -6,7 +6,7 @@


 % test notes, which will be shown in different styles:
-testnotes = { \autoBeamOff c'8 d'16 c'32 d'64 \acciaccatura {c'8} d'64
+testnotes = { \autoBeamOff c'8 d'16 c'32 d'64 \acciaccatura {c'8} d'64
c''8 d''16 c''32 d''64 \acciaccatura {\stemDown c''8 \stemNeutral}  
d''64  }


 {
@@ -21,7 +21,7 @@ testnotes = { \autoBeamOff c'8 d'16 c'32 d'64  
\acciaccatura {c'8} d'64

   \testnotes
 %
 %   \mark custom slant
-% %   Custom straight flag. The parameters are:
+% %   Custom straight flag. The parameters are:
 % %flag thickness and spacing
 % %up-flag angle and length
 % %   down-flag angle and length
Index: input/regression/markup-note-styles.ly
diff --git a/input/regression/markup-note-styles.ly  
b/input/regression/markup-note-styles.ly
index  
224eaf69d0886fbdc3415874fc4227004ecdc690..f5fbc9b60d2671170f61920fbf415b857e78e8b7  
100644

--- a/input/regression/markup-note-styles.ly
+++ b/input/regression/markup-note-styles.ly
@@ -61,3 +61,12 @@ all note head styles and straight flags.
 \show-note-styles #'(default)
   }
 }
+
+\markup {
+  \column {
+\combine \null \vspace #1
+\underline Flat-flag:
+\override #'(flag-style . flat-flag)
+\show-note-styles #'(default)
+  }
+}
Index: input/regression/markup-note.ly
diff --git a/input/regression/markup-note.ly  
b/input/regression/markup-note.ly
index  
307336799b8493a96a3b366b2fb87577db2fd384..c600fab42f9d83e08242ea82fe5d4cecf8b96a56  
100644

--- a/input/regression/markup-note.ly
+++ b/input/regression/markup-note.ly
@@ -58,6 +58,7 @@ mrkp =
 \override #'(style . mensural) \mrkp
 \override #'(flag-style . modern-straight-flag) \mrkp
 \override #'(flag-style . old-straight-flag) \mrkp
+\override #'(flag-style . flat-flag) \mrkp
 }
 }
 \override NoteHead.style = #'triangle
Index: scm/define-markup-commands.scm
diff --git a/scm/define-markup-commands.scm b/scm/define-markup-commands.scm
index  
6de7c9a8f42fad8ca68f9909901862e78cb9323f..5326629534aa49525f16cffc21de6d555beefa1f  
100644

--- a/scm/define-markup-commands.scm
+++ b/scm/define-markup-commands.scm
@@ -3381,7 +3381,9 @@ Supported flag-styles are @code{default},  
@code{old-straight-flag} and

(raw-length (if stem-up upflag-length downflag-length))
(angle (if stem-up upflag-angle downflag-angle))
(flag-length (+ (* raw-length factor) half-stem-thickness))
-   (flag-end (polar-rectangular flag-length angle))
+   (flag-end (if (= angle 0)
+ (cons flag-length (* half-stem-thickness dir))
+ (polar-rectangular flag-length angle)))
(thickness (* flag-thickness factor))
(thickness-offset (cons 0 (* -1 thickness dir)))
(spacing (* -1 flag-spacing factor dir))
@@ -3389,9 +3391,11 @@ Supported flag-styles are @code{default},  
@code{old-straight-flag} and

;; The points of a round-filled-polygon need to be given in
;; clockwise order, otherwise the polygon will be enlarged by
;; blot-size*2!
-   (points (if stem-up (list start flag-end
- (offset-add flag-end thickness-offset)
- (offset-add start thickness-offset))
+   (points (if stem-up
+   (list start
+ flag-end
+ (offset-add flag-end thickness-offset)
+ (offset-add start thickness-offset))
(list start
  (offset-add start thickness-offset)
  (offset-add flag-end thickness-offset)
@@ -3456,10 +3460,12 @@ Supported flag-styles are @code{default},  
@code{old-straight-flag} and

  ;; Straight-flags. Values taken from /scm/flag-style.scm
  (modern-straight-flag (straight-flag-mrkp 0.55 1 -18 1.1 22 1.2  
dir))

  (old-straight-flag 

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 d''_\mp^\! }

Same error as in 2.17.27.
Any chance to make it work?

And what is parent spanner here?
The old dynamic-engraver seemed to collect Hairpins/DynamicText in the
DynamicLineSpanner (I suppose this was the former parent spanner),
does the new one different?

At least this coding prints the same with and without your patch:

{
  \override DynamicLineSpanner #'after-line-breaking =
#(lambda (grob)
   (display (ly:grob-object grob 'elements)))

  c''1_\mf^\ d''_\mp^\!
}

Or am I completely wrong and this patch has nothing to do with the
problem above?

https://codereview.appspot.com/14460043/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


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.

https://codereview.appspot.com/14460043/

___
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, it is now possible to use another font here. For now
the font-argument is used only to make
  \override MetronomeMark #'font-name = ...
possible.

- predefining several formatter and providing the possibility to let
them
customize by the user.

- adding a regtest for different 'metronomeMarkFormatter'.

- Limitation: There is still need to specify, what note-head/flag-style
should be taken.

Please review this at https://codereview.appspot.com/14807043/

Affected files (+162, -39 lines):
  A input/regression/metronome-mark-formatter.ly
  M scm/translation-functions.scm



___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


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

text-interface), but

that seems reasonably straightforward.

Again, I don't know how. Afair, I didn't try something like that before.
Would be great, if you could point me to an example where someone did
similar before.
I then could study how to do it.

https://codereview.appspot.com/14807043/

___
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


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


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
end-of-line,
middle of line, and start-of-line

Please review this at https://codereview.appspot.com/96650050/

Affected files (+7, -2 lines):
  M scm/define-grob-properties.scm


Index: scm/define-grob-properties.scm
diff --git a/scm/define-grob-properties.scm b/scm/define-grob-properties.scm
index  
de05ace923e7f933e1c9cc59b6aa3b3797227446..5bddf26f979ed150804537257fdd4953c92fad68  
100644

--- a/scm/define-grob-properties.scm
+++ b/scm/define-grob-properties.scm
@@ -138,7 +138,7 @@ vector of length@tie{}3, where each element is one  
order for

 end-of-line, middle of line, and start-of-line, respectively.  An
 order is a list of symbols.

-For example, clefs are put after key signatures by setting
+For example, clefs are always put after key signatures by setting

 @example
 \\override Score.BreakAlignment #'break-align-orders =
@@ -148,7 +148,12 @@ For example, clefs are put after key signatures by  
setting

 key
 clef
 time-signature))
-@end example)
+@end example
+
+For an example to get different orders for end-of-line, middle of line,
+and start-of-line,
+see `separating-key-cancellations-from-key-signature-changes.ly' in
+@rlsr{Pitches})
  (break-align-symbol ,symbol? This key is used for aligning and
 spacing breakable items.)
  (break-align-symbols ,list? A list of symbols that determine



___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


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 be no links from the code to the

documentation.  The

links should always go from the documentation to the code.


I did some grepping through the scm-files and found
15 occurences of @rinternals (not included the ones from
document-context-mods.scm)
and
3 occurences of @ruser (not included the ones from
document-translation.scm)
and
2 instances in define-grob-properties.scm where an example is given,
both more or less related to BreakAlignment.



The appropriate way for a user to come to understand this (if they

haven't found

the snippet) is to find the appendix, then search the manual for
break-align-orders.


I'm not sure I understand what we expect from an average user here.
Apart from A.17 Layout properties there is only one occurence of
BreakAlignment in the NR, but without explaing why it is nessacary at
all.
I doubt the user has the chance to figure that he has to override
BreakAlignment.break-align-orders, if he wants to change the order of
the items.

More, I have to confess that I was not aware of 'Separating key
cancellations from key signature changes' before I did some git grep.

The whole point of this patch is linking the user to some usable
example-code without explaining the characteristics of (make-vector ...)

If it is not acceptable to link from the description in
define-grob-properties.scm, then we _should_ link and/or explain it in
the NR.
Though, where? I think doing it in section '1.1.3 Pitches' is not
appropriate, I feel BreakAlignment has only a loose relationship to
pitches.
Maybe in '1.6 Staff notation', though, I'm not convinced either.
And our general policy is not to use overrides in NR.
Or in section '5. Changing defaults' as an example-code?

I'm a bit helpless.
Suggestions?



Trying to maintain bidirectional linkage between the code and the

documentation

is a recipe for bitrot, I believe.



I am not in favor of this patch.



Thanks,



Carl


Thanks for review, I highly apreciate your thoughts



https://codereview.appspot.com/96650050/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


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
affecting the fret-label-indication, like the existing
`fret-label-vertical-offset'
 - paren-padding
affecting the padding of the parenthesis

Extending relevant reg-tests.

Documenting it in NR, fretted-strings.itely

Please review this at https://codereview.appspot.com/102450043/

Affected files (+233, -51 lines):
  M Documentation/notation/fretted-strings.itely
  M input/regression/fret-diagrams-dots.ly
  M input/regression/fret-diagrams-fingering.ly
  M input/regression/fret-diagrams-fret-label.ly
  M scm/define-grob-properties.scm
  M scm/fret-diagrams.scm



___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


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):



https://codereview.appspot.com/102450043/diff/1/Documentation/notation/fretted-strings.itely#newcode1002

Documentation/notation/fretted-strings.itely:1002: color.
We need a new (obvious) paragraph for this new text I think, so give a

line

space after the previous one.



The syntax is a bit uncomfortable I think. Are they really just called

'dots'?


Here's my suggestion:



Fingering indication dots can be colored as well as parenthesized;

the

parenthesis's color can also be altered independently.



The example will show the subtleties of 'inversion' and

'modification'.

done

I highly appreciate any suggestion to improve my wording, syntax and
grammar...
Sometimes I feel it's more difficult to write documentation than code.

https://codereview.appspot.com/102450043/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


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

scm/fret-diagrams.scm:119: ;; parantheses
typo: parentheses or -is


done


https://codereview.appspot.com/102450043/diff/1/scm/fret-diagrams.scm#newcode1096

scm/fret-diagrams.scm:1096: The values for @var{string-number},
@var{fret-number}, and @var{finger}
This description is not precise IMHO. The first values have to be in

that order.

Hope it's better now.


Do I have to code a value for finger before I use one ore more of the

optional

arguments?


no


Would (place-fret 1 1 parenthesized) work?


yes, it will work



https://codereview.appspot.com/102450043/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


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 and 'thin-kern _are_ used differently.
Look at the output from:

\version 2.19.8

\paper { ragged-right = ##f }

{
  a'1
  \mark default
  \bar :|.S
  b'
  \once \override Staff.BarLine #'kern = #10
  \mark 'kern 10
  \bar :|.S
  c'
  \once \override Staff.BarLine #'thin-kern = #10
  \mark 'thin-kern 10
  \bar :|.S
  d'
  \once \override Staff.BarLine #'kern = #10
  \once \override Staff.BarLine #'thin-kern = #10
  \mark \markup \column { 'kern 10 'thin-kern 10 }
  \bar :|.S
  e'
}

More:
http://code.google.com/p/lilypond/issues/detail?id=3995

https://codereview.appspot.com/105560044/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


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 does nothing.



- Mark


Look at the underlaying double-bar-line.
I'll post an image, with a colored double-bar on the tracker.

https://codereview.appspot.com/105560044/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


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 the option to change the distance of the segno double bar

line?

I'd say yes.
I'm always against loosing functionality, and deleting 'thin-kern would
limit the tweaking-possibilities.


IIRC, this is a leftover from the bar line reorganisation work, the

double

bar line || was formerly spaced with 'thin-kern as well, but as we
implemented the stacking algorithm for bar lines, all constituents are

now

spaced with 'kern.


Yep


I'd remove 'thin-kern entirely. In case someone wants to modify the

appearance

of the segno symbol, he can use a multiplier and create his own

stencil easily.

It's very easy to create custom-bar-lines nowadays -- for us!
Though, I doubt the average user has the scheme-knowledge to do so.

In the NR we don't explain how to code new barline-procedures like
`make-segno-bar-line' (only the hint to look into bar-line.scm) and
afaik nowhere else.
There's a regtest doing so, but we don't expect users to look there.

Until we can come up with an interface to create new barline-procedures,
which is much easier for the average user (although I've no clue right
now, how it should look), we should keep all tweaking/overriding
possibilities, imho.

https://codereview.appspot.com/105560044/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


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
tablature.scm

Personally I'd prefer the double stem centered on the TabNoteHead.
Something at the lines of:

(define-public (tabvoice::draw-double-stem-for-half-notes grob)
  (let ((stem (ly:stem::print grob)))
;; is the note a (dotted) half note?
(if (= 1 (ly:grob-property grob 'duration-log))
;; yes - draw double stem
(ly:stencil-add
(ly:stencil-translate-axis stem -0.315 X)
(ly:stencil-translate-axis stem 0.315 X))
;; no - draw simple stem
stem)))

And while on it, the value 0.315 could be taken from a property or an
optional argument.


https://codereview.appspot.com/110840044/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


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/

Affected files (+2, -2 lines):
  M lily/align-interface.cc
  M lily/page-layout-problem.cc


Index: lily/align-interface.cc
diff --git a/lily/align-interface.cc b/lily/align-interface.cc
index  
f9e0e06345187c2534cc69528d130e5bd40359fb..36bfec72a4854cfe7be89f50ceae08fde4b76bdb  
100644

--- a/lily/align-interface.cc
+++ b/lily/align-interface.cc
@@ -367,7 +367,7 @@ Align_interface::set_ordered (Grob *me)
 ADD_INTERFACE (Align_interface,
Order grobs from top to bottom, left to right, right to  
left

 or bottom to top.  For vertical alignments of staves, the
-@code{break-system-details} of the left
+@code{line-break-system-details} of the left
 @rinternals{NonMusicalPaperColumn} may be set to tune
 vertical spacing.,

Index: lily/page-layout-problem.cc
diff --git a/lily/page-layout-problem.cc b/lily/page-layout-problem.cc
index  
52720d54490738c1579e09d218b0a6cf688f786b..3a1a84cfc5add27da5165e2f8bcffe7399040bbf  
100644

--- a/lily/page-layout-problem.cc
+++ b/lily/page-layout-problem.cc
@@ -35,7 +35,7 @@
 #include text-interface.hh

 /*
- Returns the number of footntoes associated with a given line.
+ Returns the number of footnotes associated with a given line.
 */

 vectorGrob *



___
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
Documentation/notation/input.itely:2541: This will extract a single
fragment of the input file @code{starting}
Should @code{starting} be @emph{starting} ?

https://codereview.appspot.com/186640043/

___
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
explained/demonstrated in the docs.


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
input/regression/minimum-length-after-break.ly:15: \override
Tie.minimum-length-after-break = 20
I'd use \once \override ...
Though, that's only me

https://codereview.appspot.com/201140043/diff/1/input/regression/minimum-length-after-break.ly#newcode16
input/regression/minimum-length-after-break.ly:16: a1~
I assume it works for chords as well.
I'd add at least one example with chords and Tie or Glissando

https://codereview.appspot.com/201140043/

___
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


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
input/regression/minimum-length-after-break.ly:15: \override
Tie.minimum-length-after-break = 20
On 2015/02/05 22:51:06, david.nalesnik wrote:

On 2015/02/05 22:31:33, thomasmorley651 wrote:
 I'd use \once \override ...
 Though, that's only me



I'm happy to change that.  Would you use \once with the other

overrides in the

file?


I'd always use \once. But again, it's only me, what do others think?

https://codereview.appspot.com/201140043/diff/1/input/regression/minimum-length-after-break.ly#newcode16
input/regression/minimum-length-after-break.ly:16: a1~
On 2015/02/05 22:51:06, david.nalesnik wrote:

On 2015/02/05 22:31:34, thomasmorley651 wrote:
 I assume it works for chords as well.
 I'd add at least one example with chords and Tie or Glissando



Instead of the one regtest, I could have two.



The first would show a tied chord, and I would show how you can use
'minimum-length and 'minimum-length-after-break in various

combinations.


The second would be this regtest, and I could take out the tie example

so

there's no overlap with the other test.



What do you think?


I can't see an advantage in having two regtests. I'd prefer to extend
this one.

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


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
diff --git a/scm/output-lib.scm b/scm/output-lib.scm
index  
db4fb9b022255a7bc277706ff1774f4bb7fefa61..fdcea5de6488fed13954634e79442d21f3f0234c  
100644

--- a/scm/output-lib.scm
+++ b/scm/output-lib.scm
@@ -1367,7 +1367,7 @@ parent or the parent has no setting.
 
 ;; measure counter

-(define (measure-counter-stencil grob)
+(define-public (measure-counter-stencil grob)
   Print a number for a measure count.  The number is centered using
 the extents of @code{BreakAlignment} grobs associated with the left and
 right bounds of a @code{MeasureCounter} spanner.  Broken measures are



___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


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/

___
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-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.



https://codereview.appspot.com/199460043/diff/20001/ly/performer-init.ly

File ly/performer-init.ly (right):



https://codereview.appspot.com/199460043/diff/20001/ly/performer-init.ly#newcode174

ly/performer-init.ly:174: \accepts ChordNames
I wonder whether it makes sense to sort the many \accept lines

alphabetically...

I'd happily do it, though, why alpabetically? And why only for the lines
with \accept, could be done for the context-defs as well.
And it's also thinkable to sort it following context-hierarchy: first
bottom-contexts like Voice, then Staff up to Score and Global.
Or vice versa...

What do others think?

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-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: 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


  1   2   3   4   5   >