ChordPro / chordpro

Reference implementation of the ChordPro standard for musical lead sheets.
Other
310 stars 51 forks source link

{define ... diagram ...} values not being aggregated? #387

Closed gwyndaf closed 2 months ago

gwyndaf commented 3 months ago

Curious issue (or user error) when trying to control diagram display...

{title: Alt. chord diagrams}
#
{define: F† copyall F}
{define: Bb† copyall Bb}
{define: F diagram off}
{define: Bb diagram off}
{define-guitar: F† copy F@8}
{define-guitar: Ab copy Ab@4}
{define-guitar: Eb copy Eb@6}
{define-guitar: Bb† copy Bb@6}
{define-guitar: F diagram on}
{define-guitar: Bb diagram on}

[F†]  [Ab]  [Eb]  [Bb†]

[F] [Bb]  [F]
[F] [Am]  [Bb]

Results in fewer diagrams than expected: Screenshot from 2024-07-03 13-33-50

I expected (hoped) that I'd get the F† and B♭† diagrams shown first, followed those for standard F and B♭ shapes. If I omit all {define ... diagram ... } lines, results are as expected: Screenshot from 2024-07-03 13-38-18

It seems like the diagram parameter in the 2nd applicable {define} is maybe being ignored. I'd expected that the {define-guitar: diagram on} directive would replace the default (off) value for guitar.

My overall aim is to show alternative diagrams for instruments where they're available (guitar, ukulele), and can achieve that with no {define ... diagram ... } directives. But, I also want diagrams displayed in the order they first occur so, if no F† is defined (mandolin, banjo), the default F diagram is still displayed first. That is, the F† gets displayed first as F, and the later F diagram gets suppressed.

As usual, this is a small and pedantic presentational issue, but maybe worth flagging. Or maybe there's a better way.

sciurius commented 3 months ago

Good catch. It's a bug. Setting diagram "on" is ignored since it is assumed on by default.

In the mean time you could try:

{define: F† copyall F}
{define: Bb† copyall Bb}
{define-guitar!: F diagram off}
{define-guitar!: Bb diagram off}
{define-guitar: F† copy F@8}
{define-guitar: Ab copy Ab@4}
{define-guitar: Eb copy Eb@6}
{define-guitar: Bb† copy Bb@6}
gwyndaf commented 3 months ago

Thanks for the workaround, Johan. I can see that not approach working for this snippet, but the real-world case involves conditional definitions for guitar, ukulele in three tunings and guitalele, so not any one of those could match with others, and more logic than my brain can currently process! It's not urgent, so I'll probably just wait for the fix. :)

sciurius commented 3 months ago

And this:

{meta-guitar showf on}
{meta-ukulele showf off}
{define F display %{showf.1}}

I haven't tried but you get the idea.

gwyndaf commented 3 months ago

Nice one, thanks Johan. I've not really exploited the power of {meta} within songs, so this is a useful exercise.

Turns out I need to use index -1 to access the right value, because showalt could have 1 or 2 values, depending on whether or not it's been redefined for a specific instrument or just the default, but this seems to work:

{meta: showalt off}
{meta-guitar: showalt on}
{meta-ukulele: showalt on}
...
{define: F copyall F diagram %{showalt.-1} }
{define: Bb copyall Bb diagram %{showalt.-1} }
sciurius commented 2 months ago

Addressing your original problem:

{define-guitar: F diagram on}

This defines a new chord with name F and property diagram set to on (which it is by default). Note the word 'new'. There are no strings defined, hence there will be no diagram shown. Likewise:

{define: F diagram off}

defines a new chord with name F without strings so no diagram. Even simpler:

{define: F}

So what you must do is copy the properties:

{define: F copy F diagram off}
{define-guitar: F copy F diagram on}

The first directive will effectively set the diagram property off, while keeping the rest of the chord properties. The second directive will effectively set the diagram property on, while keeping the rest of the chord properties.

Does this make sense?

gwyndaf commented 2 months ago

Thanks Johan. I'd maybe assumed wrongly that the diagram property can be applied (on or off) for existing chords, but that explanation makes sense.

Your solution works nicely. I now realise that if my first definition is
{define: F diagram off}, I can't then use
{define-guitar: F copy F diagram on}
because it'll copy an 'empty' definition, so has no strings to show, even if diagram is on. So I need a copy F in the first one as well.

That seems to be an issue of my understanding, so maybe there's no actual bug.

sciurius commented 2 months ago

The main point is that {define F ...} always defines a new chord, just as the name implies.