Re: Add the command \offset to LilyPond (issue 8647044)

[david . nalesnik]

https://codereview.appspot.com/8647044/diff/5001/input/regression/offsets.ly
File input/regression/offsets.ly (right):

https://codereview.appspot.com/8647044/diff/5001/input/regression/offsets.ly#newcode5
input/regression/offsets.ly:5: the @code{\\offset} command.  These
properties are limited to immutable
On 2013/04/23 20:24:57, dak wrote:
> What does "immutable" mean here?

Hopefully this rewrite is less opaque.

https://codereview.appspot.com/8647044/diff/5001/input/regression/offsets.ly#newcode8
input/regression/offsets.ly:8: in its default appearance.  The command
is used both as a tweak and an
On 2013/04/23 20:24:57, dak wrote:
> "demonstrated as a tweak and as an override".  The double "as" is
important, and
> I'd remove "both" since otherwise the impression arises that it is
both at the
> same time.

Done.

https://codereview.appspot.com/8647044/diff/5001/scm/c++.scm
File scm/c++.scm (right):

https://codereview.appspot.com/8647044/diff/5001/scm/c++.scm#newcode30
scm/c++.scm:30: (every number-pair? x)))
On 2013/10/04 01:17:08, david.nalesnik wrote:
> On 2013/04/23 20:24:57, dak wrote:
> > Isn't it dangerous to call "every" on something that is not
necessarily a
> list?
> > Like (cons 2 3)?

> I would think so... However,
> (every number-pair? (cons 2 3))
> returns #f

Fixed.  No more reliance on undefined behavior.

https://codereview.appspot.com/8647044/diff/5001/scm/music-functions.scm
File scm/music-functions.scm (right):

https://codereview.appspot.com/8647044/diff/5001/scm/music-functions.scm#newcode2103
scm/music-functions.scm:2103: ; head of the alist.  We reverse the alist
so our search will return
Thank you for your explanations!

On 2013/10/04 05:59:15, dak wrote:

> It is probably more interesting how the function offsetter is then
ended:

> (define (offsetter property offsets)
>    (define (self grob) .............)
>    self)

This is what stumped me.  I was attempting to return (self grob).
--------
There is a major problem with this patch set which I don't know how to
address.

The examples below represent my efforts to test the effects of multiple
applications of \offset.  You can see that some accumulation is
possible.  However, the last two examples (commented out) will cause a
crash.  All I can think of is that this relates to the multiple
instances of "self" in the properties alist.  I've verified that the
instances aren't identical despite having the same name, so I don't
understand why there should be a problem.

%%%%
\relative c' {
  %% TESTS FOR ACCUMULATION %%

  % default
  <c e g b>1\arpeggio

  \override Arpeggio.positions = #'(-3.5 . 0.5)
  <c e g b>1\arpeggio

  % values created by override are offset
  \offset #'positions #'(-2 . 2) Arpeggio
  <c e g b>1\arpeggio

  % This is the result of new offset and override still in effect;
  % two offsets have not accumulated.
  \offset #'positions #'(-1 . 1) Arpeggio
  <c e g b>1\arpeggio

  % override + last offset + offset tweak
  <c e g b>1-\offset #'positions #'(-0.5 . 0.5) \arpeggio
}

\relative c' {
  %% MORE TESTS FOR ACCUMULATION %%

  % default
  <c e g b>1\arpeggio

  \once \offset #'positions #'(-2 . 2) Arpeggio
  <c e g b>1\arpeggio

  % This accumulates:
  \offset #'positions #'(-2 . 2) Arpeggio
  <c e g b>1-\offset #'positions #'(-2 . 2) \arpeggio

  %%%%%%%%%%%%%%%%%%% This causes a crash:

  %\offset #'positions #'(-2 . 2) Arpeggio
  %\once \offset #'positions #'(-2 . 2) Arpeggio
  %<c e g b>1\arpeggio

  %%%%%%%%%%%%%%%%%% This causes a crash:
  %\once \offset #'positions #'(-2 . 2) Arpeggio
  %\temporary \offset #'positions #'(-2 . 2) Arpeggio
  %<c e g b>1\arpeggio
}

https://codereview.appspot.com/8647044/