"m...@mikesolomon.org" <m...@mikesolomon.org> writes:
> On 20 févr. 2013, at 11:35, d...@gnu.org wrote:
>
>> Is there a particular reason you ignored this comment, judging from the
>> commit you pushed to staging on your own initiative?
>
> I changed the name of the function twice in two successive patch sets
> to better reflect what is going on.
It's currently grob::all-heights-from-stencil. That suggests different
types of height. You could call this a review error of mine: I've
simply gotten too exhausted to look again closely.
The saving grace is that there is actually a comment for the definition
now.
;; Using this as a callback for a grob's Y-extent promises
;; that the grob's stencil does not depend on line-spacing.
;; We use this promise to figure the space required by Clefs
;; and such at the note-spacing stage.
(define-public grob::all-heights-from-stencil
(ly:make-unpure-pure-container
ly:grob::stencil-height
(lambda (grob start end) (ly:grob::stencil-height grob))))
Now we all know that comments are a scarce resource and should see as
much use of possible. So how about following through with my suggestion
and creating a creating function and documenting that?
(define-public (constant-grob-callback callback)
"Wraps a grob @var{callback} into an unpure-pure-container
delivering the same results for pure and unpure calculations. This
means that the result of the callback must not depend on the line
spacing."
(ly:make-unpure-pure-container
callback
(lambda (grob start end) (callback grob))))
Then look-and-behold, we can define
(define-public grob::constant-height-from-stencil
(constant-grob-callback grob::stencil-height))
Now look-and-behold, we can get along without a separate comment for
each such definition since constant-grob-callback already tells us all
that we need to know (by virtue of its comment).
Is that where we can stop? Actually, no. If we take a look at
ly:make-unpure-pure-container, we see
LY_DEFINE (ly_make_unpure_pure_container, "ly:make-unpure-pure-container",
1, 1, 0, (SCM unpure, SCM pure),
"Make an unpure-pure container. @var{unpure} should be an unpure"
" expression, and @var{pure} should be a pure expression. If
@var{pure}"
" is ommitted, the value of @var{unpure} will be used twice.")
Of course, "an unpure expression" and "a pure expression" is pure
gobbledygook not even having any tangible meaning, since an expression
is not pure or unpure but rather gets used in a particular context.
Anyway, we already _have_ an API for using the same value in an
unpure-pure-container twice. Why don't we _use_ it? The superficial
answer is that pure and unpure callbacks are called in a different
manner: the unpure callback gets more arguments (or was that the other
way round? Since the unpure/pure naming is not really related to what
this does, I can't remember). So in the case of a callback, we can't
just reuse the callback like we do with fixed values.
Right? Wrong. All we need to do is changing the line
if (pure == SCM_UNDEFINED)
pure = unpure;
in the definition of ly_make_unpure_pure_container into
if (pure == SCM_UNDEFINED && !scm_is_procedure (unpure))
pure = unpure;
and then let ly_unpure_pure_container_unpure_part and/or
ly_unpure_pure_container_pure_part deal with the situation of having
only one procedure with a single argument stored by concocting a
three-argument procedure on-the-fly (or change all callers of the
callback procedure to deal with the one-argument situation for half-full
containers. That makes the change less encapsulated by making external
stuff responsible for dealing with half-full containers, but may be
conceptually simpler).
And lo-and-behold, all we need to write as a callback _directly_ is
(ly:make-unpure-pure-container ly:grob::stencil-height)
and that's it. Now apart from the drawback that either your
ly:grob::all-heights-from-stencil or the purported
constant-grob-callback functions are much better documented than
ly:make-unpure-pure-container, this is quite more readable and obvious
because it works the same way with callbacks as with fixed values:
specify only the pure and the unpure will follow suit.
We have the advantage that we don't have to look for the best name for
an additional function because the existing functions can perfectly well
be made to do the job.
Yes, I should have thought of that previously, but it is really
exhausting to do this kind of review.
And frankly, I still don't understand why this is not equivalent to
ly:grob::stencil-height anyway. But at least this is not opening yet
another can of worms but just making do with the already open ones.
--
David Kastrup
_______________________________________________
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
