Add Bulk Exporter

[psyomn]

Here's a mock. I'm almost done with a draft of the implementation. I was thinking of having the code replicate the src fs tree into the destination tree.

ie: source looks like this:

src/a/1.ptb
src/b/2.ptb
src/c/3.ptb
src/c/d/4.ptb

into:

dest/a/1.pt2
dest/b/2.pt2
dest/c/3.pt2
dest/c/d/4.pt2

What this implies is if src is dest, the files would be generated alongside in the same directory. Is this sane behavior?

Also here's a mock:

The last box should show messages if some file could not be read, or something weird happens.

Does this course of development seem ok so far? Will implement the actual conversion algo today probably.

---

TODO (to complete this PR):

• [x] reconsider directory walking implementation?
• [x] rename bulk convert
• [x] add informative error message on stdout + edit box on files which fail to be converted

[psyomn]

Btw, ignore some of the formatting: I'm going to fix it when things are ready. My editor also removed some whitespace on save; I can revert those changes if you want me to!

Edit: also about filesystem, do you have usage preference? Since we're using C++17, I think there's kind of support for filesystems, but I'm not sure how many compilers do this right now. Should I use boost, or Qt instead?

[cameronwhite]

I think that seems like a reasonable behaviour! For reference, Guitar Pro has explicit toggles for whether to traverse sub-directories, and whether to generate in the source directory or allow selecting a different folder.

We haven't yet switched to the standard filesystem library, although that's something to look into soon probably. Currently boost filesystem is used for anything in the core libraries which don't depend on Qt, but Qt is fine for anything related to file dialogs etc.

A couple notes: source/app/paths.h has conversion functions for QString -> boost::filesystem, and you could look at PowerTabEditor::setPreviousDirectory() for how to access the directory that file dialogs should be initially set to (this should no longer be cached in that class if it's going to be updated from elsewhere)

[psyomn]

So I think I'm 90% done. I still need to incorporate some of your feedback, but it's getting there. I'm kind of considering re-writing some of this, by defining some filesystem iterator that could be generic enough to be reused elsewhere here. Unless you think that's overkill.

Anyway I'm going to loop over my code a little more and make sure that I can clean it as much as possible! Thanks for the feedback so far!

[cameronwhite]

Sounds good, let me know when you'd like me to take another look!

By filesystem iterator, were you referring to the logic in walkAndConvert()? I did notice that boost::filesystem seems to have a recursive_directory_iterator and some utility functions for building relative paths, so perhaps those could be used if they work for your use case?

[psyomn]

Hey there, sorry for the silence -- it's been busy at work. I should have some free time soon to continue and finish this PR!

[cameronwhite]

No problem!

[psyomn]

By filesystem iterator, were you referring to the logic in walkAndConvert()? I did notice that boost::filesystem seems to have a recursive_directory_iterator and some utility functions for building relative paths, so perhaps those could be used if they work for your use case?

Yep that's what I was talking about! I'll have to take a closer look at what that exactly does, and will swap to it if it simplifies the code.

I still need to go over some of your other feedback, but in the meantime if you see something that is extremely wrong, let me know!

I'm changing this to ready for review, so I can add the finishing touches, and hopefully we can merge this soon!

[psyomn]

Should be ready for one more pass whenever you have the time!

[psyomn]

Hey there I've applied the changes you requested! I think it works properly, will be running some more quick checks but I think this is done!

[cameronwhite]

Great! I'll do some testing of it later this weekend

[cameronwhite]

This looks great!

Just a couple minor things from testing it out:

• The menu item's label should be Bulk Converter... (with the ellipsis) since it opens a new dialog
• Maybe there should be some output in the message box if everything completed successfully, just to make it obvious that something happened?
• The dialog's title just showed up as "Dialog" for me

[psyomn]

Will do!

The layout looks a little strange. The edit boxes and progress bar seemed cropped. Is this normal?

Thanks for the kind words!

[cameronwhite]

Yeah, the buttons do look odd - maybe try removing some of the fixed widths / heights in the .ui file?

[cameronwhite]

The changes look good, thank you! I've still got a couple layout issues on macOS, but it's probably easier for me to just make those changes since I can test them out

[psyomn]

Hey there, thanks for the merge! I was planning to investigate it a little more today (yesterday I was playing with some of the values like you suggested), but I couldn't figure out what was wrong. Feel free to poke me, and I'll be more than happy to try and help out.

In the mean time I'm going to try out some profiling stuff on memory we talked about in another issue (leak + massif). If there's something you'd prefer I take a look please let me know!

Also, are refactors welcome? For example I think there's a bunch of C++ features we could leverage on, to reduce some of the usage of boost. Would patches like these be welcome?

Thanks for all the support!

[cameronwhite]

From looking at it a bit more, I think the best fix might be to refactor the layout to use layouts (QHBoxLayout / QVBoxLayout, etc) to arrange the widgets instead of fixed sizes and positions? That should make it more tolerant of the different styles on each platform's widgets

Refactors are definitely welcome! I'm always happy to eliminate some of the boost usage :)

[psyomn]

Thanks! I'll start working on the refactoring this evening!