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

[Ian Hulin]

Hi David,
I don't want to get into a flame war here, as I think you're trying to
amend a section of manual here that needs a re-think/re-write.

I came to this conclusion when I noticed one place where it looked
like you were thinking in German, and made a common mistake in the
English version through tiredness/whatever ("wenn translated as
"when", rather than "if").

I then looked at the rest of the paragraph you'd written and it was
really good info you'd gathered from working in music-functions.scm
and the parser, but was quite different to understand for someone who
hadn't done the work you had.
Then I looked at the rest of the section, and saw that the text
surrounding the bits you changed suffered from a similar problem.
There were some bits of English-as-a-second-language crustiness in
style, and it attempted to develop the eventual resulting music
function definition bit-by-bit in Scheme.  There was also phrasing
which wasn't clear unless you'd already done a trawl through the Hello
Scheme! bit of the Guile manual.

So when you wanted to add your extra it of information starting
"Note that this code works @emph{destructively} ..."
you faced an almost impossible job to phrase it clearly, be precise,
translate it from thinking in German, *and* fit it into the flow of
the existing text.

I can have a look at this section and think about a redraft. Carl, do
you have enough time to review a draft for Scheme-fu if I write the
first draft in OpenOffice Write?

David, can you send me something stating the point you wanted to make
about the stuff about writing Scheme within music function definition?
I'll incorporate it into the first draft.  Don't worry about how it
will fit into info.  Wenn es leichter ist, um klippenklar zu sein, daß
Sie es auf deutsch schreiben, machen Sie es so, und ich werde es
übersetzen. (If it's easier to be crystal-clear writing it in German,
do it that way and I'll translate).

Then I'll then get a version together in info and put it through review.

See below for some specific responses.

Cheers

Ian

On 29/10/11 00:49, address@hidden wrote:
> 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.
Nevertheless, there were two @example blocks you had changed and your
comments referencing define-music-function issues were apparently
referencing the first example block which didn't yet include the
define-music-function statement. This, combined with your desire to be
technically precise about the music function issues, made
comprehension of your text very challenging.
The intermediate definition
> of a Scheme function is just a temporary simplification: the
> function is never used or demonstrated.
So why do you apparently reference it in your new text?  The
assumption, especially when reading a piece of prose like this
section, is that you when you write "this", you mean to reference
something above in the text, rather than below.  In this case, your
new text is ambiguous about which @example you mean.
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,