Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(994)

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 5 months ago by dak
Modified:
10 years, 2 months ago
Reviewers:
pkx166h
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Make make-relative able to deal with music rather than just pitches I'm leaning towards letting this slip into 2.18 still. The reason is that make-relative in its current form is only useful for a quite limited number of cases, can't be easily coded as a subcase of the new behavior, and the convert-ly rule only covers the most important use cases. Due to its limited usefulness, it has not seen much exposure on the mailing lists and is likely not in use much, so retaining the function name and obliterating the old, less useful behavior, makes sense before 2.18. Afterwards, this would be harder. Consists of commits (reverse order): Add regtest for music argument semantics of make-relative macro Run convert-ly on input/regression/make-relative.ly convert-ly rule for new make-relative semantics accepting music arguments This only works when the "reference pitch" is the first or last of the list. Make make-relative able to deal with music rather than just pitches The old behavior of make-relative was too cumbersome to use in many cases. This variant allows, for example: withOctave = (ly:music?) (make-relative (music) music #{ \context Bottom << $music \transpose c c' $music >> #})) \relative \new Staff \withOctave { \partial 4. c'8 e g | c2 e,4 g | c,8 c' b a <g d'> <f c'> <e b'> <d a'> | <c g'>1 | \bar "|." }

Patch Set 1 #

Total comments: 1

Patch Set 2 : Fix texidoc comment #

Patch Set 3 : Fix an obscure copying bug, explain user-visible part, provide regtest #

Unified diffs Side-by-side diffs Delta from patch set Stats (+140 lines, -26 lines) Patch
M input/regression/make-relative.ly View 2 chunks +2 lines, -2 lines 0 comments Download
A input/regression/make-relative-copies.ly View 1 2 1 chunk +29 lines, -0 lines 0 comments Download
A input/regression/make-relative-music.ly View 1 1 chunk +32 lines, -0 lines 0 comments Download
M python/convertrules.py View 1 chunk +11 lines, -0 lines 0 comments Download
M scm/music-functions.scm View 1 2 1 chunk +66 lines, -24 lines 0 comments Download

Messages

Total messages: 4
pkx166h
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.ly#newcode8 input/regression/make-relative-music.ly:8: well inside and outside of @code{\\relative}." This causes a ...
10 years, 5 months ago (2013-11-22 17:01:25 UTC) #1
dak
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.ly#newcode8 > ...
10 years, 5 months ago (2013-11-22 17:07:54 UTC) #2
dak
Fix texidoc comment
10 years, 5 months ago (2013-11-22 17:14:28 UTC) #3
dak
10 years, 5 months ago (2013-11-25 11:24:24 UTC) #4
Fix an obscure copying bug, explain user-visible part, provide regtest
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b