Fix: Handle CRLF in syslinux and grub configs#2613
Conversation
WalkthroughThis pull request updates boot parameter configuration parsing functions in a shell script to handle CRLF (DOS-style) line endings. The changes apply carriage return stripping via Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
🔧 PR Test Plugin AvailableA test plugin has been generated for this PR that includes the modified files. Version: 📥 Installation Instructions:Install via Unraid Web UI:
Alternative: Direct Download
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
emhttp/plugins/dynamix/scripts/manage_boot_params.sh (1)
753-763: Avoid re-reading and re-normalizing the same file three times invalidate_config().At Line 754, Line 758, and Line 762,
tr -d '\r' < "$cfg_file"is repeated. You can normalize once and run all checks against that result for simpler/faster validation.♻️ Suggested simplification
- if ! tr -d '\r' < "$cfg_file" | grep -qi "^label unraid OS$"; then + local normalized_cfg + normalized_cfg="$(tr -d '\r' < "$cfg_file")" + + if ! grep -qi "^label unraid OS$" <<< "$normalized_cfg"; then return 1 fi - if ! tr -d '\r' < "$cfg_file" | grep -qi "^label unraid OS GUI Mode$"; then + if ! grep -qi "^label unraid OS GUI Mode$" <<< "$normalized_cfg"; then return 1 fi - if ! tr -d '\r' < "$cfg_file" | grep -qi "^label unraid OS Safe Mode"; then + if ! grep -qi "^label unraid OS Safe Mode" <<< "$normalized_cfg"; then return 1 fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@emhttp/plugins/dynamix/scripts/manage_boot_params.sh` around lines 753 - 763, In validate_config(), avoid calling tr -d '\r' < "$cfg_file" three times; instead normalize the file once (e.g., read the file into a variable or a temp buffer with tr -d '\r') and run the three grep -qi checks against that normalized content for the labels "label unraid OS", "label unraid OS GUI Mode", and "label unraid OS Safe Mode" (preserving the existing return 1 behavior if any check fails); reference validate_config(), cfg_file, and the three label strings to locate where to replace the repeated tr invocations.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@emhttp/plugins/dynamix/scripts/manage_boot_params.sh`:
- Around line 753-763: In validate_config(), avoid calling tr -d '\r' <
"$cfg_file" three times; instead normalize the file once (e.g., read the file
into a variable or a temp buffer with tr -d '\r') and run the three grep -qi
checks against that normalized content for the labels "label unraid OS", "label
unraid OS GUI Mode", and "label unraid OS Safe Mode" (preserving the existing
return 1 behavior if any check fails); reference validate_config(), cfg_file,
and the three label strings to locate where to replace the repeated tr
invocations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a55bddcd-d499-466f-8314-e5c032b656d0
📒 Files selected for processing (1)
emhttp/plugins/dynamix/scripts/manage_boot_params.sh
Summary by CodeRabbit