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

💡 RFC: Support new_key: directive #53

Closed
1 task done
isaiahdahl opened this issue Dec 7, 2021 · 0 comments · Fixed by #54
Closed
1 task done

💡 RFC: Support new_key: directive #53

isaiahdahl opened this issue Dec 7, 2021 · 0 comments · Fixed by #54
Assignees
Milestone

Comments

@isaiahdahl
Copy link

isaiahdahl commented Dec 7, 2021

Background & Motivation

After playing around with the handy new transpose I have realized that I made a mistake in my interpretation of the transpose directive and what we use in our system for the nk:{key} directive.

Our system uses the new key directive, not as a way to actually transpose the chords to a different key. But to power the exclusions for that section of chords. So what we actually need is a directive with a totally separate purpose.

This directive would only assign the line key, but it wouldn't do any transposing. This way a person could write a chart, chord for chord, matching exactly what they hear and just make note of the key change. Using the transpose directive, you'd have to remain writing in the first key, but transposed. Ie you hear a D but you write a C because the song is in C and this part of the song is transposed to D.

They both have purposes. But I confused the two directives as achieving the same thing. I was totally wrong on that.

When the song itself changes key, and we transpose the whole song. We also need to transpose the key in the new_key: directive.

For example take this song

{t:song}
{key: C}
[C] Let it be
{nk: D}
[D] Let it be

The song has a key change, but the chords are taken as they are written. If you wanted to achieve this using the transpose directive you would write:

{t:song}
{key: C}
[C] Let it be
{transpose: D}
[C] Let it be

Now if we take the first chordpro example and change the whole key of the song programmatically #22 the new chordpro should be:

{t:song}
{key: D}
[D] Let it be
{nk: E}
[E] Let it be

Notice how the new_key directive also changed? In theory this should also happen with the transpose directive but that might be a task we cross of when we tackle #22.

The point is, we gotta figure out how to split up the roles of setting a line_key, and transposing.

Proposed Solution

I am thinking that a line should have a

transpose_key and a line_key. the line_key is what powers the exclusions for enharmonic rules purposes. and the transpose_key determines the transpose steps.

I think this will require some further thought on how these two directives work together.

Detailed Design

No response

Help make it happen!

  • I am willing to submit a PR to implement this change.
@isaiahdahl isaiahdahl added this to the PC Editor MVP milestone Dec 7, 2021
isaiahdahl added a commit that referenced this issue Dec 7, 2021
… based on the line key without transposing
@isaiahdahl isaiahdahl self-assigned this Dec 8, 2021
martijnversluis added a commit that referenced this issue Jan 13, 2022
resolves #53, add nk directive to so that we can normalize enharmonics…
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

Successfully merging a pull request may close this issue.

1 participant