fix: add aliases for all php_debugger_ functions - #107

@claude review

Claude finished @carlos-granados's task in 1m 41s —— View job

Claude PR Review

• Read PR diff and changed files
• Analyze stub, arginfo, and C changes
• Review new test files
• Post review feedback

Overall: this looks solid and well-scoped. Adding aliases for the four remaining public functions is exactly what CLAUDE.md calls out ("When adding a new public function, register both names") and the diagnostic-message change is a nice touch — the notice now names whichever alias was actually called instead of always saying xdebug_break().

What I checked

• Stub / arginfo consistency — php_xdebug.stub.php uses @alias and the regenerated php_xdebug_arginfo.h has matching ZEND_FALIAS entries plus per-alias arginfo_php_debugger_* #defines. Hash comment updated. Also confirmed the alias for xdebug_info was upgraded from reusing arginfo_xdebug_info to the new arginfo_php_debugger_info #define — cosmetic, but consistent with the others now.
• Dispatch mechanics — ZEND_FALIAS registers a new zend_function with the alias name that dispatches to ZEND_FN(xdebug_*). get_active_function_name() reads it off EG(current_execute_data)->func->common.function_name, so it will correctly return php_debugger_break when called via the alias and xdebug_break otherwise. Good use of the pattern.
• Tests — Both .phpt files cover the right ground: existence + arity + parity of return values in php_debugger_aliases.phpt, and alias-aware diagnostic wording in php_debugger_aliases_diagnostics.phpt.

Suggestions (all minor / non-blocking)

1. php_debugger_aliases.phpt doesn't exercise xdebug_notify / php_debugger_notify parity. The other functions in the alias table get a runtime equality check (xdebug_info === php_debugger_info, xdebug_is_debugger_active === php_debugger_is_debugger_active), but xdebug_notify doesn't. Since notify has an argument and a non-trivial return, adding e.g. var_dump(xdebug_notify('x') === php_debugger_notify('x')); would round out the coverage. xdebug_break / xdebug_connect_to_client are inherently covered by the diagnostics test, so this is really only the notify gap.

2. Stub-comment wording — "namespace" is misleading. The comments say "Alias of xdebug_break() in the php_debugger namespace", but PHP namespaces are a real concept (\Foo\Bar) and these are just prefixed global functions. Consider "php_debugger_ prefix" or "php_debugger_() naming"* to avoid confusion. Nit — not worth blocking on.

3. Consistency with the mode-dispatch pattern. PHP_FUNCTION(xdebug_break) starts with RETURN_FALSE_IF_MODE_IS_NOT(XDEBUG_MODE_STEP_DEBUG); (debugger.c:1211). When mode is off, both aliases return false silently. The diagnostic test sets xdebug.mode=debug so it's fine, but worth noting: aliases behave identically to canonical names across the entire mode-dispatch surface — no code change needed for that, just a confirmation that the surface stays uniform.

4. xdebug_log_ex in the diagnostic path. The log call uses a hardcoded "xdebug_break() ignored: ..." → now "%s() ignored: ...". Fine, but for anyone grep'ing xdebug logs for the historical xdebug_break() string, the log line will now vary based on which alias the user called. That's the desired behavior for the user-facing E_NOTICE, but for log parsers it's a mild break. Probably worth calling out in release notes rather than reverting.

Correctness / risk

Nothing in the C change looks risky:

• get_active_function_name() cannot return null in a user-invoked internal function call — safe to printf %s without a guard.
• No new INI/global state; no allocation; no lifetime concerns.
• ZEND_FALIAS is standard PHP-internal machinery, used identically to the existing php_debugger_info alias that has been in the codebase since the fork.

Looks good to merge once you decide on the notify parity check.
· Branch: fix/add-missing-functions

@claude I fixed 1 and 2 and preferred not to do anything about 3 and 4, can you review again?

Claude finished @carlos-granados's task in 58s —— View job

Claude PR Re-review

• Fetch latest changes and diff since last review
• Verify fix for suggestion 1 (notify parity check)
• Verify fix for suggestion 2 (stub comment wording)
• Post updated review

Both fixes look good. LGTM.

Verified

• Suggestion 1 (notify parity) — tests/library/php_debugger_aliases.phpt:33 now has var_dump(xdebug_notify('x') === php_debugger_notify('x')); and expects bool(true). This is well-defined: PHP_FUNCTION(xdebug_notify) at src/debugger/debugger.c:1287-1290 short-circuits to RETURN_FALSE when no debug connection is active, so both sides return false and the identity check holds. The test intentionally does not open a DBGp connection, which is the right call — keeps this as a pure alias-parity test rather than pulling in DebugClient boilerplate.

• Suggestion 2 (comment wording) — All five stub comments now read "Alias of xdebug_X() under the php_debugger_ prefix" (php_xdebug.stub.php:10, 19, 29, 41, 50). Clearer and no longer implies a PHP namespace. Stub hash in php_xdebug_arginfo.h:2 was regenerated to match.

• Suggestions 3 & 4 — Acknowledged as intentional non-changes. Both were non-blocking observations; skipping them is fine.

Nothing else outstanding

Diff is minimal (three files, 8/6), no C-side changes in this follow-up, no arginfo drift beyond the hash bump. Ready to merge from my end.
· Branch: fix/add-missing-functions