Skip to content

Set default start_with_request to yes and remove the "default" option - #34

Merged
carlos-granados merged 3 commits into
php-debugger:mainfrom
carlos-granados:start_with_request_mode
Apr 14, 2026
Merged

carlos-granados merged 3 commits into
php-debugger:mainfrom
carlos-granados:start_with_request_mode

Conversation

@carlos-granados

@carlos-granados carlos-granados commented Mar 29, 2026 •

Copy link
Copy Markdown
Collaborator
  • Change the default value of "start_with_request" to "yes" so that by default the debugger always tries to connect
  • Remove the "default" value for xdebug.start_with_request configuration option as this only made sense when there were several modes for Xdebug
  • Simplify trigger checking logic by removing mode parameter dependencies since now there are not different modes
  • Clean up test suite by removing obsolete tests for the "default" option which has been removed

Comment thread xdebug.c Outdated
#endif

static const char *xdebug_start_with_request_types[5] = { "", "default", "yes", "no", "trigger" };
static const char *xdebug_start_with_request_types[5] = { "", "yes", "no", "trigger" };

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

should the size be 4?

Comment thread src/lib/lib.c Outdated
Comment on lines 483 to 492
if (trigger_name) {
xdebug_log(XLOG_CHAN_CONFIG, XLOG_INFO, "Trigger value for 'XDEBUG_TRIGGER' not found, falling back to '%s'", trigger_name);
trigger_value = xdebug_lib_find_in_globals(trigger_name, &found_in_global);
}

/* Also try PHP_DEBUGGER_SESSION alias */
if (!trigger_value && XDEBUG_MODE_IS(XDEBUG_MODE_STEP_DEBUG) && (for_mode == XDEBUG_MODE_STEP_DEBUG)) {
if (!trigger_value) {
trigger_name = "PHP_DEBUGGER_SESSION";
trigger_value = xdebug_lib_find_in_globals(trigger_name, &found_in_global);
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

These conditions can be simplified as I undersand, because trigger_name is always set, right?

Comment thread src/lib/lib.c Outdated
Comment on lines +538 to +540
/* Returns 1 if the mode is 'trigger', or 'default', where the default mode for
* a feature is to trigger. Does not check whether a trigger is present. */
int xdebug_lib_start_if_mode_is_trigger(int for_mode)
int xdebug_lib_start_if_mode_is_trigger()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Update comment

@carlos-granados
carlos-granados requested a review from pronskiy April 2, 2026 17:02
@carlos-granados
carlos-granados force-pushed the start_with_request_mode branch 2 times, most recently from 2f857a7 to 91627a0 Compare April 7, 2026 23:13
Comment thread src/lib/lib.c Outdated
}

int xdebug_lib_start_with_request(int for_mode)
int xdebug_lib_start_with_request()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
int xdebug_lib_start_with_request()
int xdebug_lib_start_with_request(void)

let's keep it consistent with the rest of the codebase and the modern style

Does it also make sense to add these to config.m4?
-Wstrict-prototypes
-Wold-style-definition
-Wmissing-prototypes

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Updated. I worked with the compiler warnings in another branch

Comment thread src/lib/lib.h
#define XDEBUG_START_WITH_REQUEST_TRIGGER 4
#define XDEBUG_START_WITH_REQUEST_YES 1
#define XDEBUG_START_WITH_REQUEST_NO 2
#define XDEBUG_START_WITH_REQUEST_TRIGGER 3

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is it safe to change the values here?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes, it is safe. These values are only used internally and not exposed to the user. In the xdebug_lib_set_start_with_request() function the actual value entered by the user, which will be a string will be transformed in one of these flags

@carlos-granados
carlos-granados requested a review from pronskiy April 9, 2026 16:38
@carlos-granados
carlos-granados force-pushed the start_with_request_mode branch from c6b4bf9 to 940159d Compare April 9, 2026 16:58
# Conflicts:
#	tests/debugger/bug02122.phpt
#	tests/debugger/start_with_request_default_break.phpt
#	tests/debugger/start_with_request_default_config.phpt
@carlos-granados
carlos-granados force-pushed the start_with_request_mode branch from 940159d to 73e4488 Compare April 13, 2026 17:20
@carlos-granados

Copy link
Copy Markdown
Collaborator Author

Rebased and ready to be merged

@carlos-granados
carlos-granados merged commit 037a996 into php-debugger:main Apr 14, 2026
11 checks passed
@carlos-granados
carlos-granados deleted the start_with_request_mode branch April 15, 2026 14:39
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