Re: Add the command \offset to LilyPond (issue 8647044)
On 2013/10/20 20:35:20, david.nalesnik wrote:
OK, that makes sense. I've rewritten offset according to these
guidelines. A
side benefit
is that it was no trouble to allow directed tweaks, so the following
is
possible:
{
4
4
}
Oh. Indeed.
https://codereview.appspot.com/8647044/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add the command \offset to LilyPond (issue 8647044)
On 2013/10/20 16:15:56, dak wrote:
On 2013/10/20 15:51:57, david.nalesnik wrote:
>
https://codereview.appspot.com/8647044/diff/48001/scm/music-functions.scm
> File scm/music-functions.scm (right):
>
>
https://codereview.appspot.com/8647044/diff/48001/scm/music-functions.scm#newcode2118
> scm/music-functions.scm:2118: (let* ((immutable
(ly:grob-basic-properties
grob))
> I just noticed something unfortunate while attempting to write
documentation
for
> \offset. Basically, `property' needs to be specified using the
older notation
> -- i.e., with #' prefixed.
I suggest looking at the code in issue 3625. That "sloppy" approach
let's you
concatenate the material first (requiring the symbol? check, though)
and then
pull the whole thing through the property path checker. In this case,
it would
appear that you want to call it with #:min 3 #:max 3 #:default 'Bottom
as
anything but a toplevel property would be a cause for problems.
And you should be able to get at your property using (third p) or so.
A similar
recipe should be good for tweaks, just using a different #:start value
for the
check.
OK, that makes sense. I've rewritten offset according to these
guidelines. A side benefit
is that it was no trouble to allow directed tweaks, so the following is
possible:
{
4
4
}
Tweaks following tweaks work, too:
{
4
}
Here's what I come up for offset:
offset =
#(define-music-function (parser location property offsets item)
(symbol-list-or-symbol? scheme? symbol-list-or-music?)
(_i "Offset the default value of @var{property} of @var{item} by
@var{offsets}. If @var{item} is a string, the result is
@code{\\override} for the specified grob type. If @var{item} is
a music expression, the result is the same music expression with an
appropriate tweak applied.")
(if (ly:music? item)
; In case of a tweak, our grob property path
; is Grob.property.
(let ((prop-path (check-grob-path
(if (symbol? property)
(list property)
property)
parser location
#:start 1 #:default #t #:min 2 #:max 2)))
(if prop-path
; If the head of the grob property path is a
; symbol--i.e., a grob name--produce a directed
; tweak. Otherwise, create an ordinary tweak.
(if (symbol? (car prop-path))
#{
\tweak #prop-path
#(offsetter (second prop-path) offsets) #item
#}
#{
\tweak #(second prop-path)
#(offsetter (second prop-path) offsets) #item
#})
(make-music 'Music)))
; In case of an override, our grob property path
; is Context.Grob.property
(let ((prop-path (check-grob-path
(append item
(if (symbol? property)
(list property)
property))
parser location
#:default 'Bottom #:min 3 #:max 3)))
(if prop-path
#{
\override #prop-path =
#(offsetter (third prop-path) offsets)
#}
(make-music 'Music)
%%
Thank you very much for your help!
https://codereview.appspot.com/8647044/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add the command \offset to LilyPond (issue 8647044)
On 2013/10/20 15:51:57, david.nalesnik wrote: https://codereview.appspot.com/8647044/diff/48001/scm/music-functions.scm File scm/music-functions.scm (right): https://codereview.appspot.com/8647044/diff/48001/scm/music-functions.scm#newcode2118 scm/music-functions.scm:2118: (let* ((immutable (ly:grob-basic-properties grob)) I just noticed something unfortunate while attempting to write documentation for \offset. Basically, `property' needs to be specified using the older notation -- i.e., with #' prefixed. A quick fix would be to add the line: (property (if (symbol-list? property) (car property) property))) to the let-block here. I don't know if that's adequate, however. I suggest looking at the code in issue 3625. That "sloppy" approach let's you concatenate the material first (requiring the symbol? check, though) and then pull the whole thing through the property path checker. In this case, it would appear that you want to call it with #:min 3 #:max 3 #:default 'Bottom as anything but a toplevel property would be a cause for problems. And you should be able to get at your property using (third p) or so. A similar recipe should be good for tweaks, just using a different #:start value for the check. https://codereview.appspot.com/8647044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add the command \offset to LilyPond (issue 8647044)
https://codereview.appspot.com/8647044/diff/48001/scm/music-functions.scm File scm/music-functions.scm (right): https://codereview.appspot.com/8647044/diff/48001/scm/music-functions.scm#newcode2118 scm/music-functions.scm:2118: (let* ((immutable (ly:grob-basic-properties grob)) I just noticed something unfortunate while attempting to write documentation for \offset. Basically, `property' needs to be specified using the older notation -- i.e., with #' prefixed. A quick fix would be to add the line: (property (if (symbol-list? property) (car property) property))) to the let-block here. I don't know if that's adequate, however. https://codereview.appspot.com/8647044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add the command \offset to LilyPond (issue 8647044)
2013/10/15 : > On 2013/10/15 18:04:43, janek wrote: > Well, that's the slightly icky thing. The \offset is with respect to > the situation you'll get when doing \revert Arpeggio.positions > afterwards. So if you want to make a _relative_ offset to the first > setting, you have to do > \temporary \offset #'positions #'(-1 . 2) Arpeggio > > I'm not overly enthusiastic about how the \offset command ends up when > entered as an override. Maybe we should allow > \offset #'(-1 . 2) Arpeggio.positions here? It seems that the offset > list should not be confusable with a symbol? or whatever is used in the > front? So we could make that first argument optional, and complain when > the last argument is not a symbol list making up for it? No idea :-( ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add the command \offset to LilyPond (issue 8647044)
On 2013/10/15 18:04:43, janek wrote:
Hi,
an afterthought.
On 2013/10/06 01:15:12, david.nalesnik wrote:
> The examples below represent my efforts to test the effects of
multiple
> applications of \offset. You can see that some accumulation is
possible.
> [...]
>
> \relative c' {
> %% TESTS FOR ACCUMULATION %%
>
> % default
> 1\arpeggio
>
> \override Arpeggio.positions = #'(-3.5 . 0.5)
> 1\arpeggio
>
> % values created by override are offset
> \offset #'positions #'(-2 . 2) Arpeggio
> 1\arpeggio
I'm not sure if i haven't missed something, but to your realize that
in this
case the offset isn't applied on top of the override (as the comment
suggests),
but replaces it? This is self-evident in the example below:
\relative c' {
% default
1\arpeggio
\override Arpeggio.positions = #'(-5 . 5)
1\arpeggio
\offset #'positions #'(-1 . 2) Arpeggio
1\arpeggio
}
with current master (a82d8622e6b1be36169de7d2fe1f9aa88618933b,
containing offset
patch) the last arpeggio is shorter than the second, while it should
be longer
in both directions.
Well, that's the slightly icky thing. The \offset is with respect to
the situation you'll get when doing \revert Arpeggio.positions
afterwards. So if you want to make a _relative_ offset to the first
setting, you have to do
\temporary \offset #'positions #'(-1 . 2) Arpeggio
I'm not overly enthusiastic about how the \offset command ends up when
entered as an override. Maybe we should allow
\offset #'(-1 . 2) Arpeggio.positions here? It seems that the offset
list should not be confusable with a symbol? or whatever is used in the
front? So we could make that first argument optional, and complain when
the last argument is not a symbol list making up for it?
https://codereview.appspot.com/8647044/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add the command \offset to LilyPond (issue 8647044)
Hi,
an afterthought.
On 2013/10/06 01:15:12, david.nalesnik wrote:
The examples below represent my efforts to test the effects of
multiple
applications of \offset. You can see that some accumulation is
possible.
[...]
\relative c' {
%% TESTS FOR ACCUMULATION %%
% default
1\arpeggio
\override Arpeggio.positions = #'(-3.5 . 0.5)
1\arpeggio
% values created by override are offset
\offset #'positions #'(-2 . 2) Arpeggio
1\arpeggio
I'm not sure if i haven't missed something, but to your realize that in
this case the offset isn't applied on top of the override (as the
comment suggests), but replaces it? This is self-evident in the example
below:
\relative c' {
% default
1\arpeggio
\override Arpeggio.positions = #'(-5 . 5)
1\arpeggio
\offset #'positions #'(-1 . 2) Arpeggio
1\arpeggio
}
with current master (a82d8622e6b1be36169de7d2fe1f9aa88618933b,
containing offset patch) the last arpeggio is shorter than the second,
while it should be longer in both directions.
best,
Janek
https://codereview.appspot.com/8647044/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add the command \offset to LilyPond (issue 8647044)
https://codereview.appspot.com/8647044/diff/48001/ly/music-functions-init.ly File ly/music-functions-init.ly (right): https://codereview.appspot.com/8647044/diff/48001/ly/music-functions-init.ly#newcode705 ly/music-functions-init.ly:705: (if (ly:music? item) Ok, I am annoyed at the necessity for those distinctions. Issue 3603 might help. Not sure about the error message quality, though. https://codereview.appspot.com/8647044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add the command \offset to LilyPond (issue 8647044)
On 2013/10/07 14:10:09, dak wrote:
On 2013/10/06 01:15:12, david.nalesnik wrote:
> 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.
I suppose the problem is with
+(define (find-value-to-offset prop self alist)
+ "Return the first value of the property @var{prop} in the property
alist @var
+that is not @var{self}."
+ (let loop ((ls alist))
+(if (null? ls)
+#f
+(if (eq? prop (car (first ls)))
+(if (eq? self (cdr (first ls)))
+(loop (cdr ls))
+(cdr (first ls)))
+(loop (cdr ls))
First: try using GUILE srfi-1 functions for that kind of thing: they
are more
readable than explicit loops.
I've used member, and I've also incorporated Lily's assoc-get.
But what you need here is "return the first value of the property
@var{prop} in
the property
alist @var{alist} @em{after} having found @var{self}."
You _always_ have to _first_ pass self before looking for anything
else. If you
find a valid property _before_ finding self, ignore it and continue
looking.
Otherwise you end up in a mutual recursion.
Changes made--works like a charm! There are now more possibilities for
accumulation. Possibly another regtest `offset-accumulation.ly' is
warranted? (As far as documenting the abilities for accumulation: this
may be more confusing than worthwhile.)
If you don't find self at all, its head-scratching time: after all,
who called
you then? I lean toward throwing an error: continuation strategies
might be
fragile.
Well, if self isn't found, then we're dealing with a tweak, which
wouldn't have added to the alist. In that case, the function
`find-value-to-offset' as rewritten will simply return the first
instance of the property in the alist. `offsetter' should catch
situations where the user tries to offset something not in the basic
properties alist.
https://codereview.appspot.com/8647044/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add the command \offset to LilyPond (issue 8647044)
https://codereview.appspot.com/8647044/diff/35001/input/regression/offsets.ly File input/regression/offsets.ly (right): https://codereview.appspot.com/8647044/diff/35001/input/regression/offsets.ly#newcode31 input/regression/offsets.ly:31: On 2013/10/07 14:10:32, dak wrote: Spurious space in otherwise empty line. Done. https://codereview.appspot.com/8647044/diff/35001/scm/c++.scm File scm/c++.scm (right): https://codereview.appspot.com/8647044/diff/35001/scm/c++.scm#newcode30 scm/c++.scm:30: (not (null? x)) On 2013/10/07 14:13:32, dak wrote: Why not null? An empty list is also a list, so this makes the predicate a bit of a misnomer. OK, above condition removed so predicate returns #t for empty list. https://codereview.appspot.com/8647044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add the command \offset to LilyPond (issue 8647044)
https://codereview.appspot.com/8647044/diff/35001/scm/c++.scm File scm/c++.scm (right): https://codereview.appspot.com/8647044/diff/35001/scm/c++.scm#newcode30 scm/c++.scm:30: (not (null? x)) Why not null? An empty list is also a list, so this makes the predicate a bit of a misnomer. https://codereview.appspot.com/8647044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add the command \offset to LilyPond (issue 8647044)
https://codereview.appspot.com/8647044/diff/35001/input/regression/offsets.ly File input/regression/offsets.ly (right): https://codereview.appspot.com/8647044/diff/35001/input/regression/offsets.ly#newcode31 input/regression/offsets.ly:31: Spurious space in otherwise empty line. https://codereview.appspot.com/8647044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add the command \offset to LilyPond (issue 8647044)
On 2013/10/06 01:15:12, david.nalesnik wrote:
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.
I suppose the problem is with
+(define (find-value-to-offset prop self alist)
+ "Return the first value of the property @var{prop} in the property
alist @var
+that is not @var{self}."
+ (let loop ((ls alist))
+(if (null? ls)
+#f
+(if (eq? prop (car (first ls)))
+(if (eq? self (cdr (first ls)))
+(loop (cdr ls))
+(cdr (first ls)))
+(loop (cdr ls))
First: try using GUILE srfi-1 functions for that kind of thing: they are
more readable than explicit loops.
And nice to have accurate documentation: this tells us exactly what is
wrong:
+ "Return the first value of the property @var{prop} in the property
alist @var
+that is not @var{self}."
But what you need here is "return the first value of the property
@var{prop} in the property
alist @var{alist} @em{after} having found @var{self}."
You _always_ have to _first_ pass self before looking for anything else.
If you find a valid property _before_ finding self, ignore it and
continue looking. Otherwise you end up in a mutual recursion.
If you don't find self at all, its head-scratching time: after all, who
called you then? I lean toward throwing an error: continuation
strategies might be fragile.
https://codereview.appspot.com/8647044/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add the command \offset to LilyPond (issue 8647044)
On 2013/10/06 06:06:40, lemzwerg wrote: LGTM. Maybe you can somewhere add a sentence like Note that \tweak and friends provide absolute positioning, not relative. to the documentation. It took me a while to recognize the difference. OK--will do. (I just made some changes to the Rietveld description so at least it's clearer to developers looking at the issue.) https://codereview.appspot.com/8647044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add the command \offset to LilyPond (issue 8647044)
Just before I forget: this is not a real comment on this issue. David N posted some update on this issue maybe a week ago (details are not all that important) after it lay dormant for months. I replied to that issue, and then figured I apparently didn't properly reply since Rietveld flagged "unpublished drafts". So I did the "Publish all my drafts" routine. Turns out that the unpublished drafts were detailed advice to technical questions that I wrote months ago, about the time when the issue became dormant. And it probably became dormant not least of all because David N actually could have moved forward with that information. It's not the first time that I had accidentally let reviews of mine go unpublished: for some reason, Rietveld is designed to require an additional step after a step that already quite feels like being the final step to me. What to make of that? No idea. https://codereview.appspot.com/8647044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add the command \offset to LilyPond (issue 8647044)
LGTM. Maybe you can somewhere add a sentence like Note that \tweak and friends provide absolute positioning, not relative. to the documentation. It took me a while to recognize the difference. https://codereview.appspot.com/8647044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add the command \offset to LilyPond (issue 8647044)
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
1\arpeggio
\override Arpeggio.positions = #'(-3.5 . 0.5)
1\arpeggio
% values created by override are offset
\offset #'positions #'(-2 . 2) Arpeggio
1\arpeggio
% This is the result of new offset and override still in effect;
% two offsets have not accumulated.
\offset #'positions #'(-1 . 1) Arpeggio
1\arpeggio
% override + last offset + offset tweak
1-\offset #'positions #'(-0.5 . 0.5) \arpeggio
}
\relative c' {
%% MORE TESTS FOR ACCUMULATION %%
% default
1\arpeggio
\once \offset #'positions #'(-2 . 2) Arpeggio
1\arpeggio
% This accumulates:
\offset #'positions #'(-2 . 2) Arpeggio
1-\offset #'positions #'(-2 . 2) \arpeggio
%%% This causes a crash:
%\offset #'positions #'(-2 . 2) Arpeggio
%\once \offset #'positions #'(-2 . 2) Arpeggio
%1\arpeggio
%% This causes a crash:
%\once \offset #'positions #'(-2 . 2) Arpeggio
%\temporary \offset #'positions #'(-2 . 2) Arpeggio
%1\arpeggio
}
https://codereview.appspot.com/8647044/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add the command \offset to LilyPond (issue 8647044)
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 On 2013/04/25 13:48:55, david.nalesnik wrote: On 2013/04/23 22:09:04, dak wrote: > On 2013/04/23 20:24:57, dak wrote: > "If our search returns an anonymous procedure" is quite strained. We don't need > to _speculate_ about the identity of the anonymous procesure. If we want to be > sure, we can just remember it: > > (define (offsetter property offsets) > (define (self grob) In view of the way offsetter is called, the first line of the above would still need to be (define ((offsetter property offsets) grob) Not at all. I see the value of the procedure recognizing itself, but I'm afraid I don't understand how to implement this from the start you've given me. Could you give me a further hint as to what (define (self grob) ... would contain? The same as it does now, except that it can compare the callbacks in the alist to "self" with "eq?". It is probably more interesting how the function offsetter is then ended: (define (offsetter property offsets) (define (self grob) .) self) The basics, namely having a function returning a function (or rather a closure since it uses "property" and "offsets" internally) remain the same. It is just that the closure is no longer anonymous but rather called "self" (or whatever else) and thus is able to recognize itself. https://codereview.appspot.com/8647044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add the command \offset to LilyPond (issue 8647044)
On 2013/10/04 01:17:08, david.nalesnik wrote: 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/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 Well, relying on undefined behavior that happens to do what we want now seems imprudent. https://codereview.appspot.com/8647044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add the command \offset to LilyPond (issue 8647044)
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/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 https://codereview.appspot.com/8647044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add the command \offset to LilyPond (issue 8647044)
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
On 2013/04/23 22:09:04, dak wrote:
On 2013/04/23 20:24:57, dak wrote:
"If our search returns an anonymous procedure" is quite strained. We
don't need
to _speculate_ about the identity of the anonymous procesure. If we
want to be
sure, we can just remember it:
(define (offsetter property offsets)
(define (self grob)
In view of the way offsetter is called, the first line of the above
would still need to be
(define ((offsetter property offsets) grob)
I see the value of the procedure recognizing itself, but I'm afraid I
don't understand how to implement this from the start you've given me.
Could you give me a further hint as to what
(define (self grob) ...
would contain?
and we can now check through both mutable and immutable grob
properties, find
self, and look behind self in the aliast to see whether we find
another
occurence of the property which, if found, we duly evaluate
recursively. That
should allow multiple offsetter calls to work recursively.
Huh. Maybe not the best idea since
\offset Y-offset ... Grob
\offset Y-offset ... Grob
would not accumulate while
\offset Y-offset ... Grob
\temporary\offset Y-offset ... Grob
or even
\offset Y-offset ... Grob
\once\offset Y-offset ... Grob
would.
But still seems better than forgetting all overrides except the first
one
(likely in the grob definition).
Accumulation of offsets, while nice in theory, does not seem to me to
have practical application. (And its usage would lead to rather
confusing documentation, judging by the above.)
Here's an example of remembering which is already possible:
\relative c'' {
c8 d e f
\offset #'positions #'(-1 . -3) Beam
c d e f
\temporary \offset #'positions #'(-5 . -5) Beam
c d e f
\revert Beam.positions
c d e f
}
In view of this, I could simply look for "self" at the head of the
properties alist and use that for comparison, rather than my opaque use
of procedure-name.
https://codereview.appspot.com/8647044/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add the command \offset to LilyPond (issue 8647044)
David, Thank you for your reviews. It will take me some time to make the changes you propose, so I've changed the status of the patch to "needs work". https://codereview.appspot.com/8647044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add the command \offset to LilyPond (issue 8647044)
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 On 2013/04/23 20:24:57, dak wrote: Why would tweak/override add to the _immutable_ properties? How could they? Is there something I don't understand here? To answer myself: the basic properties would contain overrides (set there from the respectively changed context property) but not tweaks (which are applied after initialization of the grob). "If our search returns an anonymous procedure" is quite strained. We don't need to _speculate_ about the identity of the anonymous procesure. If we want to be sure, we can just remember it: (define (offsetter property offsets) (define (self grob) ... and we can now check through both mutable and immutable grob properties, find self, and look behind self in the aliast to see whether we find another occurence of the property which, if found, we duly evaluate recursively. That should allow multiple offsetter calls to work recursively. Huh. Maybe not the best idea since \offset Y-offset ... Grob \offset Y-offset ... Grob would not accumulate while \offset Y-offset ... Grob \temporary\offset Y-offset ... Grob or even \offset Y-offset ... Grob \once\offset Y-offset ... Grob would. But still seems better than forgetting all overrides except the first one (likely in the grob definition). https://codereview.appspot.com/8647044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add the command \offset to LilyPond (issue 8647044)
Sorry for the late review.
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
What does "immutable" mean here?
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
"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.
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)))
Isn't it dangerous to call "every" on something that is not necessarily
a list? Like (cons 2 3)?
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
Why would tweak/override add to the _immutable_ properties? How could
they? Is there something I don't understand here?
https://codereview.appspot.com/8647044/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add the command \offset to LilyPond (issue 8647044)
On 2013/04/23 19:38:09, dak wrote: On 2013/04/23 19:03:18, david.nalesnik wrote: > On 2013/04/23 13:14:23, janek wrote: > > anyway, this patch LTGM. > > Thanks, Janek! > > The patch has passed countdown, but in view of the fact that a stable release is > on the horizon, it may not be appropriate to push it at this time. > > On the other hand, an argument for pushing it right away might be to guard it > against loss of functionality, as happened recently regarding Stem.length and > Y-offset for certain grobs. Huh? I don't understand that argument. I mean simply that future changes would need to take the functionality of \offset into account. There's no real guarantee that something isn't going to be pushed between the present and the date the the stable release actually comes out that won't affect it. On the other hand, the patch is missing a Changes entry and any integration into the user documentation, so it is mostly inaccessible to users anyway (and the means to discuss/review its interface design are limited) and there seems little point to include it in this state. If we don't have documentation for the next unstable release, the chances for translations etc to arrive timely are slim. I'm not entirely clear what you mean here. Are you saying that the patch should be withdrawn and resubmitted with full documentation--if it is to be considered for the _stable_ release? Thanks, David https://codereview.appspot.com/8647044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add the command \offset to LilyPond (issue 8647044)
On 2013/04/23 19:03:18, david.nalesnik wrote: On 2013/04/23 13:14:23, janek wrote: > anyway, this patch LTGM. Thanks, Janek! The patch has passed countdown, but in view of the fact that a stable release is on the horizon, it may not be appropriate to push it at this time. On the other hand, an argument for pushing it right away might be to guard it against loss of functionality, as happened recently regarding Stem.length and Y-offset for certain grobs. Huh? I don't understand that argument. On the other hand, the patch is missing a Changes entry and any integration into the user documentation, so it is mostly inaccessible to users anyway (and the means to discuss/review its interface design are limited) and there seems little point to include it in this state. If we don't have documentation for the next unstable release, the chances for translations etc to arrive timely are slim. https://codereview.appspot.com/8647044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add the command \offset to LilyPond (issue 8647044)
On 2013/04/23 13:14:23, janek wrote: anyway, this patch LTGM. Thanks, Janek! The patch has passed countdown, but in view of the fact that a stable release is on the horizon, it may not be appropriate to push it at this time. On the other hand, an argument for pushing it right away might be to guard it against loss of functionality, as happened recently regarding Stem.length and Y-offset for certain grobs. What do you think about this? If it's OK to push, I can supply a patch to whoever is willing to take it on. -David https://codereview.appspot.com/8647044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add the command \offset to LilyPond (issue 8647044)
anyway, this patch LTGM. https://codereview.appspot.com/8647044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add the command \offset to LilyPond (issue 8647044)
Janek Warchoł writes: > Sorry for the delay, got little lilypond-time recently... > > 2013/4/16 : >> On 2013/04/16 14:33:50, janek wrote: >>> >>> 2013/4/16 : >>> > nothing-or-music in the last position only works if "nothing" is >>> > _explicitly_ specified with \default >>> > >>> > There is no way that the last argument of a music function can be >>> > "nothing" silently. >>> >>> pity. would it possible (and reasonable) to change this? >> >> >> Uh, a call of offset can be followed by music quite naturally. So this >> is not just a "technical restriction". It is one of logic. > > So, do i understand correctly that \offset could have a syntax like > > \offset grob-property-path value music > > And then, if grob-property-path contained only property, > the function would be used as a tweak (with "music" being > what's tweaked), and if the function was being used as an > override (grob-property-path contains a grobname), "music" > would be sort-of-unused? Possible, but "sort-of-unused" is not a reasonable interface. -- David Kastrup ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add the command \offset to LilyPond (issue 8647044)
Sorry for the delay, got little lilypond-time recently... 2013/4/16 : > On 2013/04/16 14:33:50, janek wrote: >> >> 2013/4/16 : >> > nothing-or-music in the last position only works if "nothing" is >> > _explicitly_ specified with \default >> > >> > There is no way that the last argument of a music function can be >> > "nothing" silently. >> >> pity. would it possible (and reasonable) to change this? > > > Uh, a call of offset can be followed by music quite naturally. So this > is not just a "technical restriction". It is one of logic. So, do i understand correctly that \offset could have a syntax like \offset grob-property-path value music And then, if grob-property-path contained only property, the function would be used as a tweak (with "music" being what's tweaked), and if the function was being used as an override (grob-property-path contains a grobname), "music" would be sort-of-unused? best, Janek ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add the command \offset to LilyPond (issue 8647044)
2013/4/16 :
>> and there's too many "or-nothing"s there (i suppose
>> that the biggest problem is an optional argument
>> followed by non-optional one ("value")). However,
>> in case of \offset, we can have either a \shape-like
>> syntax
>>
>> \offset property value grobname-or-music
>>
>> or
>>
>> \offset grobwithproperty-or-property value nothing-or-music
>>
>> since the first argument is not optional (i.e there's
>> always at least the property to specify), i guess that
>> it should be easier to do?
>
> nothing-or-music in the last position only works if "nothing" is
> _explicitly_ specified with \default
>
> There is no way that the last argument of a music function can be
> "nothing" silently.
pity. would it possible (and reasonable) to change this?
JAnek
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add the command \offset to LilyPond (issue 8647044)
On 2013/04/16 14:33:50, janek wrote: 2013/4/16 : > nothing-or-music in the last position only works if "nothing" is > _explicitly_ specified with \default > > There is no way that the last argument of a music function can be > "nothing" silently. pity. would it possible (and reasonable) to change this? Uh, a call of offset can be followed by music quite naturally. So this is not just a "technical restriction". It is one of logic. https://codereview.appspot.com/8647044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add the command \offset to LilyPond (issue 8647044)
and there's too many "or-nothing"s there (i suppose
that the biggest problem is an optional argument
followed by non-optional one ("value")). However,
in case of \offset, we can have either a \shape-like
syntax
\offset property value grobname-or-music
or
\offset grobwithproperty-or-property value nothing-or-music
since the first argument is not optional (i.e there's
always at least the property to specify), i guess that
it should be easier to do?
nothing-or-music in the last position only works if "nothing" is
_explicitly_ specified with \default
There is no way that the last argument of a music function can be
"nothing" silently.
I guess that was the design decision also to be made regarding the
other functions.
https://codereview.appspot.com/8647044/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add the command \offset to LilyPond (issue 8647044)
On 2013/04/16 13:03:25, dak wrote:
On 2013/04/16 12:50:47, david.nalesnik wrote:
> Here I kept the same pattern as \alterBroken and \shape,
> as they have been revised by David Kastrup. I agree that
> the syntax is a little awkward, and that
> it would be nice if the pattern you give were workable.
> However, IIRC, this syntax is the only one currently feasible.
> I'm not an expert here, though.
For \alterBroken and \shape, the syntax is actually
\shape ... item where item is either music
(which is then tweaked) _or_ a grob
(which then gets an override). The syntax
is only due to this double-function.
If \offset does not have the same characteristic (nor intends to have
it), then
one should be able to make do with a single specification.
I have not reviewed this so far, so I can't tell.
I've always thought that the syntax of \shape was done
this way because there was no "property" argument (as
it always concerned control-points) - in other words,
\shape's syntax is
\shape value grobname-or-music
because we need to keep the number and order of arguments
quite constant. The alternative (specifying grobname before
value) would be bad because then we'd have
\shape grobname-or-nothing value nothing-or-musc
and there's too many "or-nothing"s there (i suppose
that the biggest problem is an optional argument
followed by non-optional one ("value")). However,
in case of \offset, we can have either a \shape-like
syntax
\offset property value grobname-or-music
or
\offset grobwithproperty-or-property value nothing-or-music
since the first argument is not optional (i.e there's
always at least the property to specify), i guess that
it should be easier to do?
best,
Janek
https://codereview.appspot.com/8647044/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add the command \offset to LilyPond (issue 8647044)
On 2013/04/16 12:50:47, david.nalesnik wrote: On 2013/04/13 21:39:40, janek wrote: > There's one thing that puzzles me. Current syntax is > > \offset property offset-value grob-name > > I understand that grob-name is at the end because it's optional, and we want to > omit it when we're using \offset as a tweak. > However, i find this syntax awkward. Since David K's change that allowed to use > dot-separated list for specyfying grobs "together" with properties, couldn't we > process both the property and grobname as one argument, and therefore keep the > usual order? In other words, what about syntax like this: > > \offset grob-property-path offset-value > > where grob-property-path would be either Grob.property (when using \offset as an > override) or just property (when using it as a tweak)? Here I kept the same pattern as \alterBroken and \shape, as they have been revised by David Kastrup. I agree that the syntax is a little awkward, and that it would be nice if the pattern you give were workable. However, IIRC, this syntax is the only one currently feasible. I'm not an expert here, though. For \alterBroken and \shape, the syntax is actually \shape ... item where item is either music (which is then tweaked) _or_ a grob (which then gets an override). The syntax is only due to this double-function. If \offset does not have the same characteristic (nor intends to have it), then one should be able to make do with a single specification. I have not reviewed this so far, so I can't tell. https://codereview.appspot.com/8647044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add the command \offset to LilyPond (issue 8647044)
On 2013/04/13 21:39:40, janek wrote: There's one thing that puzzles me. Current syntax is \offset property offset-value grob-name I understand that grob-name is at the end because it's optional, and we want to omit it when we're using \offset as a tweak. However, i find this syntax awkward. Since David K's change that allowed to use dot-separated list for specyfying grobs "together" with properties, couldn't we process both the property and grobname as one argument, and therefore keep the usual order? In other words, what about syntax like this: \offset grob-property-path offset-value where grob-property-path would be either Grob.property (when using \offset as an override) or just property (when using it as a tweak)? Here I kept the same pattern as \alterBroken and \shape, as they have been revised by David Kastrup. I agree that the syntax is a little awkward, and that it would be nice if the pattern you give were workable. However, IIRC, this syntax is the only one currently feasible. I'm not an expert here, though. https://codereview.appspot.com/8647044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add the command \offset to LilyPond (issue 8647044)
I've considerably shortened the regtests, removing redundancies and unnecessary line breaks. Thanks for the comments! https://codereview.appspot.com/8647044/diff/1/input/regression/offsets.ly File input/regression/offsets.ly (right): https://codereview.appspot.com/8647044/diff/1/input/regression/offsets.ly#newcode11 input/regression/offsets.ly:11: On 2013/04/13 21:39:41, janek wrote: generally, i'd make the regtest shorter. Done. https://codereview.appspot.com/8647044/diff/1/input/regression/offsets.ly#newcode40 input/regression/offsets.ly:40: 1-\offset #'positions #'(-2 . 2) \arpeggio On 2013/04/13 21:39:41, janek wrote: for example, one instance of arpeggio should suffice here. Done. https://codereview.appspot.com/8647044/diff/1/input/regression/offsets.ly#newcode50 input/regression/offsets.ly:50: \once \offset #'X-offset #0.5 DynamicText On 2013/04/13 21:39:41, janek wrote: similarly here Done. https://codereview.appspot.com/8647044/diff/1/input/regression/offsets.ly#newcode57 input/regression/offsets.ly:57: c-\offset #'padding #1.5 \f On 2013/04/13 21:39:41, janek wrote: ditto Done. https://codereview.appspot.com/8647044/diff/1/input/regression/offsets.ly#newcode102 input/regression/offsets.ly:102: \once \offset #'Y-offset #2 BreathingSign On 2013/04/13 21:39:41, janek wrote: ditto Done. https://codereview.appspot.com/8647044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Add the command \offset to LilyPond (issue 8647044)
There's one thing that puzzles me. Current syntax is \offset property offset-value grob-name I understand that grob-name is at the end because it's optional, and we want to omit it when we're using \offset as a tweak. However, i find this syntax awkward. Since David K's change that allowed to use dot-separated list for specyfying grobs "together" with properties, couldn't we process both the property and grobname as one argument, and therefore keep the usual order? In other words, what about syntax like this: \offset grob-property-path offset-value where grob-property-path would be either Grob.property (when using \offset as an override) or just property (when using it as a tweak)? best, Janek BTW, nice comments! https://codereview.appspot.com/8647044/diff/1/input/regression/offsets.ly File input/regression/offsets.ly (right): https://codereview.appspot.com/8647044/diff/1/input/regression/offsets.ly#newcode11 input/regression/offsets.ly:11: generally, i'd make the regtest shorter. https://codereview.appspot.com/8647044/diff/1/input/regression/offsets.ly#newcode40 input/regression/offsets.ly:40: 1-\offset #'positions #'(-2 . 2) \arpeggio for example, one instance of arpeggio should suffice here. https://codereview.appspot.com/8647044/diff/1/input/regression/offsets.ly#newcode50 input/regression/offsets.ly:50: \once \offset #'X-offset #0.5 DynamicText similarly here https://codereview.appspot.com/8647044/diff/1/input/regression/offsets.ly#newcode57 input/regression/offsets.ly:57: c-\offset #'padding #1.5 \f ditto https://codereview.appspot.com/8647044/diff/1/input/regression/offsets.ly#newcode102 input/regression/offsets.ly:102: \once \offset #'Y-offset #2 BreathingSign ditto https://codereview.appspot.com/8647044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
