GeezerVison

@pronskiy seems that the Claude Code review is not working 😞

@claude review

Claude finished @pronskiy's task in 4m 47s —— View job

Claude PR Review

Nice cleanup — 2486 deletions for 3 additions, and the heavy lifting (deleting the three var_export_* output formats, file.c, normalize_path.c) is justified: DBGp uses XML, so the HTML/line/text formatters were genuinely orphaned. I cross-checked every removed symbol against the remaining sources, the php_xdebug.stub.php/arginfo, and CI workflows. Two issues, one of them a real build breakage in a path the project documents.

🔴 1. tests/ctest/Makefile still references the deleted normalize_path.c — broken C-test build

This PR deletes src/lib/normalize_path.c / .h, but tests/ctest/Makefile keeps it as a hard dependency:

26: xdebuglib.a: hash.o llist.o normalize_path.o str.o trim.o xdebug_strndup.o
27: 	ar -rc xdebuglib.a hash.o llist.o normalize_path.o str.o trim.o xdebug_strndup.o
35: normalize_path.o: ../../src/lib/normalize_path.c
36: 	gcc -c ${CPPFLAGS} -o normalize_path.o ../../src/lib/normalize_path.c
48: 	rm xdebuglib.a hash.o llist.o normalize_path.o str.o trim.o xdebug_strndup.o ctest afl-test-parser afl-test-mapper

Running make (or make test / make fuzz) in tests/ctest/ will now fail at the normalize_path.o rule. CI doesn’t exercise this Makefile (I checked .github/workflows/*.yml), so green CI doesn't catch it — but CLAUDE.md explicitly documents this directory as the path-map parser unit-test entry point.

This is also safe to fix: parser.c already carries its own self-contained normalize_path_xdebug_str_in_place (Win-only, plus a no-op macro elsewhere), so the static lib doesn't need a normalize_path.o at all. Just drop those 4 references from the Makefile.

Fix this →

🟡 2. Orphaned output_is_tty plumbing left behind

The only consumer of output_is_tty was xdebug_is_output_tty(), which this PR deleted from xdebug.c. The rest of the plumbing is still there:

• php_xdebug.h:46-48 — OUTPUT_NOT_CHECKED / OUTPUT_IS_TTY / OUTPUT_NOT_TTY macros
• src/base/base_globals.h:49 — int output_is_tty; field on xdebug_base_globals_t
• xdebug.c:358 — xg->output_is_tty = OUTPUT_NOT_CHECKED; in init
• src/base/base.c:896 — XG_BASE(output_is_tty) = OUTPUT_NOT_CHECKED; in RINIT

Since the only reader is gone, the writes and the field/macros are dead. Worth dropping in this PR for consistency with everything else you’ve cleaned up.

Fix this →

✅ Verified clean

I grepped each removed symbol against the rest of the tree — all of these are genuinely orphaned with no remaining callers:

• xdebug_str_to_str, xdebug_get_compiled_variable_name (compat.c/h)
• xdebug_hash_key_to_str (hash.c/h)
• xdebug_llist_insert_prev, xdebug_llist_remove_next, xdebug_llist_remove_prev, xdebug_llist_jump (llist.c/h)
• xdebug_set_remove, xdebug_set_dump (set.c/h)
• xdebug_str_add_uint64 (str.c/h)
• xdebug_strrstr, xdebug_format_output_filename, xdebug_format_file_link, xdebug_format_filename (usefulstuff.c/h)
• xdebug_error_type_simple, xdebug_get_property_type, xdebug_var_get_nolimit_options, xdebug_add_variable_attributes (var.c/h)
• xdebug_var_xml_attach_static_var_with_contents (var_export_xml.h)
• xdebug_debugger_reset_ide_key (debugger.c/h)
• xdebug_dbgp_log declaration (handler_dbgp.h)
• xdebug_init_oparray — registered as op_array_ctor_func_t, replaced with NULL in zend_extension_entry. NULL is the canonical "not interested" value for that slot, so the behavior is unchanged and you save one indirection per oparray construction.
• XDEBUG_MAKE_STD_ZVAL, HASH_KEY_SIZEOF macros (compat.h) — the surviving HASH_KEY_STRLEN, HASH_KEY_IS_NUMERIC, HASH_APPLY_KEY_*, STR_NAME_* macros all still have active users; the two you removed didn't.
• The #if 0 block in debugger.c and the dead else arm in mm.h are correct deletions.
• var_export_html.c / var_export_line.c / var_export_text.c and their headers — searched for any consumer (get_zval_value_html/line/text, XDEBUG_VAR_TYPE_TEXT/HTML/LINE, the .h includes) and found none. Safe.
• php_xdebug.stub.php / php_xdebug_arginfo.h are unaffected — no PHP-visible function signatures depended on the removed C helpers.
• config.m4 and config.w32 are correctly trimmed of the deleted source files.

Summary

The removals themselves are correct and the diff is well-scoped. The blocker is tests/ctest/Makefile referencing a deleted file — this PR breaks make in that directory. The output_is_tty leftovers are a smaller polish item but logically belong in the same cleanup. Once those two are addressed this looks ready to merge.

• Read all PR diffs
• Verify removed functions are truly unused (cross-check against remaining sources, tests, and stub file)
• Check that removed files are not referenced in config.m4 / config.w32 / Makefile
• Verify xdebug.c removals don't break INI handling or registration
• Check php_xdebug.stub.php and arginfo for any orphaned references
• Look for transitive dead code unlocked by these removals
• Post review
  · branch: refactor/remove-unused-code

@claude review again

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

Claude PR Re-Review

Re-reviewed against c5ac653. Both blockers from the previous round are fixed and I can't find anything new that warrants holding the PR.

✅ 1. tests/ctest/Makefile — fixed

normalize_path.o is gone from the xdebuglib.a rule, the build rule, and the clean target (verified at tests/ctest/Makefile:26-27, :35-36 deleted, :42). The remaining static lib (hash.o llist.o str.o trim.o xdebug_strndup.o) is sufficient — parser.c:389-400 still defines its own self-contained normalize_path_xdebug_str_in_place (Win-only, no-op elsewhere), so nothing in the C-test build needs the deleted src/lib/normalize_path.c. make, make test, and make fuzz should all work again.

✅ 2. output_is_tty plumbing — fully removed

Verified all four call sites are gone:

• php_xdebug.h — OUTPUT_NOT_CHECKED / OUTPUT_IS_TTY / OUTPUT_NOT_TTY macros removed
• src/base/base_globals.h — int output_is_tty; field removed
• xdebug.c — init line in xdebug_init_base_globals removed
• src/base/base.c — RINIT write removed

grep -rn "output_is_tty\|OUTPUT_NOT_CHECKED\|OUTPUT_IS_TTY\|OUTPUT_NOT_TTY" across *.c/*.h/*.cpp returns zero hits.

✅ Other spot-checks

• No source file #includes any of the deleted headers (file.h, normalize_path.h, var_export_html.h, var_export_line.h, var_export_text.h).
• No remaining references to get_zval_value_{html,line,text} or XDEBUG_VAR_TYPE_{HTML,LINE,TEXT}.
• xdebug_init_oparray has zero remaining references.
• php_xdebug.stub.php / php_xdebug_arginfo.h untouched — no PHP-visible signatures depended on the removed C helpers.

Summary

LGTM — ready to merge. The two issues raised in the prior review are addressed, and the cleanup is now self-consistent.

• Pull latest branch state and check what changed since last review
• Verify the tests/ctest/Makefile normalize_path.o issue is resolved
• Verify the output_is_tty plumbing issue is resolved
• Re-scan removed symbols for any new dead code
• Post review
  · branch: refactor/remove-unused-code