Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Volta renders wrong #22

Closed
kimo-k opened this issue Oct 28, 2019 · 2 comments
Closed

Volta renders wrong #22

kimo-k opened this issue Oct 28, 2019 · 2 comments

Comments

@kimo-k
Copy link

kimo-k commented Oct 28, 2019

> x %>% volta(2,c(e1,e2))
<Musical phrase>
\repeat volta 3 { <c\5>4 <e\4 c'\3 g'\2>4 <e\4 c'\3 g'\2>2 | }
\alternative {
  { <a\5>1 <b\5>1 | }
}

Looks like the extra set of brackets in \alternative are causing the volta to render improperly

@leonawicz
Copy link
Owner

Ah, okay. See the help docs for volta. endings takes a list type object. list(e1,e2) vs. c(e1,e2).

> volta(p("c ec'g' ec'g'", "4 4 2", "5 432 432"), n = 2, endings = list(p("a", 1, 5), p("b", 1, 5)))

The extra curly braces are proper: http://lilypond.org/doc/v2.18/Documentation/notation/long-repeats

Using c worked in this case before (unintended) but no longer will and isn't meant to. In general phrase objects always needed to be listed with list to avoid coercing them to strings. Previously, using c for basic concatenation fell back on the default method of coercing them to simple character strings (an atomic vector, unlike a list). That just happened to be what this function was going to do to those endings phrases anyway so it slipped through.

Now that a custom method has been written for c for the phrase class (it binds them together into a longer phrase object), you have to use the argument as intended to make a list of that object type. In R help docs list means list type and not an atomic vector like character or numeric. The addition of the c.phrase method is one of those breaking changes that was bound to hit some users. Probably one of the last ones there will be in the package that can have such sweeping consequences. Really, this function should have rejected the non-list before, but didn't.

These repeat functions will probably get some refining later. For example, I may remove the aggressive interpretation of basic strings as potentially valid phrases and simply require that a legit phrase object be provided. It's convenient when I just want to say "r1" but I may be giving these repeat functions a bit too much power there. In other places in the package I have been limiting which functions eagerly try to turn character to a tabr class object. Tending toward reserving that eagerness turning a simple class into a more complex one for functions that are explicitly about class coercion like as_noteworthy and a handful of others.

@kimo-k
Copy link
Author

kimo-k commented Oct 29, 2019

Right, I understand. Funny, I forgot how voltas work in ly.
I'm excited to see the potential of these new class methods.

So far I've been doing straight transcription, but soon I may start using tabr as an educational tool, using more of the analytic capabilities.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants