Re: Add `-dfont-ps-resdir` option to embed fonts to PDFs later (issue 577900043 by truer...@gmail.com)
Please read my suggested changes carefully and amend, as I've no knowledge of the subject matter. https://codereview.appspot.com/577900043/diff/577910043/Documentation/usage/running.itely File Documentation/usage/running.itely (right): https://codereview.appspot.com/577900043/diff/577910043/Documentation/usage/running.itely#newcode662 Documentation/usage/running.itely:662: Note: Unlikely @code{font-ps-resdir}, Unlike ... https://codereview.appspot.com/577900043/diff/577910043/Documentation/usage/running.itely#newcode668 Documentation/usage/running.itely:668: because embedding TrueType fonts later cause garbled characters. ... causes ... https://codereview.appspot.com/577900043/diff/577910043/Documentation/usage/running.itely#newcode670 Documentation/usage/running.itely:670: @code{gs-never-embed-fonts} embeds TrueType fonts despite its name. To avoid garbling characters, use @code{gs-never-embed-fonts}, as this embeds TrueType fonts despite its name. https://codereview.appspot.com/577900043/diff/577910043/Documentation/usage/running.itely#newcode679 Documentation/usage/running.itely:679: and then embed the fonts with Ghostscript as shown below. @item @code{font-ps-resdir} = @var{directory} Set @var{directory} to build a subset of the PostScript resource directory to be used for embedding fonts later. This is useful when you want to create a PDF without embedded fonts first and later embed the fonts with Ghostscript as shown below. https://codereview.appspot.com/577900043/diff/577910043/Documentation/usage/running.itely#newcode694 Documentation/usage/running.itely:694: Note: Unlikely @code{font-export-dir}, Unlike ... https://codereview.appspot.com/577900043/diff/577910043/Documentation/usage/running.itely#newcode702 Documentation/usage/running.itely:702: @code{gs-never-embed-fonts} embeds TrueType fonts despite its name. See corresponding change above. https://codereview.appspot.com/577900043/
Re: \compressFullBarRests should be renamed (issue 553750044 by v.villen...@gmail.com)
Thanks for picking this up and taking it over the finishing line, Valentin. LGTM apart from a couple of nits. Trevor https://codereview.appspot.com/553750044/diff/561590043/Documentation/notation/staff.itely File Documentation/notation/staff.itely (right): https://codereview.appspot.com/553750044/diff/561590043/Documentation/notation/staff.itely#newcode1559 Documentation/notation/staff.itely:1559: to be explicitely printed, using the syntax described in explicitly https://codereview.appspot.com/553750044/diff/561590043/Documentation/notation/staff.itely#newcode1589 Documentation/notation/staff.itely:1589: it requires to be followed by a music expression: requires to -> must https://codereview.appspot.com/553750044/
Re: Doc: Added documentation for fill-line line-width (issue 583340043 by davidgrant...@gmail.com)
LGTM Trevor https://codereview.appspot.com/583340043/
Re: Documentation: no \layout in markup-nested scores (issue 548590043 by v.villen...@gmail.com)
LGTM Trevor https://codereview.appspot.com/548590043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Allow Scheme/identifiers for duration multipliers (issue 346810043 by d...@gnu.org)
The proposed documentation looks fine to me. Trevor https://codereview.appspot.com/346810043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Clarify notation for slurs and beams (issue 343060043 by carl.d.soren...@gmail.com)
https://codereview.appspot.com/343060043/diff/20001/Documentation/learning/fundamental.itely File Documentation/learning/fundamental.itely (right): https://codereview.appspot.com/343060043/diff/20001/Documentation/learning/fundamental.itely#newcode495 Documentation/learning/fundamental.itely:495: @rlearning{Adding text}. On 2018/04/30 15:51:16, Carl wrote: On 2018/04/30 15:47:26, Trevor Daniels wrote: > Not sure if it makes any difference but we usually use > @ref for references within the same manual. Fundamentals is in a different source file than common notation. If I use @ref, it breaks the info build. So I had to use @rlearning to get it to pass make doc. If I'm doing something wrong, and there's some way to get @ref to work, I'd be happy to know about it. @ref definitely should work. It may be that the heading for one of them is "Articulation and dynamics", not "Articulations and dynamics" - no "s". Using @ruser bypasses the check, I think. https://codereview.appspot.com/343060043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Clarify notation for slurs and beams (issue 343060043 by carl.d.soren...@gmail.com)
Hi Carl Another minor nit! Otherwise looks fine. Trevor https://codereview.appspot.com/343060043/diff/20001/Documentation/learning/fundamental.itely File Documentation/learning/fundamental.itely (right): https://codereview.appspot.com/343060043/diff/20001/Documentation/learning/fundamental.itely#newcode495 Documentation/learning/fundamental.itely:495: @rlearning{Adding text}. Not sure if it makes any difference but we usually use @ref for references within the same manual. https://codereview.appspot.com/343060043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Clarify notation for slurs and beams (issue 343060043 by carl.d.soren...@gmail.com)
Hi Carl LGTM, with a couple of minor comments. Trevor https://codereview.appspot.com/343060043/diff/1/Documentation/learning/fundamental.itely File Documentation/learning/fundamental.itely (right): https://codereview.appspot.com/343060043/diff/1/Documentation/learning/fundamental.itely#newcode488 Documentation/learning/fundamental.itely:488: @end lilypond I'd prefer \relative or nothing in this example. \absolute has not been introduced earlier, in fact it's not used at all in the LM. https://codereview.appspot.com/343060043/diff/1/Documentation/learning/fundamental.itely#newcode492 Documentation/learning/fundamental.itely:492: @ruser{Expressive marks}, Might be good to point to the relevant sections in the LM as well, in Section 2.1 https://codereview.appspot.com/343060043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Doc: NR - rhythms.itely minor edits to Durations (issue 336030043 by pkx1...@gmail.com)
https://codereview.appspot.com/336030043/diff/1/Documentation/notation/rhythms.itely File Documentation/notation/rhythms.itely (right): https://codereview.appspot.com/336030043/diff/1/Documentation/notation/rhythms.itely#newcode93 Documentation/notation/rhythms.itely:93: music sequence will take their pitch from the preceding note or chord. On 2017/11/10 18:44:34, pkx166h wrote: Where is this repeated more fully I am not seeing it. Sorry James - I must have skimmed this too quickly and conflated "durations without a pitch" and "pitches without a duration". It all looks fine! :( https://codereview.appspot.com/336030043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Doc: NR - rhythms.itely minor edits to Durations (issue 336030043 by pkx1...@gmail.com)
Apart from a duplicated para this LGTM. Trevor https://codereview.appspot.com/336030043/diff/1/Documentation/notation/rhythms.itely File Documentation/notation/rhythms.itely (right): https://codereview.appspot.com/336030043/diff/1/Documentation/notation/rhythms.itely#newcode93 Documentation/notation/rhythms.itely:93: music sequence will take their pitch from the preceding note or chord. This is repeated more fully below - please delete https://codereview.appspot.com/336030043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Response to Issue #4603: Syntax change from all instances "partcombine" to "partCombine" and conver… (issue 323040043 by chazwi...@gmail.com)
Looks OK as far as it goes, but the Docs have been missed. Trevor https://codereview.appspot.com/323040043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Create engravers for merging rests (issue 321930043 by horndud...@gmail.com)
On 2017/05/21 17:12:26, thomasmorley651 wrote: I'd like to mention another point: What to do with pitched rests and rests with user-set staff-position, merge them automatically to the zero-position? If a user has explicitly set the position of a rest this should be honoured by default, I think ... I'd say using suspendRestMerging-property is sufficient to cover this case, but this is only me. Other opinions? ... unless some property (mergePitchedRests?) is set true. https://codereview.appspot.com/321930043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add a \voicify command (issue 320820043 by d...@gnu.org)
LGTM David, you've found an excellent way of squaring the circle! Trevor https://codereview.appspot.com/320820043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Doc: LM - Fundamental - 3.3.2 Creating contexts (issue 312540043 by pkx1...@gmail.com)
LGTM Trevor https://codereview.appspot.com/312540043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
CG: Update info - how to request access to tracker (issue 320180043 by pkx1...@gmail.com)
LGTM, apart from a minor nit. As Phil says - no need to wait for a review cycle. https://codereview.appspot.com/320180043/diff/1/Documentation/contributor/quick-start.itexi File Documentation/contributor/quick-start.itexi (right): https://codereview.appspot.com/320180043/diff/1/Documentation/contributor/quick-start.itexi#newcode534 Documentation/contributor/quick-start.itexi:534: same email address that you want to use LilyPond Developer mailing list ...use in the LilyPond ... https://codereview.appspot.com/320180043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: LM: Replace \set Staff.instrumentName (issue 314390043 by pkx1...@gmail.com)
LGTM Trevor https://codereview.appspot.com/314390043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: NR 1.2.1.d: Split note correctly (issue 319940043 by thomasmorle...@gmail.com)
LGTM Trevor https://codereview.appspot.com/319940043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Automatic LyricExtenders (issue 313240043 by perpeduumimmob...@gmail.com)
LGTM, with the trivial suggestions indicated below to the NR, but I'm not competent to check the code sections. The use of "__" has been expunged completely: should we not retain a brief mention perhaps for use when the user has turned auto extenders off? Trevor https://codereview.appspot.com/313240043/diff/20001/Documentation/notation/vocal.itely File Documentation/notation/vocal.itely (right): https://codereview.appspot.com/313240043/diff/20001/Documentation/notation/vocal.itely#newcode639 Documentation/notation/vocal.itely:639: syllable to the last note of the melisma. Lilypond detects places The convention we use is two spaces between sentences. https://codereview.appspot.com/313240043/diff/20001/Documentation/notation/vocal.itely#newcode853 Documentation/notation/vocal.itely:853: to num staffspaces. No lyric extenders shorter than this will be automatically @var{num} https://codereview.appspot.com/313240043/diff/20001/Documentation/notation/vocal.itely#newcode857 Documentation/notation/vocal.itely:857: @code{\earlyExtender #num} generates a lyric extender that starts num @var{num} https://codereview.appspot.com/313240043/diff/20001/Documentation/notation/vocal.itely#newcode860 Documentation/notation/vocal.itely:860: @code{\forceExtender} forces a lyric extenders where none would be generated "extender" https://codereview.appspot.com/313240043/diff/20001/Documentation/notation/vocal.itely#newcode863 Documentation/notation/vocal.itely:863: @code{\forceExtenderTo #num} forces a lyric extenders of length num "extender"; @var{num} https://codereview.appspot.com/313240043/diff/20001/Documentation/notation/vocal.itely#newcode870 Documentation/notation/vocal.itely:870: num staffspaces left from the normal endpoint. @var{num} https://codereview.appspot.com/313240043/diff/20001/Documentation/notation/vocal.itely#newcode1285 Documentation/notation/vocal.itely:1285: \set associatedVoice = "melody" This \set associatedVoice is also no longer necessary https://codereview.appspot.com/313240043/diff/20001/Documentation/notation/vocal.itely#newcode1445 Documentation/notation/vocal.itely:1445: @funindex \repeatTie @funindex \earlyExtender @cindex extender, early @funindex \shortenExtender @cindex extender, shorten https://codereview.appspot.com/313240043/diff/20001/Documentation/notation/vocal.itely#newcode1497 Documentation/notation/vocal.itely:1497: around the lyrics and @code{\skip} commands need to be inserted manually, as Replace \skip command https://codereview.appspot.com/313240043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Doc: Add a section about handling MIDI dynamics with multiple voices (issue 302930043 by ht.lilypond.developm...@gmail.com)
Much improved over the first attempt, thanks. LGTM. Trevor https://codereview.appspot.com/302930043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Don't use @code inside of @example (issue 304810043 by d...@gnu.org)
LGTM Trevor https://codereview.appspot.com/304810043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: NR: Mention standalone accidentals in figuremode (issue 299510043 by g...@ursliska.de)
LGTM Trevor https://codereview.appspot.com/299510043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Remove unescaped @funindex entries (issue 295570043 by d...@gnu.org)
Excellent! I've wanted to do this for years, but lacked the scripting skills to do it. Trevor https://codereview.appspot.com/295570043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Doc: CG - replace all references of Google Code (issue 294820044 by pkx1...@gmail.com)
See below. LGTM now. Trevor https://codereview.appspot.com/294820044/diff/1/Documentation/contributor/issues.itexi File Documentation/contributor/issues.itexi (right): https://codereview.appspot.com/294820044/diff/1/Documentation/contributor/issues.itexi#newcode1021 Documentation/contributor/issues.itexi:1021: @end itemize On 2016/03/27 11:39:55, pkx166h wrote: On 2016/03/26 23:09:37, Trevor Daniels wrote: > This is a large section to simply delete. I realise much of it is not currently > relevant, but rather than just deleting it perhaps it would be better to replace > it with details of the current procedure, so that when the day comes (hopefully > not soon!) to appoint a new bug-meister he will have a source of reference for > our procedures. Hello Trevor, I am not following you mean. All I can see here is more additional information (lower down) - albeit using the new Google code links - and a shuffle of one para further up than it was, (in the section above). ? Hi James Hhm, this is strange. The large deleted section only appears when viewing the delta from patch set 1, so maybe it's a misleading artifact. And I've just noticed it's all within @ignore brackets, so maybe it's less important than I thought anyway. I withdraw my comment; sorry for the noise. Trevor https://codereview.appspot.com/294820044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Doc: CG - replace all references of Google Code (issue 294820044 by pkx1...@gmail.com)
New stuff looks OK, but I'm concerned about the deleted section. https://codereview.appspot.com/294820044/diff/1/Documentation/contributor/issues.itexi File Documentation/contributor/issues.itexi (right): https://codereview.appspot.com/294820044/diff/1/Documentation/contributor/issues.itexi#newcode1021 Documentation/contributor/issues.itexi:1021: @end itemize This is a large section to simply delete. I realise much of it is not currently relevant, but rather than just deleting it perhaps it would be better to replace it with details of the current procedure, so that when the day comes (hopefully not soon!) to appoint a new bug-meister he will have a source of reference for our procedures. https://codereview.appspot.com/294820044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Improve beam count handling with subdivided beams (issue 276560043 by g...@ursliska.de)
On 2015/12/22 18:34:43, git wrote: https://codereview.appspot.com/276560043/diff/20001/Documentation/snippets/subdividing-beams.ly File Documentation/snippets/subdividing-beams.ly (right): https://codereview.appspot.com/276560043/diff/20001/Documentation/snippets/subdividing-beams.ly#newcode7 Documentation/snippets/subdividing-beams.ly:7: %% Note: this file works from version 2.19.34 So unfortunately I still don't know what to do with the snippet. This is the situation: - The snippet already exists and is used in the NR - It was not in Documentation/snippets/new before I copied it there So this seems to indicate that I should updated the snippet in the LSR. But while it will probably compile with 2.18.0 it doesn't give proper result before 2.19.34, and the modified text refers to that new functionality of my patch. So what should I do now? What you did was OK; only snippets that work correctly with 2.18.2 should be edited in the LSR. Just edit the headers as James suggested. Trevor https://codereview.appspot.com/276560043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Fix documentation of \once \partcombine* (issue 280940043 by d...@gnu.org)
LGTM, apart from a pre-existing typo. https://codereview.appspot.com/280940043/diff/1/Documentation/notation/simultaneous.itely File Documentation/notation/simultaneous.itely (right): https://codereview.appspot.com/280940043/diff/1/Documentation/notation/simultaneous.itely#newcode1030 Documentation/notation/simultaneous.itely:1030: case the @code{\partcombine} function can be overriden with one of the May as well correct this while you are there: "overridden" https://codereview.appspot.com/280940043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Implement rounded-box whiteout style (issue 274590043 by paulwmor...@gmail.com)
LGTM When this is pushed we ought to raise an issue to document whiteout properly in the NR. https://codereview.appspot.com/274590043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Doc: LM - Document tweaking of StaffSymbol and LedgerLineSpanner (issue 275490043 by pkx1...@gmail.com)
LGTM https://codereview.appspot.com/275490043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Doc: CG - Updated the Meister's section (issue 276980044 by pkx1...@gmail.com)
Much clearer, James. LGTM, apart from a couple of nits. https://codereview.appspot.com/276980044/diff/1/Documentation/contributor/administration.itexi File Documentation/contributor/administration.itexi (right): https://codereview.appspot.com/276980044/diff/1/Documentation/contributor/administration.itexi#newcode144 Documentation/contributor/administration.itexi:144: To train new volunteers in our Documentaton style and policy this Documentation https://codereview.appspot.com/276980044/diff/1/Documentation/contributor/administration.itexi#newcode145 Documentation/contributor/administration.itexi:145: includes to help organize any LilyPond Snippet Repository (LSR) work. ...policy, including organising LilyPond Snippet ... https://codereview.appspot.com/276980044/diff/1/Documentation/contributor/administration.itexi#newcode178 Documentation/contributor/administration.itexi:178: Updates all Issue statuses for all patches that are currenly in the currently https://codereview.appspot.com/276980044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Ensure one beam is left in subdivided beams (issue 278060043 by lilyli...@googlemail.com)
LGTM https://codereview.appspot.com/278060043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Implement make-bow-stencil, make-tie-stencil for use in markup-commands undertie and overtie (issue 270640043 by thomasmorle...@gmail.com)
LGTM, apart from an oversight, although this is based on just eye-balling. https://codereview.appspot.com/270640043/diff/11/scm/define-markup-commands.scm File scm/define-markup-commands.scm (right): https://codereview.appspot.com/270640043/diff/11/scm/define-markup-commands.scm#newcode623 scm/define-markup-commands.scm:623: (text-direction UP) I don't think text-direction is used now. https://codereview.appspot.com/270640043/diff/11/scm/define-markup-commands.scm#newcode654 scm/define-markup-commands.scm:654: (text-direction UP) ditto https://codereview.appspot.com/270640043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Doc: NR Clarify repeats w\ partials and barchecks (issue 277940043 by pkx1...@gmail.com)
LGTM https://codereview.appspot.com/277940043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Implement make-bow-stencil, make-tie-stencil for use in markup-commands undertie and overtie (issue 270640043 by thomasmorle...@gmail.com)
https://codereview.appspot.com/270640043/diff/60001/scm/define-markup-commands.scm#newcode623 > > scm/define-markup-commands.scm:623: (direction DOWN) > > Should this markup command be called "undertie" or should it rather be "tie", > > with "undertie" explicitly overriding `direction'? > > > > Because the rather explicit name '\undertie' seems a bit inconsistent with the > > behavior of > > > > { > > c'1^\markup \undertie hm > > } > > > > where the direction is determined by the direction specified for the > TextScript. > > Hm, this was _not_ intended. > In this case a Tie _below_ the arg should be printed. For ties above the arg > \overtie was defined. > > Though, thinking (a little) about it, we could go for > a) > two commands (under- and overtie) independant from direction-modifiers > or > b) > one tie-markup-command letting the user specify direction via \override > (direction . ...) and/or direction-modifiers c) a tie-command, with under/overtie derived from it. I think that was your proposal. > > I'm undecided for now, opinions? In the example above: c'1^\markup \undertie hm the direction modifier should indicate the placement of the markup relative to the staff, surely. The placement of the tie relative to the text needs to be specified independently of the direction modifier, so \undertie and \overtie are needed for this. I don't like \tie; \undertie is consistent with \underline. (OK, so we don't have \overline, but we could.) https://codereview.appspot.com/270640043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Doc: Usage - Update svg & svg-woff backend text (issue 274260043 by pkx1...@gmail.com)
I can't vouch for the technicalities, but LGTM apart from a few typos. https://codereview.appspot.com/274260043/diff/40001/Documentation/usage/running.itely File Documentation/usage/running.itely (right): https://codereview.appspot.com/274260043/diff/40001/Documentation/usage/running.itely#newcode469 Documentation/usage/running.itely:469: as separate encapsulated postscipt files for each page but without fonts postscript https://codereview.appspot.com/274260043/diff/40001/Documentation/usage/running.itely#newcode486 Documentation/usage/running.itely:486: Any SVG viewer will therefore require the fonts be available to it for ... fonts to be available ... https://codereview.appspot.com/274260043/diff/40001/Documentation/usage/running.itely#newcode487 Documentation/usage/running.itely:487: the proper rendering of both text and lyrics. It is recommended to not ... recommended not to use ... https://codereview.appspot.com/274260043/diff/40001/Documentation/usage/running.itely#newcode785 Documentation/usage/running.itely:785: require the fonts be available to it for the proper rendering of both .. to be available ... https://codereview.appspot.com/274260043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Issue 4504/5 update changes.tely (issue 275770043 by paulwmor...@gmail.com)
I raise a couple of questions - maybe I'm not understanding something. https://codereview.appspot.com/275770043/diff/1/scm/define-grob-properties.scm File scm/define-grob-properties.scm (right): https://codereview.appspot.com/275770043/diff/1/scm/define-grob-properties.scm#newcode1143 scm/define-grob-properties.scm:1143: (whiteout ,boolean-or-symbol? "If a number or true, the grob is Shouldn't this be boolean-or-number> https://codereview.appspot.com/275770043/diff/1/scm/define-grob-properties.scm#newcode1149 scm/define-grob-properties.scm:1149: (whiteout-style ,number? "Determines the shape of the Shouldn't this be ,symbol? https://codereview.appspot.com/275770043/diff/1/scm/define-grob-properties.scm#newcode1150 scm/define-grob-properties.scm:1150: @code{whiteout} background. Available are @code{outline} and the 'outline https://codereview.appspot.com/275770043/diff/1/scm/define-grob-properties.scm#newcode1151 scm/define-grob-properties.scm:1151: default @code{box}.") 'box https://codereview.appspot.com/275770043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Carry multi-measure rests across voices in \partcombine (issue 265410043 by nine.fierce.ball...@gmail.com)
https://codereview.appspot.com/265410043/diff/60001/lily/part-combine-iterator.cc File lily/part-combine-iterator.cc (right): https://codereview.appspot.com/265410043/diff/60001/lily/part-combine-iterator.cc#newcode146 lily/part-combine-iterator.cc:146: // get_event_length(), which reads "length"? Perhaps this question should be resolved before pushing. https://codereview.appspot.com/265410043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Create \= command for setting spanner-id (issue 268050045 by d...@gnu.org)
LGTM https://codereview.appspot.com/268050045/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Doc: Included/compile.itexi - CG 4.2 - Updated notes on reqs for compiling (issue 261430043 by pkx1...@gmail.com)
Not being overly familiar with *nix I can't vouch for the technicalities and syntax, but the general text looks fine (apart from a couple of trivial typos). So LGTM. https://codereview.appspot.com/261430043/diff/1/Documentation/included/compile.itexi File Documentation/included/compile.itexi (right): https://codereview.appspot.com/261430043/diff/1/Documentation/included/compile.itexi#newcode144 Documentation/included/compile.itexi:144: documented were run immeidiately after the initial installation Typo https://codereview.appspot.com/261430043/diff/1/Documentation/included/compile.itexi#newcode263 Documentation/included/compile.itexi:263: Download and instgall all the LilyPond build-dependencies (approximately Typo https://codereview.appspot.com/261430043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Doc: CG - remove link to R. Kainhofer's doc builds (issue 266960043 by pkx1...@gmail.com)
LGTM https://codereview.appspot.com/266960043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Doc: Usage - Updated 'Running LilyPond' intros (issue 261240043 by pkx1...@gmail.com)
You don't appear to have changed the menus in the translations, but I would strongly recommend leaving all the translations to the translators - unless you can translate the modified headings into Czech and Japanese. https://codereview.appspot.com/261240043/diff/20001/Documentation/usage/running.itely File Documentation/usage/running.itely (right): https://codereview.appspot.com/261240043/diff/20001/Documentation/usage/running.itely#newcode32 Documentation/usage/running.itely:32: convenient and simple-to-use GUI. These still require that LilyPond is "... require LilyPond to be installed ..." would be better. https://codereview.appspot.com/261240043/diff/20001/Documentation/usage/running.itely#newcode54 Documentation/usage/running.itely:54: the section @q{Running on the command-line} from here; @rweb{Windows}. From here: (should be a colon). Same in following para. https://codereview.appspot.com/261240043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Issue 4595: NR: Describe default fonts as TeX Gyre fonts (issue 268780043 by truer...@gmail.com)
LGTM https://codereview.appspot.com/268780043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Web: Download.itexi - expand on Windows install (issue 258680043 by p...@gnu.org)
LGTM https://codereview.appspot.com/258680043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Authors - Updated authors.itexi (issue 258660043 by p...@gnu.org)
https://codereview.appspot.com/258660043/diff/1/Documentation/included/authors.itexi File Documentation/included/authors.itexi (left): https://codereview.appspot.com/258660043/diff/1/Documentation/included/authors.itexi#oldcode199 Documentation/included/authors.itexi:199: Abraham Lee I think Abraham Lee has contributed to the current unstable release, hasn't he? https://codereview.appspot.com/258660043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Improve wording for an NR paragraph (issue 259710043 by simon.albre...@mail.de)
LGTM Trevor https://codereview.appspot.com/259710043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Issue 347: document that ragged-bottom = #t overrides ragged-last-bottom (issue 257280043 by d...@gnu.org)
On 2015/08/11 11:25:23, dak wrote: On 2015/08/11 10:48:24, Trevor Daniels wrote: > https://codereview.appspot.com/257280043/diff/1/Documentation/notation/spacing.itely > File Documentation/notation/spacing.itely (right): > > https://codereview.appspot.com/257280043/diff/1/Documentation/notation/spacing.itely#newcode383 > Documentation/notation/spacing.itely:383: @code{ragged-last-bottom} is set to > false. > On 2015/08/11 10:41:25, dak wrote: > > On 2015/08/11 10:16:35, Trevor Daniels wrote: > > > I'm not sure this is an improvement. I think we want to say, > > > > > > "This controls the spacing of the last page (or the last page > > > in each section created with a @code{\bookpart} block) when (and > > > only when) ragged-bottom is false. If ragged-bottom is true the > > > value of ragged-last-bottom is ignored and all pages are ragged." > > > > "This controls the spacing" does not say in which manner it controls the > > spacing. > > OK, so "In the same way as ragged-bottom, this controls ...". Don't see that I can come up with a change that could unambiguously be considered an improvement, so leaving this one open for someone else to grab. I see what you mean. The original wording is correct, and covers the point made in Issue 347, although in such a subtle way that at least one experienced developer failed to understand it. My suggested wording simply elaborates the point to make it more explicit. Not sure it justifies the extra space though. Unless there are comments encouraging me to add this change I'll simply mark Issue 347 as Invalid (with reasons, of course.) Trevor https://codereview.appspot.com/257280043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Issue 347: document that ragged-bottom = #t overrides ragged-last-bottom (issue 257280043 by d...@gnu.org)
https://codereview.appspot.com/257280043/diff/1/Documentation/notation/spacing.itely File Documentation/notation/spacing.itely (right): https://codereview.appspot.com/257280043/diff/1/Documentation/notation/spacing.itely#newcode383 Documentation/notation/spacing.itely:383: @code{ragged-last-bottom} is set to false. On 2015/08/11 10:41:25, dak wrote: On 2015/08/11 10:16:35, Trevor Daniels wrote: > I'm not sure this is an improvement. I think we want to say, > > "This controls the spacing of the last page (or the last page > in each section created with a @code{\bookpart} block) when (and > only when) ragged-bottom is false. If ragged-bottom is true the > value of ragged-last-bottom is ignored and all pages are ragged." "This controls the spacing" does not say in which manner it controls the spacing. OK, so "In the same way as ragged-bottom, this controls ...". https://codereview.appspot.com/257280043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Issue 347: document that ragged-bottom = #t overrides ragged-last-bottom (issue 257280043 by d...@gnu.org)
https://codereview.appspot.com/257280043/diff/1/Documentation/notation/spacing.itely File Documentation/notation/spacing.itely (right): https://codereview.appspot.com/257280043/diff/1/Documentation/notation/spacing.itely#newcode383 Documentation/notation/spacing.itely:383: @code{ragged-last-bottom} is set to false. I'm not sure this is an improvement. I think we want to say, "This controls the spacing of the last page (or the last page in each section created with a @code{\bookpart} block) when (and only when) ragged-bottom is false. If ragged-bottom is true the value of ragged-last-bottom is ignored and all pages are ragged." Trevor https://codereview.appspot.com/257280043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Doc: Usage - Update section on articulate.ly (issue 259130043 by pkx1...@gmail.com)
https://codereview.appspot.com/259130043/diff/20001/Documentation/usage/external.itely File Documentation/usage/external.itely (right): https://codereview.appspot.com/259130043/diff/20001/Documentation/usage/external.itely#newcode780 Documentation/usage/external.itely:780: @rweb{Easier editing} and @ruser{Working with input files}. I think what we want to say here is that some users have produced files that can be @code{\include}d in LilyPond to produce certain effects, and the ones that are listed below are part of the LilyPond distribution. https://codereview.appspot.com/259130043/diff/20001/Documentation/usage/external.itely#newcode797 Documentation/usage/external.itely:797: slurs) according toe the articulation markings attached to them; for toe->to https://codereview.appspot.com/259130043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Doc: Issue 4528/3 Document ssaattbb.ly built-in template (issue 256340043 by tdanielsmu...@googlemail.com)
Reviewers: , https://codereview.appspot.com/256340043/diff/1/input/regression/ssaattbb-template-with-changed-instrument-names.ly File input/regression/ssaattbb-template-with-changed-instrument-names.ly (left): https://codereview.appspot.com/256340043/diff/1/input/regression/ssaattbb-template-with-changed-instrument-names.ly#oldcode5 input/regression/ssaattbb-template-with-changed-instrument-names.ly:5: can be changed when using the satb built-in template." git-cl is showing the wrong base file here. While it's true I used this file as the starting point, the ssaattbb file should be new, not shown as an update of this one - and in my repository it is new and the satb file is still present and correct. ?? Description: Doc: Issue 4528/3 Document ssaattbb.ly built-in template Also amend documentation about the use of \layout and Layout, and add note to make it clear that the TwoVoicesPerStaff variables cannot be modified part-way through a score. Issue 4528/2 Add regression tests for ssaattbb.ly built-in template Issue 4528/1 Add ly/ssaattbb.ly built-in template Please review this at https://codereview.appspot.com/256340043/ Affected files (+632, -25 lines): M Documentation/learning/templates.itely A input/regression/ssaattbb-template-with-all-staves.ly A input/regression/ssaattbb-template-with-all-voices-on-one-staff.ly A + input/regression/ssaattbb-template-with-changed-instrument-names.ly A input/regression/ssaattbb-template-with-men-women-and-descant.ly A ly/ssaattbb.ly ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Issue 4338: fix revocery description of \octaveCheck (issue 255570043 by d...@gnu.org)
LTGM Trevor https://codereview.appspot.com/255570043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: NR Changing Defaults: document \etc (issue 256990043 by d...@gnu.org)
Apart from nitpicking, doc changes LGTM Trevor https://codereview.appspot.com/256990043/diff/20001/Documentation/notation/changing-defaults.itely File Documentation/notation/changing-defaults.itely (right): https://codereview.appspot.com/256990043/diff/20001/Documentation/notation/changing-defaults.itely#newcode4769 Documentation/notation/changing-defaults.itely:4769: @lilypond[quote,verbatim,ragged-right] padText = We normally write @lilypond[ ... ] on a line by itself, as in the second example below, and indent subsequently by 2 spaces. https://codereview.appspot.com/256990043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Doc: CG - Tightening up of the Bug Squad intro (issue 254370043 by pkx1...@gmail.com)
LGTM, apart from a trivial typo. Trevor https://codereview.appspot.com/254370043/diff/1/Documentation/contributor/issues.itexi File Documentation/contributor/issues.itexi (right): https://codereview.appspot.com/254370043/diff/1/Documentation/contributor/issues.itexi#newcode22 Documentation/contributor/issues.itexi:22: can be done by anyone with a web browser, an email client and the the "the the" https://codereview.appspot.com/254370043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Doc: Glossary - add notes for High Bass Clef (issue 256090043 by pkx1...@gmail.com)
LGTM Trevor https://codereview.appspot.com/256090043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Doc: NR add @knownissues for Percent Repeats (issue 250530043 by pkx1...@gmail.com)
https://codereview.appspot.com/250530043/diff/1/Documentation/notation/repeats.itely File Documentation/notation/repeats.itely (right): https://codereview.appspot.com/250530043/diff/1/Documentation/notation/repeats.itely#newcode764 Documentation/notation/repeats.itely:764: @knownissues @knownissues should come after the end of the @seealso items, which include Snippets: and Internal Reference:. See CG 5.5.2. Otherwise, LGTM. https://codereview.appspot.com/250530043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Doc: Usage - OOoLilyPond only works up to v4 (issue 255280043 by pkx1...@gmail.com)
https://codereview.appspot.com/255280043/diff/1/Documentation/usage/external.itely File Documentation/usage/external.itely (right): https://codereview.appspot.com/255280043/diff/1/Documentation/usage/external.itely#newcode726 Documentation/usage/external.itely:726: OpenOffice.org extension that converts LilyPond files it into images superfluous "it", otherwise LGTM https://codereview.appspot.com/255280043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Issue 4495 Remove unnecessary #{ .. #} constructs (issue 253310043 by tdanielsmu...@googlemail.com)
Thanks David https://codereview.appspot.com/253310043/diff/1/ly/staff-tkit.ly File ly/staff-tkit.ly (right): https://codereview.appspot.com/253310043/diff/1/ly/staff-tkit.ly#newcode60 ly/staff-tkit.ly:60: #(if dynUp On 2015/07/18 22:02:54, dak wrote: elseif cascades are bad to read in Scheme's indentation. Rather use cond here: #(cond (dynUp dynamicUp) (dynDown dynamicDown) (else dynamicNeutral)) Done. Here and elsewhere. https://codereview.appspot.com/253310043/diff/1/ly/vocal-tkit.ly File ly/vocal-tkit.ly (right): https://codereview.appspot.com/253310043/diff/1/ly/vocal-tkit.ly#newcode41 ly/vocal-tkit.ly:41: (if v1music On 2015/07/18 22:02:54, dak wrote: This does not work. #{ << #*unspecified* >> #} ignores the given element, but (make-simultanous-music (list ... will not do so. Instead do something like (delq! #f (list (make-two-voice-staff name clef v1name v2name) (and v1music ...) (and v2music ...))) Done. Aaannd ... this fixed the doc failure notified by James/Patchy last time. Thanks. https://codereview.appspot.com/253310043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Implement partial function calls. (issue 249670043 by d...@gnu.org)
If I understand this correctly it permits a variable definition to be used in some circumstances where a music function definition would previously have been required to achieve the same effect. That seems a worthwhile improvement and simplification. Trevor https://codereview.appspot.com/249670043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Doc: Issue 4059: Document MIDI mapping and MIDI effects (issue 249980043 by tdanielsmu...@googlemail.com)
Reviewers: Keith, https://codereview.appspot.com/249980043/diff/1/Documentation/notation/input.itely File Documentation/notation/input.itely (right): https://codereview.appspot.com/249980043/diff/1/Documentation/notation/input.itely#newcode3279 Documentation/notation/input.itely:3279: @ref{MIDI channel mapping}). On 2015/07/04 03:59:36, Keith wrote: This is the part that doesn't work. We could say "the MIDI channel associated with the current Staff (or the current the Voice if you have moved the Staff_performer to the Voice context). If you set midiChannelMapping to 'instrument, all staves with the same instrument are affected when you set one of these context properties. Setting midiChannelMapping to 'voice often prevents the context properties form having effect." Doesn't the paragraph under 'voice above explain explicitly what happens? That seems better than the rather vague "often prevents the context properties ...". There already is a reference back to that explanation. Trevor Description: Doc: Issue 4059: Document MIDI mapping and MIDI effects Based on original text supplied by Heikki Tauriainen Please review this at https://codereview.appspot.com/249980043/ Affected files (+214, -4 lines): M Documentation/notation/input.itely ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Python-ly to indent ../input/regression/a* (issue 249270044 by pkx1...@gmail.com)
On balance the indentation is improved, but there are a few cases which are worse, IMHO. With a follow-up hand-indentation to "uncorrect" the worst cases this would be worth doing, if just to remove the tabs. Trevor https://codereview.appspot.com/249270044/diff/1/input/regression/alignment-vertical-manual-setting.ly File input/regression/alignment-vertical-manual-setting.ly (right): https://codereview.appspot.com/249270044/diff/1/input/regression/alignment-vertical-manual-setting.ly#newcode3 input/regression/alignment-vertical-manual-setting.ly:3: texidoc = "Alignments may be changed per system by setting The texidoc string formatting is rather inconsistent. https://codereview.appspot.com/249270044/diff/1/input/regression/alter-broken.ly File input/regression/alter-broken.ly (right): https://codereview.appspot.com/249270044/diff/1/input/regression/alter-broken.ly#newcode32 input/regression/alter-broken.ly:32: (ly:stencil-rotate (ly:hairpin::print grob) -5 0 0))) Definitely don't like this https://codereview.appspot.com/249270044/diff/1/input/regression/alter-broken.ly#newcode42 input/regression/alter-broken.ly:42: ) Tie This does not seem right for either Scheme or Lily. https://codereview.appspot.com/249270044/diff/1/input/regression/auto-beam-exceptions.ly File input/regression/auto-beam-exceptions.ly (right): https://codereview.appspot.com/249270044/diff/1/input/regression/auto-beam-exceptions.ly#newcode29 input/regression/auto-beam-exceptions.ly:29: \repeat unfold 24 c32 All these indentations look wrong https://codereview.appspot.com/249270044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Doc: NR Update information for modern-cautionary (issue 252770043 by pkx1...@gmail.com)
LGTM Trevor https://codereview.appspot.com/252770043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Doc: avoid implicit \relative; issue 4371 (issue 237340043 by k-ohara5...@oco.net)
LGTM Thanks Keith https://codereview.appspot.com/237340043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Doc: avoid implicit \relative; issue 4371 (issue 237340043 by k-ohara5...@oco.net)
I'm generally happy with this, but I've made quite a few nit-picking comments. I think it is important that the manuals rigorously show a consistent style, particularly with regard to indentation and the placement of block delimiters. We frequently criticise users presenting examples on the mailing list for bad layout, so I think the manuals must be impeccable in this respect. Trevor https://codereview.appspot.com/237340043/diff/110001/Documentation/learning/common-notation.itely File Documentation/learning/common-notation.itely (right): https://codereview.appspot.com/237340043/diff/110001/Documentation/learning/common-notation.itely#newcode243 Documentation/learning/common-notation.itely:243: \key aes \major I wonder if it might be preferable to place the \key outside the \relative? This would be in keeping with the emerging opinion that we should encourage placing \relative blocks around smaller sections of notes. https://codereview.appspot.com/237340043/diff/110001/Documentation/learning/common-notation.itely#newcode646 Documentation/learning/common-notation.itely:646: | c2 \acciaccatura b16 c2 } This might look prettier but it also omits a bar check at the end of the last bar. I'm not convinced this is a good idea. https://codereview.appspot.com/237340043/diff/110001/Documentation/learning/common-notation.itely#newcode962 Documentation/learning/common-notation.itely:962: | r4 8.\p 16( 4-. ) } Last bar is not checked. Bad advice. https://codereview.appspot.com/237340043/diff/110001/Documentation/learning/common-notation.itely#newcode1420 Documentation/learning/common-notation.itely:1420: So far we have always used @code{\relative} to define pitches. This is not true. There are a couple of examples earlier where you have used absolute pitches. https://codereview.appspot.com/237340043/diff/110001/Documentation/learning/tweaks.itely File Documentation/learning/tweaks.itely (right): https://codereview.appspot.com/237340043/diff/110001/Documentation/learning/tweaks.itely#newcode330 Documentation/learning/tweaks.itely:330: b c } The LilyPond style, I believe, is to place the terminating } in line with the corresponding \ of \relative. Are you proposing we change this? https://codereview.appspot.com/237340043/diff/110001/Documentation/learning/tweaks.itely#newcode2324 Documentation/learning/tweaks.itely:2324: c4( c^\markup { \tiny \sharp } d4.) c8 } Again, using | as a prettifying aid to the detriment of its primary purpose. The first one does nothing useful and the last bar is not checked. https://codereview.appspot.com/237340043/diff/110001/Documentation/learning/tweaks.itely#newcode2439 Documentation/learning/tweaks.itely:2439: ees,2.~\)\mf ees4 r8 } Needs final bar check and } on following line. https://codereview.appspot.com/237340043/diff/110001/Documentation/notation/ancient.itely File Documentation/notation/ancient.itely (right): https://codereview.appspot.com/237340043/diff/110001/Documentation/notation/ancient.itely#newcode871 Documentation/notation/ancient.itely:871: ais bis } Misplaced } https://codereview.appspot.com/237340043/diff/110001/Documentation/notation/changing-defaults.itely File Documentation/notation/changing-defaults.itely (right): https://codereview.appspot.com/237340043/diff/110001/Documentation/notation/changing-defaults.itely#newcode357 Documentation/notation/changing-defaults.itely:357: } Can you really leave out the << ... >> here? https://codereview.appspot.com/237340043/diff/110001/Documentation/notation/changing-defaults.itely#newcode733 Documentation/notation/changing-defaults.itely:733: } Ditto https://codereview.appspot.com/237340043/diff/110001/Documentation/notation/keyboards.itely File Documentation/notation/keyboards.itely (right): https://codereview.appspot.com/237340043/diff/110001/Documentation/notation/keyboards.itely#newcode497 Documentation/notation/keyboards.itely:497: 1\treCorde } Misplaced } https://codereview.appspot.com/237340043/diff/110001/Documentation/notation/keyboards.itely#newcode525 Documentation/notation/keyboards.itely:525: \bar "|." } Misplaced } https://codereview.appspot.com/237340043/diff/110001/Documentation/notation/pitches.itely File Documentation/notation/pitches.itely (right): https://codereview.appspot.com/237340043/diff/110001/Documentation/notation/pitches.itely#newcode407 Documentation/notation/pitches.itely:407: cis'' cis'' cis''! cis''? c'' c'' c''! c''? Wouldn't \relative be preferable here? And in most of the examples in this section. https://codereview.appspot.com/237340043/diff/110001/Documentation/notation/pitches.itely#newcode1280 Documentation/notation/pitches.itely:1280: \clef "treble_8" c'4 } Misplaced } https://codereview.appspot.com/237340043/diff/110001/Documentation/notation/pitches.itely#newcode1443 Documentation/notation/pitches.itely:1443: a2 b } Misplaced } And several more following. https://codereview.appspot.com/237340043/ ___ lilypon
Re: Make music functions callable from Scheme (issue 244840043 by d...@gnu.org)
Just querying a strange sentence. https://codereview.appspot.com/244840043/diff/11/Documentation/changes.tely File Documentation/changes.tely (right): https://codereview.appspot.com/244840043/diff/11/Documentation/changes.tely#newcode69 Documentation/changes.tely:69: matching based will still be performed in the same manner as when "Argument checking and matching based will still ..." ?? Is something missing here? https://codereview.appspot.com/244840043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Make music functions callable from Scheme (issue 244840043 by d...@gnu.org)
Well, I can't say LGTM as that would imply I'd understood it and could confirm its correctness, but I can say I'm looking forward to removing lots of the #{ ... #} in the satb.ly template and its supporting cast! Trevor https://codereview.appspot.com/244840043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Docs: clean up after \relative conversion (issue 239250043 by k-ohara5...@oco.net)
Apart from a really trivial nitpick, LGTM (but I haven't checked if anything has been missed) Trevor https://codereview.appspot.com/239250043/diff/1/Documentation/notation/pitches.itely File Documentation/notation/pitches.itely (right): https://codereview.appspot.com/239250043/diff/1/Documentation/notation/pitches.itely#newcode303 Documentation/notation/pitches.itely:303: @code{@w{\relative}} is interpreted just the same as Don't need @w now https://codereview.appspot.com/239250043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Some updates to the ancient music section (issue 240810043 by philehol...@googlemail.com)
LGTM Trevor https://codereview.appspot.com/240810043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: absolute pitch entry: accept an offset octave (issue 235010043 by k-ohara5...@oco.net)
On 2015/05/17 21:09:13, dak wrote: At any rate, if we were to retain both \fixed and \absolute, I strongly prefer just two input modes, \relative and \absolute, but as I said right at the beginning: ... I'd prefer the syntax and options [of \absolute] to parallel those of \relative. That is, an optional prefix pitch to indicate the starting octave, and taking the starting octave from the first contained note if the prefix is omitted. That is then consistent, easy to explain and understand, and provides all the functionally we need (retaining the option to add the transposing cuteness at some point in the future if we wish.) Trevor https://codereview.appspot.com/235010043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Doc: avoid implicit \relative; issue 4371 (issue 237340043 by k-ohara5...@oco.net)
I'm not opposed to the change to explicit \relative; it avoids having to explain why it was omitted in the examples, and makes the example code more exactly correspond to the code obtained by clicking on the image. But I think if are to make this change it would also be good to say what leaving out \relative means. It was difficult to do this before as all the examples left it out (apparently). Now we can do it. Placing a brief explanation at the beginning of 1.2.1 would explain the mysterious 's in those first examples of code. https://codereview.appspot.com/237340043/diff/10006/Documentation/learning/tweaks.itely File Documentation/learning/tweaks.itely (right): https://codereview.appspot.com/237340043/diff/10006/Documentation/learning/tweaks.itely#newcode2124 Documentation/learning/tweaks.itely:2124: \relative c' { no leading spaces https://codereview.appspot.com/237340043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: absolute pitch entry: accept an offset octave (issue 235010043 by k-ohara5...@oco.net)
On 2015/05/17 10:44:36, dak wrote: Well, I remain unenthused about the new name. Maybe get a vote on the user list, with options \absolute x'', \fixed x'', \octave x''? I think those were pretty much the terms mentioned significantly more than once. The proper name for this would \absoluteWithFixedOctaveOffset, but that's too long and the acronym is similarly uninspiring. All three of the proposed options appear in this name, so the question is, "Which alludes most memorably to the actual function?". At the risk of going round in circles, on reconsidering in this light, I rather favour retaining \absolute. The other terms are modifiers to the primary function; the offset is both apparent and optional, "absolute" is used in the text and is an established concept. Trevor https://codereview.appspot.com/235010043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: absolute pitch entry: accept an offset octave (issue 235010043 by k-ohara5...@oco.net)
On 2015/05/17 07:36:01, Keith wrote: On 2015/05/15 06:12:38, lemzwerg wrote: > Given that we are currently producing development > releases, I suggest that this gets implemented, > then we simply wait a few months so that people can > test it in real life, and then we do a final decision. If we don't come back with another patch, the current patch will become final by default, but I do think it is a useful choice. The addition to Learning Manual: Common Notation gives us some idea of the extra complexity of documentation that comes with \fixed. A similar change to this section was suggested a few years ago at http://lists.gnu.org/archive/html/lilypond-user/2008-08/msg00384.html I'm content with this conclusion. I might even try using \fixed ;-) Trevor https://codereview.appspot.com/235010043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: absolute pitch entry: accept an offset octave (issue 235010043 by k-ohara5...@oco.net)
On 2015/05/06 15:57:21, wl_gnu.org wrote: > Probably the best name is \octave, which was used for something > similar > until version 0.1.19 > > \octave c'' {c4 e g c e g c'1} Sounds OK for me. Werner So is this proposing three entry modes: \relative (unchanged) \absolute (unchanged) \octave(as proposed here)? I'd not object to that. Trevor https://codereview.appspot.com/235010043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Update incipit snippets (issue 235980043 by philehol...@googlemail.com)
I haven't checked incipit.ly, but otherwise it looks fine apart from what appears to be an inadvertent change which needs investigating. https://codereview.appspot.com/235980043/diff/1/Documentation/snippets/conducting-signs,-measure-grouping-signs.ly File Documentation/snippets/conducting-signs,-measure-grouping-signs.ly (right): https://codereview.appspot.com/235980043/diff/1/Documentation/snippets/conducting-signs,-measure-grouping-signs.ly#newcode31 Documentation/snippets/conducting-signs,-measure-grouping-signs.ly:31: setting in @@file@{scm/time-signature-settings.scm@}: Is this change intentional? The original looks right. Looks like someone edited this snippet in Documentation/snippets without adding the change to snippets/new. https://codereview.appspot.com/235980043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Document incipit command (issue 232180043 by philehol...@googlemail.com)
On 2015/05/05 12:07:32, mail_philholmes.net wrote: I read the CG on *index entries, and experimented a fair amount, but never quite got to understand the difference. Is cindex the Command index only? Should it be @cindex \incipit or @cindex incipit? Or both? And funindex is the main index, which therefore separates the \ entries from the others with alpha sorting? It is a bit of a mess, but we never agreed on the best way to sort it out. cindex is the Concept index and feeds into App E - the general index - only. funindex feeds into the command index, App D, and also into the "\" section of App E. (Correct me if that's wrong - I'm relying on a 4-year old memory here.) Anyway, we decided to continue to add more rather than fewer index entries so if we ever decided to separate them some other way we'd have all the entries available. So in addition to your two entries I would suggest adding @cindex incipits, adding or some such. Trevor https://codereview.appspot.com/232180043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Document incipit command (issue 232180043 by philehol...@googlemail.com)
One suggestion, otherwise LGTM. https://codereview.appspot.com/232180043/diff/1/Documentation/notation/ancient.itely File Documentation/notation/ancient.itely (right): https://codereview.appspot.com/232180043/diff/1/Documentation/notation/ancient.itely#newcode2651 Documentation/notation/ancient.itely:2651: @funindex incipit @cindex entries too? https://codereview.appspot.com/232180043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: absolute pitch entry: accept an offset octave (issue 235010043 by k-ohara5...@oco.net)
On 2015/05/03 20:25:22, dak wrote: Again: I don't think that this was Keith's proposal. And I am pretty sure that none of my suggestions was Keith's proposal either: I just angle for something useful to do with \absolute f. It wasn't Keith's proposal, you're right, I rather ran on ahead. Keith's suggestion was to add a simple single octave offset. Maybe just this is the right thing to do after all. It's easy, effective, simple, non-scary, and maximises benefit/effort. I'm not too keen on creating Easter eggs - I've spent a fair fraction of the last several years documenting things previously hidden so ordinary users could benefit too. https://codereview.appspot.com/235010043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: absolute pitch entry: accept an offset octave (issue 235010043 by k-ohara5...@oco.net)
On 2015/05/03 09:02:51, dak wrote: However, I _think_ that your comment would suggest \absolute f'' { f ... to be the same as \absolute { f'' ... Correct whereas I suggested making \absolute f'' { f ... the same as \absolute { bes'' ... Now there _is_ a difference between \relative c and \relative f. With what I guess from your proposal, \absolute c and \absolute f would be the same. And so would be \absolute b. Yes, in Keith's and my model \relative sets a starting pitch, \absolute would set a starting octave only, and pitches thereafter are relative to the octave of the previous note. So the input notes are always in a clearly defined octave. Perhaps a better name for this mode of entry would be \relativeOctave. I'll use this later to be clear what I mean. Now I actually like the idea of using \absolute bes' for entering a trumpet in audible pitch using an input scale of { c d e f g ... }. That's a concept different from \transpose c' bes' { ... } or \transpose c bes' which primarily suggest a connection between _printed_ pitch and audible pitch (like \transposition does) rather than _input_ pitch and printed pitch. A nice idea (your original suggestion was too cute indeed to register as meaning this to my old brain ;) But it does rather muddy the concept of an absolute pitch, which is enshrined in >10 years of manuals. I do realize that \relative only ever touches the octave, and it seems to make little sense to have \absolute f turn { c, d, e, f g a b c d e f' ... } into one continous scale even though it would only touch the octave (like relative) and allow using as few octave marks as possible for a given tessitura. No, a continuous scale would be \relativeOctave { c, d e f g a b c' d e f ... }. The c' resets the octave. This doesn't work so well for a melody oscillating a tone or two above and below a c, of course, but it does avoid multiple ''' and ,,,. But while that would also be a consistent possibility, I don't think having e be a higher pitch than f is going to win us a lot of sympathies. No, e would never be higher than f in \relativeOctave. I prefer the transposing interpretation. I wouldn't oppose it. Indeed, the two possibilities could exist together, depending on the presence or absence of a prefix pitch. \absolute bes' { ... } transposes the input; \absolute { ... } works like \relativeOctave. Just some thoughts. We need some other views I think. Trevor https://codereview.appspot.com/235010043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Partcombiner documentation (Issue 4307) (issue 233110043 by philehol...@googlemail.com)
LGTM. Trevor https://codereview.appspot.com/233110043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Update NR font size section (Issue 3370) (issue 235920043 by philehol...@googlemail.com)
LGTM Trevor https://codereview.appspot.com/235920043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Partcombiner documentation (Issue 4307) (issue 233110043 by philehol...@googlemail.com)
Good point, Phil. Perhaps we should slightly amend the text too to cover this feature, as shown. Trevor https://codereview.appspot.com/233110043/diff/20001/Documentation/notation/simultaneous.itely File Documentation/notation/simultaneous.itely (right): https://codereview.appspot.com/233110043/diff/20001/Documentation/notation/simultaneous.itely#newcode932 Documentation/notation/simultaneous.itely:932: rhythm as chords and separates notes more than a ninth apart into ... ninth apart or when the voices cross into separate ... https://codereview.appspot.com/233110043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: absolute pitch entry: accept an offset octave (issue 235010043 by k-ohara5...@oco.net)
I'm in favour of a change like this, but I'd prefer the syntax and options to parallel those of \relative. That is, an optional prefix pitch to indicate the starting octave, and taking the starting octave from the first contained note if the prefix is omitted. That would then become an attractive alternative to \relative. https://codereview.appspot.com/235010043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Partcombiner documentation (Issue 4307) (issue 233110043 by philehol...@googlemail.com)
https://codereview.appspot.com/233110043/diff/1/Documentation/notation/simultaneous.itely File Documentation/notation/simultaneous.itely (right): https://codereview.appspot.com/233110043/diff/1/Documentation/notation/simultaneous.itely#newcode931 Documentation/notation/simultaneous.itely:931: @notation{a due} note, and separates notes more than a ninth apart into For the avoidance of doubt, perhaps insert here: @notation{a due} note, combines notes less than a ninth apart with the same rhythm as chords, and separates ... Otherwise, LGTM. https://codereview.appspot.com/233110043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Doc: NR section 3.5.x MIDI file creation tidy up (issue 120480043 by pkx1...@gmail.com)
Hi James I'm happy with this now. Thanks for being so forbearing and persistent in the face of all the criticism. So, LGTM! Trevor https://codereview.appspot.com/120480043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Doc Issue 4350: \compressFullBarRests needs a warning (issue 227470043 by tdanielsmu...@googlemail.com)
Reviewers: Jean-Charles, https://codereview.appspot.com/227470043/diff/1/Documentation/learning/fundamental.itely File Documentation/learning/fundamental.itely (right): https://codereview.appspot.com/227470043/diff/1/Documentation/learning/fundamental.itely#newcode3193 Documentation/learning/fundamental.itely:3193: r4 f8 a | cis4 f | e4 d | On 2015/04/24 17:16:28, Jean-Charles wrote: maybe add a "R2*2" to show the expansion? Thanks for looking. I thought of doing what you suggest and had even made the change, but then I realised that the snippet was part of a longer narrative showing how a score and parts was built up. So it would have required a rather more invasive change. Instead, I gave the reference to the section in the NR which does indeed give several examples of consecutive MM rests. Description: Doc Issue 4350: \compressFullBarRests needs a warning The \compressFullBarRests predef, which simply sets skipBars true, can generate incorrectly set music if used carelessly. The problem is that its action is not restricted to rests: it removes bar lines whenever a rest or a _note_ crosses a bar line. Add a warning to the NR and point to it from the LM. Use \compressFullBarRests in the LM example rather than explicitly setting skipBars, and show how the rests should be bracketed between \compressFullBarRests and \expandFullBarRests. Please review this at https://codereview.appspot.com/227470043/ Affected files (+27, -6 lines): M Documentation/learning/fundamental.itely M Documentation/notation/rhythms.itely Index: Documentation/learning/fundamental.itely diff --git a/Documentation/learning/fundamental.itely b/Documentation/learning/fundamental.itely index 6aa80af8d344a81390f7089d5ac02668cca462ac..29f7280ac029354f5c672d2f460bf9ae2233e4c8 100644 --- a/Documentation/learning/fundamental.itely +++ b/Documentation/learning/fundamental.itely @@ -3164,27 +3164,38 @@ takes 3@tie{}measures in 2/4 time R2*3 @end example -When printing the part, multi-rests -must be condensed. This is done by setting a run-time variable +When printing the part, multi-rests must be condensed. There is a +pre-defined command to do this: @example -\set Score.skipBars = ##t +\compressFullBarRests @end example @noindent This command sets the property @code{skipBars} in the -@code{Score} context to true (@code{##t}). Prepending the rest and -this option to the music above, leads to the following result +@code{Score} context to true (@code{##t}). A similar command: + +@example +\expandFullBarRests +@end example + +does the opposite. + +Full bar rests expressed with a multiplier and bracketed between these +two commands are compressed: @lilypond[quote,ragged-right] \transpose f c' \relative c { \time 2/4 - \set Score.skipBars = ##t + \compressFullBarRests R2*3 | + \expandFullBarRests r4 f8 a | cis4 f | e4 d | } @end lilypond +For more details see @ruser{Full measure rests}. + The score is made by combining all of the music together. Assuming that the other voice is in @code{bassoonNotes} in the file Index: Documentation/notation/rhythms.itely diff --git a/Documentation/notation/rhythms.itely b/Documentation/notation/rhythms.itely index f14e0dbd996297cd29f57386ff2113ed3ad59984..ee30ccef55b188f5d52df97d054a4c14582d5e50 100644 --- a/Documentation/notation/rhythms.itely +++ b/Documentation/notation/rhythms.itely @@ -880,6 +880,13 @@ r1 | R1*17 | R1*4 | R2.*2 | @end lilypond +@warning{The action of @code{@bs{}compressFullBarRests} is to remove +bars and bar lines which contain no notes or rests other than a previous +note or rest whose duration carries over one or more of the following +bars. Because it also applies to long notes it is safest to always +bracket the full bar rests between @code{@bs{}compressFullBarRests} and +@code{@bs{}expandFullBarRests} commands as shown above.} + @cindex text on multi-measure rest @cindex multi-measure rest, attaching text @@ -965,6 +972,9 @@ setting, resulting bar-check warnings may not be displayed. Music Glossary: @rglos{multi-measure rest}. +Learning Manual +@rlearning{Scores and parts}. + Notation Reference: @ref{Durations}, @ref{Text}, ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Fix issue 4355 -- broken beam subdivision (issue 226700043 by carl.d.soren...@gmail.com)
I can't compile LP at the moment, but LGTM from eye-balling. How pleasing it took just two extra lines! Trevor https://codereview.appspot.com/226700043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Issue 3799: New satb.ly built-in template and template framework (issue 225860043 by tdanielsmu...@googlemail.com)
On 2015/04/21 19:58:34, dak wrote: Is Women/Men the correct nomenclature or would it rather be female/male [voice types]? As an alto I'd prefer the least sexist designation as long as it matches typical usage. "Men" is commonly seen in vocal scores, certainly in Oxford and Novello publications. "Women" is used rarely, but I have examples in an Oxford publication. Usually a change to the S/A line is written as S/A Unis or similar above the staff, and the staff is left unnamed. This can be done easily by a user of the template if that is desired. I've never seen female/male used, so I'd prefer to keep the names unchanged. Trevor https://codereview.appspot.com/225860043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Doc: NR section 3.5.x MIDI file creation tidy up (issue 120480043 by pkx1...@gmail.com)
On 2015/04/20 16:31:52, J_lowe wrote: https://codereview.appspot.com/120480043/diff/240001/Documentation/notation/input.itely#newcode2732 Documentation/notation/input.itely:2732: accent, marcato and portato. On 2014/12/28 23:40:29, Trevor Daniels wrote: > We need a proper list here showing clearly what is supported, so users new to > midi output can see at the outset what it can do for them. The basic facilities > can then be compared with the enhancements provided by the articulate script > to see if that is going to be useful. Not all users use articulate.ly, so the point was to list that what is 'not supported without articulate.ly' in the section that has nothing to do with articulate.ly and to list that which 'is supported with the articulate script' IN the articulate script where *already* seemed to be documenting this kind of thing in the first place. If these lists are all wanted in one place then fine - pick a place - but at least it has to be clear of which features won't appear in your MIDI output if you don't use articulate.ly and then that either means you list that all at the beginning or the end (where we have the articulate script information located). Also there seemed to be a 'want' to list what does work which I cannot understand; as my personal preference for 'lists' like this is that assume it ALL works unless it is listed that it does not work. Listing things that both do and don't work again seemed overkill and what if something isn't listed? Does it work, or doesn't it? This last para seems to be the nub of our disagreement. I strongly believe we should list the supported features and you want to list the unsupported ones. I should explain why I hold this view. There are two main reasons: 1. This is a Reference manual. It is intended to show what LP can do and how to do it. That is, it has a positive view: what can be done, not what cannot be done. This is apparent from the way the contents list is laid out, and the way the rest of the document is written, but let me give just a couple of examples: a) In 1.1.1 under Note names in other languages it says: "The available languages and the note names they define are: " and then goes on to list them. It does not list what languages are not supported. b) In 1.2.1 under Durations it says: " Durations as short as 128th notes may be specified. Shorter values are possible, but only as beamed notes." A positive, not a negative statement. You can find similar lists of what is supported in many other places, and AFAICS, none that list what is not supported. Try glancing at the Appendices. 2. MIDI is a complex and extensible standard. LP, even with articulate.ly, supports only a fraction of the possibilities. So it is simply misleading to let the user believe LP supports everything that is not listed, unless you are going to have a very long list. Have a look at https://en.wikipedia.org/wiki/MIDI to see what this would entail. Look at the range of controllers, sequencers and synthesizers; look at system exclusive messages and MIDI extensions. OK, so what am I asking for? Simply a list of MIDI features supported by basic LP, and a list of additional features supported when articulate.ly is included. That's all. The two lists exist already. They're in 3.5.2 under Supported in MIDI. The lists need to be updated, but I can do that later. Where should it go? As close to the beginning of the MIDI section as is sensible. Without your revised section ordering immediately to hand I'll have to leave you to decide that. If you still can't accept this, we'll have to wait until some other developers chip in with their view. Other than that, LGTM, and thanks for working so persistently with this! Trevor https://codereview.appspot.com/120480043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Issue 3799: New satb.ly built-in template and template framework (issue 225860043 by tdanielsmu...@googlemail.com)
On 2015/04/18 15:30:06, Trevor Daniels wrote: Fix file names of two more regression tests Sorry for the extra work James. Three out of four regression tests had missing hyphens! Windows, perhaps unfortunately, is quite happy with file names containing spaces, so I saw no errors here. Trevor https://codereview.appspot.com/225860043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Issue 3799: New satb.ly built-in template and template framework (issue 225860043 by tdanielsmu...@googlemail.com)
Patch set 2 contains a number of code improvements as well as four regression tests and a (minimal) change to the documentation. The main changes from patch set 1 are: - There is now a distinction between Women/Men music variables in their own right and WomenDivided/MenDivided variables which are used when modifying the instrument names and midi instruments associated with the two-voice staves. - There is a distinction between prefixes and names which are hardwired into the code, such as InstrumentName, Music, TwoVoicesPerStaff, etc and those which may be modified or extended by the template. The latter are defined in the template itself, the former in base-tkit.ly. I have not in this patch taken up Dan's suggestion to make a score-building function, mainly because I have not yet found a way of doing this to my satisfaction. I'd like this now to be considered as a candidate for updating the code base. Trevor https://codereview.appspot.com/225860043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Issue 3799: New satb.ly built-in template and template framework (issue 225860043 by tdanielsmu...@googlemail.com)
message: On 2015/04/08 01:54:02, Dan Eble wrote: Thanks for looking, Dan To support TTBB, would you generalize this template further, or would you create something new? My intention is to add an ssaattbb template once this patch has been installed. This would have three staves for each part: s1, s2 and s, etc, with separate TwoVoicesPerStaff controls for each part. This would make it easy to produce TTBB or any other combination of doubled parts. (Of course you could produce a TTBB score now, with satb.ly, by simply writing SopranoMusic = TenorOneMusic, with SopranoInstrumentName = "Tenor 1", etc, but that's a bit clunky.) How about factoring out the parts that are used within a \score into their own file so that they can be included many times in a multi-score book? Now that's worth considering. I'll look into that. My reason for not providing a score tkit in the present submission was the large number of parameters that would need to be passed, and I didn't have a convincing use case, but you've just suggested a better approach and a possible use case. How about using the part combiner? (That reminds me I have some work to do.) If you mean for the case when TwoVoicesPerStaff is true, it would make it difficult to associate lyrics with voices. If you mean for Women or Men voices or for Piano accompaniment, then of course you can do this yourself within the provided music, but again the lyric associations would be problematic. How about supporting (to some extent) alternate lyrics above/below the staff for the outer voices? I think this would be best accommodated in the future ssaattbb template I mentioned above, since alternate lyrics usually implies split voices. Trevor https://codereview.appspot.com/225860043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Issue 3799: New satb.ly built-in template and template framework (issue 225860043 by tdanielsmu...@googlemail.com)
Reviewers: , Message: This patch is posted for comment and improvement rather than a candidate for immediate inclusion in the code base. Some caveats/issues/questions: \includes are used for testing; later, perhaps after settling on the file names, these may not be necessary. Indenting is problematic, due to the heavy mixing of Scheme and Lily code. base-tkit.ly is actually pure Scheme, and perhaps should be recast as base-tkit.scm and located elsewhere. There are no regression tests and no documentation at present. These would need to be added. Description: Issue 3799: New satb.ly built-in template and template framework Add kits as aids for providing built-in templates ("tkits") Replace the satb.ly template with one built from the tkits, which corrects some errors and extends the facilities: - fixes issue 4192 - Women and Men one-voice staves added - Soprano and Tenor music may now be omitted without error - midi instruments may be specified - midi channel mapping is by instrument - midi instruments are by voice, not staff Please review this at https://codereview.appspot.com/225860043/ Affected files (+548, -183 lines): A ly/base-tkit.ly A ly/lyrics-tkit.ly A ly/piano-tkit.ly M ly/satb.ly A ly/staff-tkit.ly A ly/vocal-tkit.ly A ly/voice-tkit.ly ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Fix TODO with markup padding and fix buggy example in LM (issue 225840043 by philehol...@googlemail.com)
LGTM Trevor https://codereview.appspot.com/225840043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Remove unused event-type general-music (issue 222090043 by david.nales...@gmail.com)
https://codereview.appspot.com/222090043/diff/1/scm/define-music-types.scm File scm/define-music-types.scm (right): https://codereview.appspot.com/222090043/diff/1/scm/define-music-types.scm#newcode343 scm/define-music-types.scm:343: (types . ()) On 2015/04/01 15:16:07, david.nalesnik wrote: Should Music be given a type? According to pattern, "music" with lowercase "m"? Well, if "general-music" is unused presumably a type of "music" would also be unused. I'm rather outside my knowledge zone here, but is an entry of Music required at all? If "general-music" is being removed I'd have thought the Music entry should be removed too. Trevor https://codereview.appspot.com/222090043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Doc: Issue 4309: Clarify the warning about bare durations (issue 203640043 by tdanielsmu...@googlemail.com)
Reviewers: , Message: Pushed to staging as 67e08daacdeb384724e18a31597d9d828ac40f51 Closing ... Description: Doc: Issue 4309: Clarify the warning about bare durations Please review this at https://codereview.appspot.com/203640043/ Affected files (+4, -4 lines): M Documentation/learning/common-notation.itely Index: Documentation/learning/common-notation.itely diff --git a/Documentation/learning/common-notation.itely b/Documentation/learning/common-notation.itely index ff5775a6167c33937f3c4c2a855271955628cbcc..1541d39d90277ade98b4a65c5722515f48ad12cd 100644 --- a/Documentation/learning/common-notation.itely +++ b/Documentation/learning/common-notation.itely @@ -290,10 +290,10 @@ g4~ 4 c2~ | 4~ 8 a~ 2 | @end lilypond This shorthand may be useful in other places where the rhythm changes -with an unchanging pitch, but remember a bare duration will attach to -the preceding pitch, making a single note, if only white space -separates them. - +with an unchanging pitch, but remember that a bare pitch followed by a +space and a bare duration will be interpreted as a single note. In +other words, @code{c4 a 8 8} would be interpreted as @code{c4 a8 a8}, +not as @code{c4 a4 a8 a8}. Write instead @code{c4 a4 8 8 }. @node Slurs @unnumberedsubsubsec Slurs ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Docs: Issue 4181: Expose \stemUp and \stemDown predefs less (issue 214870044 by tdanielsmu...@googlemail.com)
Reviewers: , Message: This patch eliminates virtually all mention of the \stemUp and \stemDown predefs from the LM. Most of the uses of these predefs in the NR are sensible, so I have left untouched the ones in ancient, editorial, percussion and staff. Apart from the bald mention in staff, these are all in rather esoteric sections and probably quite well-hidden from new users. Trevor Description: Docs: Issue 4181: Expose \stemUp and \stemDown predefs less in LM Tweaking output - change some examples to use \slurUp etc rather than \stemUp - give an example showing the use of the direction indicators - replace mention of \stemUp in text with \tieUp, etc - correct a few typos in NR Changing defaults - remove superfluous \stemUp Please review this at https://codereview.appspot.com/214870044/ Affected files (+33, -32 lines): M Documentation/learning/tweaks.itely M Documentation/notation/changing-defaults.itely Index: Documentation/learning/tweaks.itely diff --git a/Documentation/learning/tweaks.itely b/Documentation/learning/tweaks.itely index b6429dcf43be6e9df930f9fc37478245bc2b92c8..de6554579bfccb7b02863927b15ab64d08edd267 100644 --- a/Documentation/learning/tweaks.itely +++ b/Documentation/learning/tweaks.itely @@ -331,12 +331,12 @@ The @code{\once} prefix may also be used in front of many predefined commands to limit their effect to one musical moment: @lilypond[quote,verbatim,relative=1] -c4 d -\once \stemDown -e4 f | -g4 a +c4( d) +\once \slurDashed +e4( f) | +g4( a) \once \hideNotes -b c | +b( c) | @end lilypond However, predefined commands of the form @code{\@dots{}Neutral}, @@ -1869,30 +1869,30 @@ automatically when @code{direction} is set. @cindex center @cindex neutral -The following example shows in bar 1 the default behavior of stems, -with those on high notes pointing down and those on low notes pointing -up, followed by four notes with all stems forced down, four notes with -all stems forced up, and finally four notes reverted back to the -default behavior. +The following example shows the default positioning of slurs in the +first bar, with slurs starting on high notes positioned above the notes +and those starting on low notes positioned below, followed by a bar +with both slurs forced down, a bar with both slurs forced up, and +finally a bar with both slurs reverted back to the default behavior. -@cindex Stem, example of overriding +@cindex Slur, example of overriding @cindex direction property, example -@lilypond[quote,fragment,ragged-right,verbatim,relative=2] -a4 g c a | -\override Stem.direction = #DOWN -a4 g c a | -\override Stem.direction = #UP -a4 g c a | -\revert Stem.direction -a4 g c a | +@lilypond[quote,verbatim,relative=2] +a4( g) c( a) | +\override Slur.direction = #DOWN +a4( g) c( a) | +\override Slur.direction = #UP +a4( g) c( a) | +\revert Slur.direction +a4( g) c( a) | @end lilypond -Here we use the constants @code{DOWN} and @code{UP}. +Here we have used the constants @code{DOWN} and @code{UP}. These have the values @w{@code{-1}} and @code{+1} respectively, and these numerical values may be used instead. The value @code{0} may also be used in some cases. It is simply treated as meaning -@code{UP} for stems, but for some objects it means @q{center}. +@code{UP} for slurs, but for some objects it means @q{center}. There is a constant, @code{CENTER} which has the value @code{0}. However, these explicit overrides are not usually used, as there are @@ -1943,12 +1943,20 @@ the commonest. The meaning of each is stated where it is not obvious. @end multitable The neutral/normal variants of these commands are implemented -using @code{\revert} and may @strong{not} be +using @code{\revert} and these may @strong{not} be preceded by @code{\once}. If you wish to limit the effect of the other commands (which are implemented using @code{\override}) to a single timestep, you can precede them with @code{\once} like you would do with explicit overrides. +Or, if just a single layout object needs to be forced up or down, the +direction indicators, @code{^} or @code{_}, may be used: + +@lilypond[quote,verbatim,relative=2] +a4( g) c( a) | +a4^( g) c_( a) | +@end lilypond + @node Fingering @unnumberedsubsubsec Fingering @@ -3155,8 +3163,7 @@ the left, and 1.8 staff space downwards: @cindex Fingering, example of overriding @cindex extra-offset property, example -@lilypond[quote,fragment,relative=1,verbatim] -\stemUp +@lilypond[quote,relative=1,verbatim] f4-5 \once \override Fingering.extra-offset = #'(-0.3 . -1.8) f4-5 @@ -4442,10 +4449,10 @@ Let's begin by looking at some files in @file{ly/}. Open @file{ly/property-init.ly} in a text editor. The one you normally use for @code{.ly} files will be fine. This file contains the definitions of all the standard LilyPond predefined -commands, such as @code{\stemUp} and @code{\slurDotted}. You will +commands, such as @code{\tieUp} and @code{\slurDotted}. You will see that
Doc: Internals - dash-fraction when set to 0.0 (issue 198150043 by pkx1...@gmail.com)
This is definitely an improvement, so LGTM. (As David mentioned earlier, there seem to be discrepancies between what one might expect when setting dash-fraction and the actual result, but this at least makes the documentation self-consistent.) https://codereview.appspot.com/198150043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: property to set voiced-rest positions; issue 3902 (issue 188580043 by k-ohara5...@oco.net)
LGTM Trevor https://codereview.appspot.com/188580043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Doc; CG - add more specific note for Guile (issue 193870043 by pkx1...@gmail.com)
On 2015/01/02 00:22:11, J_lowe wrote: On 2015/01/02 00:09:19, Trevor Daniels wrote: > https://codereview.appspot.com/193870043/diff/1/Documentation/included/compile.itexi > File Documentation/included/compile.itexi (right): > > https://codereview.appspot.com/193870043/diff/1/Documentation/included/compile.itexi#newcode78 > Documentation/included/compile.itexi:78: (1.8.8 - version 2.x is not currently > supported) > I think we're currently shipping Guile 1.8.7. Has Lily been tested with 1.8.8? > Just a query. > > Trevor Oh, I just did guile --version on my own System to see what it was I had installed and it came up with 1.8.8. (also guile-1.8 --version returns the same thing) I *assume* therefore that when I build LP from master and run it it is using 1.8.8 I do not have an pre-installed Linux or Windows system to test this. James Fine, 1.8.8 is probably right then. 2.19.15 was released with 1.8.7 but it seems latest in master uses 1.8.8. Trevor https://codereview.appspot.com/193870043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Doc; CG - add more specific note for Guile (issue 193870043 by pkx1...@gmail.com)
https://codereview.appspot.com/193870043/diff/1/Documentation/included/compile.itexi File Documentation/included/compile.itexi (right): https://codereview.appspot.com/193870043/diff/1/Documentation/included/compile.itexi#newcode78 Documentation/included/compile.itexi:78: (1.8.8 - version 2.x is not currently supported) I think we're currently shipping Guile 1.8.7. Has Lily been tested with 1.8.8? Just a query. Trevor https://codereview.appspot.com/193870043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Doc: NR section 3.5.x MIDI file creation tidy up (issue 120480043 by pkx1...@gmail.com)
James We've now had so many iterations of this I'd lost track of where we were up to with incremental changes and started over. I think much of it is improved, but some basic information has been lost and/or buried. I've tried to indicate as clearly (starkly might be more accurate) as possible what should be done to retrieve it. Trevor https://codereview.appspot.com/120480043/diff/240001/Documentation/notation/input.itely File Documentation/notation/input.itely (right): https://codereview.appspot.com/120480043/diff/240001/Documentation/notation/input.itely#newcode2674 Documentation/notation/input.itely:2674: * Controlling MIDI dynamics:: I would put these in the order: The MIDI block Controlling MIDI dynamics MIDI instruments (taken from Enhancing MIDI output) Using repeats with MIDI Enhancing MIDI output (just the articulate script) https://codereview.appspot.com/120480043/diff/240001/Documentation/notation/input.itely#newcode2732 Documentation/notation/input.itely:2732: accent, marcato and portato. We need a proper list here showing clearly what is supported, so users new to midi output can see at the outset what it can do for them. I asked for this early in October ("The clear statement currently shown in NR 3.5.3 should be reinstated somehow, and reflected accurately in the text.") but I don't see it anywhere yet. The basic facilities can then be compared with the enhancements provided by the articulate script to see if that is going to be useful. https://codereview.appspot.com/120480043/diff/240001/Documentation/notation/input.itely#newcode2846 Documentation/notation/input.itely:2846: @rinternals{Dynamic_performer}. I don't think we need this reference here. https://codereview.appspot.com/120480043/diff/240001/Documentation/notation/input.itely#newcode2856 Documentation/notation/input.itely:2856: these should beentered in a @code{Staff} (not @code{DrumStaff}) context There seems to be a character that is not a space between "be" and "entered". https://codereview.appspot.com/120480043/diff/240001/Documentation/notation/input.itely#newcode2874 Documentation/notation/input.itely:2874: to match many articulation and tempo indications. Engraved output Most articulations are handled by the basic LilyPond performer. See and use NR 3.5.3. https://codereview.appspot.com/120480043/diff/240001/Documentation/notation/input.itely#newcode2912 Documentation/notation/input.itely:2912: @item Chords that are @emph{not} entered as chord names. Why is this basic information hidden in the articulate script and what does this item mean? Chords entered as notes _are_ supported. Consider a user who wanted microtonal chords. It says earlier microtones need a player that supports them. Ok, he says, I have that. Having tried to make it work for hours he now finds, buried in a @knownissues in a script he is not using, that microtonal chords are not supported. How is that an improvement to the documentation? What do you have against the clear lists in NR 3.5.3 presented at the top of the MIDI section? Why do you keep trying to suppress it with inaccurate and incomplete information distributed in several places? https://codereview.appspot.com/120480043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: compare LilyPond Rationals without unnecessary overflow; issue 4180 (issue 194770043 by k-ohara5...@oco.net)
LGTM Trevor https://codereview.appspot.com/194770043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel