Skip to content

Conversation

@Vija02
Copy link

@Vija02 Vija02 commented Oct 9, 2025

Attempt to add V7 support to the project. Thanks to the pointers in #1

This implementation:

  • Uses the .proto files from https://github.com/greyshirtguy/ProPresenter7-Proto as mentioned in the issue (latest one)
  • Uses buf to handle generating the TS files and also to parse the file
    • I added a postinstall script to generate these ts files from the .proto files
  • From there, mapping the data is quite straightforward. The data structure is slightly different from previous versions but not too bad. I've done the important ones.

Notes:

  • I don't really have a good v7 file so I've not included them, and also no tests yet since it wouldn't be relevant without the file.
  • I didn't touch the builder part since I don't need it. But i think it should be quite straightforward with Buf. The main thing to do would just be the data mapping
  • Not sure what you'd prefer for the eslint function complexity so i've just left the warning as it is

Let me know what you think!

@ChrisMBarr
Copy link
Owner

ChrisMBarr commented Oct 10, 2025

Oh wow, this is quite a surprise! I will sit down and take a look at all of this over the weekend sometime. Thank you very much for this!

So basically this now has a way to read but not write V7 files, and it does this by using these generated proto files which sort of translate from the original format into typescript?

@Vija02
Copy link
Author

Vija02 commented Oct 14, 2025

Yep. The .proto files did all the heavy lifting. From there, it's just a matter of using a library to parse (and also build) into a JSON format. The annoying part is mapping the data both ways

@ChrisMBarr
Copy link
Owner

Ok, I finally took a look at this, sorry for taking so long. I pushed a few minor changes into the main branch, and you'll need to pull latest into your PR branch to resolve merge conflicts on the package files.

I have to requested changes though:

  • Can you add unit tests? Your code is not complex, but it would be really nice to have "proof" that it all works as expected and will continue to do so if any changes are needed in the future.
  • Can you simplify some of the code complexity issues in the V7 parser? There are a lot of null coalescing statements which makes for a high cylomatic complexity. This is not a huge deal, but if it's possible to simplify, please do.

Also a question: This project is used in LyricConverter (http://lyricconverter.net and code at https://github.com/ChrisMBarr/LyricConverter) - Will your changes here also work in a web browser context? Not just in Node?

@Vija02
Copy link
Author

Vija02 commented Oct 21, 2025

Unit tests: I can write that, but I currently don't have access to a good V7 file for testing. I also run Linux, so it's quite a hassle to find a machine to run the software and create one. If you have a good file, that would be amazing.

Code complexity: Honestly i'm not so sure what to do about that. The types that were generated from the .proto files makes it so most fields are nullable. Meanwhile, our mapped type/model are more strongly typed. That's why most of it requires the null coalesce statements. Unless we change our model or the .proto type definitions, I'm not sure what i can do here.

Browser: Good question... Looking at the code again, i believe most of it should do. The question mark is in the Buffer.from function I used. That's only for handling a string input so it's not on the hot path right now. But i'll try to change it into something more portable

@ChrisMBarr
Copy link
Owner

ChrisMBarr commented Oct 21, 2025

Ok, that all makes sense. I have a Mac and can use the free trial will try to generate some simple Pro7 files for you. I'll make a simple one, and then try importing a pro6 file and saving that out as pro7.

Ok, makes sense about the code complexity stuff, just wanted to make sure. We can probably just add some lint ignore statements and explain there's just not a great way to simplify since they truly are all possibly null.

Another parser I made was for SongShowPlus files, works in the browser. It has a check for Buffer like this:

 let fileBuffer: ArrayBuffer;
//The Node Buffer is not available in web browsers, so we want to make sure it exists before attempting to use it
if (Buffer?.isBuffer(rawBuffer)) {
  //Convert Node Buffer to an array buffer
  //https://stackoverflow.com/a/71211814/79677
  fileBuffer = rawBuffer.buffer.slice(
    rawBuffer.byteOffset,
    rawBuffer.byteOffset + rawBuffer.byteLength
  ) as ArrayBuffer;
} else {
  fileBuffer = rawBuffer as ArrayBuffer;
}

You can see the code for that here if you want to compare: https://github.com/ChrisMBarr/SongShowPlus-parser/blob/main/src/index.ts

@Vija02
Copy link
Author

Vija02 commented Oct 23, 2025

Thanks!

I disabled the code complexity rule on the file, rebased to main & also updated the Buffer call for compatibility with browser. I used TextEncoder instead since it's quite straightforward.

@ChrisMBarr
Copy link
Owner

Wow, they now really push you do start a 14 day trial with a credit card, but they do still have a downloadable demo version thankfully. Ugh!

Ok, so I got a few simple songs imported and I took some screenshots of the UI for each one so you can see what it contains since you aren't able to run the software

Pro7 Sample Files.zip

The one for the "Feature Test" is for sure not as important, but it's a file I used for the V6 parser. The lyrics are for sure more important.

@Vija02
Copy link
Author

Vija02 commented Nov 22, 2025

Thanks @ChrisMBarr! Sorry for the delay, I just finally have the chance to work on this 😓
That should be all good, also resolved the merge conflict.

@ChrisMBarr
Copy link
Owner

Hey nice, that's great! Sorry the workflow didn't run automatically when you submitted this, I've made the settings change so that it will now. Looks like there are a few areas of code not covered by unit tests. Could you add those, or if not possible/reasonable to test then add the istanbul-ignore statements?

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 this pull request may close these issues.

2 participants