issue #2538: Adding global messagebox and commandline parameter parsing classes.

[pgScorpio]

Short description of changes

Added global messagebox and commandline parameter parsing classes to global.h/main.c
Adapted all messageboxes to use the new CMsgBoxes class.

CHANGELOG:

Context: Fixes an issue?

General improvement and preparation for sound-redesign.

Does this change need documentation? What needs to be documented and how?

Status of this Pull Request

Working...

What is missing until this pull request can be merged?

Checklist

• I've verified that this Pull Request follows the general code principles
• I tested my code and it does what I want
• My code follows the style guide
• I waited some time after this Pull Request was opened and all GitHub checks completed without errors.
• I've filled all the content above

[ann0see]

Thanks for your PR!

@pgScorpio Yes, I think this PR is ok - although merging from your master branch to our master branch is discouraged: https://blog.jasonmeridth.com/posts/do-not-issue-pull-requests-from-your-master-branch/

If you don't push any other changes to master, it should be ok.

BTW: Please run nmake clang_format: https://github.com/jamulussoftware/jamulus/blob/master/CONTRIBUTING.md#source-code-consistency on your computer (after having installed clang format of course) to pass the styling check.

Also your code doesn't seem to build on Linux: https://github.com/jamulussoftware/jamulus/runs/5664602526?check_suite_focus=true#step:9:365

[pgScorpio]

@ann0see

@pgScorpio Yes, I think this PR is ok

I hope, but I understand it's still not the way it should be done....

although merging from your master branch to our master branch is discouraged: https://blog.jasonmeridth.com/posts/do-not-issue-pull-requests-from-your-master-branch/
If you don't push any other changes to master, it should be ok.

This was the only way I could find to make it work I'm afraid....
And I did fetch a clean master branch before adding the changes as I would always do.
It's just the way I'm used to work for over 30 years (PVCS and Microsoft VCS, not GitHub):
Fetch a clean (master-)branch with all latest changes, apply your changes, and upload your changes....
Creating a new branch for every change request, seems to be terrible overkill to me.

If the (unclear) procedure from your link has to be followed for every PR I'm afraid I won't be doing much PR's, there should be a much simpler way, but as said I find the documentation unclear and confusing especially for a newbie to GitHub.

BTW: Please run nmake clang_format: https://github.com/jamulussoftware/jamulus/blob/master/CONTRIBUTING.md#source-code-consistency on your computer (after having installed clang format of course) to pass the styling check.

nmake clang_format also doesn't seem to work for me either:

But, as far as I know, I did run clang-format "file" on every file before committing, but, as seen before, my clang-format seems to do different things than the autobuild clang-format.

Also your code doesn't seem to build on Linux: https://github.com/jamulussoftware/jamulus/runs/5664602526?check_suite_focus=true#step:9:365

Also strange, because the same code does build on my ubuntu virtual machine using Qt Creator:

I hope you can understand that all these issues contribute to my github frustrations....

[ann0see]

If you don’t push other changes to your master branch you can work as you want to work.
Git and GitHub confuses many people 😀. Sometimes it makes sense only after reading multiple articles or watching videos.

If nmake clang_format doesn’t work (maybe since it’s not available in the Linux shell) you can also run clang-format -i src/*.cpp via PowerShell/CMD. If you want, I can also fix it for you by pushing to your repo (this is possible if you didn’t disable the checkbox at the right side bar allowing maintainers to push to your branch.

I hate to say that, but personally I dislike developing on Windows (too much hassle to set up tools which clearly don’t integrate well), so I quickly moved to linux.

Concerning the failed linux build, I‘d wait for some of the others to review.

[ann0see]

@pgScorpio I think you forgot running it on some files? Never mind: I've just updated your commit.

Before working further on this, you probably need to git reset --hard origin/master locally.

BTW: It does also build on my linux machine. Maybe some incompatibilities with old versions of Qt have been introduced.

[pgScorpio]

@ann0see

you can also run clang-format -i src/*.cpp
I think you forgot running it on some files?

running clang-format -i "file" for every failed file is exactly what I did before the latest commit, resulting in these changes. But they still do not seem to be OK according autobuild.

After that I also tried it on linux: with clang-format no change, and make clang_format doesn't work there either:

Wouldn't it be more handy if the autobuild (or better github on every push) just did clang_format instead of just checking....

I hate to say that, but personally I dislike developing on Windows (too much hassle to set up tools which clearly don’t integrate well), so I quickly moved to linux.

That's a matter of taste I guess, no problem, but when you are used to work on Windows since the very first version it's hard to switch. Also I personally like the Windows tooling much better (Especially the older versions of VS) simply because I can use them blindly, so making it 10x faster to work with. (And those keyboard shortcuts are so well burned in that I keep messing things up on other platforms, especially on mac, that's really a tragedy for me.)

.

[pgScorpio]

BTW: It does also build on my linux machine. Maybe some incompatibilities with old versions of Qt have been introduced.

I think it has to do with the headless build.... I'll take a look.

[softins]

Wouldn't it be more handy if the autobuild (or better github on every push) just did clang_format instead of just checking....

No, we had that conversation some time ago when first looking at clang-format. The problem is that if a file gets modified by GitHub at the time you commit or push, your local repo is immediately out of sync with the remote, meaning you would have to do a git pull after every push.

There is a make clang_format target in the makefile now, which does clang-format -i on all the valid source files (but not the auto-generated ones), but again, we didn't want to make that automatically happen on every make.

[pgScorpio]

There is a make clang_format target in the makefile now, which does clang-format -i on all the valid source files (but not the auto-generated ones), but again, we didn't want to make that automatically happen on every make.

See my earlier post... I get "No rule to make target 'clang_format'" ????

[softins]

There is a make clang_format target in the makefile now, which does clang-format -i on all the valid source files (but not the auto-generated ones), but again, we didn't want to make that automatically happen on every make.

See my earlier post... I get "No rule to make target 'clang_format'" ????

OK, that probably means your branch was branched from master before make clang_format was added. It is post-3.8.2, added to master on 4 Mar in commit 51b8997

[pgScorpio]

OK, that probably means your branch was branched from master before make clang_format was added. It is post-3.8.2, added to master on 4 Mar in commit 51b8997

No, I have all the files as in 51b8997

But I've got it working....
You have to run qmake jamulus.pro before you can use make clang_format, but that is nowhere documented.

[ann0see]

You have to run qmake jamulus.pro before you can use make clang_format, but that is nowhere documented.

Ok. So can you add it to the file (in another PR of course).

[pgScorpio]

@ann0see
Concerning the still failed Coding Style Check on settings.cpp:

1. This is the result of clang-format -i src/settings.cpp! (I did run it again and no change.) So there still seems to be a difference in my local clang-format and the one used in auto-build. (FYI: I'm using clang-format LLVM version 13.0.1)
2. The auto-build "modified" version, and the current master version, do not conform the style guide (alignment of the braces with the ' if' ), my version does.
3. As I noted before there are more differences in my local clang-format and auto-build clang-format (Like auto-build clang-format breaking long lines too soon.)
4. I think it maybe has to do with the somewhat strange use of clang-format on and clang-format off in the middle of a if else without using braces, but I didn't change anything to this part of the code....

So I don't think this should need a code-change but needs a look at the clang-format version.
(At least documenting which version of clang-format to use and where to get it.)

[ann0see]

Agree. @passing @hoffie you were both involved in the clang format development. Could someone please have a look at this PR?

[pgScorpio]

@ann0see

Ok. So can you add it to the file (in another PR of course).

No, since there are a lot more prerequisites missing that even I don't know what they should be (still encountering problems myself.), but I can open an issue if you wish so someone else can pick it up.

[ann0see]

Yes. An issue would be great.

[pgScorpio]

@ann0see

Yes. An issue would be great.

It's there. See #2553

[ann0see]

@pgScorpio
Sorry for the git related comment which I strongly suggest you to follow:

I think you should not use merge to update your PRs since it will introduce huge confusion later on. Use rebasing instead. See https://www.maxwellantonucci.com/posts/2017/09/24/the-git-rebase-introduction-i-wish-i'd-had/

Therefore:

Undo your merge commit: checkout the last commit before the merge with git checkout b2810a73145ea57b96787dc38005b83dc0408213 your current local state (= "HEAD" in git terms) to your repo git push --force origin HEAD:<nameOfRemoteBranchToUpdate> .

Now fetch the latest upstream.

Afterwards, instead of git merge upstream/master do a git rebase -i: git rebase -i upstream/master. Afterwards force push it to your remote branch: git push --force origin master

[pgScorpio]

@ann0see

I think you should not use merge to update your PRs since it will introduce huge confusion later on. Use rebasing instead. See https://www.maxwellantonucci.com/posts/2017/09/24/the-git-rebase-introduction-i-wish-i'd-had/

Thanks for the link,

Unfortunately I saw your post too late, In the mean time I had quite some trouble getting the rebasing done (even the same commands as given in the link you provided initially gave me a lot of errors), And ended up with a no longer building tree. I hope it is all corrected now.
At least I seem to be up-to-date now, I can push again and the builds are working again... Though I'm still not shure if all changes are correct, since I even encountered conflicts in files I never changed myself and I hope I also made the right choices there.