File format: store excerpts by a unique and safe name #20210
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Decouples the actual Excerpt name (shown in UI, and in the Excerpt itself) from the name under which it is saved in the MSC file. This way, the Excerpt name can be whatever the user wants, but still the excerpt can safely be saved in the file.
…standard" For example, when opening pre-4.2 files. This also gives us more freedom to easily change the excerpt file name generation algorithm in the future.
cbjeukendrup
force-pushed
the
excerpts_store_by_escaped_name
branch
from
c911118 to
8e18026
Compare
|
This PR should be fully ready now! |
| m_fileName = fileName; | ||
| } | ||
|
|
||
| static inline bool isValidExcerptFileNameCharacter(char16_t c) |
There was a problem hiding this comment.
It's better to use mu::io::isAllowedFileName / io::path_t mu::io::escapeFileName
RomanPudashkin
approved these changes
Dec 8, 2023
Merged
|
@DmitryArefiev I found a fix for that super quickly, so I created a PR already: #20404 |
|
This seems to crash (well, abort on a failed assertion) when importing a Mu3 score with parts |
| @@ -153,6 +156,59 @@ async::Notification Excerpt::nameChanged() const | |||
| return m_nameChanged; | |||
| } | |||
|
|
|||
| const String& Excerpt::fileName() const | |||
| { | |||
| IF_ASSERT_FAILED(!m_fileName.empty()) { | |||
There was a problem hiding this comment.
here it aborts when opening a Mu3 score with parts
There was a problem hiding this comment.
(in Debug builds only)
|
@Jojo-Schmitz Please check #22123 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The uniqueness of the names is guaranteed because each name is prefixed with the index of the excerpt. This has the additional advantage that we can eventually use those numbers to preserve the order of the excerpts across launches.
This PR can still read files that have already been created in older 4.2 builds.
However, for all older files, the viewstate.json file of existing excerpts won't be read, until you re-save the file in the build from this PR.I think I know a way around that, which I'm still planning to implement, so therefore I'm marking this as draft for now. Feedback is already welcome though.
Resolves: #14477
Resolves: #19054
Resolves: #19301