Message ID | 1484058324-5368-1-git-send-email-guitton@adacore.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 78556 invoked by alias); 10 Jan 2017 14:25:42 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 78542 invoked by uid 89); 10 Jan 2017 14:25:41 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=265, 6 X-HELO: smtp.eu.adacore.com Received: from mel.act-europe.fr (HELO smtp.eu.adacore.com) (194.98.77.210) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 10 Jan 2017 14:25:39 +0000 Received: from localhost (localhost [127.0.0.1]) by filtered-smtp.eu.adacore.com (Postfix) with ESMTP id E5F4082FC1; Tue, 10 Jan 2017 15:25:37 +0100 (CET) Received: from smtp.eu.adacore.com ([127.0.0.1]) by localhost (smtp.eu.adacore.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id STkcvHxQk5Kh; Tue, 10 Jan 2017 15:25:37 +0100 (CET) Received: from chelles.act-europe.fr (chelles.act-europe.fr [IPv6:2a02:2ab8:224:1:d6be:d9ff:fef8:4565]) by smtp.eu.adacore.com (Postfix) with ESMTP id D405382FBF; Tue, 10 Jan 2017 15:25:37 +0100 (CET) Received: by chelles.act-europe.fr (Postfix, from userid 560) id CDC801EA0067; Tue, 10 Jan 2017 15:25:37 +0100 (CET) From: Jerome Guitton <guitton@adacore.com> To: gdb-patches@sourceware.org Cc: Jerome Guitton <guitton@adacore.com> Subject: [RFA] candidates for ambiguous command in upper case Date: Tue, 10 Jan 2017 15:25:24 +0100 Message-Id: <1484058324-5368-1-git-send-email-guitton@adacore.com> |
Commit Message
Jerome Guitton
Jan. 10, 2017, 2:25 p.m. UTC
If you type an ambiguous command in lower case, gdb tells the command is ambiguous and tells you which one could match. If you type the same but in upper case, gdb also says it is ambiguous, but shows an empty list of commands: (gdb) ex Ambiguous command "ex": exec-file, expression. (gdb) EX Ambiguous command "EX": . Simple fix in attachment, with an additional test. Tested on x86-linux. OK to apply? gdb/ChangeLog: * cli-decode.c (lookup_cmd): case insensitive match when looking up candidates for ambigous command. gdb/testsuite/ChangeLog: * gdb.base/completion.exp: Add test for ambiguous upper case command. --- gdb/cli/cli-decode.c | 2 +- gdb/testsuite/gdb.base/completion.exp | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-)
Comments
On 17-01-10 15:25:24, Jerome Guitton wrote: > If you type an ambiguous command in lower case, gdb tells the command > is ambiguous and tells you which one could match. If you type the same > but in upper case, gdb also says it is ambiguous, but shows an empty > list of commands: > > (gdb) ex > Ambiguous command "ex": exec-file, expression. > (gdb) EX > Ambiguous command "EX": . > IMO, there is nothing wrong. There is no command starts from "EX". In bash, I type "make" and tab, I get, $ make make makeglossaries makeindex makeinfo makejvf make-ssl-cert make_strings If type "MAKE", and type, I get, $ MAKEDEV
Yao Qi (qiyaoltc@gmail.com):
> IMO, there is nothing wrong. There is no command starts from "EX".
This is a bit weird to accept upper-case EXEC-FIL then... isn't it?
(gdb) exec-fil
No executable file now.
(gdb) EXEC-FIL
No executable file now.
On 2017-01-10 10:19, Jerome Guitton wrote: > Yao Qi (qiyaoltc@gmail.com): > >> IMO, there is nothing wrong. There is no command starts from "EX". > > This is a bit weird to accept upper-case EXEC-FIL then... isn't it? > > (gdb) exec-fil > No executable file now. > (gdb) EXEC-FIL > No executable file now. I agree that if GDB accepts commands in upper case, the ambiguous command message should work accordingly. The message as it is, with an empty list, doesn't make sense. I also noticed that tab completion was case sensitive, should that be fixed as well? For example, (gdb) inf<tab><tab> shows "inferior info", but (gdb) INF<tab><tab> shows nothing. Should it show "inferior info", or "INFERIOR INFO"?
Simon Marchi (simon.marchi@polymtl.ca): > I also noticed that tab completion was case sensitive, should that > be fixed as well? I guess so. It seems that it could be changed by a similar patch in complete_on_cmdlist.
On 01/10/2017 03:28 PM, Simon Marchi wrote: > On 2017-01-10 10:19, Jerome Guitton wrote: >> Yao Qi (qiyaoltc@gmail.com): >> >>> IMO, there is nothing wrong. There is no command starts from "EX". >> >> This is a bit weird to accept upper-case EXEC-FIL then... isn't it? >> >> (gdb) exec-fil >> No executable file now. >> (gdb) EXEC-FIL >> No executable file now. > > I agree that if GDB accepts commands in upper case, the ambiguous > command message should work accordingly. Agreed. I thought that the manual mentioned that gdb accepts commands in either case, but I can't find it now. > The message as it is, with an empty list, doesn't make sense. > > I also noticed that tab completion was case sensitive, should that be > fixed as well? > > For example, > > (gdb) inf<tab><tab> > > shows "inferior info", but > > (gdb) INF<tab><tab> > > shows nothing. Should it show "inferior info", or "INFERIOR INFO"? I think the former. I.e., show the canonical lowercase. That is likely to end up much easier and saner to implement. I've been playing with completion recently [1], and I've noticed that (gdb) handle sigu<tab> completes to: (gdb) handle SIGU<bell> (gdb) handle sig<tab> Or really: (gdb) handle sigu M-? SIGURG SIGUSR1 SIGUSR2 In my branch I've completely changed how GDB hands over completion matches to readline, and I've had to make sure to preserve that behavior, as caught by some test. [1] - this WIP branch gets rid of the need to quote linespecs as in "b 'function(int<tab>" among other things: https://github.com/palves/gdb/commits/palves/cp-linespec Thanks, Pedro Alves
On 17-01-10 17:00:28, Pedro Alves wrote: > On 01/10/2017 03:28 PM, Simon Marchi wrote: > > On 2017-01-10 10:19, Jerome Guitton wrote: > >> Yao Qi (qiyaoltc@gmail.com): > >> > >>> IMO, there is nothing wrong. There is no command starts from "EX". > >> > >> This is a bit weird to accept upper-case EXEC-FIL then... isn't it? > >> > >> (gdb) exec-fil > >> No executable file now. > >> (gdb) EXEC-FIL > >> No executable file now. > > > > I agree that if GDB accepts commands in upper case, the ambiguous > > command message should work accordingly. > > Agreed. I thought that the manual mentioned that gdb accepts > commands in either case, but I can't find it now. > I don't find gdb accepts commands in either case in the manual, and I am surprised that gdb does so. Actually, gdb does so since 1988! commit 7b4ac7e1ed2c4616bce56d1760807798be87ac9e Author: gdb-2.4+.aux.coff <gdb@fsf.org> Date: Sat Jan 16 04:39:57 1988 +0000 gdb-2.4+.aux.coff in lookup_cmd function, + /* Find end of command name. */ + + p = *line; + while (*p == '-' + || (*p >= 'a' && *p <= 'z') + || (*p >= 'A' && *p <= 'Z') + || (*p >= '1' && *p <= '9')) + { + if (*p >= 'A' && *p <= 'Z') + *p += 'a' - 'A'; + p++; + } however, I don't see any reason to do so. At least, we need to be clear that whether gdb accepts upper case commands or not.
On 01/11/2017 11:25 AM, Yao Qi wrote: > On 17-01-10 17:00:28, Pedro Alves wrote: >> On 01/10/2017 03:28 PM, Simon Marchi wrote: >>> On 2017-01-10 10:19, Jerome Guitton wrote: >>>> Yao Qi (qiyaoltc@gmail.com): >>>> >>>>> IMO, there is nothing wrong. There is no command starts from "EX". >>>> >>>> This is a bit weird to accept upper-case EXEC-FIL then... isn't it? >>>> >>>> (gdb) exec-fil >>>> No executable file now. >>>> (gdb) EXEC-FIL >>>> No executable file now. >>> >>> I agree that if GDB accepts commands in upper case, the ambiguous >>> command message should work accordingly. >> >> Agreed. I thought that the manual mentioned that gdb accepts >> commands in either case, but I can't find it now. >> > > I don't find gdb accepts commands in either case in the manual, and > I am surprised that gdb does so. Actually, gdb does so since 1988! > > commit 7b4ac7e1ed2c4616bce56d1760807798be87ac9e > Author: gdb-2.4+.aux.coff <gdb@fsf.org> > Date: Sat Jan 16 04:39:57 1988 +0000 > > gdb-2.4+.aux.coff > > in lookup_cmd function, > > + /* Find end of command name. */ > + > + p = *line; > + while (*p == '-' > + || (*p >= 'a' && *p <= 'z') > + || (*p >= 'A' && *p <= 'Z') > + || (*p >= '1' && *p <= '9')) > + { > + if (*p >= 'A' && *p <= 'Z') > + *p += 'a' - 'A'; > + p++; > + } > > however, I don't see any reason to do so. At least, we need to be > clear that whether gdb accepts upper case commands or not. > I think it makes for a more coherent command-handling environment. So load would be the same as LOAD, Load, LoAd, lOaD. I can't think of a scenario where differentiating uppercase/lowercase commands in GDB's CLI would make a difference. Though i've noticed that MI commands are case-sensitive, which is incoherent. We ought to fix that too.
On 01/11/2017 05:25 PM, Yao Qi wrote: > On 17-01-10 17:00:28, Pedro Alves wrote: >> On 01/10/2017 03:28 PM, Simon Marchi wrote: >>> On 2017-01-10 10:19, Jerome Guitton wrote: >>>> Yao Qi (qiyaoltc@gmail.com): >>>> >>>>> IMO, there is nothing wrong. There is no command starts from "EX". >>>> >>>> This is a bit weird to accept upper-case EXEC-FIL then... isn't it? >>>> >>>> (gdb) exec-fil >>>> No executable file now. >>>> (gdb) EXEC-FIL >>>> No executable file now. >>> >>> I agree that if GDB accepts commands in upper case, the ambiguous >>> command message should work accordingly. >> >> Agreed. I thought that the manual mentioned that gdb accepts >> commands in either case, but I can't find it now. >> > > I don't find gdb accepts commands in either case in the manual, and > I am surprised that gdb does so. Actually, gdb does so since 1988! I guess I probably discovered it by typing some command with Caps Lock on by accident, and seeing it work. I can only guess on original motivation -- maybe to make GDB usable with uppercase-only terminals? Those were common at some point in the past. I think Linux's terminal subsystem might still have support for those. But then symbol searching was not case-insensitive, ("set case-sensitive off" support was only added much later), so I can't quite imagine how that'd be much usable. Thanks, Pedro Alves
Pedro Alves (palves@redhat.com): > I can only guess on original motivation -- maybe to make GDB usable > with uppercase-only terminals? Those were common at some point in > the past. I think Linux's terminal subsystem might still have > support for those. But then symbol searching was not case-insensitive, > ("set case-sensitive off" support was only added much later), so I > can't quite imagine how that'd be much usable. I'm not sure of the original motivation; but I guess that if it has been there for thirty years it is now considered as the standard interface by a few users. In any case, even if we decide to change that, consistency would be good, at least for CLI; the same policy for casing should apply in all cases of completion. Documenting it in the user manual would be good indeed; I'm not sure where the proper place would be though.
On 01/12/2017 10:18 AM, Jerome Guitton wrote: > Pedro Alves (palves@redhat.com): > >> I can only guess on original motivation -- maybe to make GDB usable >> with uppercase-only terminals? Those were common at some point in >> the past. I think Linux's terminal subsystem might still have >> support for those. But then symbol searching was not case-insensitive, >> ("set case-sensitive off" support was only added much later), so I >> can't quite imagine how that'd be much usable. > > I'm not sure of the original motivation; but I guess that if it has > been there for thirty years it is now considered as the standard > interface by a few users. Dunno, if it doesn't make sense, and nobody uses it, then it's one less thing to maintain and test. We don't treat command options as case insensitive. E.g.: (gdb) watch -locaTION *main No symbol "locaTION" in current context. (gdb) p /x obj $1 = 0x4007a0 (gdb) p /X obj No symbol "X" in current context. Etc. If we removed support for case insensitive commands, then it may open up interesting uses. > In any case, even if we decide to change that, consistency would be > good, at least for CLI; the same policy for casing should apply in all > cases of completion. > > Documenting it in the user manual would be good indeed; I'm not sure > where the proper place would be though. The place where I had expected to find it was where we talk about unambiguous command abbreviations. I.e., the very first thing the manual talks about, once GDB is up and running: https://sourceware.org/gdb/current/onlinedocs/gdb/Commands.html#Commands Thanks, Pedro Alves
Pedro Alves (palves@redhat.com): > Dunno, if it doesn't make sense, and nobody uses it, then it's one > less thing to maintain and test. Hard to figure out if someone uses it, I guess. This originally got reported by one of our users as a minor inconsistancy, who also said that he would be OK if gdb commands were case sensitive. The patch that I'm suggesting is fairly simple, but my feeling is that it would be just as easy to remove the feature. I've tested the opposite: making the match case sensitive is a matter of removing 4 characters in cli-decode.c. Not much consequences in the testsuite. "handle sigq" still completes to "handle SIGQUIT". So, what do we want to do? 1. Remove the feature? 2. Improve its consistency? 3. Keep things as is?
On 01/16/2017 04:32 PM, Jerome Guitton wrote: > Pedro Alves (palves@redhat.com): > >> Dunno, if it doesn't make sense, and nobody uses it, then it's one >> less thing to maintain and test. > > Hard to figure out if someone uses it, I guess. This originally got > reported by one of our users as a minor inconsistancy, who also said > that he would be OK if gdb commands were case sensitive. > > The patch that I'm suggesting is fairly simple, but my feeling is that > it would be just as easy to remove the feature. > > I've tested the opposite: making the match case sensitive is a matter > of removing 4 characters in cli-decode.c. Not much consequences in the > testsuite. "handle sigq" still completes to "handle SIGQUIT". > > So, what do we want to do? > 1. Remove the feature? > 2. Improve its consistency? > 3. Keep things as is? > I vote #1. Thanks, Pedro Alves
On 01/16/2017 07:58 PM, Pedro Alves wrote: > On 01/16/2017 04:32 PM, Jerome Guitton wrote: >> Pedro Alves (palves@redhat.com): >> >>> Dunno, if it doesn't make sense, and nobody uses it, then it's one >>> less thing to maintain and test. >> >> Hard to figure out if someone uses it, I guess. This originally got >> reported by one of our users as a minor inconsistancy, who also said >> that he would be OK if gdb commands were case sensitive. >> >> The patch that I'm suggesting is fairly simple, but my feeling is that >> it would be just as easy to remove the feature. >> >> I've tested the opposite: making the match case sensitive is a matter >> of removing 4 characters in cli-decode.c. Not much consequences in the >> testsuite. "handle sigq" still completes to "handle SIGQUIT". >> >> So, what do we want to do? >> 1. Remove the feature? >> 2. Improve its consistency? >> 3. Keep things as is? >> > > I vote #1. I don't see a problem with #1 as long as we keep it consistent throughout from now on.
On 01/17/2017 04:29 PM, Luis Machado wrote: > > I don't see a problem with #1 as long as we keep it consistent > throughout from now on. Can you expand on exactly what you mean by the "as long" part? Thanks, Pedro Alves
On 01/17/2017 10:34 AM, Pedro Alves wrote: > On 01/17/2017 04:29 PM, Luis Machado wrote: >> >> I don't see a problem with #1 as long as we keep it consistent >> throughout from now on. > > Can you expand on exactly what you mean by the "as long" part? > > Thanks, > Pedro Alves > > Meaning that, if we go with #1, we should make sure we are not doing case-insensitive comparisons in gdb unless it is clearly stated we should do it (like handling of Windows paths i suppose). For the record, i think #3 is not acceptable and we don't seem to have a good justification for keeping the behavior of #2 other than "we've been doing this since ...". So i think #1 would be the best. I just wanted to point out we may want to make sure there are no other gdb places with the same undocumented behavior. Does that make sense?
On 01/17/2017 04:51 PM, Luis Machado wrote: > On 01/17/2017 10:34 AM, Pedro Alves wrote: >> On 01/17/2017 04:29 PM, Luis Machado wrote: >>> >>> I don't see a problem with #1 as long as we keep it consistent >>> throughout from now on. >> >> Can you expand on exactly what you mean by the "as long" part? > Meaning that, if we go with #1, we should make sure we are not doing > case-insensitive comparisons in gdb unless it is clearly stated we > should do it (like handling of Windows paths i suppose). Would you say that we should stop completing "handle sig" to "handle SIG"? "handle" doesn't accept lowercase signal names, but it could be argued that the completer is helping the user here. Grepping for strncasecmp doesn't find all that many hits, and several are in symbol lookup or path handling stuff where they're probably rightly used. The ones in tracepoint.c do look like of the inconsistent-user-interface sort. I had never realized we accept $REG etc. in tracepoint actions. Thanks, Pedro Alves
On 01/17/2017 11:04 AM, Pedro Alves wrote: > On 01/17/2017 04:51 PM, Luis Machado wrote: >> On 01/17/2017 10:34 AM, Pedro Alves wrote: >>> On 01/17/2017 04:29 PM, Luis Machado wrote: >>>> >>>> I don't see a problem with #1 as long as we keep it consistent >>>> throughout from now on. >>> >>> Can you expand on exactly what you mean by the "as long" part? > >> Meaning that, if we go with #1, we should make sure we are not doing >> case-insensitive comparisons in gdb unless it is clearly stated we >> should do it (like handling of Windows paths i suppose). > > Would you say that we should stop completing "handle sig" to "handle SIG"? > "handle" doesn't accept lowercase signal names, but it could be > argued that the completer is helping the user here. > That sounds like a one-off case where it is useful. > Grepping for strncasecmp doesn't find all that many hits, and several > are in symbol lookup or path handling stuff where they're probably rightly > used. > I'll go through the code to see if there are any other occurrences. > The ones in tracepoint.c do look like of the inconsistent-user-interface sort. > I had never realized we accept $REG etc. in tracepoint actions. > Neither did i. This one looks like it could be dropped.
diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c index d3be93c..d59fe9b 100644 --- a/gdb/cli/cli-decode.c +++ b/gdb/cli/cli-decode.c @@ -1550,7 +1550,7 @@ lookup_cmd (const char **line, struct cmd_list_element *list, char *cmdtype, ambbuf[0] = 0; for (c = local_list; c; c = c->next) - if (!strncmp (*line, c->name, amb_len)) + if (!strncasecmp (*line, c->name, amb_len)) { if (strlen (ambbuf) + strlen (c->name) + 6 < (int) sizeof ambbuf) diff --git a/gdb/testsuite/gdb.base/completion.exp b/gdb/testsuite/gdb.base/completion.exp index 4a3ee4b..9a7a221 100644 --- a/gdb/testsuite/gdb.base/completion.exp +++ b/gdb/testsuite/gdb.base/completion.exp @@ -265,6 +265,19 @@ gdb_test_multiple "" "$test" { } } +set test "complete 'info T '" +send_gdb "info T \t" +gdb_test_multiple "" "$test" { + -re "^info T \\\x07$" { + send_gdb "\n" + gdb_test_multiple "" "$test" { + -re "Ambiguous info command \"T \": target, tasks, terminal, threads, tp, tracepoints, tvariables, (type-printers, )?types\\..*$gdb_prompt $" { + pass "$test" + } + } + } +} + set test "complete 'info t '" send_gdb "info t \t" gdb_test_multiple "" "$test" {