Re: quarter tones in tablature (issue 272320043 by address@hidden)

[dak]

On 2015/11/08 20:19:16, thomasmorley651 wrote:
> On 2015/11/08 14:22:26, dak wrote:


> >
https://codereview.appspot.com/272320043/diff/1/scm/translation-functions.scm
> > File scm/translation-functions.scm (right):
> >
> >

https://codereview.appspot.com/272320043/diff/1/scm/translation-functions.scm#newcode237
> > scm/translation-functions.scm:237: ((determine-frets #:optional
> > (support-non-integer-fret? #f))
> > That's pretty bad.  determine-frets was a public function
previously, changing
> > it to a hook it to a function generator is incompatible.

> I don't understand. Why incompatible? At least it survived our
regtests and my
> local testings.

If someone uses it, for example as a component in his own
fret-determining function, it will behave differently from before.  It
is an exported function so people might have used it.

> > support-non-integer-fret belongs into a context property, and
determine-fret
> > gets passed a context anyway, so that's quite straightforward to do.
 No need
> to
> > change the call interface to determine-frets.

> I could create a follow up, introducing "support-non-integer-fret?" as
a
> context-property and change determine-fret-code accordingly.
> I think that's your suggestion.

Yes.  Its name should be chosen to fit with existing properties
(question marks are likely a killer in property names anyway), but
that's the idea.  It might also be an idea to make this a property
valid-fret-predicate and set it index? by default (and one could
override with number? instead).  Though one would need to check how this
combines with instruments with irregular fret arrangement since those
have different valid frets on each string and I think we also support
that in some manner.

https://codereview.appspot.com/272320043/