feat(language): add Unraid OS language management system#71
feat(language): add Unraid OS language management system#71Ajit-Mehrotra wants to merge 92 commits intounraid:mainfrom
Conversation
* main window in pretty good shape - now onto the popup boxes * hw popup + delegate, os popup + delegate, and dst popup + delegate pretty much in a good spot. based on how the icons are displayed, didn't end up needing orange pngs, because the dark ones can be changed using ColorOverlay * about to start working on options popup * think the settings menu seems okay now * language/keyboard selector & progress bar theming * removal of a few now unused rpi pngs * replace instances of "#2B2A29" with new UnColors singleton member mediumGray * replace instances of "#FF8C2F" with new UnColors singleton member orange * replace instances of "#1C1B1B" with new UnColors singleton member darkGray * replace instances of "#FFFFFF" with color name white
* removes "use saved settings" pop up and always displays options popup instead
* ico for windows * removed rpi-imager.ico and rpi-image.png; mods to CMakeLists.txt to swap from rpi to unraid for windows RC file and copying to 128x128 app icon folder, will need a few other changes to make this work with linux still * renaming application from rpi-imager to unraid-imager * changes for linux * mac icns
* changed os list url in config.h * default select unraid device on load
* seems to be in a good place now for both ident and network config * added in the boot img stuff from the old usb creator (this also ensures that the volume label gets changed to UNRAID) * get rid of unnecessary if inside else clause * works on windows, though i did have to disable secure boot * some misc cleanup: *help button linking to website *info button opening "about" popup *putenv to enable darkmode title bar on windows *remove obe img gz files *minor text edits *changed logo background from white to darkgray * pushing to check on linux and mac
* local commit before switching branches * pushing to check on linux * working on linux * committing new files * completely disable devices in list that have an invalid guid
misc updates: * shrink help/info icons + add text * hyperlink urls in about popup *adjust text in about popup *adjust wording of formatting text *adjust wording of custom img select *limit custom img select to zip files *adjust wording of invalid guid text *add text for missing guid
* renaming unraid-imager -> unraid-usb-creator * more renaming + new icons * guid working on mac, troubleshooting make bootable * rename on format for mac * rather than trying to make the make bootable script work on all platforms (since it's only needed for legacy boot, and we don't let the user select that as an option right now), simply display a message telling the user to run the script after burn successful * fix embedded links in info popup * adding back in linux drive renaming * deb file updates
Co-authored-by: Jessica Reid <jlr@Jessicas-Mini-875.hsd1.va.comcast.net>
Always Run make_bootable.bat on Windows
The legacy url currently being linked to is going away
Fix incorrect link to docs
# Conflicts: # src/imagewriter.cpp # src/linux/org.raspberrypi.rpi-imager.desktop
part 3 up to line 480
* is it this easy? * let's try this again * once more with feeling * let's see if these replace the "2" qml modules from qt5 * try updating control file for debuild * needed for linguist tools? * see if this fixes Qt6::lprodump cmake error * revert previous changes * trying with install-qt-action * syntax error * try specifying something other than latest for os * once more * upload deb artifact * see if we can hack something together here * pick a runner that it can actually find * keep running up that hill * just gonna keep trying some stuff * maybe? * just need to re-trigger the github action * remove changes to mac yml * remove trailing whitespace from yml
- cmake 3.31 (<4.0) - Qt 5 (not Qt6 or latest) - add dmg wrapping for .app for drag and drop installation **Important**: codesigning is still pending automation. Currently dev must: 1. double click the .dmg (drop .app into Applications folder) 2. then finally remove quarantine from .app in Applications folder: `xattr -d com.apple.quarantine /Applications/unraid-usb-creator.app`
…e management and installation
…eManager to perform patching operations works for macos, coded for linux (needs testing), windows TODO
…address unmounting a temporary mount on linux
…uage error message
📝 WalkthroughWalkthroughThis update introduces comprehensive Unraid OS language selection and installation support throughout the application. It adds a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant QML_UI
participant ImageWriter
participant UnraidLanguageManager
participant DownloadExtractThread
User->>QML_UI: Selects Unraid OS language from dropdown
QML_UI->>ImageWriter: setUnraidOSLanguage(language)
QML_UI->>ImageWriter: startWrite()
ImageWriter->>DownloadExtractThread: Start extraction (with changedDefaultLanguage flag)
DownloadExtractThread-->>ImageWriter: Extraction complete
alt Language is not English
ImageWriter->>UnraidLanguageManager: installUnraidOSLanguage(selectedLanguage, mountPoint)
UnraidLanguageManager->>ImageWriter: progressUpdated / done / error
ImageWriter->>QML_UI: unraidOSLanguageStatusUpdate / unraidLanguageInstallationComplete
end
ImageWriter->>QML_UI: success
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (9)
src/config.h (1)
42-42: Consider grouping new constants with existing Unraid-specific URLs
UNRAID_LANGUAGES_URLis currently added far from the other Unraid‐specific constant (UNRAID_GUID_URL, line 40). For readability and to keep related settings together, place the new macro immediately after or beforeUNRAID_GUID_URL, or group them under a dedicated “Unraid URLs” section..github/workflows/build-macos.yml (2)
22-23: Update toactions/checkout@v4to silence actionlint and get perf/security fixesActionlint flags
@v3as outdated. Switching to v4 is drop-in and aligns with GH-recommended version.- - uses: actions/checkout@v3 + - uses: actions/checkout@v4
31-35: Duplicatebrew updateslows the job
brew updateis already executed once (line 33). Removing the second call saves ~30 s per run.- brew update - brew install create-dmg + brew install create-dmgsrc/CMakeLists.txt (1)
145-145: Verify the WIN32 condition change for console window behavior.The condition now restricts the WIN32 flag to Release builds only, which means console windows will appear in Debug builds on Windows. Please confirm this is intentional for debugging purposes.
Do you want me to verify how this affects the user experience in Debug vs Release builds?
src/main.qml (1)
1-2: Remove unnecessary blank lines at the start of the file.Starting files with blank lines is unconventional and serves no purpose.
- - /*src/imagewriter.h (1)
155-160: Add documentation for the public language management methods.These Q_INVOKABLE methods are part of the public QML interface and should be documented.
// Unraid OS Language Management + /** + * @brief Get list of available Unraid OS languages + * @return List of language names suitable for display + */ Q_INVOKABLE QStringList getUnraidOSLanguages(); + + /** + * @brief Get the currently selected language name + * @return Display name of the selected language + */ Q_INVOKABLE QString getSelectedUnraidOSLanguageName(); + + /** + * @brief Set the selected Unraid OS language + * @param languageName Display name of the language to select + */ Q_INVOKABLE void setUnraidOSLanguage(const QString &languageName); + + /** + * @brief Install the selected language onto the target device + */ Q_INVOKABLE void installUnraidOSLanguage(); - // Q_INVOKABLE bool isUnraidOSLanguageAvailable();src/unraidlanguagemanager.cpp (1)
32-43: Add network timeout configuration.The network requests don't have explicit timeout configuration, which could cause the application to hang on slow connections.
void UnraidLanguageManager::requestUnraidLanguagesJson() { - emit progressUpdated("Downloading JSON file containing supported language information..."); QNetworkRequest request(m_unraidLanguagesURL); + request.setTransferTimeout(30000); // 30 second timeout QNetworkReply *jsonReply = m_networkManager.get(request); QObject::connect(jsonReply, &QNetworkReply::finished, this, [this, jsonReply]() { onLanguagesJsonRequest(jsonReply); }); }Apply similar timeout configuration to
requestLanguageXmlandrequestLanguageZipmethods.src/imagewriter.cpp (2)
84-86: Remove emojis from the comment for professional code style.While the initialization is correct, the comment should maintain a professional tone.
- //Default language for unraid os is english 🇺🇸🦅 + // Default language for Unraid OS is English (en_US) _unraidLanguageManager.setCurrentLanguageCode("en_US");
302-318: Consider handling multiple mount points.This method returns only the first mount point when a drive might have multiple. Per the PR objectives, this is a known issue on Linux.
Consider either:
- Returning all mount points and letting the caller decide
- Adding logic to select the most appropriate mount point
- Documenting this limitation clearly
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
.github/workflows/build-macos.yml(2 hunks).github/workflows/build-windows.yml(1 hunks).gitignore(1 hunks)src/CMakeLists.txt(3 hunks)src/config.h(1 hunks)src/downloadextractthread.cpp(6 hunks)src/downloadextractthread.h(2 hunks)src/imagewriter.cpp(8 hunks)src/imagewriter.h(5 hunks)src/main.cpp(1 hunks)src/main.qml(67 hunks)src/qml.qrc(1 hunks)src/qmlcomponents/OSLanguageDropdown.qml(1 hunks)src/qmlcomponents/qmldir(1 hunks)src/unraidlanguagemanager.cpp(1 hunks)src/unraidlanguagemanager.h(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/build-macos.yml
22-22: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (24)
.github/workflows/build-windows.yml (1)
16-16: Pin the container image to a digest for reproducible buildsUsing
latestrisks unexpected breakage when the base image is updated. Pin the tag to an immutable digest (e.g.@sha256:…) or a versioned tag so CI builds remain deterministic.- container: ghcr.io/unraid/usb-creator-next/usb-creator-mxe:latest + # Pin to specific digest to avoid silent breaking changes + container: ghcr.io/unraid/usb-creator-next/usb-creator-mxe@sha256:<digest>.gitignore (1)
1-20: LGTM – broader ignore rules make senseThe refined patterns cover build artefacts, Qt Creator metadata and macOS packaging outputs without over-matching. No issues spotted.
src/qmlcomponents/qmldir (1)
2-3: Registration looks correct
OSLanguageDropdownis properly added with version 1.0. No further action needed.src/qml.qrc (1)
61-61: LGTM! Resource addition is correct.The new QML component is properly added to the resource system, making it available for use in the application.
src/main.cpp (1)
350-353: LGTM! Signal-slot connection is properly implemented.The connection for language status updates follows the established pattern and correctly links the imageWriter backend to the QML interface.
src/CMakeLists.txt (1)
24-24: LGTM! Source files properly added to build.The new UnraidLanguageManager source and header files are correctly included in the build configuration.
Also applies to: 97-97
src/downloadextractthread.h (1)
26-26: LGTM! Language tracking parameter and member variable are well-designed.The new
changedDefaultLanguageparameter with default valuefalsemaintains backward compatibility while enabling language state tracking. The private member variable follows the established naming convention.Also applies to: 63-63
src/qmlcomponents/OSLanguageDropdown.qml (3)
7-36: LGTM! Component structure and properties are well-designed.The language dropdown component has a clean API with appropriate property aliases and signal emission. The languageChanged signal correctly passes the selected text to parent components.
85-108: Well-implemented popup with dynamic height calculation.The popup implementation correctly calculates available space and adapts the height dynamically while maintaining a maximum of 200px. The Material Dark theme and scroll indicator provide good UX.
27-27: Styling objects confirmed available in QML context
- A FontLoader with
id: robotois declared in src/main.qml, providingroboto.name.- The
UnColorssingleton is defined in qmlcomponents/UnColors.qml (orange, darkGray, mediumGray) and registered in qmldir.No further changes are needed.
src/main.qml (3)
243-273: LGTM! Language dropdown integration is well-implemented.The OSLanguageDropdown component is properly integrated with appropriate signal handling and state management.
1597-1601: LGTM! Language status update handler is properly implemented.The status update function correctly logs messages and updates the UI progress text.
2107-2130: LGTM! Language selection functions and signal connections are well-implemented.The helper functions and Connections object properly handle language updates from the backend.
src/downloadextractthread.cpp (1)
46-48: LGTM! Constructor properly extended for language change tracking.The addition of the
changedDefaultLanguageparameter and its initialization follow proper C++ practices.src/unraidlanguagemanager.h (1)
1-74: LGTM! Well-designed class interface following Qt patterns.The UnraidLanguageManager class has a clean public interface with appropriate signals for asynchronous operations and proper encapsulation of implementation details.
src/imagewriter.h (1)
225-231: LGTM! Helper methods properly encapsulated.The disk operation helper methods are appropriately scoped as private and support the language installation workflow.
src/unraidlanguagemanager.cpp (1)
216-251: LGTM! Comprehensive error handling in JSON parsing.The
parseLanguageMapmethod has thorough error checking at each step with descriptive error messages.src/imagewriter.cpp (7)
56-59: LGTM!The Linux-specific headers are properly guarded and necessary for mount operations.
66-81: Initialization looks correct.The UnraidLanguageManager is properly initialized with the parent object.
215-236: Signal connections are properly implemented.Good use of both old-style and new-style Qt connections. The error handling lambda provides clear user feedback.
247-275: Well-implemented offline handling and lazy loading.The method properly handles offline scenarios and implements an effective lazy loading pattern for language data.
349-368: Well-structured language installation method.Good validation of preconditions and clear error messaging.
943-952: Good integration of language installation flow.The early return pattern properly ensures language installation completes before signaling success.
986-990: LGTM!Standard signal relay implementation.
| Image { | ||
| id: iconimage | ||
| source: (icon == "https://craftassets.unraid.net/static/favicon/favicon.ico") ? (bgrect.mouseOver ? "unraid/icons/un-mark-dark-gray.svg" : "unraid/icons/un-mark-gradient.svg") : (icon == "icons/erase.png" ? (bgrect.mouseOver ? "unraid/icons/erase_dark_gray.svg" : "unraid/icons/erase_orange.svg") : (icon == "icons/use_custom.png" ? (bgrect.mouseOver ? "unraid/icons/use_custom_dark_gray.svg" : "unraid/icons/use_custom_orange.svg") : icon)) | ||
| source: (icon == "https://craftassets.unraid.net/static/favicon/favicon.ico") ? (bgrect.mouseOver ? "unraid/icons/un-mark-dark-gray.svg" : "unraid/icons/un-mark-gradient.svg") : (icon == "icons/erase.png" ? (bgrect.mouseOver ? "unraid/icons/erase_dark_gray.svg" : "unraid/icons/erase_orange.svg") : (icon == "icons/use_custom.png" ? (bgrect.mouseOver ? "unraid/icons/use_custom_dark_gray.svg" : "unraid/icons/use_custom_orange.svg") : icon)) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor complex ternary operator for better readability.
This deeply nested ternary operator is difficult to read and maintain. Consider extracting it into a function.
-source: (icon == "https://craftassets.unraid.net/static/favicon/favicon.ico") ? (bgrect.mouseOver ? "unraid/icons/un-mark-dark-gray.svg" : "unraid/icons/un-mark-gradient.svg") : (icon == "icons/erase.png" ? (bgrect.mouseOver ? "unraid/icons/erase_dark_gray.svg" : "unraid/icons/erase_orange.svg") : (icon == "icons/use_custom.png" ? (bgrect.mouseOver ? "unraid/icons/use_custom_dark_gray.svg" : "unraid/icons/use_custom_orange.svg") : icon))
+source: getIconSource(icon, bgrect.mouseOver)Add this helper function to the component:
function getIconSource(iconPath, isMouseOver) {
if (iconPath === "https://craftassets.unraid.net/static/favicon/favicon.ico") {
return isMouseOver ? "unraid/icons/un-mark-dark-gray.svg" : "unraid/icons/un-mark-gradient.svg"
} else if (iconPath === "icons/erase.png") {
return isMouseOver ? "unraid/icons/erase_dark_gray.svg" : "unraid/icons/erase_orange.svg"
} else if (iconPath === "icons/use_custom.png") {
return isMouseOver ? "unraid/icons/use_custom_dark_gray.svg" : "unraid/icons/use_custom_orange.svg"
}
return iconPath
}🤖 Prompt for AI Agents
In src/main.qml at line 1103, the source property uses a deeply nested ternary
operator that reduces readability and maintainability. Refactor this by creating
a helper function named getIconSource that takes the icon path and mouseOver
state as parameters and returns the appropriate icon path based on the
conditions. Replace the ternary expression with a call to this function, passing
the icon and bgrect.mouseOver as arguments.
|
|
||
| //this is a temporary fix. The fix should be apart of a larger refactor of linux mountpoints. | ||
| // Current mountpoint logic in this method is not able to pick up auto-mountpoint on linux consistently (or at all.../media/usr/USB_Name) despite GUI picking it up. | ||
| // need to consolidate how we are getting mountpoints on linux throughout codebase and make it more robust and consistent than currently is. | ||
|
|
||
| // if user changed default language, we don't want to close this mountpoint as we want the UnraidLanguageManager to use it first. | ||
| //ImageWriter will then close it once UnraidLanguageManager has completed making the required patches to update the language. | ||
|
|
||
| //ALSO, 90% sure this unmounting also does not work (idk why tho). See what I did in ImageWriter::onUnraidLanguageDone(). That one works | ||
| if (manualmount && !_changedDefaultLanguage) | ||
| { | ||
| QStringList args; | ||
| args << folder; | ||
|
|
||
| qDebug() << "Unmounting temp mounting directory" << args; | ||
|
|
||
| QProcess::execute("umount", args); | ||
| QDir d; | ||
| d.rmdir(folder); | ||
| qDebug() << "Deleting temp mounting directory" << folder; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor the mount/unmount logic for robustness.
The comments acknowledge multiple issues:
- Mount detection doesn't work on Linux
- Unmounting might not work correctly
- The conditional logic is a temporary workaround
This fragile mount handling needs a comprehensive refactor.
Would you like me to help design a more robust mount point management system that properly handles both automatic and manual mounts across platforms?
🤖 Prompt for AI Agents
In src/downloadextractthread.cpp around lines 492 to 512, the current mount and
unmount logic is fragile and unreliable on Linux, with inconsistent mount
detection and questionable unmount behavior. To fix this, refactor the
mountpoint management by consolidating mount detection and unmounting into a
dedicated, cross-platform module or utility that consistently handles both
automatic and manual mounts. Replace the temporary conditional logic with calls
to this new robust system to ensure reliable mountpoint handling and cleanup.
|
|
||
| // This code is unable to find the automatically mounted partitions for a device on Linux. | ||
| // As such, it ALWAYS leads to creating a temporary mount (the logic for removing said temp mount later on below also doesn't work hahah). | ||
|
|
||
| /* See if OS auto-mounted the device */ | ||
| for (int tries = 0; tries < 3; tries++) | ||
| { | ||
| qDebug() << "Attempt #" << tries << " to find mountpoint folder: "; | ||
| QThread::sleep(1); | ||
| auto l = Drivelist::ListStorageDevices(); | ||
| for (const auto& i : l) | ||
| { | ||
| if (QByteArray::fromStdString(i.device).toLower() == devlower && i.mountpoints.size() == 1) | ||
| { | ||
| folder = QByteArray::fromStdString(i.mountpoints.front()); | ||
| qDebug() << "Found existing mountpoint folder: " << folder; | ||
|
|
||
| // breh this legit only breaks from the first for loop... would need a flag, I think | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Address the Linux mount detection issue.
The comment indicates that the code cannot detect automatically mounted partitions on Linux, always creating temporary mounts. This is a significant limitation that should be properly addressed.
Consider implementing a more robust mount detection mechanism. Would you like me to open an issue to track this technical debt?
🏁 Script executed:
#!/bin/bash
# Check if there are any existing mount detection patterns in the codebase
rg -A 5 "mountpoint|mount.*detect|auto.*mount" --type cpp --type hLength of output: 18764
Refactor Linux mount detection in downloadextractthread.cpp
The current retry loop (lines ~228–250) with ad-hoc break logic and temp-mount hacks is brittle and duplicates logic spread across linuxdrivelist.cpp and udisks2api.cpp. We should consolidate into a single, robust helper that:
- Leverages
Drivelist::ListStorageDevices()orudisks2api::mountPoints()for auto-mount discovery - Correctly breaks out of both loops when a mountpoint is found
- Falls back cleanly to manual mount only when no auto-mount exists
- Ensures paired cleanup/unmount logic actually triggers
Please update these locations:
- src/downloadextractthread.cpp (retry loops &
isMountPointcheck) - src/linux/linuxdrivelist.cpp (auto-mount filtering)
- src/linux/udisks2api.cpp (use of
mountPoints()and unmount flow)
And open a tracker issue to refactor Linux mountpoint detection across the codebase for consistency and reliability.
🤖 Prompt for AI Agents
In src/downloadextractthread.cpp around lines 228 to 250, refactor the Linux
mount detection retry loop to properly break out of both nested loops when a
mountpoint is found by using a flag or restructuring the loops. Consolidate the
mountpoint detection logic by leveraging either Drivelist::ListStorageDevices()
or udisks2api::mountPoints() consistently. Ensure the fallback to manual
mounting only occurs if no auto-mount is detected, and verify that the
cleanup/unmount logic is correctly paired and triggered. Also update related
logic in src/linux/linuxdrivelist.cpp and src/linux/udisks2api.cpp to unify
mount detection and unmount flows, and create a tracker issue to improve Linux
mountpoint detection consistency across the codebase.
| void UnraidLanguageManager::onLanguagesJsonFetched() { | ||
| if (!m_installPending) return; | ||
| m_installPending = false; | ||
| continueLanguageInstall(m_currentLanguageCode, m_usbPath); | ||
| qDebug() <<"Resetting m_installPending: " <<m_installPending << " to true "; | ||
| m_installPending = true; | ||
| } |
There was a problem hiding this comment.
Fix the install pending flag logic.
The m_installPending flag is being reset to true after installation continues, which seems incorrect and could cause issues if the method is called again.
void UnraidLanguageManager::onLanguagesJsonFetched() {
if (!m_installPending) return;
m_installPending = false;
continueLanguageInstall(m_currentLanguageCode, m_usbPath);
- qDebug() <<"Resetting m_installPending: " <<m_installPending << " to true ";
- m_installPending = true;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| void UnraidLanguageManager::onLanguagesJsonFetched() { | |
| if (!m_installPending) return; | |
| m_installPending = false; | |
| continueLanguageInstall(m_currentLanguageCode, m_usbPath); | |
| qDebug() <<"Resetting m_installPending: " <<m_installPending << " to true "; | |
| m_installPending = true; | |
| } | |
| void UnraidLanguageManager::onLanguagesJsonFetched() { | |
| if (!m_installPending) return; | |
| m_installPending = false; | |
| continueLanguageInstall(m_currentLanguageCode, m_usbPath); | |
| } |
🤖 Prompt for AI Agents
In src/unraidlanguagemanager.cpp around lines 89 to 95, the m_installPending
flag is set to false before continuing the language install but then immediately
reset to true, which is incorrect. Remove the line that sets m_installPending
back to true after calling continueLanguageInstall to ensure the flag correctly
reflects the install state and prevents repeated or conflicting installs.
| /* | ||
| * Since QSettings strips literal quotes in INI values, we: | ||
| * 1) Wrap the value in a unique marker (e.g. "__QUOTE__") | ||
| * 2) Let QSettings write the file normally | ||
| * 3) After sync, reopen the INI and replace marker + value + marker → "value" | ||
| * This restores real quotation marks in the final file. | ||
| */ | ||
|
|
||
| void UnraidLanguageManager::patchDynamixConfig(const QString &languageCode) | ||
| { | ||
| const QString cfgPath = m_usbPath + "/config/plugins/dynamix/dynamix.cfg"; | ||
|
|
||
| // Marker must be something that will NEVER appear in your real data | ||
| const QString quoteMarker = "__QUOTE__"; | ||
|
|
||
| QString placeholderValue = quoteMarker + languageCode + quoteMarker; | ||
|
|
||
| if (!QFile::exists(cfgPath)) { | ||
| qWarning() << "Config file does not exist:" << cfgPath; | ||
| emit error(QString("Config file does not exist: %1").arg(cfgPath)); | ||
| return; | ||
| } | ||
|
|
||
| { | ||
| QSettings settings(cfgPath, QSettings::IniFormat); | ||
|
|
||
| settings.beginGroup("display"); | ||
| settings.setValue("locale", placeholderValue); | ||
| settings.endGroup(); | ||
|
|
||
| settings.sync(); | ||
|
|
||
| // Check if sync was successful | ||
| QSettings::Status status = settings.status(); | ||
| switch (status) { | ||
| case QSettings::NoError: | ||
| qDebug() << "Successfully patched dynamix config file:" << cfgPath; | ||
| break; | ||
| case QSettings::AccessError: | ||
| qWarning() << "Access error writing to:" << cfgPath; | ||
| // Try to get more specific error information | ||
| if (!QFileInfo(cfgPath).isWritable()) { | ||
| qWarning() << "File is not writable (permissions issue)"; | ||
| emit error(QString("Config file is not writable: %1").arg(cfgPath)); | ||
| } | ||
| break; | ||
| case QSettings::FormatError: | ||
| qWarning() << "Format error in file:" << cfgPath; | ||
| emit error(QString("Format Error in file: %1").arg(cfgPath)); | ||
| break; | ||
| default: | ||
| qWarning() << "Unknown error occurred while writing to:" << cfgPath | ||
| << "Status code:" << status; | ||
|
|
||
|
|
||
| // Additional diagnostic information | ||
| QFileInfo fileInfo(cfgPath); | ||
| qWarning() << "File exists:" << fileInfo.exists() | ||
| << "Is readable:" << fileInfo.isReadable() | ||
| << "Is writable:" << fileInfo.isWritable() | ||
| << "File size:" << fileInfo.size(); | ||
| emit error(QString("Unknown error occurred while writing to:").arg(cfgPath)); | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| // Post-process the file: swap marker → real quotes | ||
| { | ||
| QFile file(cfgPath); | ||
| if (!file.open(QIODevice::ReadOnly | QIODevice::Text)) { | ||
| qWarning() << "Could not reopen for post-processing:" << cfgPath; | ||
| emit error(QString("Could not reopen for post-processing:").arg(cfgPath)); | ||
| } else { | ||
| QString contents = QString::fromUtf8(file.readAll()); | ||
| file.close(); | ||
|
|
||
| // Replace __QUOTE__langCode__QUOTE__ → "langCode" | ||
| const QString target = quoteMarker + languageCode + quoteMarker; | ||
| const QString replaced = QString("\"%1\"").arg(languageCode); | ||
| contents.replace(target, replaced); | ||
|
|
||
| if (!file.open(QIODevice::WriteOnly | QIODevice::Truncate | QIODevice::Text)) { | ||
| qWarning() << "Could not reopen for swapping quotes in dynamix.cfg:" << cfgPath; | ||
| emit error(QString("Could not reopen dynamix.cfg file: ").arg(cfgPath)); | ||
| } else { | ||
| file.write(contents.toUtf8()); | ||
| file.close(); | ||
| qDebug() << "Patched quotes in dynamix config to actual quotation marks."; | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Consider a more robust approach for handling quoted values.
While the marker-based workaround for QSettings quote stripping is clever, it's fragile if the marker ever appears in real data. Consider alternative approaches:
- Use a different configuration format (JSON, custom INI writer)
- Use a more unique marker (UUID-based)
- Write the file directly without QSettings
🏁 Script executed:
#!/bin/bash
# Check if there are other uses of QSettings that might have similar issues
rg -B 2 -A 2 'QSettings.*IniFormat' --type cppLength of output: 305
Strengthen QSettings quote workaround in src/unraidlanguagemanager.cpp
The fixed __QUOTE__ marker approach can collide with real data. To harden this implementation (lines 345–436):
- Generate a truly unique placeholder at runtime (e.g. a UUID) instead of the constant
__QUOTE__. - Or bypass QSettings for this entry and write the INI line directly, or migrate to a format that preserves quotes natively (JSON, TOML).
- Alternatively, extend QSettings with a custom IniFormat handler that escapes and restores quotation marks without markers.
🤖 Prompt for AI Agents
In src/unraidlanguagemanager.cpp lines 345 to 436, the current fixed marker
"__QUOTE__" used to preserve quotes in QSettings can conflict with real data. To
fix this, replace the constant marker with a runtime-generated unique
placeholder such as a UUID to ensure it never appears in actual data. Generate
this unique marker at the start of patchDynamixConfig and use it consistently
for wrapping and replacing the languageCode value. This will harden the quote
workaround and prevent accidental collisions.
| void ImageWriter::onUnraidLanguageDone() | ||
| { | ||
|
|
||
| emit success(); | ||
|
|
||
| #ifdef Q_OS_LINUX | ||
|
|
||
| // this is code for unmounting temp directory that was created (if it was created) | ||
| // sometimes hangs up the gui. Not sure why. | ||
| // For now, temp mounts will remain unmounted on linux if they choose a separate language | ||
|
|
||
| // QString mnt = getMountPointForSelectedDrive() | ||
| // qDebug() << "Attempting to unmount temporary mountpoint: " << mnt << " --> on device: " << _dst; | ||
|
|
||
| // if (mnt.isEmpty()) { | ||
| // qDebug() << "Nothing to unmount."; | ||
| // } else{ | ||
| // QtConcurrent::run([mnt]() { | ||
| // // Force‐lazy unmount | ||
| // ::umount2(mnt.toUtf8().constData(), MNT_DETACH|MNT_FORCE); | ||
| // QDir().rmdir(mnt); | ||
| // qDebug() << "Background unmount of" << mnt << "complete"; | ||
| // }); | ||
|
|
||
| // } | ||
|
|
||
| #endif | ||
|
|
||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove commented-out code.
Large blocks of commented code should be tracked in issues rather than left in the codebase.
void ImageWriter::onUnraidLanguageDone()
{
-
emit success();
-
-#ifdef Q_OS_LINUX
-
- // this is code for unmounting temp directory that was created (if it was created)
- // sometimes hangs up the gui. Not sure why.
- // For now, temp mounts will remain unmounted on linux if they choose a separate language
-
- // QString mnt = getMountPointForSelectedDrive()
- // qDebug() << "Attempting to unmount temporary mountpoint: " << mnt << " --> on device: " << _dst;
-
- // if (mnt.isEmpty()) {
- // qDebug() << "Nothing to unmount.";
- // } else{
- // QtConcurrent::run([mnt]() {
- // // Force‐lazy unmount
- // ::umount2(mnt.toUtf8().constData(), MNT_DETACH|MNT_FORCE);
- // QDir().rmdir(mnt);
- // qDebug() << "Background unmount of" << mnt << "complete";
- // });
-
- // }
-
-#endif
-
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| void ImageWriter::onUnraidLanguageDone() | |
| { | |
| emit success(); | |
| #ifdef Q_OS_LINUX | |
| // this is code for unmounting temp directory that was created (if it was created) | |
| // sometimes hangs up the gui. Not sure why. | |
| // For now, temp mounts will remain unmounted on linux if they choose a separate language | |
| // QString mnt = getMountPointForSelectedDrive() | |
| // qDebug() << "Attempting to unmount temporary mountpoint: " << mnt << " --> on device: " << _dst; | |
| // if (mnt.isEmpty()) { | |
| // qDebug() << "Nothing to unmount."; | |
| // } else{ | |
| // QtConcurrent::run([mnt]() { | |
| // // Force‐lazy unmount | |
| // ::umount2(mnt.toUtf8().constData(), MNT_DETACH|MNT_FORCE); | |
| // QDir().rmdir(mnt); | |
| // qDebug() << "Background unmount of" << mnt << "complete"; | |
| // }); | |
| // } | |
| #endif | |
| } | |
| void ImageWriter::onUnraidLanguageDone() | |
| { | |
| emit success(); | |
| } |
🤖 Prompt for AI Agents
In src/imagewriter.cpp around lines 319 to 347, remove the entire block of
commented-out code inside the onUnraidLanguageDone() function under the
Q_OS_LINUX conditional. This includes all commented lines related to unmounting
the temp directory and the associated debug statements. Keep only the active
code and comments that are necessary, deleting the commented-out code to clean
up the codebase.
| bool _changedDefaultLanguage = _unraidLanguageManager.getCurrentLanguageCode() != "en_US"; | ||
|
|
||
| qDebug() << "Changed Default Language (ImgWriter): " << _changedDefaultLanguage; | ||
| _thread = new DownloadExtractThread(urlstr, _dst.toLatin1(), _expectedHash, _changedDefaultLanguage, this); | ||
| if (_repo.toString() == OSLIST_URL) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix variable naming convention.
Local variables should not start with underscore.
- bool _changedDefaultLanguage = _unraidLanguageManager.getCurrentLanguageCode() != "en_US";
+ bool changedDefaultLanguage = _unraidLanguageManager.getCurrentLanguageCode() != "en_US";
- qDebug() << "Changed Default Language (ImgWriter): " << _changedDefaultLanguage;
- _thread = new DownloadExtractThread(urlstr, _dst.toLatin1(), _expectedHash, _changedDefaultLanguage, this);
+ qDebug() << "Changed Default Language (ImgWriter): " << changedDefaultLanguage;
+ _thread = new DownloadExtractThread(urlstr, _dst.toLatin1(), _expectedHash, changedDefaultLanguage, this);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| bool _changedDefaultLanguage = _unraidLanguageManager.getCurrentLanguageCode() != "en_US"; | |
| qDebug() << "Changed Default Language (ImgWriter): " << _changedDefaultLanguage; | |
| _thread = new DownloadExtractThread(urlstr, _dst.toLatin1(), _expectedHash, _changedDefaultLanguage, this); | |
| if (_repo.toString() == OSLIST_URL) | |
| bool changedDefaultLanguage = _unraidLanguageManager.getCurrentLanguageCode() != "en_US"; | |
| qDebug() << "Changed Default Language (ImgWriter): " << changedDefaultLanguage; | |
| _thread = new DownloadExtractThread(urlstr, _dst.toLatin1(), _expectedHash, changedDefaultLanguage, this); | |
| if (_repo.toString() == OSLIST_URL) |
🤖 Prompt for AI Agents
In src/imagewriter.cpp around lines 464 to 468, the local variable
_changedDefaultLanguage starts with an underscore, which violates the naming
convention for local variables. Rename _changedDefaultLanguage to
changedDefaultLanguage by removing the leading underscore throughout its
declaration and usage in this code block.
| void ImageWriter::setUnraidOSLanguage(const QString &languageName) | ||
| { | ||
| QMap<QString, QString> availableLanguages = _unraidLanguageManager.getAvailableLanguages(); | ||
|
|
||
| if (!availableLanguages.empty()) { | ||
| qDebug() << "Setting Language to: " << availableLanguages.key(languageName, QString()); | ||
| _unraidLanguageManager.setCurrentLanguageCode( | ||
| availableLanguages.key(languageName, QString())); | ||
| } else { | ||
| //default to english | ||
| _unraidLanguageManager.setCurrentLanguageCode("en_US"); | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix potential bug when language name is not found.
If languageName is not found in availableLanguages, key() returns an empty QString which gets passed to setCurrentLanguageCode().
void ImageWriter::setUnraidOSLanguage(const QString &languageName)
{
QMap<QString, QString> availableLanguages = _unraidLanguageManager.getAvailableLanguages();
if (!availableLanguages.empty()) {
- qDebug() << "Setting Language to: " << availableLanguages.key(languageName, QString());
- _unraidLanguageManager.setCurrentLanguageCode(
- availableLanguages.key(languageName, QString()));
+ QString langCode = availableLanguages.key(languageName, QString());
+ if (!langCode.isEmpty()) {
+ qDebug() << "Setting Language to: " << langCode;
+ _unraidLanguageManager.setCurrentLanguageCode(langCode);
+ } else {
+ qDebug() << "Language not found: " << languageName << ", defaulting to English";
+ _unraidLanguageManager.setCurrentLanguageCode("en_US");
+ }
} else {
//default to english
_unraidLanguageManager.setCurrentLanguageCode("en_US");
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| void ImageWriter::setUnraidOSLanguage(const QString &languageName) | |
| { | |
| QMap<QString, QString> availableLanguages = _unraidLanguageManager.getAvailableLanguages(); | |
| if (!availableLanguages.empty()) { | |
| qDebug() << "Setting Language to: " << availableLanguages.key(languageName, QString()); | |
| _unraidLanguageManager.setCurrentLanguageCode( | |
| availableLanguages.key(languageName, QString())); | |
| } else { | |
| //default to english | |
| _unraidLanguageManager.setCurrentLanguageCode("en_US"); | |
| } | |
| } | |
| void ImageWriter::setUnraidOSLanguage(const QString &languageName) | |
| { | |
| QMap<QString, QString> availableLanguages = _unraidLanguageManager.getAvailableLanguages(); | |
| if (!availableLanguages.empty()) { | |
| QString langCode = availableLanguages.key(languageName, QString()); | |
| if (!langCode.isEmpty()) { | |
| qDebug() << "Setting Language to: " << langCode; | |
| _unraidLanguageManager.setCurrentLanguageCode(langCode); | |
| } else { | |
| qDebug() << "Language not found: " << languageName << ", defaulting to English"; | |
| _unraidLanguageManager.setCurrentLanguageCode("en_US"); | |
| } | |
| } else { | |
| //default to english | |
| _unraidLanguageManager.setCurrentLanguageCode("en_US"); | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/imagewriter.cpp around lines 283 to 295, the code uses
availableLanguages.key(languageName, QString()) which returns an empty QString
if languageName is not found, causing setCurrentLanguageCode to receive an
invalid code. Fix this by first checking if languageName exists in
availableLanguages keys; if not found, default to "en_US" before calling
setCurrentLanguageCode to ensure a valid language code is always set.
6 participants
Description
Not everyone's first language is English. This PR implements a language management system for Unraid OS installer, allowing users to select and install different languages during the USB creation process.
Note
Language installation can be completely bypassed by selecting "English" as default language.
Features Added:
Technical Implementation Overview
Files Created:
unraidlanguagemanager.h/cpp→ main class for downloading, parsing, patching, and state managementqmlcomponents/OSLanguageDropdown.qml→ Language dropdown componentFiles Modified:
imagewriter.h/cpp→ main class integrating UnraidLanguageManager into existing codebasemain.cpp→ integrates signal connections for gui progress updates and errorsmain.qml→ adds dropdown and connection between imagewriter and gui for lazy-loading dropdownCMakeLists.txt→ Added new source files and tweaked windows build configconfig.h→ AddedUNRAID_LANGUAGES_URLconstantqmlcomponents/qmldir→ Component registrationNote
Changes to these files were made to integrate language installation on Linux. Relates to temporary mounting/unmounting. Please refer to Known Issues - Linux
downloadextractthread.h/cpp→ integrate with Linux temp mounting/unmountingdownloadthread.h/cpp→ integrate with Linux temp mounting/unmountingProgram Flow
user opens app
User opens app → ImageWriter::getUnraidOSLanguages() → UnraidLanguageManager::requestUnraidLanguagesJson()user chooses language
User chooses language → ImageWriter::setUnraidOSLanguage() → UnraidLanguageManager::setCurrentLanguageCode()user formats usb
User clicks "Continue" → ImageWriter::startWrite() → If language != "en_US" → ImageWriter::installUnraidOSLanguage() → UnraidLanguageManager::installUnraidOSLanguage()post-install cleanup
Language installation complete → ImageWriter::onUnraidLanguageDone()Encountered, Unfixed Issues
All Platforms
DownloadExtractThread::extractMultiFileRunalways loops 3 times to find mountpoints even if it already found the mountpoint because thebreak;statement only breaks the firstforloop (it's a nested for-loop).Linux
ImageWriter::getMountPointForSelectedDrive()→DriveList::ListStorageDevices()→linuxdrivelist.cpplinuxdrivelist→lsblkin aQProcessand sometimes fails. Ends up passing empty mountpoint toinstallUnraidOSLanguageand causes error.DownloadExtractThread::extractMultiFileRuneither doesn't work or takes too long, freezing up the usb up despite successful message on GUI.Notes & Other Changes
CMakeList.txt.Next Steps
Connectionat the bottom ofmain.qmlto be with the other signal connections inmain.cppto maintain codebase consistency and centralize similar code.Summary by CodeRabbit
New Features
Bug Fixes
Chores
.gitignorerules to cover additional build and platform-specific files.