* [PATCH 1/3] gdb: use intrusive_list for cmd_list_element aliases list @ 2021-12-01 16:41 Simon Marchi 2021-12-01 16:41 ` [PATCH 2/3] gdb: change some alias functions parameters to const-reference Simon Marchi ` (2 more replies) 0 siblings, 3 replies; 10+ messages in thread From: Simon Marchi @ 2021-12-01 16:41 UTC (permalink / raw) To: gdb-patches Change the manually-implemented linked list to use intrusive_list. This is not strictly necessary, but it makes the code much simpler. Change-Id: Idd08090ebf2db8bdcf68e85ef72a9635f1584ccc --- gdb/cli/cli-decode.c | 82 ++++++++++++++++++++------------------------ gdb/cli/cli-decode.h | 18 +++++++--- 2 files changed, 50 insertions(+), 50 deletions(-) diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c index 030cba443386..39f9eb54dc20 100644 --- a/gdb/cli/cli-decode.c +++ b/gdb/cli/cli-decode.c @@ -30,12 +30,10 @@ static void undef_cmd_error (const char *, const char *); -static struct cmd_list_element *delete_cmd (const char *name, - struct cmd_list_element **list, - struct cmd_list_element **prehook, - struct cmd_list_element **prehookee, - struct cmd_list_element **posthook, - struct cmd_list_element **posthookee); +static cmd_list_element::aliases_list_type delete_cmd + (const char *name, cmd_list_element **list, cmd_list_element **prehook, + cmd_list_element **prehookee, cmd_list_element **posthook, + cmd_list_element **posthookee); static struct cmd_list_element *find_cmd (const char *command, int len, @@ -171,20 +169,24 @@ do_add_cmd (const char *name, enum command_class theclass, { struct cmd_list_element *c = new struct cmd_list_element (name, theclass, doc); - struct cmd_list_element *p, *iter; /* Turn each alias of the old command into an alias of the new command. */ c->aliases = delete_cmd (name, list, &c->hook_pre, &c->hookee_pre, &c->hook_post, &c->hookee_post); - for (iter = c->aliases; iter; iter = iter->alias_chain) - iter->alias_target = c; + + for (cmd_list_element &alias : c->aliases) + alias.alias_target = c; + if (c->hook_pre) c->hook_pre->hookee_pre = c; + if (c->hookee_pre) c->hookee_pre->hook_pre = c; + if (c->hook_post) c->hook_post->hookee_post = c; + if (c->hookee_post) c->hookee_post->hook_post = c; @@ -195,7 +197,7 @@ do_add_cmd (const char *name, enum command_class theclass, } else { - p = *list; + cmd_list_element *p = *list; while (p->next && strcmp (p->next->name, name) <= 0) { p = p->next; @@ -296,8 +298,7 @@ add_alias_cmd (const char *name, cmd_list_element *target, c->allow_unknown = target->allow_unknown; c->abbrev_flag = abbrev_flag; c->alias_target = target; - c->alias_chain = target->aliases; - target->aliases = c; + target->aliases.push_front (*c); return c; } @@ -1195,14 +1196,13 @@ add_setshow_zuinteger_cmd (const char *name, command_class theclass, show_list); } -/* Remove the command named NAME from the command list. Return the - list commands which were aliased to the deleted command. If the - command had no aliases, return NULL. The various *HOOKs are set to - the pre- and post-hook commands for the deleted command. If the - command does not have a hook, the corresponding out parameter is - set to NULL. */ +/* Remove the command named NAME from the command list. Return the list + commands which were aliased to the deleted command. The various *HOOKs are + set to the pre- and post-hook commands for the deleted command. If the + command does not have a hook, the corresponding out parameter is set to + NULL. */ -static struct cmd_list_element * +static cmd_list_element::aliases_list_type delete_cmd (const char *name, struct cmd_list_element **list, struct cmd_list_element **prehook, struct cmd_list_element **prehookee, @@ -1211,7 +1211,7 @@ delete_cmd (const char *name, struct cmd_list_element **list, { struct cmd_list_element *iter; struct cmd_list_element **previous_chain_ptr; - struct cmd_list_element *aliases = NULL; + cmd_list_element::aliases_list_type aliases; *prehook = NULL; *prehookee = NULL; @@ -1238,21 +1238,14 @@ delete_cmd (const char *name, struct cmd_list_element **list, /* Update the link. */ *previous_chain_ptr = iter->next; - aliases = iter->aliases; + aliases = std::move (iter->aliases); /* If this command was an alias, remove it from the list of aliases. */ if (iter->is_alias ()) { - struct cmd_list_element **prevp = &iter->alias_target->aliases; - struct cmd_list_element *a = *prevp; - - while (a != iter) - { - prevp = &a->alias_chain; - a = *prevp; - } - *prevp = iter->alias_chain; + auto it = iter->alias_target->aliases.iterator_to (*iter); + iter->alias_target->aliases.erase (it); } delete iter; @@ -1351,11 +1344,9 @@ static void fput_aliases_definition_styled (struct cmd_list_element *cmd, struct ui_file *stream) { - for (cmd_list_element *iter = cmd->aliases; - iter != nullptr; - iter = iter->alias_chain) - if (!iter->default_args.empty ()) - fput_alias_definition_styled (iter, stream); + for (cmd_list_element &alias : cmd->aliases) + if (!alias.default_args.empty ()) + fput_alias_definition_styled (&alias, stream); } @@ -1370,17 +1361,17 @@ fput_command_names_styled (struct cmd_list_element *c, bool always_fput_c_name, const char *postfix, struct ui_file *stream) { - if (always_fput_c_name || c->aliases != nullptr) + if (always_fput_c_name || !c->aliases.empty ()) fput_command_name_styled (c, stream); - for (cmd_list_element *iter = c->aliases; iter; iter = iter->alias_chain) + for (cmd_list_element &alias : c->aliases) { fputs_filtered (", ", stream); wrap_here (" "); - fput_command_name_styled (iter, stream); + fput_command_name_styled (&alias, stream); } - if (always_fput_c_name || c->aliases != nullptr) + if (always_fput_c_name || !c->aliases.empty ()) fputs_filtered (postfix, stream); } @@ -1453,14 +1444,15 @@ apropos_cmd (struct ui_file *stream, print_doc_of_command (c, prefix, verbose, regex, stream); /* Try to match against the name of the aliases. */ - for (cmd_list_element *iter = c->aliases; - returnvalue < 0 && iter; - iter = iter->alias_chain) + for (const cmd_list_element &alias : c->aliases) { - name_len = strlen (iter->name); - returnvalue = regex.search (iter->name, name_len, 0, name_len, NULL); + name_len = strlen (alias.name); + returnvalue = regex.search (alias.name, name_len, 0, name_len, NULL); if (returnvalue >= 0) - print_doc_of_command (c, prefix, verbose, regex, stream); + { + print_doc_of_command (c, prefix, verbose, regex, stream); + break; + } } } if (c->doc != NULL && returnvalue < 0) diff --git a/gdb/cli/cli-decode.h b/gdb/cli/cli-decode.h index f7945ba2bf5f..1d3a3db786ef 100644 --- a/gdb/cli/cli-decode.h +++ b/gdb/cli/cli-decode.h @@ -26,6 +26,7 @@ #include "gdb_regex.h" #include "cli-script.h" #include "completer.h" +#include "gdbsupport/intrusive_list.h" /* Not a set/show command. Note that some commands which begin with "set" or "show" might be in this category, if their syntax does @@ -246,11 +247,18 @@ struct cmd_list_element aliased command can be located in case it has been hooked. */ struct cmd_list_element *alias_target = nullptr; - /* Start of a linked list of all aliases of this command. */ - struct cmd_list_element *aliases = nullptr; - - /* Link pointer for aliases on an alias list. */ - struct cmd_list_element *alias_chain = nullptr; + /* Node to link aliases on an alias list. */ + using aliases_list_node_type + = intrusive_list_node<cmd_list_element>; + aliases_list_node_type aliases_list_node; + + /* Linked list of all aliases of this command. */ + using aliases_list_member_node_type + = intrusive_member_node<cmd_list_element, + &cmd_list_element::aliases_list_node>; + using aliases_list_type + = intrusive_list<cmd_list_element, aliases_list_member_node_type>; + aliases_list_type aliases; /* If non-null, the pointer to a field in 'struct cli_suppress_notification', which will be set to true in cmd_func -- 2.33.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/3] gdb: change some alias functions parameters to const-reference 2021-12-01 16:41 [PATCH 1/3] gdb: use intrusive_list for cmd_list_element aliases list Simon Marchi @ 2021-12-01 16:41 ` Simon Marchi 2021-12-03 16:13 ` Pedro Alves 2021-12-01 16:41 ` [PATCH 3/3] gdb: don't show deprecated aliases Simon Marchi 2021-12-03 16:13 ` [PATCH 1/3] gdb: use intrusive_list for cmd_list_element aliases list Pedro Alves 2 siblings, 1 reply; 10+ messages in thread From: Simon Marchi @ 2021-12-01 16:41 UTC (permalink / raw) To: gdb-patches Now that we use intrusive list to link aliases, it becomes easier to pass cmd_list_element arguments by const-reference rather than by pointer to some functions, change a few. Change-Id: Id0df648ed26e9447da0671fc2c858981cda31df8 --- gdb/cli/cli-decode.c | 64 ++++++++++++++++++++++---------------------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c index 39f9eb54dc20..b8be3f54921f 100644 --- a/gdb/cli/cli-decode.c +++ b/gdb/cli/cli-decode.c @@ -86,7 +86,7 @@ lookup_cmd_with_subcommands (cmd_list_element **subcommands, } static void -print_help_for_command (struct cmd_list_element *c, +print_help_for_command (const cmd_list_element &c, bool recurse, struct ui_file *stream); static void @@ -1314,39 +1314,39 @@ add_com_suppress_notification (const char *name, enum command_class theclass, /* Print the prefix of C followed by name of C in title style. */ static void -fput_command_name_styled (struct cmd_list_element *c, struct ui_file *stream) +fput_command_name_styled (const cmd_list_element &c, struct ui_file *stream) { std::string prefixname - = c->prefix == nullptr ? "" : c->prefix->prefixname (); + = c.prefix == nullptr ? "" : c.prefix->prefixname (); fprintf_styled (stream, title_style.style (), "%s%s", - prefixname.c_str (), c->name); + prefixname.c_str (), c.name); } /* Print the definition of alias C using title style for alias and aliased command. */ static void -fput_alias_definition_styled (struct cmd_list_element *c, +fput_alias_definition_styled (const cmd_list_element &c, struct ui_file *stream) { - gdb_assert (c->is_alias ()); + gdb_assert (c.is_alias ()); fputs_filtered (" alias ", stream); fput_command_name_styled (c, stream); fprintf_filtered (stream, " = "); - fput_command_name_styled (c->alias_target, stream); - fprintf_filtered (stream, " %s\n", c->default_args.c_str ()); + fput_command_name_styled (*c.alias_target, stream); + fprintf_filtered (stream, " %s\n", c.default_args.c_str ()); } /* Print the definition of the aliases of CMD that have default args. */ static void -fput_aliases_definition_styled (struct cmd_list_element *cmd, +fput_aliases_definition_styled (const cmd_list_element &cmd, struct ui_file *stream) { - for (cmd_list_element &alias : cmd->aliases) + for (const cmd_list_element &alias : cmd.aliases) if (!alias.default_args.empty ()) - fput_alias_definition_styled (&alias, stream); + fput_alias_definition_styled (alias, stream); } @@ -1357,21 +1357,21 @@ fput_aliases_definition_styled (struct cmd_list_element *cmd, */ static void -fput_command_names_styled (struct cmd_list_element *c, +fput_command_names_styled (const cmd_list_element &c, bool always_fput_c_name, const char *postfix, struct ui_file *stream) { - if (always_fput_c_name || !c->aliases.empty ()) + if (always_fput_c_name || !c.aliases.empty ()) fput_command_name_styled (c, stream); - for (cmd_list_element &alias : c->aliases) + for (const cmd_list_element &alias : c.aliases) { fputs_filtered (", ", stream); wrap_here (" "); - fput_command_name_styled (&alias, stream); + fput_command_name_styled (alias, stream); } - if (always_fput_c_name || !c->aliases.empty ()) + if (always_fput_c_name || !c.aliases.empty ()) fputs_filtered (postfix, stream); } @@ -1380,7 +1380,7 @@ fput_command_names_styled (struct cmd_list_element *c, otherwise print only one-line help for command C. */ static void -print_doc_of_command (struct cmd_list_element *c, const char *prefix, +print_doc_of_command (const cmd_list_element &c, const char *prefix, bool verbose, compiled_regex &highlight, struct ui_file *stream) { @@ -1396,12 +1396,12 @@ print_doc_of_command (struct cmd_list_element *c, const char *prefix, { fputs_filtered ("\n", stream); fput_aliases_definition_styled (c, stream); - fputs_highlighted (c->doc, highlight, stream); + fputs_highlighted (c.doc, highlight, stream); fputs_filtered ("\n", stream); } else { - print_doc_line (stream, c->doc, false); + print_doc_line (stream, c.doc, false); fputs_filtered ("\n", stream); fput_aliases_definition_styled (c, stream); } @@ -1441,7 +1441,7 @@ apropos_cmd (struct ui_file *stream, /* Try to match against the name. */ returnvalue = regex.search (c->name, name_len, 0, name_len, NULL); if (returnvalue >= 0) - print_doc_of_command (c, prefix, verbose, regex, stream); + print_doc_of_command (*c, prefix, verbose, regex, stream); /* Try to match against the name of the aliases. */ for (const cmd_list_element &alias : c->aliases) @@ -1450,7 +1450,7 @@ apropos_cmd (struct ui_file *stream, returnvalue = regex.search (alias.name, name_len, 0, name_len, NULL); if (returnvalue >= 0) { - print_doc_of_command (c, prefix, verbose, regex, stream); + print_doc_of_command (*c, prefix, verbose, regex, stream); break; } } @@ -1461,7 +1461,7 @@ apropos_cmd (struct ui_file *stream, /* Try to match against documentation. */ if (regex.search (c->doc, doc_len, 0, doc_len, NULL) >= 0) - print_doc_of_command (c, prefix, verbose, regex, stream); + print_doc_of_command (*c, prefix, verbose, regex, stream); } /* Check if this command has subcommands. */ if (c->is_prefix ()) @@ -1524,8 +1524,8 @@ help_cmd (const char *command, struct ui_file *stream) /* If the user asked 'help somecommand' and there is no alias, the false indicates to not output the (single) command name. */ - fput_command_names_styled (c, false, "\n", stream); - fput_aliases_definition_styled (c, stream); + fput_command_names_styled (*c, false, "\n", stream); + fput_aliases_definition_styled (*c, stream); fputs_filtered (c->doc, stream); fputs_filtered ("\n", stream); @@ -1664,7 +1664,7 @@ help_all (struct ui_file *stream) fprintf_filtered (stream, "\nUnclassified commands\n\n"); seen_unclassified = 1; } - print_help_for_command (c, true, stream); + print_help_for_command (*c, true, stream); } } @@ -1716,23 +1716,23 @@ print_doc_line (struct ui_file *stream, const char *str, If RECURSE is non-zero, also print one-line descriptions of all prefixed subcommands. */ static void -print_help_for_command (struct cmd_list_element *c, +print_help_for_command (const cmd_list_element &c, bool recurse, struct ui_file *stream) { fput_command_names_styled (c, true, " -- ", stream); - print_doc_line (stream, c->doc, false); + print_doc_line (stream, c.doc, false); fputs_filtered ("\n", stream); - if (!c->default_args.empty ()) + if (!c.default_args.empty ()) fput_alias_definition_styled (c, stream); fput_aliases_definition_styled (c, stream); if (recurse - && c->is_prefix () - && c->abbrev_flag == 0) + && c.is_prefix () + && c.abbrev_flag == 0) /* Subcommands of a prefix command typically have 'all_commands' as class. If we pass CLASS to recursive invocation, most often we won't see anything. */ - help_cmd_list (*c->subcommands, all_commands, true, stream); + help_cmd_list (*c.subcommands, all_commands, true, stream); } /* @@ -1789,7 +1789,7 @@ help_cmd_list (struct cmd_list_element *list, enum command_class theclass, as this would show the (possibly very long) not very useful list of sub-commands of the aliased command. */ print_help_for_command - (c, + (*c, recurse && (theclass != class_alias || !c->is_alias ()), stream); continue; -- 2.33.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] gdb: change some alias functions parameters to const-reference 2021-12-01 16:41 ` [PATCH 2/3] gdb: change some alias functions parameters to const-reference Simon Marchi @ 2021-12-03 16:13 ` Pedro Alves 2021-12-03 21:49 ` Simon Marchi 0 siblings, 1 reply; 10+ messages in thread From: Pedro Alves @ 2021-12-03 16:13 UTC (permalink / raw) To: Simon Marchi, gdb-patches On 2021-12-01 16:41, Simon Marchi via Gdb-patches wrote: > Now that we use intrusive list to link aliases, it becomes easier to > pass cmd_list_element arguments by const-reference rather than by > pointer to some functions, change a few. > > Change-Id: Id0df648ed26e9447da0671fc2c858981cda31df8 LGTM. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] gdb: change some alias functions parameters to const-reference 2021-12-03 16:13 ` Pedro Alves @ 2021-12-03 21:49 ` Simon Marchi 0 siblings, 0 replies; 10+ messages in thread From: Simon Marchi @ 2021-12-03 21:49 UTC (permalink / raw) To: Pedro Alves, gdb-patches On 2021-12-03 11:13, Pedro Alves wrote: > On 2021-12-01 16:41, Simon Marchi via Gdb-patches wrote: >> Now that we use intrusive list to link aliases, it becomes easier to >> pass cmd_list_element arguments by const-reference rather than by >> pointer to some functions, change a few. >> >> Change-Id: Id0df648ed26e9447da0671fc2c858981cda31df8 > > LGTM. > Thanks, I pushed the first two patches, since they don't introduce a change in behavior, while I wait for feedback from Philippe on the 3rd one. Simon ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/3] gdb: don't show deprecated aliases 2021-12-01 16:41 [PATCH 1/3] gdb: use intrusive_list for cmd_list_element aliases list Simon Marchi 2021-12-01 16:41 ` [PATCH 2/3] gdb: change some alias functions parameters to const-reference Simon Marchi @ 2021-12-01 16:41 ` Simon Marchi 2021-12-03 16:16 ` Pedro Alves 2021-12-03 16:13 ` [PATCH 1/3] gdb: use intrusive_list for cmd_list_element aliases list Pedro Alves 2 siblings, 1 reply; 10+ messages in thread From: Simon Marchi @ 2021-12-01 16:41 UTC (permalink / raw) To: gdb-patches I don't think it's very useful to show deprecated aliases to the user. It encourages the user to use them, when the goal is the opposite. For example, before: (gdb) help set index-cache enabled set index-cache enabled, set index-cache off, set index-cache on alias set index-cache off = set index-cache enabled off alias set index-cache on = set index-cache enabled on Enable the index cache. When on, enable the use of the index cache. (gdb) help set index-cache on Warning: 'set index-cache on', an alias for the command 'set index-cache enabled', is deprecated. Use 'set index-cache enabled on'. set index-cache enabled, set index-cache off, set index-cache on alias set index-cache off = set index-cache enabled off alias set index-cache on = set index-cache enabled on Enable the index cache. When on, enable the use of the index cache. After: (gdb) help set index-cache enabled Enable the index cache. When on, enable the use of the index cache. (gdb) help set index-cache on Warning: 'set index-cache on', an alias for the command 'set index-cache enabled', is deprecated. Use 'set index-cache enabled on'. Enable the index cache. When on, enable the use of the index cache. Change-Id: I989b618a5ad96ba975367e9d16db95523cd57a4c --- gdb/cli/cli-decode.c | 29 ++++++++++++++++++++++++++--- gdb/testsuite/gdb.base/commands.exp | 22 ++++++++++++++++++++++ 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c index b8be3f54921f..9b965ea99891 100644 --- a/gdb/cli/cli-decode.c +++ b/gdb/cli/cli-decode.c @@ -1345,7 +1345,7 @@ fput_aliases_definition_styled (const cmd_list_element &cmd, struct ui_file *stream) { for (const cmd_list_element &alias : cmd.aliases) - if (!alias.default_args.empty ()) + if (!alias.cmd_deprecated && !alias.default_args.empty ()) fput_alias_definition_styled (alias, stream); } @@ -1361,17 +1361,40 @@ fput_command_names_styled (const cmd_list_element &c, bool always_fput_c_name, const char *postfix, struct ui_file *stream) { - if (always_fput_c_name || !c.aliases.empty ()) + /* First, check if we are going to print something. That is, either if + ALWAYS_FPUT_C_NAME is true or if there exists at least one non-deprecated + alias. */ + + auto print_alias = [] (const cmd_list_element &alias) + { + return !alias.cmd_deprecated; + }; + + bool print_something = always_fput_c_name; + if (!print_something) + for (const cmd_list_element &alias : c.aliases) + { + if (!print_alias (alias)) + continue; + + print_something = true; + break; + } + + if (print_something) fput_command_name_styled (c, stream); for (const cmd_list_element &alias : c.aliases) { + if (!print_alias (alias)) + continue; + fputs_filtered (", ", stream); wrap_here (" "); fput_command_name_styled (alias, stream); } - if (always_fput_c_name || !c.aliases.empty ()) + if (print_something) fputs_filtered (postfix, stream); } diff --git a/gdb/testsuite/gdb.base/commands.exp b/gdb/testsuite/gdb.base/commands.exp index 6785f9532b11..1dca419f0fb7 100644 --- a/gdb/testsuite/gdb.base/commands.exp +++ b/gdb/testsuite/gdb.base/commands.exp @@ -709,6 +709,27 @@ maintenance deprecate set qqq_aaa" file delete $file1 } +# Test that the help for a command does not show deprecated aliases. + +proc_with_prefix deprecated_command_alias_help_test {} { + gdb_test_multiline "define real_command" \ + "define real_command" "End with a line saying just \"end\".." \ + "print 1" "" \ + "end" "" + + gdb_test_no_output "alias alias_command = real_command" + gdb_test_no_output "alias alias_with_args_command = real_command 123" + + gdb_test "help real_command" \ + "real_command, alias_with_args_command, alias_command\r\n alias alias_with_args_command = real_command 123\r\nUser-defined." \ + "help real_command, before" + gdb_test_no_output "maintenance deprecate alias_command" + gdb_test_no_output "maintenance deprecate alias_with_args_command" + gdb_test "help real_command" \ + "User-defined." \ + "help real_command, after" +} + proc_with_prefix bp_deleted_in_command_test {} { global gdb_prompt @@ -1203,6 +1224,7 @@ user_defined_command_manyargs_test watchpoint_command_test test_command_prompt_position deprecated_command_test +deprecated_command_alias_help_test bp_deleted_in_command_test temporary_breakpoint_commands stray_arg0_test -- 2.33.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] gdb: don't show deprecated aliases 2021-12-01 16:41 ` [PATCH 3/3] gdb: don't show deprecated aliases Simon Marchi @ 2021-12-03 16:16 ` Pedro Alves 2021-12-03 21:48 ` Simon Marchi 0 siblings, 1 reply; 10+ messages in thread From: Pedro Alves @ 2021-12-03 16:16 UTC (permalink / raw) To: Simon Marchi, gdb-patches On 2021-12-01 16:41, Simon Marchi via Gdb-patches wrote: > I don't think it's very useful to show deprecated aliases to the > user. It encourages the user to use them, when the goal is the > opposite. > > For example, before: > > (gdb) help set index-cache enabled > set index-cache enabled, set index-cache off, set index-cache on > alias set index-cache off = set index-cache enabled off > alias set index-cache on = set index-cache enabled on > Enable the index cache. > When on, enable the use of the index cache. > > (gdb) help set index-cache on > Warning: 'set index-cache on', an alias for the command 'set index-cache enabled', is deprecated. > Use 'set index-cache enabled on'. > > set index-cache enabled, set index-cache off, set index-cache on > alias set index-cache off = set index-cache enabled off > alias set index-cache on = set index-cache enabled on > Enable the index cache. > When on, enable the use of the index cache. > I agree, this looks crowded, unclear and confusing. > After: > > (gdb) help set index-cache enabled > Enable the index cache. > When on, enable the use of the index cache. > (gdb) help set index-cache on > Warning: 'set index-cache on', an alias for the command 'set index-cache enabled', is deprecated. > Use 'set index-cache enabled on'. > > Enable the index cache. > When on, enable the use of the index cache. > > Change-Id: I989b618a5ad96ba975367e9d16db95523cd57a4c LGTM. > --- > gdb/cli/cli-decode.c | 29 ++++++++++++++++++++++++++--- > gdb/testsuite/gdb.base/commands.exp | 22 ++++++++++++++++++++++ > 2 files changed, 48 insertions(+), 3 deletions(-) > > diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c > index b8be3f54921f..9b965ea99891 100644 > --- a/gdb/cli/cli-decode.c > +++ b/gdb/cli/cli-decode.c > @@ -1345,7 +1345,7 @@ fput_aliases_definition_styled (const cmd_list_element &cmd, > struct ui_file *stream) > { > for (const cmd_list_element &alias : cmd.aliases) > - if (!alias.default_args.empty ()) > + if (!alias.cmd_deprecated && !alias.default_args.empty ()) > fput_alias_definition_styled (alias, stream); > } > > @@ -1361,17 +1361,40 @@ fput_command_names_styled (const cmd_list_element &c, > bool always_fput_c_name, const char *postfix, > struct ui_file *stream) > { > - if (always_fput_c_name || !c.aliases.empty ()) > + /* First, check if we are going to print something. That is, either if > + ALWAYS_FPUT_C_NAME is true or if there exists at least one non-deprecated > + alias. */ > + > + auto print_alias = [] (const cmd_list_element &alias) > + { > + return !alias.cmd_deprecated; > + }; > + > + bool print_something = always_fput_c_name; > + if (!print_something) > + for (const cmd_list_element &alias : c.aliases) > + { > + if (!print_alias (alias)) > + continue; > + > + print_something = true; > + break; > + } > + > + if (print_something) > fput_command_name_styled (c, stream); > > for (const cmd_list_element &alias : c.aliases) > { > + if (!print_alias (alias)) > + continue; > + > fputs_filtered (", ", stream); > wrap_here (" "); > fput_command_name_styled (alias, stream); > } > > - if (always_fput_c_name || !c.aliases.empty ()) > + if (print_something) > fputs_filtered (postfix, stream); > } > > diff --git a/gdb/testsuite/gdb.base/commands.exp b/gdb/testsuite/gdb.base/commands.exp > index 6785f9532b11..1dca419f0fb7 100644 > --- a/gdb/testsuite/gdb.base/commands.exp > +++ b/gdb/testsuite/gdb.base/commands.exp > @@ -709,6 +709,27 @@ maintenance deprecate set qqq_aaa" > file delete $file1 > } > > +# Test that the help for a command does not show deprecated aliases. > + > +proc_with_prefix deprecated_command_alias_help_test {} { > + gdb_test_multiline "define real_command" \ > + "define real_command" "End with a line saying just \"end\".." \ > + "print 1" "" \ > + "end" "" > + > + gdb_test_no_output "alias alias_command = real_command" > + gdb_test_no_output "alias alias_with_args_command = real_command 123" > + > + gdb_test "help real_command" \ > + "real_command, alias_with_args_command, alias_command\r\n alias alias_with_args_command = real_command 123\r\nUser-defined." \ > + "help real_command, before" > + gdb_test_no_output "maintenance deprecate alias_command" > + gdb_test_no_output "maintenance deprecate alias_with_args_command" > + gdb_test "help real_command" \ > + "User-defined." \ > + "help real_command, after" > +} > + > proc_with_prefix bp_deleted_in_command_test {} { > global gdb_prompt > > @@ -1203,6 +1224,7 @@ user_defined_command_manyargs_test > watchpoint_command_test > test_command_prompt_position > deprecated_command_test > +deprecated_command_alias_help_test > bp_deleted_in_command_test > temporary_breakpoint_commands > stray_arg0_test > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] gdb: don't show deprecated aliases 2021-12-03 16:16 ` Pedro Alves @ 2021-12-03 21:48 ` Simon Marchi 2021-12-04 12:33 ` Philippe Waroquiers 0 siblings, 1 reply; 10+ messages in thread From: Simon Marchi @ 2021-12-03 21:48 UTC (permalink / raw) To: Pedro Alves, gdb-patches, Philippe Waroquiers On 2021-12-03 11:16, Pedro Alves wrote: > On 2021-12-01 16:41, Simon Marchi via Gdb-patches wrote: >> I don't think it's very useful to show deprecated aliases to the >> user. It encourages the user to use them, when the goal is the >> opposite. >> >> For example, before: >> >> (gdb) help set index-cache enabled >> set index-cache enabled, set index-cache off, set index-cache on >> alias set index-cache off = set index-cache enabled off >> alias set index-cache on = set index-cache enabled on >> Enable the index cache. >> When on, enable the use of the index cache. >> >> (gdb) help set index-cache on >> Warning: 'set index-cache on', an alias for the command 'set index-cache enabled', is deprecated. >> Use 'set index-cache enabled on'. >> >> set index-cache enabled, set index-cache off, set index-cache on >> alias set index-cache off = set index-cache enabled off >> alias set index-cache on = set index-cache enabled on >> Enable the index cache. >> When on, enable the use of the index cache. >> > > I agree, this looks crowded, unclear and confusing. > >> After: >> >> (gdb) help set index-cache enabled >> Enable the index cache. >> When on, enable the use of the index cache. >> (gdb) help set index-cache on >> Warning: 'set index-cache on', an alias for the command 'set index-cache enabled', is deprecated. >> Use 'set index-cache enabled on'. >> >> Enable the index cache. >> When on, enable the use of the index cache. >> >> Change-Id: I989b618a5ad96ba975367e9d16db95523cd57a4c > > LGTM. Thanks. Philippe, do you have an opinion on this change of behavior? Simon ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] gdb: don't show deprecated aliases 2021-12-03 21:48 ` Simon Marchi @ 2021-12-04 12:33 ` Philippe Waroquiers 2021-12-04 14:07 ` Simon Marchi 0 siblings, 1 reply; 10+ messages in thread From: Philippe Waroquiers @ 2021-12-04 12:33 UTC (permalink / raw) To: Simon Marchi, Pedro Alves, gdb-patches On Fri, 2021-12-03 at 16:48 -0500, Simon Marchi wrote: > > On 2021-12-03 11:16, Pedro Alves wrote: > > On 2021-12-01 16:41, Simon Marchi via Gdb-patches wrote: > > > > > > > After: > > > > > > (gdb) help set index-cache enabled > > > Enable the index cache. > > > When on, enable the use of the index cache. > > > (gdb) help set index-cache on > > > Warning: 'set index-cache on', an alias for the command 'set index-cache enabled', is deprecated. > > > Use 'set index-cache enabled on'. > > > > > > Enable the index cache. > > > When on, enable the use of the index cache. > > > > > > Change-Id: I989b618a5ad96ba975367e9d16db95523cd57a4c > > > > LGTM. > > Thanks. > > Philippe, do you have an opinion on this change of behavior? > > Simon I agree that showing deprecated aliases is not useful, and the 'After' output is much better. Thanks Philippe ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] gdb: don't show deprecated aliases 2021-12-04 12:33 ` Philippe Waroquiers @ 2021-12-04 14:07 ` Simon Marchi 0 siblings, 0 replies; 10+ messages in thread From: Simon Marchi @ 2021-12-04 14:07 UTC (permalink / raw) To: Philippe Waroquiers, Pedro Alves, gdb-patches On 2021-12-04 07:33, Philippe Waroquiers wrote: > On Fri, 2021-12-03 at 16:48 -0500, Simon Marchi wrote: >> >> On 2021-12-03 11:16, Pedro Alves wrote: >>> On 2021-12-01 16:41, Simon Marchi via Gdb-patches wrote: >>> >>> >>>> After: >>>> >>>> (gdb) help set index-cache enabled >>>> Enable the index cache. >>>> When on, enable the use of the index cache. >>>> (gdb) help set index-cache on >>>> Warning: 'set index-cache on', an alias for the command 'set index-cache enabled', is deprecated. >>>> Use 'set index-cache enabled on'. >>>> >>>> Enable the index cache. >>>> When on, enable the use of the index cache. >>>> >>>> Change-Id: I989b618a5ad96ba975367e9d16db95523cd57a4c >>> >>> LGTM. >> >> Thanks. >> >> Philippe, do you have an opinion on this change of behavior? >> >> Simon > I agree that showing deprecated aliases is not useful, > and the 'After' output is much better. > > Thanks > Philippe > > Ok, thanks, pushed. Simon ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] gdb: use intrusive_list for cmd_list_element aliases list 2021-12-01 16:41 [PATCH 1/3] gdb: use intrusive_list for cmd_list_element aliases list Simon Marchi 2021-12-01 16:41 ` [PATCH 2/3] gdb: change some alias functions parameters to const-reference Simon Marchi 2021-12-01 16:41 ` [PATCH 3/3] gdb: don't show deprecated aliases Simon Marchi @ 2021-12-03 16:13 ` Pedro Alves 2 siblings, 0 replies; 10+ messages in thread From: Pedro Alves @ 2021-12-03 16:13 UTC (permalink / raw) To: Simon Marchi, gdb-patches On 2021-12-01 16:41, Simon Marchi via Gdb-patches wrote: > Change the manually-implemented linked list to use intrusive_list. This > is not strictly necessary, but it makes the code much simpler. LGTM. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-12-04 14:07 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-12-01 16:41 [PATCH 1/3] gdb: use intrusive_list for cmd_list_element aliases list Simon Marchi 2021-12-01 16:41 ` [PATCH 2/3] gdb: change some alias functions parameters to const-reference Simon Marchi 2021-12-03 16:13 ` Pedro Alves 2021-12-03 21:49 ` Simon Marchi 2021-12-01 16:41 ` [PATCH 3/3] gdb: don't show deprecated aliases Simon Marchi 2021-12-03 16:16 ` Pedro Alves 2021-12-03 21:48 ` Simon Marchi 2021-12-04 12:33 ` Philippe Waroquiers 2021-12-04 14:07 ` Simon Marchi 2021-12-03 16:13 ` [PATCH 1/3] gdb: use intrusive_list for cmd_list_element aliases list Pedro Alves
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).