Re: scheme-tutorial.itely: avoid unnecessary copying (issue 5314065)

[dak]

Reviewers: carl.d.sorensen_gmail.com, Ian Hulin (gmail),

Message:
On 2011/10/28 23:04:31, Ian Hulin (gmail) wrote:
> David,
> I think you've updated an example in two places, and added material
which needs
> to reference the second example after the first one.

I disagree.  I see only one example here.  The intermediate definition
of a Scheme function is just a temporary simplification: the function
is never used or demonstrated.  There is also no information how one
would actually get the information for passing into the Scheme
function at the Scheme level.

Since no attempt to do a complete example in Scheme is made, it would
be nonsensical to add complications only required for a full Scheme
version, only to drop them again later before even using them.

Since I don't have the time and energy to do this in the desired way
(I was just trying to correct an example in discord with reality,
making people think that about half of music-function-init.ly is
defective), I am marking this as Patch_abandoned and Frog, and maybe
somebody else can be interested in working on it until it meets the
required standards.


Description:
scheme-tutorial.itely: avoid unnecessary copying

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

Affected files:
  M Documentation/extending/scheme-tutorial.itely


Index: Documentation/extending/scheme-tutorial.itely
diff --git a/Documentation/extending/scheme-tutorial.itely  
b/Documentation/extending/scheme-tutorial.itely
index  
4c180538f5e9ed8246bfbb821063fdb0341f825d..2177856324208ab1283a3c3db9240a9c6cc1f623  
100644
--- a/Documentation/extending/scheme-tutorial.itely
+++ b/Documentation/extending/scheme-tutorial.itely
@@ -256,7 +256,7 @@ Abelson, see
 @subheading Lists

 A very common Scheme data structure is the @emph{list}.  Formally, a
-list is defined as either the empty list (represented as @code{'()},
+list is defined as either the empty list (represented as @code{'()}),
 or a pair whose @code{cdr} is a list.

 There are many ways of creating lists.  Perhaps the most common is
@@ -1004,7 +1004,7 @@ and its inner expressions are stored as a list in its  
@code{'elements}
 property.  A note is represented as an @code{EventChord} expression,
 containing a @code{NoteEvent} object (storing the duration and
 pitch properties) and any extra information (in this case, an
address@hidden with a @code{"f"} text property.
address@hidden with a @code{"f"} text property).

 @funindex{\void}
 @code{\displayMusic} returns the music it displays, so it will get
@@ -1230,12 +1230,11 @@ To build this function, we begin with
 (define (add-marcato event-chord)
   "Add a marcato ArticulationEvent to the elements of `event-chord',
   which is supposed to be an EventChord expression."
-  (let ((result-event-chord (ly:music-deep-copy event-chord)))
-    (set! (ly:music-property result-event-chord 'elements)
-          (cons (make-music 'ArticulationEvent
-                  'articulation-type "marcato")
-                (ly:music-property result-event-chord 'elements)))
-    result-event-chord))
+  (set! (ly:music-property event-chord 'elements)
+        (cons (make-music 'ArticulationEvent
+                'articulation-type "marcato")
+              (ly:music-property event-chord 'elements)))
+  event-chord)
 @end example

 The first line is the way to define a function in Scheme: the function
@@ -1252,25 +1251,20 @@ too!)
 is a description of what the function does.  This is not strictly
 necessary, but just like clear variable names, it is good practice.

address@hidden
-(let ((result-event-chord (ly:music-deep-copy event-chord)))
address@hidden example
-
address@hidden is used to declare local variables.  Here we use one local
-variable, named @code{result-event-chord}, to which we give the value
address@hidden(ly:music-deep-copy event-chord)}.  @code{ly:music-deep-copy} is
-a function specific to LilyPond, like all functions prefixed by
address@hidden:}.  It is use to make a copy of a music
-expression.  Here we copy @code{event-chord} (the parameter of the
-function).  Recall that our purpose is to add a marcato to an
address@hidden expression.  It is better to not modify the
address@hidden which was given as an argument, because it may be
-used elsewhere.
-
-Now we have a @code{result-event-chord}, which is a
address@hidden expression and is a copy of
address@hidden  We add the marcato to its @code{'elements}
-list property.
+Note that this code works @emph{destructively} on @code{event-chord},
+modifying the original.  When this is a problem, we can use
address@hidden:music-deep-copy} to create a complete copy of the music.
+However, we are going to use this inside of a music function, and
+everything a music function receives @emph{is} either original
+material or a full copy as long as you are not working in Scheme:
address@hidden hands you a copy, while @code{#music} would give you the
+original content of the variable.  So for efficiency's sake, we avoid
+creating another copy and rely on the user not bypassing Lilypond's
+mechanisms for dealing with music.
+
+Now we have @code{event-chord}, which is hopefully a
address@hidden expression.  We add the marcato to its
address@hidden'elements} list property.

 @example
 (set! place new-value)
@@ -1302,7 +1296,7 @@ plus the new @code{ArticulationEvent} expression.   
The order
 inside the @code{'elements} property is not important here.

 Finally, once we have added the marcato articulation to its @code{elements}
-property, we can return @code{result-event-chord}, hence the last line of
+property, we can return @code{event-chord}, hence the last line of
 the function.

 Now we transform the @code{add-marcato} function into a music
@@ -1313,12 +1307,11 @@ addMarcato = #(define-music-function (parser  
location event-chord)
                                      (ly:music?)
     "Add a marcato ArticulationEvent to the elements of `event-chord',
     which is supposed to be an EventChord expression."
-    (let ((result-event-chord (ly:music-deep-copy event-chord)))
-      (set! (ly:music-property result-event-chord 'elements)
-            (cons (make-music 'ArticulationEvent
-                    'articulation-type "marcato")
-                  (ly:music-property result-event-chord 'elements)))
-      result-event-chord))
+    (set! (ly:music-property event-chord 'elements)
+          (cons (make-music 'ArticulationEvent
+                  'articulation-type "marcato")
+                (ly:music-property event-chord 'elements)))
+    event-chord)
 @end example

 We may verify that this music function works correctly,