Re: [python] [patch] PR python/13329

[Phil Muldoon <pmuldoon@redhat.com>]

Tom Tromey <tromey@redhat.com> writes:

> Phil> +/* 0 if we should not print the stack when catching a Python error,
> Phil> +   1 to print the full stack, 2 to just print the message.  */
> Phil> +static int gdbpy_should_print_stack = 2;
>
> It is more normal, though not exclusively the case (I think), to just
> use the string constants here.  Because they are constant strings you
> can just compare them with ==.
>
> That is, you can just remove this variable.
>
> Phil> +/* The extended exception printing variable.  'full' if we want to
> Phil> +   print the error message and stack, 'none' if we want to print
> Phil> +   nothing, and 'message' if we only want to print the error
> Phil> +   message.  */
> Phil> +static const char *gdbpy_should_print_stack_extended =
> Phil> +  python_excp_message;
>
> ... and rename this to gdbpy_should_print_stack and use it directly.
>
> Phil> +/* When setting the 'python print-stack' with an enum, set the
> Phil> +   gdbpy_should_print_stack correspondingly.  */
> Phil> +static void
> Phil> +set_python_excp_mode (char *args, int i, struct cmd_list_element *c)

I have to keep/add these functions around to be compatible with "maint
set python print-stack" which is a boolean parameter.  So it was my view
we still operate on the gdbpy_should_print_stack variable in
gdbpy_print_stack function, and have the "set function" just set
gdbpy_should_print_stack from the enum command.  I can do what you wish,
but I would have to write a "set" function to set the enum constant
instead.

> With the above change you won't need this function.
>
> Phil> +  add_setshow_enum_cmd ("print-stack", no_class, python_excp_enums,
> Phil> +			&gdbpy_should_print_stack_extended, _("\
> Phil> +Set mode for Python stack dump on error."), _("\
> Phil> +Show the mode of Python stack printing on error."), _("\
> Phil> +none  == no stack or message will be printed.\n\
> Phil> +full == a message and a stack will be printed.\n\
> Phil> +message == an error message without a stack will be printed."),
> Phil> +			set_python_excp_mode, NULL,
> Phil> +			&user_set_python_list,
> Phil> +			&user_show_python_list);
>
> I'm surprised this is backward compatible.

The user command "set python print-stack" has not been published in a
release so it is not compatible; I believe it is within our API pledge
to change whatever we like until that feature is released.  "maint set
python print-stack" still works, but is deprecated. For the variable
gdbpy_should_print_stack, 0 is still off, 1 is full, 2 is message only.
The "maint set python print-stack" will only ever set off or full.

> Phil> -gdb_test_no_output "set python print-stack on"
> Phil> +gdb_test_no_output "set python print-stack full"
>
> You should leave in some compatibility tests for the old values.

Do you mean use "maint set python print-stack" here?

> I didn't look but you should try the old code to see exactly what values
> it accepted.  I thought the boolean set/shows took more than just on/off.

I will, but the result remains the same. 0 for off, > 0 for on.

Cheers

Phil