Re: Function to display the rhythmic location of a grob (issue 197690044 by address@hidden)

[david . nalesnik]

https://codereview.appspot.com/197690044/diff/20001/scm/output-lib.scm
File scm/output-lib.scm (right):

https://codereview.appspot.com/197690044/diff/20001/scm/output-lib.scm#newcode29
scm/output-lib.scm:29: (define-public (grob::rhythmic-location grob)
On 2015/02/22 07:59:40, dak wrote:
> The function is misnamed since rhythmic-location is an established
term in the
> code base with a number of operators that will not work on the result
from this
> function.  A "rhythmic location" is just the cadr of what you return
here.

> So you should split this function into two, one for returning the
global
> timestep (there probably is an established term for that, but if not,
grob::when
> would be an obvious candidate), and one for returning the rhythmic
location.

> There is likely some duplication of code then, but since it will be
the
> exception rather than rule that one needs both values, the resulting
code will
> likely be more rather than less efficient for the typical use case.

Done.

https://codereview.appspot.com/197690044/diff/20001/scm/output-lib.scm#newcode30
scm/output-lib.scm:30: "Return the rhythmic position of grob @var{g} as
a list.  The
On 2015/02/22 03:01:04, pwm wrote:
> Do you mean @var{grob} ?  That would be more in line with other
functions in
> this file.

Done.

https://codereview.appspot.com/197690044/diff/20001/scm/output-lib.scm#newcode42
scm/output-lib.scm:42: (list when rl))
On 2015/02/22 03:01:04, pwm wrote:
> Instead of this let block I'd probably just do:

> (list (ly:grob-property col 'when)
>    (ly:grob-property col 'rhythmic-location))

> But either way is fine with me.

Done--in a way!

https://codereview.appspot.com/197690044/