Issue 7538043: Change \transpose to \relative in ancient.itely
|
Created:
12 years ago by aleksandr.andreev Modified:
12 years ago CC:
lilypond-devel_gnu.org Visibility:
Public. |
DescriptionChange \transpose to \relative in ancient.itely
Change \transpose c c' syntax to \relative c' syntax in examples in the ancient notation chapter of notation manual.
Patch Set 1 #
Total comments: 3
Patch Set 2 : Updated based on comments from Trevor #
Total comments: 9
MessagesTotal messages: 12
Please review
Sign in to reply to this message.
LGTM, apart from a couple of nitpicks. Thanks! Trevor https://codereview.appspot.com/7538043/diff/1/Documentation/notation/ancient.... File Documentation/notation/ancient.itely (right): https://codereview.appspot.com/7538043/diff/1/Documentation/notation/ancient.... Documentation/notation/ancient.itely:1686: drop the blank line https://codereview.appspot.com/7538043/diff/1/Documentation/notation/ancient.... Documentation/notation/ancient.itely:1687: \relative c' { indent https://codereview.appspot.com/7538043/diff/1/Documentation/notation/ancient.... Documentation/notation/ancient.itely:2440: c4 c c c c2 b\longa no tab, please
Sign in to reply to this message.
Updated based on comments from Trevor
Sign in to reply to this message.
some white mensural examples change, but that doesn't matter (if anybody prefers not changing an example, I can fix those). I hope Greogrian examples don't change - I don't know whether it would matter. https://codereview.appspot.com/7538043/diff/5001/Documentation/notation/ancie... File Documentation/notation/ancient.itely (right): https://codereview.appspot.com/7538043/diff/5001/Documentation/notation/ancie... Documentation/notation/ancient.itely:952: @c @end example please make the two examples above and below match
Sign in to reply to this message.
https://codereview.appspot.com/7538043/diff/5001/Documentation/notation/ancie... File Documentation/notation/ancient.itely (right): https://codereview.appspot.com/7538043/diff/5001/Documentation/notation/ancie... Documentation/notation/ancient.itely:952: @c @end example On 2013/03/07 19:04:52, benko.pal wrote: > please make the two examples above and below match Sorry, I am confused which examples you are referring to. The above example is commented out. Should we eliminate it completely?
Sign in to reply to this message.
https://codereview.appspot.com/7538043/diff/5001/Documentation/notation/ancie... File Documentation/notation/ancient.itely (right): https://codereview.appspot.com/7538043/diff/5001/Documentation/notation/ancie... Documentation/notation/ancient.itely:952: @c @end example On 2013/03/07 21:16:18, aleksandr.andreev wrote: > On 2013/03/07 19:04:52, benko.pal wrote: > > please make the two examples above and below match > > Sorry, I am confused which examples you are referring to. The above example is > commented out. Should we eliminate it completely? sorry, you are right. I mistook @c for code. please ignore me.
Sign in to reply to this message.
On 2013/03/07 21:25:04, benko.pal wrote: > https://codereview.appspot.com/7538043/diff/5001/Documentation/notation/ancie... > File Documentation/notation/ancient.itely (right): > > https://codereview.appspot.com/7538043/diff/5001/Documentation/notation/ancie... > Documentation/notation/ancient.itely:952: @c @end example > On 2013/03/07 21:16:18, aleksandr.andreev wrote: > > On 2013/03/07 19:04:52, benko.pal wrote: > > > please make the two examples above and below match > > > > Sorry, I am confused which examples you are referring to. The above example is > > commented out. Should we eliminate it completely? > > sorry, you are right. I mistook @c for code. please ignore me. It does not make sense to keep outdated comments around. The purpose of comments is to help people, not to confuse them. If the code might be getting used in future, it should be made to match. If it doesn't, it should get removed.
Sign in to reply to this message.
LGTM
Sign in to reply to this message.
LGTM Janek PS i dedicate all my code reviews to Graham Percival. +1 for Graham!
Sign in to reply to this message.
So many pitches are wrong in this patch that I think we should revert it and have it redone carefully. Possibly using some automatism (like Frescobaldi's absolute/relative conversions, making sure that one gets the starting pitch right). As it stands, this just changes too much. https://codereview.appspot.com/7538043/diff/5001/Documentation/notation/ancie... File Documentation/notation/ancient.itely (right): https://codereview.appspot.com/7538043/diff/5001/Documentation/notation/ancie... Documentation/notation/ancient.itely:224: \[ g c a f d' \] This looks wrong. Shouldn't it be \[ g c, a' f d'' etc? It would appear that you replaced \transpose c c' with \relative c'' without bothering to convert _anything_ except the first note from absolute to relative notation. https://codereview.appspot.com/7538043/diff/5001/Documentation/notation/ancie... Documentation/notation/ancient.itely:378: \new MensuralVoice = "discantus" \relative c' { Why not \relative c'' if the starting pitch is c''? https://codereview.appspot.com/7538043/diff/5001/Documentation/notation/ancie... Documentation/notation/ancient.itely:952: @c @end example Where is the point in leaving random outcommented code in the document and letting it get more and more outdated? https://codereview.appspot.com/7538043/diff/5001/Documentation/notation/ancie... Documentation/notation/ancient.itely:1645: \[ b \] The original had b', but this would appear to be b instead. https://codereview.appspot.com/7538043/diff/5001/Documentation/notation/ancie... Documentation/notation/ancient.itely:1657: \relative c' { Again, this is now b rather than b' as previously. https://codereview.appspot.com/7538043/diff/5001/Documentation/notation/ancie... Documentation/notation/ancient.itely:1688: \[ \auctum \ascendens b \] This is b instead of b' now.
Sign in to reply to this message.
On 2013/03/16 09:09:29, dak wrote: > So many pitches are wrong in this patch that I think we should revert it and > have it redone carefully. > I do not have push access, so I cannot do much at this point. Can you revert it then? > Possibly using some automatism (like Frescobaldi's absolute/relative > conversions, making sure that one gets the starting pitch right). > I've never used Frescobaldi, so I don't know much about this. > As it stands, this just changes too much. > Sorry, I should have checked the output more carefully. (Then again, Pál said above that the changes "didn't matter"). Anyway, if you want to revert this, that is OK with me.
Sign in to reply to this message.
On 2013/03/18 00:00:13, aleksandr.andreev wrote: > Sorry, I should have checked the output more carefully. (Then again, Pál said > above that the changes "didn't matter"). Ancient music usually tries very hard to avoid ledger lines which is one of the reasons for its multitude of clefs. So being an octave off tends to make the examples quite untypical.
Sign in to reply to this message.
|
