Stop using ob functions to echo pro global settings

[Crabcyborg]

$more_html is never used, so there's no point in not just triggering do_action.

[coderabbitai]

Walkthrough

The PR removes the $more_html parameter from the FrmSettingsController::maybe_render_currency_selector() method signature and updates the corresponding call site in the general settings view. Output buffering logic in the view is also removed.

Changes

Cohort / File(s)	Summary
Controller Method Signature Update classes/controllers/FrmSettingsController.php	Updated maybe_render_currency_selector() method signature to remove the $more_html parameter; removed corresponding @param docblock entry.
View Call Site Update classes/views/frm-settings/general.php	Removed output buffering (ob_start(), ob_get_clean(), echo) around the "Other" section; updated maybe_render_currency_selector() call to pass only $frm_settings parameter instead of ($frm_settings, $more_html).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The pull request title accurately describes the main change: removing output buffering functions to simplify the pro global settings echo logic.
Description check	✅ Passed	The description clearly explains the rationale for the change: the $more_html parameter was unused, so output buffering was unnecessary and a do_action trigger could be used directly instead.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 73f68f6 and 8f0d1cc.

📒 Files selected for processing (2)

• classes/controllers/FrmSettingsController.php
• classes/views/frm-settings/general.php

🧰 Additional context used

🧬 Code graph analysis (1)

classes/views/frm-settings/general.php (1)

classes/controllers/FrmSettingsController.php (2)

• FrmSettingsController (6-540)
• maybe_render_currency_selector (292-300)

🪛 PHPMD (2.15.0)

classes/controllers/FrmSettingsController.php

292-292: Avoid unused parameters such as '$frm_settings'. (undefined)

(UnusedFormalParameter)

⏰ 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). (6)

• GitHub Check: PHP 7.4 tests in WP 6.9
• GitHub Check: PHP 8 tests in WP 6.9
• GitHub Check: Cypress
• GitHub Check: Cypress
• GitHub Check: PHP 7.4 tests in WP 6.9
• GitHub Check: PHP 8 tests in WP 6.9

🔇 Additional comments (2)

classes/views/frm-settings/general.php (1)

36-45: LGTM! Simplified call removes unnecessary complexity.

The updated call correctly matches the new method signature. The removal of output buffering (as mentioned in the PR description) simplifies the code since $more_html was never actually used. The do_action('frm_settings_form', $frm_settings) on line 42 remains unchanged, ensuring Pro add-ons can still hook into the settings rendering.

classes/controllers/FrmSettingsController.php (1)

292-300: No issues found. The method signature change is safe and the code is correct.

The single call to this method in the codebase (general.php line 44) already uses the new signature with one parameter, and $more_html has no references elsewhere. Additionally, $frm_settings is not unused—it's consumed by the included template (_currency.php) which accesses $frm_settings->currency. This is a standard PHP pattern where included templates have access to parent scope variables.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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