Menu
▾
▴
#2240 Patch: Don't wrap EventChord around rhythmic events by default.
Originally created by: *anonymous
Originally created by: dak@gnu.org
Originally owned by: dak@gnu.org
Don't wrap EventChord around rhythmic events by default.
This changes quite a number of things and required quite a few code
changes. It is obvious that there is a lot of code duplication in
Lilypond.
A number of regtest differences show up: most of them actually
demonstrate bugs in the preexisting code base that fails to typeset
things like <c-.> with the same carefulness as c-. is typeset. Since
c-. is no longer treated like <c>-. this becomes obvious in a number
of cases.
The part combiner has several problems, the tablature code has a few,
relying on string numbers getting lost by default does no longer work,
the fingering engraver is affected.
Displaying music obviously is no longer working; this is not a defect
of the old code but rather needs work to match the new realities.
All in all, things could be worse.
This is merely a first sketch and should at least provide for
interesting experiments.
Originally posted by: dak@gnu.org
Issue 2070 has been merged into this issue.
Originally posted by: dak@gnu.org
The description is obviously no longer accurate: everything should work fine by now.
Owner: dak@gnu.org
Status: Started
Originally posted by: mts...@gmail.com
You can change the description through edit issue on Rietveld.
Originally posted by: gra...@percival-music.ca
(No comment was entered for this change.)
Labels: -Type-Enhancement Type-Critical
Originally posted by: gra...@percival-music.ca
Sorry, I tried it twice, checked git manually, deleted the old build dir, etc. Results are still the same: loses the second FAT from text-spanner-attachment-alignment.ly, loses the dim from part-combine-solo-end.ly, etc.
Maybe it's not rebuilding something it needs to rebuild after applying the patch?
Labels: -Patch-new Patch-needs_work
Originally posted by: dak@gnu.org
The symptom would totally be what happens if [re8f544af29c93145d122efa8dcfc0548d9b84f0b] is not present.
Try deleting lily/out/stream-event.o before rebuilding.
Originally posted by: dak@gnu.org
Ok, here is another theory about your failure: you have old versions of lily/rhythmic-music-iterator.cc and lily/include/rhythmic-music-iterator.hh in your tree, and if you do
git apply patch-from-Rietveld
you get an error message that they are already there, and they don't get updated. Remove them before calling git apply.
Here is how to avoid this in future:
Always when you use git-apply, use git apply --index. Never anything else.
Why does this help?
It means that the patch is not just applied to the working directory, but also is something git knows about _independently_.
If you afterwards to git reset --hard, git will _not_ touch files in the working it does not know about. In this case, the old versions of the rhythmic music iterator. So they stick around perpetually.
If you did git apply --index instead, git _does_ know about those files. When the patch created them, git reset --hard will remove them again.
So please check again after removing those unregistered files from your work directory manually.
Labels: -Patch-needs_work Patch-new
Originally posted by: dak@gnu.org
Don't wrap EventChord around rhythmic events by default.
This changes quite a number of things and required quite a few code
changes. It is obvious that there is a lot of code duplication in
Lilypond.
A number of regtest differences show up: most of them actually
demonstrate bugs in the preexisting code base that fails to typeset
things like <c-.> with the same carefulness as c-. is typeset. Since
c-. is no longer treated like <c>-. this becomes obvious in a number
of cases.
The part combiner has several problems, the tablature code has a few,
relying on string numbers getting lost by default does no longer work,
the fingering engraver is affected.
Displaying music obviously is no longer working; this is not a defect
of the old code but rather needs work to match the new realities.
All in all, things could be worse.
This is merely a first sketch and should at least provide for
interesting experiments.
http://codereview.appspot.com/5440084
Originally posted by: dak@gnu.org
Manually changed to Patch-review after very extensive manual testing since it would appear that Patchy's automated testing got confused by stale files in its work tree and will not be accurate in a timely manner.
If you are testing from a Rietveld patch (actually any patch), please use
git apply --index
for applying the patch so that git sees newly added files in the worktree and will remove them when you do
git reset --hard
If you tested this patch previously, please make sure that you don't have stale files in your work tree before applying the patch again. Running
git clean
might work for that.
Labels: -Patch-new Patch-review
Originally posted by: dak@gnu.org
(No comment was entered for this change.)
Blocking: 2247
Originally posted by: ColinPKC...@gmail.com
(No comment was entered for this change.)
Labels: -Patch-review Patch-countdown
Originally posted by: ColinPKC...@gmail.com
Counted down to 20120124, please push.
Labels: -Patch-countdown Patch-push
Originally posted by: dak@gnu.org
Pushed as a number of commits to staging, including documentation fixes that were necessary to keep the docs building. The whole sequence as pushed (partly as merge commits to keep the main line compilable at every stage) is
* commit [rf1b7a60cdb4c2f1d41329a1b3a6a01f4306f6467] (HEAD, origin/staging)
| Author: David Kastrup <dak@gnu.org>
| Date: Sun Jan 22 15:08:37 2012 +0100
|
| parser.yy: avoid creating empty articulations
|
* commit [rdeda82d71f1604f2a73825f222398a8a7300d7d1]
|\ Merge: 67c9951 1f0a00b
| | Author: David Kastrup <dak@gnu.org>
| | Date: Wed Jan 25 17:16:10 2012 +0100
| |
| | Don't wrap EventChord around rhythmic events outside of music lists, ada
| |
| * commit [r1f0a00b69403290b7fc7527b9ab100f95533f954]
| | Author: David Kastrup <dak@gnu.org>
| | Date: Mon Jan 23 21:23:29 2012 +0100
| |
| | Documentation fixes in the wake of making EventChord optional
| |
| * commit [rbab44018918beed5d01350fa6c4fed7dfffcf19c]
|/ Author: David Kastrup <dak@gnu.org>
| Date: Thu Dec 1 18:32:15 2011 +0100
|
| Don't wrap EventChord around rhythmic events outside of music lists.
|
* commit [r67c99515c55da486945a7e461f6733b002b8d60a]
| Author: David Kastrup <dak@gnu.org>
| Date: Sat Jan 21 14:24:01 2012 +0100
|
| Let \relative run through articulations for the sake of pitched trills
|
* commit [r24b50a014e90df93d3670a782cf7be23233ead43]
|\ Merge: 8019ff7 c49a3e5
| | Author: David Kastrup <dak@gnu.org>
| | Date: Wed Jan 25 15:28:39 2012 +0100
| |
| | Implements rhythmic-music-iterator, and fixes
| |
| * commit [rc49a3e5b7af2a83e0f87c6f13fdd80673d926209]
| | Author: David Kastrup <dak@gnu.org>
| | Date: Thu Jan 19 18:58:38 2012 +0100
| |
| | Fixes to Rhythmic-music-iterator
| |
| * commit [r868a063d831ce4bf065ce1ec0226f4831bf3c3fc]
|/ Author: Mike Solomon <mike@apollinemike.com>
| Date: Wed Jan 18 20:12:13 2012 +0100
|
| Implements rhythmic-music-iterator
|
Thanks for the support of doing it. I checked and rechecked the regtests at the end and most in-between stages and the docs at the end, and everything should work without any user-visible change except of the to-xml test which, in correspondence with reality and its purpose, shows fewer EventChord tags.
Of course there will be fallout because of the changed internal representation. The changes.tely file contains some of the reasons why this is, in the overall scheme of things, still a change to the better.
Labels: -Patch-push Fixed_2_15_28
Status: Fixed
Originally posted by: dak@gnu.org
(No comment was entered for this change.)
Blocking: -2247
Originally posted by: gra...@percival-music.ca
(No comment was entered for this change.)
Status: Verified