Skip to content

Fix PHP errors if file doesn't exist my_parse_ini_file#2372

Merged
limetech merged 2 commits into
unraid:masterfrom
Squidly271:patch-2
Sep 17, 2025
Merged

Fix PHP errors if file doesn't exist my_parse_ini_file#2372
limetech merged 2 commits into
unraid:masterfrom
Squidly271:patch-2

Conversation

@Squidly271

@Squidly271 Squidly271 commented Sep 17, 2025

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • Bug Fixes
    • Suppresses spurious PHP warnings when reading configuration files for cleaner logs.
    • Avoids noisy output for missing or unreadable INI files without changing behavior.
    • Ensures plugin not-installed checks consistently notify post-install hooks of the not-installed state, improving plugin check reliability.

@coderabbitai

coderabbitai Bot commented Sep 17, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

Suppress PHP warning on INI file reads by using @file_get_contents in my_parse_ini_file; and in the plugin not-installed path, call post_hooks(1) (constant) instead of post_hooks($error), leaving surrounding behavior unchanged.

Changes

Cohort / File(s) Summary
INI parsing wrapper
emhttp/plugins/dynamix/include/Wrappers.php
Replaced file_get_contents($file) with @file_get_contents($file) inside my_parse_ini_file to suppress PHP warnings; data still forwarded to my_parse_ini_string with no signature or parsing changes.
Plugin install check / post-hooks
emhttp/plugins/dynamix.plugin.manager/scripts/plugin
In the not-installed branch, changed post_hooks($error) to post_hooks(1) so hook invocation receives constant 1 as the error argument; write/exit flow unchanged.

Sequence Diagram(s)

sequenceDiagram
  participant Manager as plugin script
  participant Hooks as post_hooks

  Manager->>Manager: check plugin installed?
  alt not installed
    Manager->>Manager: write "not installed" message
    Manager->>Hooks: post_hooks(1)   %% changed: now a constant 1 is passed
    Manager->>Manager: exit(1)
  else installed
    Manager->>Manager: continue normal flow
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~15–30 minutes

Poem

I twitch my nose at warning cries,
I tuck them under silent skies.
One change, one hop, a gentle tweak—
Hooks get a one, and files don't squeak.
Thump-thump, I guard the quiet peaks. 🐰✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title clearly summarizes the primary change — preventing PHP warnings when my_parse_ini_file is called for a non-existent file — and directly corresponds to the Wrappers.php modification to suppress file_get_contents warnings, so it accurately reflects the PR's main intent.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

github-actions Bot commented Sep 17, 2025

Copy link
Copy Markdown

🔧 PR Test Plugin Available

A test plugin has been generated for this PR that includes the modified files.

Version: 2025.09.17.0025
Build: View Workflow Run

📥 Installation Instructions:

Install via Unraid Web UI:

  1. Go to Plugins → Install Plugin
  2. Copy and paste this URL:
https://preview.dl.unraid.net/pr-plugins/pr-2372/webgui-pr-2372.plg
  1. Click Install

Alternative: Direct Download

⚠️ Important Notes:

  • Testing only: This plugin is for testing PR changes
  • Backup included: Original files are automatically backed up
  • Easy removal: Files are restored when plugin is removed
  • Conflicts: Remove this plugin before installing production updates

📝 Modified Files:

Click to expand file list
emhttp/plugins/dynamix.plugin.manager/scripts/plugin
emhttp/plugins/dynamix/include/Wrappers.php

🔄 To Remove:

Navigate to Plugins → Installed Plugins and remove webgui-pr-2025.09.17.0025, or run:

plugin remove webgui-pr-2025.09.17.0025

🤖 This comment is automatically generated and will be updated with each new push to this PR.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ada892e and 51925bf.

📒 Files selected for processing (1)
  • emhttp/plugins/dynamix/include/Wrappers.php (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: Squidly271
PR: unraid/webgui#0
File: :0-0
Timestamp: 2025-05-31T05:10:13.120Z
Learning: Squidly271 prefers not to receive CodeRabbit reviews or feedback while PRs are in draft status. Only provide reviews after PRs are moved out of draft.

Comment thread emhttp/plugins/dynamix/include/Wrappers.php

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
emhttp/plugins/dynamix.plugin.manager/scripts/plugin (1)

733-733: Pass a descriptive error string to post_hooks() instead of a bare integer

post_hooks($error='') expects a string (docs: "error (empty if none)") and other callers pass strings — replace the numeric literal with a short message and still exit 1.

-    post_hooks(1);
+    $error = 'not installed';
+    post_hooks($error);
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 51925bf and 94425ec.

📒 Files selected for processing (1)
  • emhttp/plugins/dynamix.plugin.manager/scripts/plugin (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: Squidly271
PR: unraid/webgui#0
File: :0-0
Timestamp: 2025-05-31T05:10:13.120Z
Learning: Squidly271 prefers not to receive CodeRabbit reviews or feedback while PRs are in draft status. Only provide reviews after PRs are moved out of draft.

@limetech limetech merged commit 7debbde into unraid:master Sep 17, 2025
2 checks passed
@Squidly271 Squidly271 deleted the patch-2 branch September 17, 2025 04:38
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.

2 participants