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

Issue 109072: Implement framework for post-fix text (de)cresc spanners (backend only) (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
14 years, 8 months ago by Reinhold
Modified:
14 years, 7 months ago
Reviewers:
Neil Puttock
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Implement framework for post-fix text (de)cresc spanners (backend only) The main problem why \decr or \dim were prefix operators was that the new dynamic engraver was reading only the properties of the Engraver itself, so there was no way to create a CrescendoEvent and at the same time specify the hairpin/text behavior. This patch makes the New_dynamic_engraver also look at the properties of the (Dec|C)recendoEvent and prefer a setting from there. If the event does not have a property (which will be the case most of the time), the Engraver's settings are used (i.e. same behavior as now). With this change, e.g. \dim can be defined as dim = #(make-music 'DecrescendoEvent 'span-direction START 'span-type text 'span-text "dim.") and used as postfix like a4\dim without the need of any overrides before the affected note. This is just the backend commit, implementing processing of the event's properties. The definition of \cresc is still unchanged, leaving the old prefix notation in place, until we find good upgrade path. Also add regtests for text cresc spanners

Patch Set 1 #

Total comments: 14

Patch Set 2 : Updated patch, regtests added to snippets; Han-Wen's SCMs comment not included (don't understand it) #

Patch Set 3 : Use char const* instead of const char*; patch is finished from my side #

Unified diffs Side-by-side diffs Delta from patch set Stats (+124 lines, -4 lines) Patch
A Documentation/snippets/new/dynamics-custom-text-spanner-postfix.ly View 1 chunk +26 lines, -0 lines 0 comments Download
A Documentation/snippets/new/dynamics-text-spanner-postfix.ly View 1 chunk +27 lines, -0 lines 0 comments Download
A input/regression/dynamics-custom-text-spanner-postfix.ly View 1 1 chunk +26 lines, -0 lines 0 comments Download
A input/regression/dynamics-text-spanner-postfix.ly View 1 1 chunk +27 lines, -0 lines 0 comments Download
M lily/new-dynamic-engraver.cc View 1 2 4 chunks +14 lines, -2 lines 0 comments Download
M ly/music-functions-init.ly View 1 chunk +1 line, -1 line 0 comments Download
M scm/define-music-properties.scm View 1 chunk +3 lines, -1 line 0 comments Download

Messages

Total messages: 1
Neil Puttock
14 years, 8 months ago (2009-08-22 00:26:56 UTC) #1
LGTM.

Don't forget to add 'span-text to all-music-properties.

http://codereview.appspot.com/109072/diff/1/2
File input/regression/dynamics-custom-text-spanner-postfix.ly (right):

http://codereview.appspot.com/109072/diff/1/2#newcode1
Line 1: \version "2.13.1"
2.13.4

http://codereview.appspot.com/109072/diff/1/2#newcode4
Line 4: texidoc = "Postfix functions for custom crescendo text spanners. The
spanners should
two spaces after full stop

http://codereview.appspot.com/109072/diff/1/2#newcode13
Line 13: )
lonely parenthesis

(same below)

http://codereview.appspot.com/109072/diff/1/2#newcode18
Line 18: \relative c' { c4-\mycresc "custom cresc" c4 c4 c4 |
newline after {

http://codereview.appspot.com/109072/diff/1/3
File input/regression/dynamics-text-spanner-postfix.ly (right):

http://codereview.appspot.com/109072/diff/1/3#newcode1
Line 1: \version "2.13.1"
2.13.4

http://codereview.appspot.com/109072/diff/1/3#newcode5
Line 5: produce one text spanner. Defining custom spanners is also easy."
two spaces after full stop

http://codereview.appspot.com/109072/diff/1/3#newcode9
Line 9: crpoco = #(make-music 'CrescendoEvent 'span-direction START 'span-type
'text 'span-text "cresc. poco a poco")
long lines

http://codereview.appspot.com/109072/diff/1/3#newcode14
Line 14: \relative c' { c4\cresc d4 e4 f4 |
newline after {

http://codereview.appspot.com/109072/diff/1/4
File lily/new-dynamic-engraver.cc (right):

http://codereview.appspot.com/109072/diff/1/4#newcode22
Line 22: #include "misc.hh"
You're not using camel_case_to_lisp_identifier (), so this can be removed

http://codereview.appspot.com/109072/diff/1/4#newcode37
Line 37: SCM get_property_setting (Stream_event *evt, const char *evprop, const
char *ctxprop);
char const *

shorten line

http://codereview.appspot.com/109072/diff/1/4#newcode77
Line 77: //   SCM spanner_type = evt->get_property (name);
remove

http://codereview.appspot.com/109072/diff/1/4#newcode79
Line 79: if (spanner_type == SCM_EOL) {
remove { }

http://codereview.appspot.com/109072/diff/1/4#newcode122
Line 122: (start_type + "Spanner").c_str ());
fix indent

http://codereview.appspot.com/109072/diff/1/4#newcode131
Line 131: (start_type + "Text").c_str ());
fix indent
Sign in to reply to this message.

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