[ChordProParser] Not Recognizing Custom Sections, and other section-related issues

[edonv]

I've got a few feature requests relating to parsing different section types from ChordPro:

• Section labels support the following syntax {start_of_verse: label="Verse 1"}, not just {start_of_verse: Verse 1}. Currently ChordProParser doesn't like this and reads label="Verse 1" as the section label.
• Per the ChordPro spec, it's allowed to name a section anything, as long as the directive starts with start_of_ and end_of_. I use this a lot (i.e. start_of_pre_chorus), and because of this, ChordProParser just ignores the label altogether.
• I can't speak for the other formatters, but in my experience with HtmlTableFormatter, grid sections are rendered with the section label inline with its content. Is it possible to at least render the section label in the same way as other sections, separated?
  • Actually, it looks like HtmlDivFormatter does format the label, but it's still inline with the section content.
  • Additionally, it'd be nice if the . syntax (cells spacing) in grid sections could be rendered as separate table cells or something, like how it's rendered in ChordPro, but I'm not sure if that's possible with the way ChordSheetJS stores parsed songs.

See here for more details on this.

I'm sure I'll run into more things later, but this is it for now. I think most of this should be easily addable, no?

[edonv]

Oh, and I'll add that it seems both HTML formatters are adding a comma to the end of a line that has whitespace then a chord after the last word.

Example:

When you need[C#m]  an escape  [/C]

Screenshot:

[SamuMazzi]

Oh, and I'll add that it seems both HTML formatters are adding a comma to the end of a line that has whitespace then a chord after the last word.

I think this is relatd to #1409

[martijnversluis]

Hey @edonv. Thanks for your feedback! I created 3 separate issues and put them on the board.

[martijnversluis]

@edonv I just released 10.7.0 that adds support for attributes 👍

[martijnversluis]

@edonv I just released 10.9.0 that adds support for custom section delimiters ({start_of_x} / {end_of_x}). Please let me know if this is useful 👍

[edonv]

Thanks for all of your super quick and organized work on all this!

I do have a follow-up question: I'm having success with the label attribute, but when combined with custom section (i.e. {start_of_pre_chorus: label="Pre-Chorus"}...{end_of_pre_chorus}), I still don't get a section label. To be honest, this means it behaves the same as before. Like before, it's "failing" to parse a custom section type, but due to something, it's not parsing the label attribute (and before, the inline label) when the section is not a standard type. Any idea why?

And if it's helpful, I did a bit of debugging, just inspecting the parsed Song's label property, and it returned null for any section that isn't a "standard" section type.

[martijnversluis]

@edonv Thanks for checking. Eventually it turned out the function that determines whether a label should be rendered, wasn't updated when I introduced the custom section support.

I just published 10.9.3 which should fix that. Could you let me know if that works for you? 👍

[edonv]

Yes, that worked! Thanks!

[martijnversluis]

@edonv I just published 10.11.0 which changes how labels are rendered. Please let me know if this is helpful! 👍

[edonv]

That works great!

Separately, is there any chance of the formatters mapping grid structures with . characters into spaced grids in HTML? I have code that's doing this manually as part of the grid delegate function for HTMLDivFormatter, creating a <table> for it.

I'd be happy to contribute that if you'd like, just let me know where it'd go in your codebase and I'll fork.

[martijnversluis]

@edonv Yeah that would be a great contribution! I suppose we could have a src/formatter/delegates/grid.ts that default exports a function doing that. Ideally something along the line of:

class GridDelegate {
  call(content) {
    // perform the table rendering, nicely broken up in some private methods if necessary
  }
}

export default (content) => new GridDelegate().call(content);

Thanks in advance for the effort!