Re: Fix 1770: revert caused a crash in displayLilyMusic. (issue4805043)

[reinhold . kainhofer]

Reviewers: Neil Puttock, J_lowe,


http://codereview.appspot.com/4805043/diff/2002/scm/define-music-display-methods.scm
File scm/define-music-display-methods.scm (right):

http://codereview.appspot.com/4805043/diff/2002/scm/define-music-display-methods.scm#newcode867
scm/define-music-display-methods.scm:867: ;; nested properties are
written #'prop1 #'prop2:
On 2011/07/21 17:47:24, Neil Puttock wrote:
> we encourage users to use list syntax for overrides, so I'd prefer
this to be
> left as it is

Really? The NR (section 5.3.6 "Modifying alists" as well as 4.4.1
"Flexible vertical spacing within systems") only gives examples of the
form #'staff-staff-spacing #'basic-distance... I didn't even know that
the list syntax was there, let alone that it is the syntax we are
advocating.

Description:
Fix 1770: revert caused a crash in displayLilyMusic.

Handle the property names similar to OverrideProperty (i.e. first check
grob-property-path and then grob-property to get the name of the
property).

Please review this at http://codereview.appspot.com/4805043/

Affected files:
  M input/regression/display-lily-tests.ly
  M scm/define-music-display-methods.scm


Index: input/regression/display-lily-tests.ly
diff --git a/input/regression/display-lily-tests.ly  
b/input/regression/display-lily-tests.ly
index  
620289dda3b87789d54a827bc3c59d36f21d4ce3..cda991945940a29d2248a2999254fa3e32f09a62  
100644
--- a/input/regression/display-lily-tests.ly
+++ b/input/regression/display-lily-tests.ly
@@ -219,6 +219,10 @@ stderr of this run."
 \test "" ##[ \once \override Beam #'beam-thickness = #0.6 #]
 \test "" ##[ \revert Staff . Stem #'thickness #]     % RevertProperty
 \test "" ##[ \revert Beam #'beam-thickness #]
+\test "NOT A BUG" ##[ \oneVoice #]   % resetting a bunch of properties
+\test "" ##[ \override StaffGrouper #'staff-staff-spacing #'basic-distance  
= #7 #] % nested properties
+\test "" ##[ \revert StaffGrouper #'(staff-staff-spacing basic-distance)  
#] % nested properties
+

 %% \applyOutput
 \test "" ##[ \applyOutput #'Foo #(lambda (arg) (list)) #]
Index: scm/define-music-display-methods.scm
diff --git a/scm/define-music-display-methods.scm  
b/scm/define-music-display-methods.scm
index  
c0041ba2a98344efe4fdef5fe097a9e0f832e0cb..e762d099e62b6504ae69f8b4614f0f516db9fada  
100644
--- a/scm/define-music-display-methods.scm
+++ b/scm/define-music-display-methods.scm
@@ -854,16 +854,20 @@ Otherwise, return #f."

 ;;; Layout properties

+;; Convert a list of (nested) property names to a property string
+(define (format-properties-list props)
+  (string-join (map (lambda (p) (format #f "#'~a" p)) props) " "))
+
 (define-display-method OverrideProperty (expr parser)
   (let* ((symbol         (ly:music-property expr 'symbol))
-        (property-path   (ly:music-property expr 'grob-property-path))
-        (properties      (if (pair? property-path)
-                             property-path
-                             (list (ly:music-property expr 'grob-property))))
+        (properties   (ly:music-property expr 'grob-property-path
+                                             (list (ly:music-property  
expr 'grob-property))))
         (value   (ly:music-property expr 'grob-value))
-        (once    (ly:music-property expr 'once)))
+        (once    (ly:music-property expr 'once))
+         ;; nested properties are written #'prop1 #'prop2:
+         (prop-string (format-properties-list properties)))

-    (format #f "~a\\override ~a~a #'~a = ~a~a"
+    (format #f "~a\\override ~a~a ~a = ~a~a"
            (if (or (null? once)
                    (not once))
                ""
@@ -872,23 +876,22 @@ Otherwise, return #f."
                ""
                (format #f "~a . " (*current-context*)))
            symbol
-           (if (null? (cdr properties))
-               (car properties)
-               properties)
+           prop-string
            (property-value->lily-string value parser)
            (new-line->lily-string))))

 (define-display-method RevertProperty (expr parser)
-  (let ((symbol (ly:music-property expr 'symbol))
-       (properties (ly:music-property expr 'grob-property-path)))
+  (let* ((symbol (ly:music-property expr 'symbol))
+         (properties (ly:music-property expr 'grob-property-path
+                                             (list (ly:music-property  
expr 'grob-property)))))
     (format #f "\\revert ~a~a #'~a~a"
            (if (eqv? (*current-context*) 'Bottom)
                ""
                (format #f "~a . " (*current-context*)))
            symbol
            (if (null? (cdr properties))
-               (car properties)
-               properties)
+               (car properties)
+               properties)
            (new-line->lily-string))))

 (define-display-method TimeSignatureMusic (expr parser)