markup in chord definition ignored (related to truesf?)

[gwyndaf]

New issue occurring with dev releases 026 through to 028

I have a chord defined with:

{title: Chord markup test}
{define: Bm7 copy Bm7 display "Bm<span size=80% rise=30%>7</span>"}
Just a [Bm7]

but the Bm7 doesn't display superscript 7 as in earlier versions. Same behaviour across a larger test sample.

It seems config-related, and I think I've narrowed it down: the problem seems caused by having truesf set to true and goes away if set to false.

[sciurius]

There is a limit to the simple heuristics to find out whether the character b must or must not be replaced by a flat symbol. I'll see if it can be further improved but currently I'm inclined to drop truesf in favour of \u266d. (Note that a \u266d and ♭ will be replaced by a chordprosymbol if the current font doesn't seem to have the glyphs.)

[gwyndaf]

It seems to be a regression in recent development versions. The release version still handles markup in chord names as expected.

It doesn't appear to relate directly to chords with sharps/flats, simply that the truesf setting seems somehow related, and affects (in git versions) whether or not the markup issue arises.

[sciurius]

Sorry, I was unclear... truesf means replacing appropriate b in chord names with a true flat symbol. This went fine, until markup was added to chord names... Guess what happens with <span color="blue">Bb</span>. So I improved the heuristics to only look at the chord without markup. So far so good, until you get something like Bm<sup>7</sup>. Since the chord Bm7 is broken the algorithm gets confused and the markup gets lost. The result is Bm7 instead of Bm⁷.

[gwyndaf]

Ah that makes sense. I hit that issue with using blue in {define} markup a while back and thought that was the reason: ended up using teal instead! Now I see how it relates.

I noticed that {define A copy A display "<span size=85%>Am</span>"} works without problems, so can see the issue is where markup splits the full chord name.

Might this offer a way through it: search/replace #/b only outside markup, i.e. after > (or start of string) and before < (or end of string)?

That assumes that no C# or b5 or /G# type of elements in chord names would be split by markup (but could each carry markup). Perhaps that's a safe assumption?

I don't know perl but maybe that approach could be achieved with some regex lookbehind and lookahead. Will try to figure out what might work...

[sciurius]

Might this offer a way through it: search/replace #/b only outside markup,

Yes, that is what 6.000_029 does. Can you verify?

[gwyndaf]

That's fantastic, thanks.

I updated my test to include some potentially troublesome chords and colour names, and all seems well:

{title: Chord markup test}
{define A copy A display "<span size=85%>Am</span>"}
{define: Bm7 copy Bm7 display "<span size=85%>Bm<span size=80% rise=30% foreground=#00Bb00>7</span></span>"}
{define: "Ebm7b5/Bb" copy Ebm7b5 display "<b>Ebm</b><sup>7</sup><span foreground=blue>b5</span><span size=80% foreground=darkslateblue>/Bb</span>"}

Just a [Bm7] [A] [Ebm7b5/Bb]

now gives (as expected) Chord_markup (guitar) git.pdf I'll keep testing with a bigger bunch of songbooks, but this looks great.

It would be a pity to lose the truesf feature, as it seems a useful and easy way of getting nice output while keeping source files backward-compatible and portable (as durable assets), and simple to input for users who don't know Unicode. But I appreciate it might be a PITA for coding! :)

[sciurius]

Thanks. Keep testing!

[gwyndaf]

Mostly error-free across a batch of songbooks, and visually OK from brief examination.

However, {chord: D frets 2 2 2 4 3 4} seems to output the chord diagram as expected but now (from dev v029) gives an error:

Use of uninitialized value in string ne at /media/nfs/Shared/Music/Technology/ChordPro/git/chordpro/script/../lib/App/Music/ChordPro/Chords/Parser.pm line 976.

As {chord: } and {define: } directives are similar, it might perhaps be related.

Message doesn't arise when running with -X so there's clearly something now troublesome in my configs. Still trying to narrow that down...

[gwyndaf]

Found it: it is indeed also related to settings.truesf. Message arises when true, but not when false. Output as expected in both cases.

Additional observation: with truesf=true, message also arises with {chord: D copy D} but not with {chord: D display D}. I'm not sure what to infer from that, but it might give you some clues.

[sciurius]

Please try 031.

[sciurius]

I was playing with this, but I think I played too much... Campania.pdf

[gwyndaf]

Message now seems to have gone (in both test sample and batch of books).

Campania looks interesting, though it might take a while to get my head around it!

I haven't really used Roman or Nashville chords in ChordPro, so I don't how they're handled, even for basic triads or simple variants, without even getting into the examples you've got :)

[sciurius]

I haven't really used Roman or Nashville either, I added limited support on demand, but I'm not sure anyone is even using it.

The interesting part of Campania is that it does the formatting itself. See https://github.com/MarcSabatella/Campania. For ChordPro this is an interesting complication since it would mean that for this font no formatting whatsoever must be done on how the chords are displayed.

A similar issue with the MuseJazzText font, that has glyphs for chord qualities but this must be handled with glyphs in the display string. E.g.

      { "name" : "G#m7",   "copy" : "G#m7",   "display" : "G\ue10c\ue181\ue197" },

which yields

[gwyndaf]

Both fonts seem like interesting approaches. As you say, seems like they'd require custom instrument configs tied to the output font, but at least the possibility seems feasible.

I generate my instrument configs from spreadsheets that hold the master data, so it wouldn't be too difficult to add and combine columns for different display settings, to generate multiple instrument/font configs of, e.g. guitar-musejazz.json.

A slightly related question: are there metadata variables for chord root, qualifier and extension? I vaguely remember seeing mention of something for root, but not sure about the others.

If those exist, they might offer an efficient way to specify chord display settings in configs or {define: } directives, and potentially more flexibility when transposing.

[sciurius]

In chord display strings a number of additional meta variables are available, but these are not all well-defined and may be present or not depending on the situation.

In general, it is safe to use %{name} (the original name, e.g. Am7), %{root} (the root, e.g. A), %{qual} (qualifier, e.g. m) and %{ext} (the rest, e.g 7). %{root}%{qual}%{ext} always yields the complete chord name. There is also %{qual_canon} that holds normalized qualifiers. In the case of Am7 %{qual} will be m and %{qual_canon} will be -.

Note that these can only be used from the config. So you could write something like

  { "name" : "A",   ..., "display" : "%{root}%{qual}%{ext|<sup>%{}</sup>}"},
  { "name" : "Am",  ..., "display" : "%{root}%{qual}%{ext|<sup>%{}</sup>}"},
  { "name" : "Am7", ..., "display" : "%{root}%{qual}%{ext|<sup>%{}</sup>}"},

[gwyndaf]

Thank you.

I tried using %{root} in {define: display } with no success, but it seems like that's because they don't work in that context.

Still, it offers useful possibilities for rationalising my instrument configs.

My main headache is chords like 7sus4, which I display as C<sup>7</sup>sus<sup>4</sup> on the rationale of using regular type for the basic major or modified triad (m, dim, aug, sus), and superscript for additional tones (7, maj7, add9 etc.).

I don't know if there's any standard for such things, but seems like Chordpro treats dim, aug and sus as extensions (not qualifiers), so I might have to stick with my less elegant approach for now!

[sciurius]

I tried using %{root} in {define: display }

Remember that %{...} substitutions are made when processing the line. At the time the define is processed the chord is not yet 'active' (can't think of a better term atm).

What should work (in 6.000_032 ☺) is

{define ... display "\%{root}\%{qual}\%ext"}

Now the display string is correctly defined as "%{root}%{qual}%ext" and will later get the appropriate substitution values.

ChordPro treats these as qualities: - min m maj + aug 0 o dim. Most music notation methods seem to agree on that. Suspension is generally not considered a quality, hence C⁷sus4. It is more like add9 etc.

[gwyndaf]

Thanks. I might revised my approach to sus, which will also simplify things!

Chord metadata works nicely, but seems to create issues for transposition:

{title: Chord metadata test}
{define: Am7 copy Am7 display "\%{root}\%{qual}\%{ext|<sup>\%{}</sup>}" }
{define Bm7 copy Bm7 display "<span foreground=blue>\%{name}</span>" }
{define D#m7 copy D#m7 display "D#m7"}

[Am7] [Bm7] [C#m7] [D#m7]

{transpose: -2}
[Am7] [Bm7] [C#m7] [D#m7]

results in metadata.pdf

It seems like any custom chord with a display value doesn't get transposed, regardless of whether it uses chord metadata or fixed values.

I'm not sure I fully understand the process, but suspect it's something to do with how/when the metadata values get substituted, in relation to when the chords gets transposed.

[gwyndaf]

Maybe it's one to defer if it's going to cause problems?

I'm interested in the potential of using chord metadata in definitions, but more as a future possibility rather than any immediate need.

[sciurius]

It is definitely not ready for prime time, mostly related to issues with copy.

But it is good to give it some thinking.

{define: Am copy Am display "\%{root}\%{qual}\%{ext|<sup>\%{}</sup>}" }

What would you expect %{root} to be? Easy isn't it: A. Ok.

{define: C/6 copy Am display "\%{root}\%{qual}\%{ext|<sup>\%{}</sup>}\%{bass|/\%{}}" }

You would expect root to be C. It isn't. At least, not now. I can fix this by parsing the to-be-defined name. But then

{define: C* copy Am display "\%{root}\%{qual}\%{ext|<sup>\%{}</sup>}\%{bass|/\%{}}" }

C* cannot be parsed since it is not a valid chord name. Still you would expect root to be C, not A. Moreover, you would expect it to transpose +2 to D*.

To be continued tomorrow.

[gwyndaf]

Well, you've pre-empted my curiosity about how the bass gets parsed in slash chords! Although considering that question had me wondering if surplus trailing characters get captured, which is maybe still relevant for the C* question.

I can see how copy creates ambiguity, i.e. whether it's the defined or the copied chord name that gets parsed. I suppose it's most intuitive if it is the chord being defined.

Which then raises interesting questions: 1) If the copied chord has display already defined in the config, should that continue to be the default (as currently) unless display is set in {define}? I think maybe it should, for compatibility with current behaviour. 2) In your last, transposition, example, would the chord name being copied also be transposed, in that case to Bm? Although that seems logical, it's possible that the Bm voicing might not be a good substitute for D, even if Am was for C. But I suppose such cases need some human/musical judgement, and show the limits of how far it can be done automatically.

I suppose this approach could potentially allow a default chord display format to be specified once, and remove the need to set it for every chord, whether in config or {define} :)

[gwyndaf]

... although maybe transposition should operate on the defined chord only if display includes `%{root}. Otherwise it could produce unexpected results with chords that are explicitly defined.

[gwyndaf]

A bit of testing on release 6.010 for diagram display....

{define: Am/C copy Am  }
{define: C6 copy Am   }
Slash chord [Am/C] and root position chord [Am]
Alternate chord [C6]

Displays a single diagram, named 'Am', so it looks like duplicate diagrams are now automatically suppressed.

However,

{define: Am/C copy Am display "Am (Am/C)"  }
{define: C6 copy Am display "Am (C6)"  }
Slash chord [Am/C] and root position chord [Am]
Alternate chord [C6]

also displays only a single chord diagram, named 'Am', with 'Am' for all song body chords. Expected result would be three different chord diagrams names (same grid, different names), yes? Test.pdf

Another interesting observation: with an explicit {define: Am}, that's used as the copy source for the other two chords, even though defined after them. Possibly that's to with the order in which it's processed:

{define: Am/C copy Am display "Am (Am/C)"  diagram none}
{define: C6 copy Am display "Am (C6)"  diagram none}
{define: Am copy Am display "Am (orig)" diagram none}
Slash chord [Am/C] and root position chord [Am]
Alternate chord [C6]

Test2.pdf

diagramdoesn't seem to have any effect, although I'm guessing at the syntax, or if it involves any global config setting.

A thought: although my original thinking was to automatically suppress chord that are fully duplicated (grid and name), it's more useful for that to be possible than to be automatic.

Since the discussion then got onto the diagram option, that seems a far more flexible approach than hiding duplicates automatically, and it might create unnecessary complexity to have both.

[sciurius]

I pushed out 6.010 to consolidate a number of things and to have a solid playground for further experimenting. No changes w.r.t. "copy" yet.

[gwyndaf]

OK, that makes sense: I'll hold off testing that functionality for now.

Just flagging that some chords seem to get suppressed unexpectedly in 6.010, but they do indeed use {define copy}.

diagram false seems to work nicely in {define} for explicit chord definitions. :)

[sciurius]

Yes, currently {define copy is a bit of a mess. I'm trying to sort it out.

[sciurius]

I made some changes for define copy. Please try 6.010_006.

Also, diagram can now take a colour name/code.

[gwyndaf]

Great. Initial tests on simple samples now output as expected.

My initial scenario of Am, Am/C, C6. all redefined with copy Am now outputs three identical Am chords (when using my own config, which includes display "Am"). As expected with previous behaviour, except now I can disable the redundant chords successfully with diagram false. Working nicely so far and seems safer, more effective and flexible than automatic suppression of duplicates.

diagram #888888 nicely gives a grey diagram, including chord name. If I colour markup the chord name with display <span foreground=blue>Am</span>, that's reflected in the diagram name (but grid remains grey). That's all as expected, so looking good so far.

Testing to continue on more complex real-life examples...

[gwyndaf]

Mostly looking good in real examples, but I found unexpected behaviour with conditional (or combined?) {define} directives:

{title: Conditional define and diagram for %{instrument.type}}
{define-keyboard!: G/B copy G diagram #00FF00}
{define-guitar: G/B copy G frets x 2 0 0 0 3 display "G/B" diagram #FF0000}
The chord [G/B]

This outputs a green G diagram if instrument.type=ukulele and red G/B if it's guitar, both as expected.

However, if I replace the first {define} with

{define-keyboard!: G/B copy G diagram false}

this gives no diagram for ukulele (as expected), but also no diagram for guitar - unexpected: it's hidden when I expect it to be red.

Reversing those diagram values works as expected, i.e. green diagram for ukulele and none for guitar.

It looks like something to do with how successive, matching {define} directives get handled. I generally write mine on the assumption that only the last matching {define-} for the specified instrument and chord is used. But it looks like maybe they're combined, with each subsequent {define} updating relevant values from previous (or config).

If that's the case, it seems like an initial diagram value of false isn't being updated with subsequent values, whether true, default or colour value.

[gwyndaf]

OK, I think I've now figured out my understanding of how {define} values 'inherit', which is that they don't! It's down to copy I think.

{define Am7 copy Am7 display "Am7general"}
{define-guitar Am7 frets x x 2 0 1 0}
This is [Am7]

If I run with -X (default guitar config), I get a simple Am7: old-school, using only information from {define-guitar} (e.g. muted A and E strings), without using config data.

If I change it to {define-guitar Am7 copy Am7 frets x x 2 0 1 0} I get the same diagram, but it's named Am7general

From that, I infer that copy copies from the prevailing definition of the chord. In simple cases, that'll be straight from the config, but if earlier {define} directives apply to the instrument/chord, those values will be copied, and updated where the latest {define} redefines them.

Is that correct? Sorry for not understanding this originally, but I can test more clearly now I think I understand how it should work.

As far as I can tell, each {define} parameter is redefined by wholly replacing the previous value, yes?

{define: Dm7 copy Dm7 display "<span foreground=red>Dm7</span>"}
{define: Dm7 copy Dm7 display "<span size=150%>Dm7</span>"}
Show [Dm7]

gives me a large (150%) black display name for Dm7, not a red one, i.e. the display value has been fully replaced, not the previous styling augmented. That seems a sane approach!

[gwyndaf]

Slightly related question: is a chord's display name available as metadata, e.g. %{displayname} to represent the chord's current define.display setting (including markup)?

Potential application is chords being redefined as smaller or a different colour (e.g. passing or other special chords). Being able to access the existing display value avoids redefining any existing (e.g. <sup>) markup.

For instance , {define Csus4 copy Csus4 display <small>\%{displayname}</small>} instead of {define Csus4 copy Csus4 display <small>C</sup>sus4</sup></small>}. And is potentially more adaptable for transposition.

[sciurius]

Thanks for the feedback. As you have noticed, it boils down to the case where you define a chord that was defined earlier. For example:

{define C/6 copy Am ...}
{define C/6 copy Am ...}

The second define is picking up things from the earlier define. I'm currently trying to tackle that. And, indeed, it is copy, not inheritance.

But I keep wondering what we would expect copy to do w.r.t. things like root, qual etc. A define can be anything, not just something that resembles a chord. Think {define "|"}.

In cases where it resembles a chord we can set the parts accordingly. In {define C/6 copy Am ...} I'd expect root to be C, qual and ext empty, and bass to be 6. I'd even expect it to transpose +2 to D/6. It should copy frets and fingers. But should it copy the display string? I don't think so. Your suggestion for %{displayname} is, however. intriguing...

I try to work from that.

It would be a great help if you can turn your test cases in small .cho files similar to the ones in the t/cho folder.

[gwyndaf]

OK. I can see that using a) only the last {define} that matches the chord and instrument/condition is simpler in use than b) combining a chain of multiple {define copy}. b) might allow slightly more efficient chord definitions in complex cases, but a) avoids confusion. TBH I'd previously assumed a) anyway, and that the copy source will be the instrument config, not a previous {define}.

I suspect a) would also avoid/reduce confusion when using transposition or chord metadata like root, which seem to pose enough questions already!

The case of 'non-musical' chords, like | or N.C. connects with two questions I was pondering:

1) Whether there should be a way of flagging {define} chords as non-transposable. 2) If any 'trailing' data in the chord name is parsed and captured in metadata,

1) As I think you suggest, it would be great if transpose could operate intelligently on {define}, e.g. {define C/6 copy Am}would transpose +2 to {define D/6 copy Bm}. However, if the display value isn't also copied, it would output with a display name of Am but use Bm voicing, if the config includes display values, e.g. brandtroemer

But there might still be cases where the {define} is for a distinctive voicing that works for that chord in that key, and doesn't make musical/ergonomic sense when transposed. So, although transposing {define} seems a great idea, it's maybe a bit risky unless there's also a way of 'protecting' some definitions from transpose. And if that's the case, that approach might also suit non-musical 'chords' like | or N.C.

I suspect that if {define} contains explicit values for frets or fingers (even if it includes copy), that might be enough to indicate that it shouldn't be transposed, although it's not an approach that would deal with | 'chords', and is maybe too obscure/subtle to be clear in use.

2) If chords are parsed to capture any trailing characters, beyond the understood chord, e.g. with [C7sus4*], the trailing (best name I've got right now) would be *. Similarly, if a [|] chord is parsed and finds no musically understandable chord form, it could return null for root (and bypass transposing?) and yet | for trailing. However, I'm not sure what are the implications for name and how it's handled: whether name would be null or | for [|]

Regarding how copy interacts with root, qual etc. more generally, that maybe depends on the order in which song chords, transposition and definitions are handled. My working assumption: 1) (If applicable) transpose all chord names used in song. That might also include chord names given explicitly in {define} if that feature happens. 2) Copy from config any chord data referenced by {define copy}, expanding any metadata references within the config, e.g. a display value of %{root}%{qual}<sup>%{ext}</sup in the config would be expanded and copied as Bm<sup>7</sup>. 3) Update/replace any copied values with those specified in {define}, e.g. fingers 1 2 1 3 1 4. This is probably the point where metadata used within {define} gets evaluated. Based on current behaviour, I guess {define D6 copy Bm7} gives us name=D6, root=D, ext=6. If we've also copied a display value from the config (as per current behaviour), that probably shouldn't change those metadata values, but the name displayed would be Bm<sup>7</sup> (from copied display value. That would then imply if we have {define D6 copy Bm7 display "\%{root}\%{qual}<sup>\%{ext}</sup>" } the chord would display as D6 with superscript (not Bm7).

Sorry that's a bit long, just trying to share my thinking through as I try to understand the process.

I'll check the format of test samples you mention and try to get mine in line with those! :)

[gwyndaf]

define-tests1.zip

I hope is what you meant. I've run the samples to generate ChordPro output, in the .ref file (if as expected).

Note that cho022.ref is hand-edited, as current output isn't as expected: based on the other .ref files, it looks like all chords get a first {define}line, and those to be displayed get a second one. Not sure I completely understand the logic, but have sought to copy the effect!

[sciurius]

Thanks for your thoughts and reflections. It helps me a lot to get a clear(er) view.

This is my memory dump for now. I'll probably change my mind later ☺.

Without copy.

{define €}

This defines a new song chord, without diagram info. It doesn't look like a chord so it is not transposable, you'll get a warning if you try.

This construct is merely intended to silence "Unknown chord" warnings.

{define A*}

This defines a new song chord, A*, without a diagram. The chord name is parsed in relaxed mode, so it succeeds the 'looks like a chord' test.
name = A*, root = A, ext = *, and it is transposable.

{define A* ...frets...}

This defines a new song chord, A*, with a diagram. The chord name is parsed in relaxed mode, so it succeeds the 'looks like a chord' test.
name = A*, root = A, ext = *, and it is transposable.

{define Am ...}

This defines a new song chord, Am, with or without a diagram. The chord name is known and the current definition of the Am chord is shadowed in the song.
name = Am, root = A, qual = m, and it is transposable.

With copy.

{define A* copy Am}

This defines a new song chord, A*. The base chord must be known and a subset of its properties is copied to the new chord. The current definition of the base chord is then shadowed in the song.
name = A*, root = A, ext = *, and it is transposable.
The copied properties are base, frets, fingers, keys.
root, qual etc will get new values derived from the new chord name.
display and diagram are not copied.

Note that once A* is defined, it can be used as a source for a subsequent copy.

{define Am copy Am}

This defines a new song chord, Am. The receiver chord name and the sender chord name are known and a subset of the sender properties is copied to the new chord. The current definition of the Am chord is then shadowed in the song.
name = Am, root = A, qual = m, and it is transposable.
In this case display could be copied as well..?

Properties aka metadata

• There will always be name, with the name of the chord as used in the {define}.
• If a chord looks like a chord, there will be root, qual, ext, bass (some possibly empty).
• If a chord is known, same.
• If a chord is copied, there will be root, qual, ext, bass (some possibly empty) derived from the new name.
  base, frets, fingers, keys (some possibly empty) will be initially copied from the base chord unless any of them is specified in the {define}. The display string of the base chord will be available as base.display (or some other name).

[gwyndaf]

Thanks Johan.

Just to clarify "The chord name is parsed in relaxed mode." Does that mean temporarily adopting relaxed mode when parsing {define A* }, even if settings.chordnames = strict so - even in strict mode - [A*] would be accepted as a valid chord because it's been defined? Or, that {define A*} is parsed only when settings.chordname = relaxed?

I don't have a particular view, just wanted to understand clearly.

With {define copy}, I think it might be confusing if {define A* copy Am} doesn't copy display (and diagram) values, but {define Am7 copy Am} does copy those values. Perhaps it's clearest if both copy the same set of values (whether or not that includes display)?

Right now, I think current behaviour is to copy display values in both cases, yes? Personally, I find that useful because I can define Am@5 in my config with a display value of Am. That makes it easy to provide alternate voicings by simply {define Am copy Am@5}, so chords are still displayed as Am in song and diagrams, even if using different voicing.

I'm wondering what might be the advantages of not copying display values from the base chord, if they exist?

Similar thing with diagram I suppose: if the value's been set in the config file, why not copy it? In reality, I suspect this is something that might usually be set at song level.

However, here's a possible use case: configs for guitar-basic.json and guitar-advanced.json, with easy chords marked as diagram false in the advanced config. That then allows the same source songs to be prepared for both basic and advanced players, simply by switching config. (This might come close to the 'easy' chords functionality that I think is now deprecated).

Just my thoughts, but happy to adapt to whatever approach you go with :)

[sciurius]

Thanks for the feedback.

Does that mean temporarily adopting relaxed mode when parsing {define A* }, even if settings.chordnames = strict

Precisely.

I think it might be confusing if {define A* copy Am} doesn't copy display (and diagram) values, but {define Am7 copy Am} does

Assume you have in your config Am with display "A<sup>MI</sup>". When you say {define C/6 copy Am} you want to see C/6 and not A^MI. So I think best is to never copy the display string. If you are 100% sure you want it, you can write display "\%{orig.display}".

Diagram info (short for frets, fingers, base and keys and not to be confused with diagram) are copied provided they are not specified in the define. I can imagine it would be useful to change fingers only, e.g.

{define D copy D fingers x x x 2 4 3}

but this assumes the chord definitions are frets x x 0 2 3 2. So I think best is to not copy the diagram info when any of them are in the define.

As for diagram: this does not exist in the config. It is a pure song thing.

with easy chords marked as diagram false in the advanced config

Isn't that what diagrams.suppress is for?

guitar-advanced.json:

{ "include" : [ "guitar-basic.json" ],
  "diagrams" : {
    "suppress" : [ "append",
        "Am", "Em", ...
      ],
  }
}

[gwyndaf]

OK, that makes sense. I agree it's best if copy behaviour is consistent.

Not copying display seems sensible for using copy to define new chords by adapting existing ones (e.g. C6 from Am), and I guess that's the most common usage. My peculiar usage sometimes substitutes less 'scary' display names for beginners (even if musically less correct), but that now becomes simple with %{orig.display} :)

Agree that diagrams should work only on a per-song basis (not be in, or copied from, config). Good point about using diagrams.suppress in an advanced guitar config, which I'd overlooked.

[sciurius]

I've reworked the chord parsing and lookup and I think I've ironed out most wrinkles. I leave it to you to try 6.010_009 and find all the edge cases that I screwed up...

[gwyndaf]

Happy to test the edges!

Good news: I ran a batch of songbooks and no additional errors arose with _009. The rest is down to visual comparison, which I'm working through.

So far, main issue is some chord diagrams missing from the page. One cause seems to be {define copy} with only fingers redefined, e.g. {define: A7 copy A7 fingers 0 0 2 0 4 0 }. Doesn't seem to be an issue with a straight copy, or redefined frets. define_test001.cho.txt define_test001.ref.txt

Testing continues...

[sciurius]

{define copy} with only fingers redefined, e.g. {define: A7 copy A7 fingers 0 0 2 0 4 0 }

It is easy to make this work, but the problem is that e.g. redefining fingers relies on how the base chord defines its frets. If the base chord changes its frets, your fingers won't fit anymore. It's about control -- I control the frets and you control the fingers.

Well, caveat emptor. 6.010_010.

[gwyndaf]

Great: that now seems to work for redefined fingers only, so the diagrams now display, using frets data copied from config.

_010 also seems to deal with a similar issue, of frets (only) being redefined, but fingers details missing on diagrams (not copied from config). Why would I do that? {define E/B copy E frets x 2 2 1 0 0} - same fingering, just skipping a string.

One new error message with _010:

Maximum number of iterations exceeded at /media/data/Shared/Music/Technology/ChordPro/git/chordpro/script/../lib/App/Music/ChordPro/Chords/Parser.pm line 1012.

Arises at PDF generation, so --verbose doesn't identify the offending song. I'll aim to narrow it down, which might help pinpoint the cause.

[sciurius]

Yes, _010 copies the base_fret, frets, fingers and keys from the base chord provided they have not been specified in the {define}.

The max. num of iterations error comes from a %{xxx} substitution probably in a display string. See https://metacpan.org/pod/String::Interpolate::Named#maxiter. Did you use %{orig.display}"?

[sciurius]

There's one bug that I know of: %{chords} will only have the chords with diagrams.

BTW: What would you expect this to mean:

{transpose 2}
{define D frets x 0 0 2 3 2}

Before the rework, it defined an E chord...

[gwyndaf]

I can see {chords} might be tricky. Would [C*] and [C] be counted as one, or two, chords, for instance? Does our old friend [N.C.] get counted? :)

I thought {define} wasn't affected by {transpose} and define and special chords (different from config) to suit the song after transposition, even if that means multiple definitions to suit several possible keys. In your example, I'd expect the chord name to remain as D.

The idea of transposing {define}automatically seems attractive in theory but, the more I think about it, the more it seems fraught with difficulty, and maybe best left unchanged. Even with {define copy}, there's usually a good reason why I've used it, usually to suggest an alternate voicing or fingering, which might not make sense if transposed.

Re the error: yes, I tried using some %{orig.display} and other chord metadata, but haven't yet noticed problems from individual songs. I've got some pretty complex display strings with %{xxx} in my songbook layout, but they're more long than deeply-nested. Will report back once I've found narrowed it down...

[gwyndaf]

Maximum number of iterations exceeded at /media/data/Shared/Music/Technology/ChordPro/git/chordpro/script/../lib/App/Music/ChordPro/Chords/Parser.pm line 1012.

seems to be caused by this (reproducible running with -X --config ukulele-ly options):

{title: Define copy conditional display issue}
{define: Cadd9 frets 0 2 0 3 display "<span size=80%>\%{orig.display}</span>"}
{define-ukulele: Cadd9 copy Cadd9 fingers 0 2 0 4 display "<span size=80%>\%{orig.display}</span>"}
Optional riff [C]-[Cadd9] 

It does indeed seem related to using \%{orig.display}. If I replace that in either define with Cadd9 the error goes away.

Observation: if I use explicit Cadd9 in the first, generic define, the name displays smaller: 80% x 80% = 64%.

This suggests that define-ukulele is referencing %{orig.display} from the previous define, which in turn is referencing it from the config (if it exists).

On the basis that display is never copied, I'm guessing it's because %{orig.display} already has a value set from processing the previous define, so it gets compounded. So maybe that's causing the iterations issue?

[sciurius]

Good catch.

The problem is that you are substituting %{orig.display} with <span size=80%>%{orig.display}</span>, which gets substituted with <span size=80%><span size=80%>%{orig.display}</span></span> etc.

Basic problem is:

{define: C7 frets 0 2 0 3 display "x\%{display}"}

If the replacement is the same as the source, e.g. %{display} → %{display} this would be detected. Now it is not and the expansion will iterate until the limit (16) is reached. I'm not sure how to deal with this...

BTW, orig.display only makes sense with copy.

{define: Cadd9 frets 0 2 0 3 display "<span size=80%>\%{orig.display}</span>"}

This is a new chord, there is no copy so it does not have a base chord, hence nowhere to get an orig.display from.

[gwyndaf]

Yes, I discovered %{orig.copy} is meaningless if there's no copy origin. Makes sense.

Here's a thought. I don't know if it directly addresses the problem, but might bypass it. It might even remove the need for orig.display (and associated issues), though I should probably think through all possible use cases.

A lot of my define display stuff is because my config includes display names with markup, e.g.

{ "name" : "F#sus4@2",   "base" : 2,   "frets" :  [ 1,3,3,3,1,1 ],   "fingers"  :  [ 1,3,3,4,1,1 ],   "display"  :  "F#sus<span size=80% rise=30%>4</span>"  }, 

So, if display isn't copied from the config, when I use {define copy} (e.g. for alternate fingering), I need to set display manually with similar markup, or use \%{orig.display}.

However, both could be avoided by having a general config setting for default chordname display format. Maybe you're already doing this?

As I understand metadata parsing, current approach would be equivalent to %{name} or %{root}%{qual}%{ext}/%{bass}. If there a general setting, I'd probably use it for something audacious like (possibly adding tests if the values exist):

"chorddisplay" : "\%{root}\%{qual}<span size=80% rise=30%>\%{ext}</span><span size=90%>\/\%{bass}</span>"

I suppose that's what I'd expect %{display} to return for any given chord, and be used automatically.

That would mean display would only be needed for a display format that differs from the chorddisplay style, or needs a chord name other than the one being defined (e.g. simpler alternative).

For me at least, I think that would remove the need for maybe 90% of my display settings, both in configs and {define} directives. And if I'm not relying on display settings from the config, that removes the need for orig.display.

[sciurius]

Excellent idea!

In 6.010_011 you can add a chord named "defaults" in the config that has default values for (some) properties, currently only display.

E.g.

    "chords" : [
      { "name" : "defaults", "display" : "%{root}%{qual}%{ext|<sup>%{}</sup>}%{bass|/%{}}" },
      { "name" : "C", "base" : 1, "frets" ... }
      ...
    ],

The entries in chords are processed sequentially, so you can insert another defaults with different defaults, etc.. The scope of the defaults is the file that contains them.

Have fun!

[gwyndaf]

Oh, that's great. Took me a while to get it working (see below), but very interesting. Here's why:

{title: test of default display}
{define Cmmaj7 copy Cm frets 0 0 2 1 1 0}
[Cadd9] [Cm7b5] [C7sus4] [Cmmaj7]

with -X --config guitar2.json (guitar2.json.zip has default display line added, and only root C chords). This outputs as expected: default display (metadata/markup) applied to chords that are defined in guitar2.json. However, not applied to Cmmaj7 (because the default display applies only within the instrument config).

But, if I add display "\%{orig.display}" to the {define copy} directive, I get the default display markup applied (superscript maj7), even though Cmmaj7 isn't in the config, which is pretty neat :)

It seems chords that are understood, but not known/defined, won't automatically adopt the default display format. This was my initial confusion, processing the above example using guitar-ly.json (modified to add default display line). None of the chords in my sample took on my default format, which I now realise is because they're not present in that simpler config file, so the default display doesn't apply to them (i.e. understood but not known).

I'm still trying to figure out how to use this feature to apply the default display format to all chords (even if not known/defined). So far, I can see how it simplifies instrument configs, but it looks like display is still needed to set the format of chord names created with {define}.

Incidental observations, which might or might not be relevant I did some chord dumps as an initial test of this, outside of a specific song.

chordpro-git -X --config guitar2.json --dump-chords --output testdump.pdf generates testdump.pdf

All C chords (from guitar2.json) output with default display setting (unless explicitly defined for a chord, i.e. C), while the remainder output as per guitar.json (processed by default, before guitar2.json I think). However, some chords output only once, while others output several times ( e.g. Cdim). None seems to cause any problems, but looks like an odd behaviour worth flagging, in case it impacts something else.

One thing that might indicate a problem: C9(11) diagram outputs with no name. I though it might be the parentheses, but others - e.g. Cm7(b5) - output ok (but with parentheses removed). Distinctive thing about C9(11) seems to be that it's the only C chord with parentheses that has an explicit definition (not copy). This isn't a problem with G9(11), so might be caused by handling the default display or parsing chord metadata.