Re: Allow \partial to occur in mid-piece (issue 21370043)

2013-11-04 Thread dak
https://codereview.appspot.com/21370043/diff/1/Documentation/notation/rhythms.itely File Documentation/notation/rhythms.itely (right): https://codereview.appspot.com/21370043/diff/1/Documentation/notation/rhythms.itely#newcode1458 Documentation/notation/rhythms.itely:1458: #end-of-line-invisible

Re: Isolated durations in music sequences now stand for unpitched notes (issue 22120043)

2013-11-09 Thread dak
https://codereview.appspot.com/22120043/diff/40001/lily/parser.yy File lily/parser.yy (right): https://codereview.appspot.com/22120043/diff/40001/lily/parser.yy#newcode649 lily/parser.yy:649: | FRACTION On 2013/11/09 17:12:52, Trevor Daniels wrote: I don't like the mixed tabs and spaces here

Re: Isolated durations in music sequences now stand for unpitched notes (issue 22120043)

2013-11-09 Thread dak
https://codereview.appspot.com/22120043/diff/40001/scm/music-functions.scm File scm/music-functions.scm (right): https://codereview.appspot.com/22120043/diff/40001/scm/music-functions.scm#newcode780 scm/music-functions.scm:780: ;;; that don't interpret them is harmless. On 2013/11/09 18:46:30, b

Re: Fill in section "\set vs \override" (issue 21820045)

2013-11-10 Thread dak
Reviewers: Trevor Daniels, benrg, Keith, Message: On 2013/11/07 00:04:43, benrg wrote: As someone who barely understands LilyPond's internals I appreciate this explanation, but it still leaves me confused. It seems to be saying that grob properties are an additional hierarchical override sys

Re: Isolated durations in music sequences now stand for unpitched notes (issue 22120043)

2013-11-10 Thread dak
On 2013/11/10 11:48:13, janek wrote: Awesome!! could you push it in a branch, so that I could read the commits in sequence? dev/issue3648 On 2013/11/09 19:05:29, dak wrote: > You need to use q here. Sorry, but it would not do to turn > > << \new Staff { 4 } >\new R

Re: Issue 3641: Keep only one Axis_group_engraver active (issue 20500043)

2013-11-10 Thread dak
Man, "unpublished drafts" _again_. I really am too stupid for that Rietveld user interface. At any rate, they seem a bit dated now. Might make sense for followup issues. https://codereview.appspot.com/20500043/diff/20001/lily/axis-group-engraver.cc File lily/axis-group-engraver.cc (right): h

Re: Issue 3641: Keep only one Axis_group_engraver active (issue 20500043)

2013-11-10 Thread dak
On 2013/11/10 13:09:16, ckenneth721 wrote: https://codereview.appspot.com/22120043/ This looks rather like your cat got your mouse. If you have a problem figuring out the user interface here, perhaps just write your comment to the lilypond-devel list directly. https://codereview.appspot.com/2

Re: Isolated durations in music sequences now stand for unpitched notes (issue 22120043)

2013-11-12 Thread dak
Some more thoughts about the chord issue: I agree that it sounds somewhat icky to have repetition not extend to chords, in particular when having a music function with a body like #{ #music 4 4. 8 #} and calling it with either \rhythm c or \rhythm , it will be quite a nuisance that the first works

Re: Isolated durations in music sequences now stand for unpitched notes (issue 22120043)

2013-11-12 Thread dak
On 2013/11/12 15:35:42, benko.pal wrote: On 2013/11/12 10:23:56, dak wrote: > a) we don't commit ourselves either way: it is undefined what a pure duration > after an intervening chord will do. > > b) just single pitches, no chords: that's what this patch proposes. &g

Re: Isolated durations in music sequences now stand for unpitched notes (issue 22120043)

2013-11-13 Thread dak
On 2013/11/12 21:58:13, janek wrote: 2013/11/12 : > It still would not be fun in connection with TabStaff, not repeating > string numbers and stuff, and such repetition is really not feasible. I don't understand what you mean here. > Of course the problem is that we have to make a decision

Re: Support articulations, slurs and breaths in MIDI (issue 26470047)

2013-11-16 Thread dak
Just that Devon does not get a wrong impression: I applaud every effort to make LilyPond's MIDI output better reflect its input, and this work is a great step in that direction. Dan, however, suggests to make LilyPond's output to reflect not the music but an imaginary performance by a human playe

Re: Support articulations, slurs and breaths in MIDI (issue 26470047)

2013-11-16 Thread dak
On 2013/11/16 00:18:40, Devon Schudy wrote: Dan Eble wrote: I'm actually a wind player, not a pianist, but MIDI is designed (and mostly used) for keyboards, so their interpretation is usually the best one to use in MIDI. Keyboard interpretation of slurs varies — sometimes it just suppresses th

Re: Doc: Changes.tely 2.18 release (issue 24180044)

2013-11-18 Thread dak
https://codereview.appspot.com/24180044/diff/1/Documentation/changes.tely File Documentation/changes.tely (right): https://codereview.appspot.com/24180044/diff/1/Documentation/changes.tely#newcode65 Documentation/changes.tely:65: There is now a new context called @code{NullVoice} which, while no

Re: Support articulations, slurs and breaths in MIDI (issue 26470047)

2013-11-19 Thread dak
On 2013/11/18 13:34:33, Devon Schudy wrote: mailto:d...@gnu.org wrote: > If you say "overlap is the one synthesizers recognize": does that > mean that there needs to be a physical gap, or is it sufficient if > the note-on command of the next note comes before the note-off > command of the previou

Re: page-breaking: allow ragged pages to be compressed; issue 3281 (issue 25710043)

2013-11-19 Thread dak
On 2013/11/19 09:16:34, Keith wrote: On 2013/11/19 07:57:36, lemzwerg wrote: > LGTM. Just to be sure: Does the warning message regarding forced compressed > pages (if ragged-bottom is active) get emitted even for single-system pages? A single system on a ragged-bottom page gets a warning fr

Re: optimal-page-breaking: signed/unsigned bug; issue 1553 (issue 22750045)

2013-11-20 Thread dak
https://codereview.appspot.com/22750045/diff/1/lily/optimal-page-breaking.cc File lily/optimal-page-breaking.cc (right): https://codereview.appspot.com/22750045/diff/1/lily/optimal-page-breaking.cc#newcode120 lily/optimal-page-breaking.cc:120: for (vsize sys_count = ideal_sys_count + 1; --sys_co

Re: Grace notes shouldn't shorten the previous note unless they overlap with it. (issue 29550043)

2013-11-21 Thread dak
https://codereview.appspot.com/29550043/diff/1/lily/note-performer.cc File lily/note-performer.cc (right): https://codereview.appspot.com/29550043/diff/1/lily/note-performer.cc#newcode100 lily/note-performer.cc:100: tie_head->length_mom_ = min (tie_head->length_mom_, Ok, I've brooded a lot over

Re: optimal-page-breaking.cc protect unsigned subtraction; issue 1553 (issue 22750045)

2013-11-21 Thread dak
https://codereview.appspot.com/22750045/diff/20001/lily/optimal-page-breaking.cc File lily/optimal-page-breaking.cc (right): https://codereview.appspot.com/22750045/diff/20001/lily/optimal-page-breaking.cc#newcode78 lily/optimal-page-breaking.cc:78: if (min_sys_count > ideal_sys_count) // subtra

Re: Make make-relative able to deal with music rather than just pitches (issue 30890043)

2013-11-22 Thread dak
Reviewers: J_lowe, Message: On 2013/11/22 17:01:25, J_lowe wrote: https://codereview.appspot.com/30890043/diff/1/input/regression/make-relative-music.ly File input/regression/make-relative-music.ly (right): https://codereview.appspot.com/30890043/diff/1/input/regression/make-relative-music.l

Re: Support articulations, slurs and breaths in MIDI (issue 26470047)

2013-11-22 Thread dak
On 2013/11/23 07:25:31, benko.pal wrote: > I would need, however, an appropriate new name for the property, and the best I > can think of now is 'strength'. 'attack'? 'velocity'. There is nothing to be gained by inventing different names from the MIDI terminology but confusion. https://c

Re: Support articulations, slurs and breaths in MIDI (issue 26470047)

2013-11-24 Thread dak
On 2013/11/24 15:11:38, Devon Schudy wrote: David Kastrup wrote: > the next goal for this sub-project may be to get the audio playback > to honour the \repeat structures by translating the volta and > tremolo flavours to unfold. I want this too, but apparently it was discussed previously, a

Re: Make convert-ly -d only ever update on changed files (issue 31830043)

2013-11-24 Thread dak
Reviewers: Keith, Message: On 2013/11/25 06:37:35, Keith wrote: I'm fine with this, but then we would probably want to run convert-ly without the '-d' option on Documentation/snippets, upon creation of version 2.18.0. Maybe there is a way to get the behavior of "round-up-to stable version"

Re: Make convert-ly -d only ever update on changed files (issue 31830043)

2013-11-25 Thread dak
https://codereview.appspot.com/31830043/diff/1/scripts/convert-ly.py#newcode306 scripts/convert-ly.py:306: last = next_stable The old code was there so that the \version ".." is updated for a new stable version Searching through the mailing list

Re: Support articulations, slurs and breaths in MIDI (issue 26470047)

2013-11-26 Thread dak
On 2013/11/26 17:54:57, Ian Hulin (gmail) wrote: On 2013/11/24 15:40:01, janek wrote: > 2013/11/24 : > > On 2013/11/24 15:11:38, Devon Schudy wrote: > > > >> David Kastrup wrote: > >> > the next goal for this sub-project may be to get the audio playback > >> > to honour the \repeat structures

script-column: earlier scripts support later scripts; issue 3683 (issue 35010043)

2013-11-29 Thread dak
https://codereview.appspot.com/35010043/diff/1/lily/script-column.cc File lily/script-column.cc (right): https://codereview.appspot.com/35010043/diff/1/lily/script-column.cc#newcode156 lily/script-column.cc:156: use all the scripts so far as support for the current grob A question of understandi

Re: Issue 185: Remove Pitch_squash_engraver (issue 35080044)

2013-11-29 Thread dak
Reviewers: J. Rohrer, Message: On 2013/11/29 19:28:44, J. Rohrer wrote: To make sure I understand correctly (unfortunately I cannot test the patch right now, I only looked over the diffs): After this change, - you can still freely mix squashed and unsquashed voices in one staff, Yes. The

Reduce offsets of \super and \sub (issue 35320043)

2013-11-30 Thread dak
https://codereview.appspot.com/35320043/diff/1/scm/define-markup-commands.scm File scm/define-markup-commands.scm (right): https://codereview.appspot.com/35320043/diff/1/scm/define-markup-commands.scm#newcode4000 scm/define-markup-commands.scm:4000: (* 0.33 baseline-skip) I'm not sure a multiple

Re: Reduce offsets of \super and \sub (issue 35320043)

2013-11-30 Thread dak
On 2013/11/30 08:45:14, Keith wrote: On Fri, 29 Nov 2013 23:59:52 -0800, wrote: > I'm not sure a multiple of baseline-skip is the best metric (possibly > for limiting height in cramped situations, but not even sure about > that). > > It seems that it would not follow mo

Re: Completion_*_engraver: add means to preserve scale factor; issue 3650 (issue 35370043)

2013-11-30 Thread dak
https://codereview.appspot.com/35370043/diff/1/scm/define-context-properties.scm File scm/define-context-properties.scm (right): https://codereview.appspot.com/35370043/diff/1/scm/define-context-properties.scm#newcode231 scm/define-context-properties.scm:231: original duration to be split, and t

Re: Reduce offsets of \super and \sub (issue 35320043)

2013-12-01 Thread dak
On 2013/12/01 09:16:14, Keith wrote: On Sat, 30 Nov 2013 23:56:09 -0800, wrote: > I have one request: this patch makes the situation better, and even if > the baseline-skip approach is wrong, it was already used that way so > it's not making things worse. I

Re: Implement \beamExceptions function fishing exceptions from beamed music. (issue 34880043)

2013-12-01 Thread dak
Reviewers: janek, Graham Percival, Message: On 2013/12/01 12:32:34, janek wrote: I very much like the simplification in the user interface! If i understand correctly, this function will overwrite beamExceptions with the pattern extracted from the music. No. It converts the given music into

Re: Support articulations, slurs and breaths in MIDI (issue 26470047)

2013-12-06 Thread dak
It took me until reading the commit message to figure out that this is actually meddling with the parser. I really should do more reviewing. It might have made sense to put the parser part, which appears reasonably independent, into a separate issue to make it more visible. The parser is notabl

Re: Metafont code cleanup (issue 38530043)

2013-12-09 Thread dak
On 2013/12/09 09:31:57, hanwenn wrote: also, for reviewing, you should do the file reorganization and the code reorganization in different changes. It's also a good idea to move files and change files in different commits since git gets worse at tracking moves/renames the more changes happen

Use standard inclusion scheme for FreeType headers. (issue 35580043)

2013-12-10 Thread dak
https://codereview.appspot.com/35580043/diff/1/lily/open-type-font.cc File lily/open-type-font.cc (right): https://codereview.appspot.com/35580043/diff/1/lily/open-type-font.cc#newcode26 lily/open-type-font.cc:26: #include FT_TRUETYPE_TABLES_H This is inscrutable programming. It relies on open-

Re: Use standard inclusion scheme for FreeType headers. (issue 35580043)

2013-12-10 Thread dak
On 2013/12/10 21:20:54, lemzwerg wrote: My patch is very simple, as you can see, just replacing the hardcoded header paths with macros (this is what I refer as `standard inclusion scheme') to make it compile. The `reckless' programming was there before me, so to say, and I admit that I hav

Re: Caches the interior skylines of vertical axis groups and systems. (issue 7185044)

2013-12-11 Thread dak
I have a headache after the first file of 30, so this is just this one file and does not imply that the others are fine. https://codereview.appspot.com/7185044/diff/164001/lily/axis-group-interface.cc File lily/axis-group-interface.cc (left): https://codereview.appspot.com/7185044/diff/164001/l

Re: Parser: make optional arguments compatible with lookahead (issue 41720043)

2013-12-12 Thread dak
Reviewers: lemzwerg, https://codereview.appspot.com/41720043/diff/1/lily/parser.yy File lily/parser.yy (right): https://codereview.appspot.com/41720043/diff/1/lily/parser.yy#newcode1073 lily/parser.yy:1073: if (!unsmob_music ($$)) On 2013/12/13 06:13:32, lemzwerg wrote: Looks like a slightly i

Re: Parser: make optional arguments compatible with lookahead (issue 41720043)

2013-12-12 Thread dak
On 2013/12/13 06:47:29, lemzwerg wrote: https://codereview.appspot.com/41720043/diff/1/lily/parser.yy File lily/parser.yy (right): https://codereview.appspot.com/41720043/diff/1/lily/parser.yy#newcode1073 lily/parser.yy:1073: if (!unsmob_music ($$)) > What's more annoying is that the copy&pas

Re: grob-property: if callback is independent of layout, call just once (issue 42190043)

2013-12-14 Thread dak
https://codereview.appspot.com/42190043/diff/1/lily/unpure-pure-container.cc File lily/unpure-pure-container.cc (right): https://codereview.appspot.com/42190043/diff/1/lily/unpure-pure-container.cc#newcode45 lily/unpure-pure-container.cc:45: On 2013/12/14 09:47:33, MikeSol wrote: Looking good.

Adds outside-staff-interface and outside-staff-axis-group-interface (issue 37950044)

2013-12-16 Thread dak
The summary seems incompatible with http://music.stackexchange.com/a/14160/8773> Once an interface is required for outside-staffing a grob, the set of grobs one can use in that manner is hardwired to the "intended" grobs. https://codereview.appspot.com/37950044/

Re: Adds outside-staff-interface and outside-staff-axis-group-interface (issue 37950044)

2013-12-16 Thread dak
On 2013/12/16 09:58:40, mike7 wrote: On Dec 16, 2013, at 11:45 AM, mailto:d...@gnu.org wrote: > The summary seems incompatible with > http://music.stackexchange.com/a/14160/8773> > > Once an interface is required for outside-staffing a grob, the set of > grobs one can use in that manner is h

Re: Issue 3641: Keep only one Axis_group_engraver active (issue 20500043)

2013-12-16 Thread dak
https://codereview.appspot.com/20500043/diff/20001/lily/vertical-align-engraver.cc File lily/vertical-align-engraver.cc (right): https://codereview.appspot.com/20500043/diff/20001/lily/vertical-align-engraver.cc#newcode99 lily/vertical-align-engraver.cc:99: On 2013/12/16 18:05:17, Keith wrote:

Re: Cleanup and generalization: get rid of Audio_column. (issue 42600043)

2013-12-20 Thread dak
https://codereview.appspot.com/42600043/diff/20001/lily/audio-item.cc File lily/audio-item.cc (right): https://codereview.appspot.com/42600043/diff/20001/lily/audio-item.cc#newcode44 lily/audio-item.cc:44: return int (moment_to_ticks (when_)); Why cast to int here? moment_to_ticks is already an

Re: Cleanup and generalization: get rid of Audio_column. (issue 42600043)

2013-12-20 Thread dak
On 2013/12/21 02:58:05, Devon Schudy wrote: OK: Issue 3744 fixes that for all score-level translators by creating the Score context before iteration starts. Creating it later was dubious anyway, because Score_engraver or Score_performer is needed for iteration to work. AFAICT it's never valid to

Re: Automatically unfold percent and tremolo repeats in MIDI output. (issue 769) (issue 40720060)

2013-12-21 Thread dak
Keith comments: The other limitation of \unfoldRepeats is that it fails to see repeats in parallel expressions << {\repeat volta 2 s1 } {c2 d2} >> while this approach using the iterators will see them and be able to expand them when requested. I think he is wrong about that. I don't see an

Re: Cleanup and generalization: get rid of Audio_column. (issue 42600043)

2013-12-21 Thread dak
On 2013/12/21 21:29:13, Devon Schudy wrote: dak wrote: Doing start_translation_timestep in mid-timestep is unclean, though, and may confuse translators that expect it to be called at the beginning. How would they know the difference? There is nothing sent to an implicit context before it

Re: Automatically unfold percent and tremolo repeats in MIDI output. (issue 769) (issue 40720060)

2013-12-22 Thread dak
On 2013/12/22 07:01:10, Keith wrote: On Sat, 21 Dec 2013 00:23:05 -0800, wrote: > Keith comments: >> The other limitation of \unfoldRepeats is that it fails to see repeats > in >> parallel expressions << {\repeat volta 2 s1 } {c2 d2} >> while this > approach >> using th

Re: Automatically unfold percent and tremolo repeats in MIDI output. (issue 769) (issue 40720060)

2013-12-22 Thread dak
On 2013/12/22 22:01:25, Keith wrote: On Sun, 22 Dec 2013 11:45:49 -0800, Devon Schudy wrote: > Unfolding during iteration doesn't extend well to voltas, because it > changes their length. Even if the length is updated, the old length > has already been used by the pa

Re: Draft of 2.18 release news (issue 45070043)

2013-12-22 Thread dak
Reviewers: J_lowe, Message: On 2013/12/22 23:03:46, J_lowe wrote: Fails make need to escape some braces here and there in the @examples - some nits. Embarrassing. I did not bother all too much about seeing Patchy complain since I was not expecting a patch to the stable branch to apply at al

Re: Draft of 2.18 release news (issue 45070043)

2013-12-22 Thread dak
On 2013/12/23 00:41:28, Devon Schudy wrote: https://codereview.appspot.com/45070043/diff/20001/Documentation/web/news-front.itexi File Documentation/web/news-front.itexi (right): https://codereview.appspot.com/45070043/diff/20001/Documentation/web/news-front.itexi#newcode52 Documentation/web

Re: Web: updated Authors.itexi (issue 44720043)

2013-12-25 Thread dak
https://codereview.appspot.com/44720043/diff/20001/Documentation/included/authors.itexi File Documentation/included/authors.itexi (right): https://codereview.appspot.com/44720043/diff/20001/Documentation/included/authors.itexi#newcode193 Documentation/included/authors.itexi:193: Devon Schudy, Uh

Re: Web: updated Authors.itexi (issue 44720043)

2013-12-25 Thread dak
/included/authors.itexi:193: Devon Schudy, On 2013/12/25 12:24:10, dak wrote: > Uh, Devon is not a core developer of 2.18 (and that's where we need the update): > as far as I can tell his work is starting in 2.19. Now I can cherrypick this > into 2.18 and do fixes to that list to d

Re: Create Score context before starting iteration. (issue 44860045)

2013-12-25 Thread dak
https://codereview.appspot.com/44860045/diff/60001/Documentation/contributor/programming-work.itexi File Documentation/contributor/programming-work.itexi (right): https://codereview.appspot.com/44860045/diff/60001/Documentation/contributor/programming-work.itexi#newcode43 Documentation/contribut

Re: Adds outside-staff-interface and outside-staff-axis-group-interface (issue 37950044)

2013-12-25 Thread dak
Spacing problems. Nothing requiring to prolong the review, but should be fixed before pushing. https://codereview.appspot.com/37950044/diff/40001/scm/define-grobs.scm File scm/define-grobs.scm (right): https://codereview.appspot.com/37950044/diff/40001/scm/define-grobs.scm#newcode355 scm/defin

Gives empty heights and widths to stencil-less accidentals. (issue 45070044)

2013-12-25 Thread dak
https://codereview.appspot.com/45070044/diff/1/lily/accidental.cc File lily/accidental.cc (right): https://codereview.appspot.com/45070044/diff/1/lily/accidental.cc#newcode65 lily/accidental.cc:65: return sten_p != SCM_EOL && sten_p != SCM_BOOL_F Please avoid adding more C++ value comparisons of

Tremolo cleanup. (issue 44890043)

2013-12-26 Thread dak
Yes, I know, I need to get into the habit of reviewing earlier. I was looking at this with the perspective "what if you had to push this yourself", quite hypothetically. Mostly cosmetic change suggestions that should not require an extended review. Feel free to send me a formatted patch. http

Re: Doc: NR improve example of \accepts (issue 44420043)

2013-12-26 Thread dak
On 2013/12/26 19:31:35, Keith wrote: On 2013/12/26 18:54:07, J_lowe wrote: > Could someone come up with a good example for \denies? I don't think there is one. We can always leave a context out of the enclosing context that would 'deny' it. The meaning of \denies is clear enough from

Re: Create Score context before starting iteration. (issue 44860045)

2014-01-03 Thread dak
On 2014/01/03 19:07:55, Devon Schudy wrote: David Kastrup wrote: > https://codereview.appspot.com/44860045/diff/60001/input/regression/initial-contexts.ly#newcode10 > input/regression/initial-contexts.ly:10: %%and iteration can't skip time > without it. > This comment is a bit of a non-sequi

Regtest enhanced for the new clefs (issue 47840043)

2014-01-04 Thread dak
https://codereview.appspot.com/47840043/diff/1/input/regression/clefs.ly File input/regression/clefs.ly (right): https://codereview.appspot.com/47840043/diff/1/input/regression/clefs.ly#newcode1 input/regression/clefs.ly:1: \version "2.19.1" This version will not make Patchy happy. We are at 2.

Re: Issue 3720: Built-in templates for SATB vocal scores (issue 41990043)

2014-01-04 Thread dak
https://codereview.appspot.com/41990043/diff/60001/ly/satb.ly File ly/satb.ly (right): https://codereview.appspot.com/41990043/diff/60001/ly/satb.ly#newcode1 ly/satb.ly:1: Missing \version number is causing an error when running convert-ly. h

Re: CG: basic cleanup (issue 46120044)

2014-01-05 Thread dak
https://codereview.appspot.com/46120044/diff/1/Documentation/contributor/administration.itexi File Documentation/contributor/administration.itexi (right): https://codereview.appspot.com/46120044/diff/1/Documentation/contributor/administration.itexi#newcode219 Documentation/contributor/administra

Re: Add Changes entries for bare rhythms and \beamExceptions (issue 47850043)

2014-01-06 Thread dak
https://codereview.appspot.com/47850043/diff/40001/Documentation/changes.tely File Documentation/changes.tely (right): https://codereview.appspot.com/47850043/diff/40001/Documentation/changes.tely#newcode69 Documentation/changes.tely:69: Well, the original is already in (since it really should g

Re: Add Changes entries for bare rhythms and \beamExceptions (issue 47850043)

2014-01-06 Thread dak
On 2014/01/06 16:24:24, t.daniels_treda.co.uk wrote: mailto:d...@gnu.org wrote Monday, January 06, 2014 3:27 PM > Maybe one should not try finding a term at all? One might write > something like > > Durations can now be written in music expressions without an > immediately preceding p

Re: Accidental no longer clashes with TupletNumber or TupletBracket (issue 45520044)

2014-01-07 Thread dak
https://codereview.appspot.com/45520044/diff/1/lily/tuplet-bracket.cc File lily/tuplet-bracket.cc (right): https://codereview.appspot.com/45520044/diff/1/lily/tuplet-bracket.cc#newcode674 lily/tuplet-bracket.cc:674: // won't be under the bracket That's merely a heuristic, isn't it? What with \n

Re: Web:Introduction: Rename "Our Goal" box (issue 48430043)

2014-01-07 Thread dak
On 2014/01/07 10:45:22, uliska wrote: https://codereview.appspot.com/48430043/diff/1/Documentation/web/introduction.itexi File Documentation/web/introduction.itexi (right): https://codereview.appspot.com/48430043/diff/1/Documentation/web/introduction.itexi#newcode14 Documentation/web/introdu

Re: python/convertrules.ly: Use bare rhythms after ties for simple cases (issue 49470049)

2014-01-10 Thread dak
Reviewers: Trevor Daniels, https://codereview.appspot.com/49470049/diff/1/Documentation/learning/fundamental.itely File Documentation/learning/fundamental.itely (right): https://codereview.appspot.com/49470049/diff/1/Documentation/learning/fundamental.itely#newcode1054 Documentation/learning/fu

Re: python/convertrules.ly: Use bare rhythms after ties for simple cases (issue 49470049)

2014-01-10 Thread dak
On 2014/01/10 20:56:26, t.daniels_treda.co.uk wrote: Actually this seems to be a consequence of not bumping _all_ version numbers to the current release. No, it is a consequence of not bumping the version number to the syntax used for adding material. A \version header of xx.yy.zz means "this

Re: python/convertrules.ly: Use bare rhythms after ties for simple cases (issue 49470049)

2014-01-10 Thread dak
On 2014/01/10 20:39:03, Trevor Daniels wrote: My preference would be no space before ties, one space after. That leaves ties properly associated with the note to which they are a post event, but with a clear separation from the following note. So: c'4~ 2~ 2 rather than c'4~2~2 or c'4 ~ 2 ~ 2

Issue 3796: Doc: Bump version number (issue 50530044)

2014-01-11 Thread dak
https://codereview.appspot.com/50530044/diff/1/Documentation/learning/templates.itely File Documentation/learning/templates.itely (right): https://codereview.appspot.com/50530044/diff/1/Documentation/learning/templates.itely#newcode11 Documentation/learning/templates.itely:11: @c \version "2.19.

Re: Issue 3798: Bad beam exceptions in bagpipe.ly (issue 50790043)

2014-01-11 Thread dak
Reviewers: carl.d.sorensen_gmail.com, Message: On 2014/01/12 03:59:20, Carl wrote: Do you want me to make a patch for the old way that can be applied to 2.18.1? I think I'll be able to insert the necessary 4 characters myself... But thanks for the offer. Description: Issue 3798: Bad beam exc

Re: Issue 3720: Built-in templates for SATB vocal scores (issue 41990043)

2014-01-12 Thread dak
On 2014/01/12 08:05:30, Devon Schudy wrote: David Kastrup wrote: > At any rate, I'm not really happy with the macro usage at top level > here: I think it would be better if the basic assembly was rather done > by music or Scheme functions I'm not really happy with it either, because users ha

Re: python/convertrules.ly: Use bare rhythms after ties for simple cases (issue 49470049)

2014-01-13 Thread dak
On 2014/01/13 22:59:21, Trevor Daniels wrote: We'll need a tracker issue to add some explanatory text about isolated durations to the LM (unless I've missed it), Uh, I think in a comparable situation we decided that the LM should not be converted along with the rest since that would lead to use

Re: Avoid macros in SATB framework. (issue 51230043)

2014-01-14 Thread dak
https://codereview.appspot.com/51230043/diff/20001/ly/satb.ly File ly/satb.ly (right): https://codereview.appspot.com/51230043/diff/20001/ly/satb.ly#newcode75 ly/satb.ly:75: #(define-once Key *unspecified*) I don't see that using some Scheme interface rather than providing that facility on the L

Re: Parser: harmonize \lyricsto and \addlyrics arguments (issue 53120044)

2014-01-16 Thread dak
On 2014/01/16 13:55:43, lemzwerg wrote: A side question: What about a syntax like \new Lyrics \with { lyricsto = "bla" } { bla al blob blob } to be in sync with other modes? How would that be "in sync with other modes"? You assign to a context variable that is not being used, an

Re: Parser: harmonize \lyricsto and \addlyrics arguments (issue 53120044)

2014-01-16 Thread dak
On 2014/01/16 19:49:16, janek wrote: So, does it mean that i'll be able to write \mus = { c'2 d'8 e' f' g' } << \new Staff \mus \new Lyrics \lyricsto \mus >> ? What would make you even think that? https://codereview.appspot.com/53120044/

Re: Parser: harmonize \lyricsto and \addlyrics arguments (issue 53120044)

2014-01-16 Thread dak
On 2014/01/16 19:57:11, janek wrote: On 2014/01/16 19:55:40, dak wrote: > On 2014/01/16 19:49:16, janek wrote: > > So, does it mean that i'll be able to write > > > > \mus = { c'2 d'8 e' f' g' } > > > > << > > \new

Re: Parser: harmonize \lyricsto and \addlyrics arguments (issue 53120044)

2014-01-16 Thread dak
On 2014/01/16 19:58:32, janek wrote: On 2014/01/16 19:57:11, janek wrote: > On 2014/01/16 19:55:40, dak wrote: > > On 2014/01/16 19:49:16, janek wrote: > > > So, does it mean that i'll be able to write > > > > > > \mus = { c'2 d'8 e' f&#

Re: Parser: harmonize \lyricsto and \addlyrics arguments (issue 53120044)

2014-01-17 Thread dak
When replying to a code review, please at least keep the URL of the review in the body of the mail, or the review will not be visible on Rietveld. From: Werner LEMBERG Subject: Re: Parser: harmonize \lyricsto and \addlyrics arguments (issue 53120044) To: d...@gnu.org, re...@codereview-hr.apps

TabNoteHead.style = #'slash supported. (issue 53290045)

2014-01-18 Thread dak
https://codereview.appspot.com/53290045/diff/1/scm/tablature.scm File scm/tablature.scm (right): https://codereview.appspot.com/53290045/diff/1/scm/tablature.scm#newcode27 scm/tablature.scm:27: ((slash) "2slash" This one is missing an else branch or the return value will be undefined (which

Doc: Issue 3807: Maintaining \version in documentation files (issue 51450044)

2014-01-19 Thread dak
https://codereview.appspot.com/51450044/diff/1/Documentation/contributor/doc-work.itexi File Documentation/contributor/doc-work.itexi (right): https://codereview.appspot.com/51450044/diff/1/Documentation/contributor/doc-work.itexi#newcode77 Documentation/contributor/doc-work.itexi:77: The @code{

Re: Doc: Issue 3807: Maintaining \version in documentation files (issue 51450044)

2014-01-19 Thread dak
On 2014/01/19 13:02:36, Trevor Daniels wrote: On 2014/01/19 12:40:55, dak wrote: > https://codereview.appspot.com/51450044/diff/1/Documentation/contributor/doc-work.itexi > File Documentation/contributor/doc-work.itexi (right): > > https://codereview.appspot.com/514

Changes.tely updated - 2.19.x before Feb 4th 2014 (issue 60490050)

2014-02-08 Thread dak
https://codereview.appspot.com/60490050/diff/1/Documentation/changes.tely File Documentation/changes.tely (right): https://codereview.appspot.com/60490050/diff/1/Documentation/changes.tely#newcode108 Documentation/changes.tely:108: Chord change detection in @code{\repeat} alternatives now happen

Re: Changes.tely updated - 2.19.x before Feb 4th 2014 (issue 60490050)

2014-02-08 Thread dak
https://codereview.appspot.com/60490050/diff/20001/Documentation/changes.tely File Documentation/changes.tely (right): https://codereview.appspot.com/60490050/diff/20001/Documentation/changes.tely#newcode64 Documentation/changes.tely:64: @code{Partcombiner}'s handing of repeated note durations h

Re: Changes.tely updated - 2.19.x before Feb 4th 2014 (issue 60490050)

2014-02-08 Thread dak
On 2014/02/08 19:38:38, J_lowe wrote: So this sounds like (and from my cursory experiments with lilypond-book compiling snippets) that bits of the NR http://lilypond.org/doc/v2.19/Documentation/notation/long-repeats are no longer needed now. --snip-- @lilypond[fragment,quote,relative=2] \

NR: 1.4.1 Replaced deprecated snippet w\ @lilypond (issue 60840048)

2014-02-08 Thread dak
https://codereview.appspot.com/60840048/diff/1/Documentation/notation/repeats.itely File Documentation/notation/repeats.itely (left): https://codereview.appspot.com/60840048/diff/1/Documentation/notation/repeats.itely#oldcode526 Documentation/notation/repeats.itely:526: {printing-a-repeat-sign-a

Re: Doc: Point-and-Click has wrong default value and ref to SVG output needs adding (issue 61630045)

2014-02-14 Thread dak
https://codereview.appspot.com/61630045/diff/20001/Documentation/usage/running.itely File Documentation/usage/running.itely (left): https://codereview.appspot.com/61630045/diff/20001/Documentation/usage/running.itely#oldcode670 Documentation/usage/running.itely:670: relevant when @code{PDF} is g

Report an error instead of crashing when iteration encounters non-music. (issue 63890043)

2014-02-14 Thread dak
Shouldn't most of those get caught earlier by making use of the types defined in scm/define-music-properties.scm ? If we are missing proper typechecks in make-music et al, maybe we should fix that there? https://codereview.appspot.com/63890043/ ___ li

Re: Doc: NR Clarify repeats w\ partials and barchecks (issue 61530043)

2014-02-17 Thread dak
https://codereview.appspot.com/61530043/diff/40001/Documentation/notation/repeats.itely File Documentation/notation/repeats.itely (right): https://codereview.appspot.com/61530043/diff/40001/Documentation/notation/repeats.itely#newcode186 Documentation/notation/repeats.itely:186: measure, measure

Re: Changes.tely updated - 2.19.x before Feb 4th 2014 (issue 60490050)

2014-02-18 Thread dak
On 2014/02/18 16:27:25, janek wrote: LGTM with a suggestion https://codereview.appspot.com/60490050/diff/90001/Documentation/changes.tely File Documentation/changes.tely (right): https://codereview.appspot.com/60490050/diff/90001/Documentation/changes.tely#newcode122 Documentation/changes

Re: Changes.tely updated - 2.19.x before Feb 4th 2014 (issue 60490050)

2014-02-18 Thread dak
On 2014/02/18 18:09:08, janek wrote: > I think that turning \coloredNotes into a context definition producer by > just removing the \layout { and matching } without any other change > would work fine and make a nice example. You mean something like this? coloredNotes = #(define-scheme-fu

Re: Changes.tely updated - 2.19.x before Feb 4th 2014 (issue 60490050)

2014-02-18 Thread dak
On 2014/02/18 18:29:22, janek wrote: sorry, i don't understand what you mean :( I quoted the original description, I quoted the code you posted, I listed the code that one arrives at when actually following the description, and I gave an example of what works with that changed code. I suggest

Count MIDI ticks from the beginning of the score, not each staff independently. (issue 73610044)

2014-03-11 Thread dak
That approach seems too contorted. The Global context is the one doing the iteration, so it would appear that it should be responsible for recording the global starting time if the Midi output needs it. While one could argue for storing this in some other context, in cases like \score { { \skip

Re: Count MIDI ticks from the beginning of the score, not each staff independently. (issue 73610044)

2014-03-19 Thread dak
I am not particularly happy with the amount of code for getting the starting position. I still think that a general override in the \midi block might be conceptually simpler, particularly when generating multiple midi files from the same source. However, as a default or fallback behavior, this i

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

2014-03-21 Thread dak
Reviewers: lemzwerg, Message: On 2014/03/21 08:27:41, lemzwerg wrote: LGTM. Does this change deserve a regtest? It deserves several regtests, change entry, documentation, and probably additional work to cover more cases. There are LSR examples and Scheme programming examples based on the pre

Re: Count MIDI ticks from the beginning of the score, not each staff independently. (issue 73610044)

2014-03-27 Thread dak
And you are right about derived_mark. That's needed. https://codereview.appspot.com/73610044/diff/40001/lily/performance.cc File lily/performance.cc (right): https://codereview.appspot.com/73610044/diff/40001/lily/performance.cc#newcode64 lily/performance.cc:64: if (start_ != SCM_EOL) Conditio

Re: Count MIDI ticks from the beginning of the score, not each staff independently. (issue 73610044)

2014-03-28 Thread dak
https://codereview.appspot.com/73610044/diff/60001/lily/score-performer.cc File lily/score-performer.cc (right): https://codereview.appspot.com/73610044/diff/60001/lily/score-performer.cc#newcode181 lily/score-performer.cc:181: performance_ = new Performance(context ()->get_property ("midiStart"

Re: Count MIDI ticks from the beginning of the score, not each staff independently. (issue 73610044)

2014-03-29 Thread dak
On 2014/03/29 00:44:17, Devon Schudy wrote: dak wrote: > If not (I actually don't know), it seems like making it rather part of > the midi block might be more appropriate. In which case we would not > need to store it separately as it would then be part of > performan

More readable \displayMusic output. (issue 82420045)

2014-03-30 Thread dak
https://codereview.appspot.com/82420045/diff/1/scm/music-functions.scm File scm/music-functions.scm (right): https://codereview.appspot.com/82420045/diff/1/scm/music-functions.scm#newcode181 scm/music-functions.scm:181: (list (ly:moment-main obj)) Two-argument form is not really ambiguous: if se

Re: More readable \displayMusic output. (issue 82420045)

2014-03-31 Thread dak
On 2014/03/31 14:42:26, Devon Schudy wrote: Fix durations. Print negative graces as rationals. Looks good. https://codereview.appspot.com/82420045/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypon

Re: Issue 3917: Add \alternatingTimeSignatures (issue 97110045)

2014-05-09 Thread dak
https://codereview.appspot.com/97110045/diff/20001/Documentation/notation/rhythms.itely File Documentation/notation/rhythms.itely (right): https://codereview.appspot.com/97110045/diff/20001/Documentation/notation/rhythms.itely#newcode1780 Documentation/notation/rhythms.itely:1780: Different from

<    9   10   11   12   13   14   15   16   17   18   >