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

[dak]

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)
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.

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.

It doesn't matter as much whether it would be more in line with other
functions (though it is of course a consideration), but as written
currently it is not even in line with the actual function argument name.

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