Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for "factorysample:" prefix #7236

Merged

Conversation

michaelgregorius
Copy link
Contributor

@michaelgregorius michaelgregorius commented May 1, 2024

Add support to upgrade files with src tags that are prefixed with factorysample:.

Fix the problem for projects that still have "bassloopes" in their source files.

See #7186 for more details.

Add support to upgrade files with `src` tags that are prefixed with "factorysample:".
{
const QString original = prefix + originalName + "." + extension;
const QString replacement = prefix + originalName + " - " + bpm + " BPM." + extension;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^Tabs hiding here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed with commit d1fa6dd which also fixes the "bassloopes" issue.

Fix projects that still reference files with the typo "bassloopes" in their name.

The upgrade is implemented in its own method because it is unrelated to the BPM renaming even if it is technically very similar in its solution.

Introduce the helper method `mapSrcAttributeInElementsWithResources` which replaces the `src` attribute in elements with resources (samples and AFP) if it can be found in a map.

Remove hidden tabs.
@michaelgregorius michaelgregorius linked an issue May 1, 2024 that may be closed by this pull request
1 task
@JohannesLorenz
Copy link
Contributor

@michaelgregorius You often do very explanative commit messages, which is very valuable. However, some of the commits in your PRs have long commit messages without newlines. gitk does not break them down, making them hard to read. This is why e.g. vim suggests to break lines at 80 chars (or even earlier). Is it OK if you go through the commit messages of your open PRs?

@michaelgregorius
Copy link
Contributor Author

@JohannesLorenz, a while ago I always made sure that my commit messages all had line breaks at around 74 characters. However, this makes it rather tedious to write commit messages depending on which editor is used, especially if text is changed a lot during writing.

I am now of the opinion that it's rather a problem of the tools that display commit messages and that these tools should not make me jump through hoops just because some of them are inadequate to display the messages. The long lines are just different paragraphs when looked at with other tools.

@Rossmaxx
Copy link
Contributor

I don't know why this was stuck in the back burner for this long. The fixes doesn't look too bad either and people had confirmed the fix. I haven't tested it tho so would be better if someone who actually test give the opinion on it. Let's get done with the small PRs please.

Copy link
Contributor

@Rossmaxx Rossmaxx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving based on testing done by others.

@michaelgregorius michaelgregorius merged commit beedbc1 into LMMS:master Jun 14, 2024
9 checks passed
@michaelgregorius
Copy link
Contributor Author

I have now merged this. It should be rather safe as it "only" upgrades files that are loaded but should have no effect on files that are written. So, if we found that the merge has led to problems then we should be able to revert it without causing more havoc.

@michaelgregorius michaelgregorius deleted the 7186-AddFactorySamplePrefixAgain branch June 14, 2024 14:25
FyiurAmron pushed a commit to FyiurAmron/lmms that referenced this pull request Jun 15, 2024
…#7236)

## Add support for "factorysample:" prefix
Add support to upgrade files with `src` tags that are prefixed with "factorysample:".

## Fix "bassloopes" typo
Fix projects that still reference files with the typo "bassloopes" in their name.

The upgrade is implemented in its own method because it is unrelated to the BPM renaming even if it is technically very similar in its solution.

Introduce the helper method `mapSrcAttributeInElementsWithResources` which replaces the `src` attribute in elements with resources (samples and AFP) if it can be found in a map.
FyiurAmron pushed a commit to FyiurAmron/lmms that referenced this pull request Jun 17, 2024
…#7236)

## Add support for "factorysample:" prefix
Add support to upgrade files with `src` tags that are prefixed with "factorysample:".

## Fix "bassloopes" typo
Fix projects that still reference files with the typo "bassloopes" in their name.

The upgrade is implemented in its own method because it is unrelated to the BPM renaming even if it is technically very similar in its solution.

Introduce the helper method `mapSrcAttributeInElementsWithResources` which replaces the `src` attribute in elements with resources (samples and AFP) if it can be found in a map.
Rossmaxx pushed a commit to Reflexe/lmms that referenced this pull request Jul 16, 2024
…#7236)

## Add support for "factorysample:" prefix
Add support to upgrade files with `src` tags that are prefixed with "factorysample:".

## Fix "bassloopes" typo
Fix projects that still reference files with the typo "bassloopes" in their name.

The upgrade is implemented in its own method because it is unrelated to the BPM renaming even if it is technically very similar in its solution.

Introduce the helper method `mapSrcAttributeInElementsWithResources` which replaces the `src` attribute in elements with resources (samples and AFP) if it can be found in a map.
This pull request was closed.
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.

Account for changed native folder/file names when loading files
4 participants