Don't give clips a hidden default name (Fix #5528)

[Spekular]

Fixes #5528 and simplifies/removes lots of name related logic by not giving clips a default name. Adds an upgrade routine to remove the default names in old projects, to avoid previously invisible clip names becoming visible.

Known issues:

• Initial testing implies the upgrade routine isn't called for .mmp files
  • This was likely a coincidence related to the version issue described below, further testing shows .mmp and .mmpz files behave the same way
• Version bump doesn't work with LMMSBot builds nor builds from a git clone, so the upgrade logic isn't guaranteed to run once. Depending on your LMMS version and project version it could run multiple times or never run. However, I have confirmed that versions like "1.2.2.686" are correctly detected as less than "1.3.0-alpha-1".
  • Fix: make a 1.3.0-alpha-1 tag when this PR is merged?

For reviewers: The commit "Automatic formatting changes" can be ignored while reviewing.

A future improvement would be to stop abusing name() to give hints in automation clips, but IMO this is out of scope for this PR. There is a displayName() function that might be appropriate, but it's used inconsistently.

[Spekular]

For testers

First and foremost, test that your old projects are correctly upgraded (I don't recommend saving over old projects when doing this!). Opening an old project should not lead to clip names becoming visible that were previously invisible. Then, for more thorough testing, you can test:

• Creating clips of these types:
  • Unnamed: Created, name not modified
  • Named: Created and given a name (other than the track name)
  • Same-named: Created and renamed to match track name
  • Reset: Name has been reset
• Renaming a track containing each type of clip
• Copying each type of clip to another track
• Resetting a clip of each type

[LmmsBot]

🤖 Hey, I'm @LmmsBot from github.com/lmms/bot and I made downloads for this pull request, click me to make them magically appear! 🎩

Linux

• Linux (AppImage): lmms-1.2.3-715%2Bg26ac891-linux-x86_64.AppImage (build link)

Windows

• Windows 32-bit: lmms-1.2.3-715%2Bg26ac89106-mingw-win32.exe (build link)
• Windows 64-bit: lmms-1.2.3-715%2Bg26ac89106-mingw-win64.exe (build link)
• Windows 32-bit: lmms-1.2.2-msvc2017-win32.exe (build link)
• Windows 64-bit: lmms-1.2.2-msvc2017-win64.exe (build link)

macOS

• macOS : lmms-1.2.3-715%2Bg26ac89106-mac10.13.dmg (build link)

🤖

{"platform_name_to_artifacts": {"Linux": [{"artifact": {"title": {"title": "(AppImage)", "platform_name": "Linux"}, "link": {"link": "https://8874-15778896-gh.circle-artifacts.com/0/lmms-1.2.3-715%2Bg26ac891-linux-x86_64.AppImage"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/8874?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}], "Windows": [{"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://8877-15778896-gh.circle-artifacts.com/0/lmms-1.2.3-715%2Bg26ac89106-mingw-win32.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/8877?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://8875-15778896-gh.circle-artifacts.com/0/lmms-1.2.3-715%2Bg26ac89106-mingw-win64.exe"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/8875?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}, {"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/11ankdhysxh4h2mt/artifacts/build/lmms-1.2.2-msvc2017-win32.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/35307328"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/dlfpirtg2b6bpksc/artifacts/build/lmms-1.2.2-msvc2017-win64.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/35307328"}], "macOS": [{"artifact": {"title": {"title": "", "platform_name": "macOS"}, "link": {"link": "https://8878-15778896-gh.circle-artifacts.com/0/lmms-1.2.3-715%2Bg26ac89106-mac10.13.dmg"}}, "build_link": "https://circleci.com/gh/LMMS/lmms/8878?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link"}]}, "commit_sha": "5d98a85d472250f4605d48b8b85222d549e9a367"}

[Spekular]

@PhysSong requesting review from you because I've seen you handle version bumps in the past, so I thought you might know why the bump to 1.3.0-alpha-1 didn't seem to work.

[PhysSong]

If you build from a Git repository(not using the tarball), LMMS uses the output of git describe --tags. It depends on tags of the repository.

[Spekular]

@PhysSong in other words, once this PR is merged we need to make a 1.3.0-alpha-1 tag if we want the version to update? If so, do you mind if I do that? (assuming I have the correct permissions and can figure it out, haha)

[Spekular]

Also, I suppose now would be a good time to confirm if the version numbering I used is actually correct. It looks like the current state of the PR would generate 1.3.0-alpha.1, but will the .1 be overridden by an automatic build number? Or will it prevent automatic build numbers from showing? Perhaps I should set VERSION_STAGE to alpha-1 and leave VERSION_BUILD unset.

SET(VERSION             "${VERSION_MAJOR}.${VERSION_MINOR}.${VERSION_RELEASE}")
IF(VERSION_STAGE)
	SET(VERSION     "${VERSION}-${VERSION_STAGE}")
ENDIF()
IF(VERSION_BUILD)
	SET(VERSION     "${VERSION}.${VERSION_BUILD}")
ENDIF()

[PhysSong]

You should use either alpha1(perhaps alpha01 in case there will be alpha10) or alpha.1 for VERSION_STAGE and leave VERSION_BUILD to "0". If you use alpha.1, ProjectVersion should be updated to allow something like 1.3.0-alpha.1.789.

[PhysSong]

See also: #3594

[Veratil]

Haven't tested, but LGTM

[Spekular]

Note to self (emphasis mine):

Precedence for two pre-release versions with the same major, minor, and patch version MUST be determined by comparing each dot separated identifier from left to right until a difference is found as follows:

Identifiers consisting of only digits are compared numerically.

Identifiers with letters or hyphens are compared lexically in ASCII sort order.

Longer strings are bigger than smaller strings if the first part is the same. Testing a few strings with this site gives, in ascending order:

• a
• aa
• alpha
• alpha01
• beta

This means that 1.3.0-alpha01 > 1.3.0-alpha.2, because alpha01 > alpha is true. In other words, if we use alpha01 now, we commit to using alpha## format until we reach beta or rc versions.

[Veratil]

https://semvercompare.azurewebsites.net/?version=1.3.0-beta&version=1.3.0-aa&version=1.3.0-alpha&version=1.3.0-alpha01&version=1.3.0-a&version=1.3.0-alpha.2&version=1.3.0&version=1.3.0-alpha.2%252Bdeadbeef

semver ordering of examples and a few others, lower in the list is > higher

[Spekular]

@PhysSong done in #5636. Should we close #3594 once that's merged?

[PhysSong]

It seems like new conflicts are not resolved properly.

[PhysSong]

It seems like the merge commit wasn't marked as a merge commit for some reason. I added a commit to fix it.

[Spekular]

@PhysSong thank you, I was going to manually transfer the changes to a new branch if the diff didn't resolve to something sensible (as it's occasionally done before).

This is ready for review now that it's no longer blocked by any other PRs.

[Spekular]

@Veratil I'm re-requesting review from you since you checked this out before. The changes are pretty minor, the relevant diff is here.

For anyone reviewing for the first time, this is the relevant diff (it excludes most of the formatting changes).

[DomClark]

Looks good to me mostly. Can you revert the formatting change in AutomationPattern.h so it isn't touched unnecessarily? BBTrack.cpp also has quite a few formatting changes, which I worry may cause conflicts with #5573.

[Spekular]

Latest commit addresses Dom's most recent comment, and I used a new test file to confirm that the behavior was wrong before and isn't now.

I also touched some code that should arguably be in a separate PR: Previously, legacyFileVersion would only be called if the attribute version existed and was set to exactly 1.0. If version didn't exist, nothing would happen and the file version would be set to the latest (18 in this case) meaning no upgrades run. I changed it so that if the version is missing OR set to 1.0, legacyFileVersion is used, and otherwise the normal parsing happens. If we want to be strict I can revert that and make a separate PR, but I was going crazy trying to figure out why my upgrade wasn't working with my new test file. It took a few qDebugs to realize that stripping out the version number wouldn't trigger legacyFileVersion.