Message ID | 1400708905-14184-1-git-send-email-simon.marchi@ericsson.com |
---|---|
State | Changes Requested, archived |
Headers |
Return-Path: <x14314964@homiemail-mx23.g.dreamhost.com> X-Original-To: siddhesh@wilcox.dreamhost.com Delivered-To: siddhesh@wilcox.dreamhost.com Received: from homiemail-mx23.g.dreamhost.com (mx2.sub5.homie.mail.dreamhost.com [208.113.200.128]) by wilcox.dreamhost.com (Postfix) with ESMTP id BE604360098 for <siddhesh@wilcox.dreamhost.com>; Wed, 21 May 2014 14:48:39 -0700 (PDT) Received: by homiemail-mx23.g.dreamhost.com (Postfix, from userid 14314964) id 63D756401C38B; Wed, 21 May 2014 14:48:39 -0700 (PDT) X-Original-To: gdb@patchwork.siddhesh.in Delivered-To: x14314964@homiemail-mx23.g.dreamhost.com Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by homiemail-mx23.g.dreamhost.com (Postfix) with ESMTPS id 3D6A46401C3C0 for <gdb@patchwork.siddhesh.in>; Wed, 21 May 2014 14:48:39 -0700 (PDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:cc:subject:date:message-id :mime-version:content-type; q=dns; s=default; b=DcOYu+zeiCxkKU3p btirZyJymICOm4SWRHpM2pMEQYh5uRcvxm+Ye/7ChDw/HeJVOH9HLrceUWJs+Yau ZsOfEJBuOXFeApFnfaRhSDrFZxmDwfIgaF7SDeiTbFSQJvz+P2BgpDK8hvU/jzWy QMvi6qKrXHc6LO1gZs5EZ4uRe6w= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:cc:subject:date:message-id :mime-version:content-type; s=default; bh=rI0YDLRoOa4OcbOtuxM2Lz mYHVA=; b=RClf3+LiXCauwFo6DyKkCEihpnwbbSSvdrn/N1vwUEQoVRT0VgZWBz N1xxlAaIY6DE9rzB3AyubE0e7Lov0DSJyiLnR2GcVwK4z96TjyQqBgwMMNyobC4y xCS9FEa1zSWS+Zx2zTPRzPgthkkN3s8QkZyWtJu/iDLGibezkQ5ps= Received: (qmail 3171 invoked by alias); 21 May 2014 21:48:37 -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-gdb=patchwork.siddhesh.in@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 3162 invoked by uid 89); 21 May 2014 21:48:36 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL, BAYES_00, SPF_PASS autolearn=ham version=3.3.2 X-HELO: usevmg20.ericsson.net Received: from usevmg20.ericsson.net (HELO usevmg20.ericsson.net) (198.24.6.45) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Wed, 21 May 2014 21:48:35 +0000 Received: from EUSAAHC008.ericsson.se (Unknown_Domain [147.117.188.96]) by usevmg20.ericsson.net (Symantec Mail Security) with SMTP id E8.F0.27529.27FCC735; Wed, 21 May 2014 18:08:19 +0200 (CEST) Received: from simark-hp.mo.ca.am.ericsson.se (147.117.188.8) by smtps-am.internal.ericsson.com (147.117.188.96) with Microsoft SMTP Server (TLS) id 14.3.174.1; Wed, 21 May 2014 17:48:30 -0400 From: Simon Marchi <simon.marchi@ericsson.com> To: <gdb-patches@sourceware.org> CC: Simon Marchi <simon.marchi@ericsson.com> Subject: [PATCH] Add comment for mi_run_cmd_full Date: Wed, 21 May 2014 17:48:25 -0400 Message-ID: <1400708905-14184-1-git-send-email-simon.marchi@ericsson.com> MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes X-DH-Original-To: gdb@patchwork.siddhesh.in |
Commit Message
Simon Marchi
May 21, 2014, 9:48 p.m. UTC
It should clear up confusion about the args parameter to mi_run_cmd_full. If you have a better wording, let me know, I am not so good at that english thing. gdb/testsuite/ChangeLog: 2014-05-21 Simon Marchi <simon.marchi@ericsson.com> * lib/mi-support.exp (mi_run_cmd_full): Add comments. --- gdb/testsuite/lib/mi-support.exp | 6 ++++++ 1 file changed, 6 insertions(+)
Comments
Hi Simon, > 2014-05-21 Simon Marchi <simon.marchi@ericsson.com> > > * lib/mi-support.exp (mi_run_cmd_full): Add comments. Overall, this looks good to me. I wasn't aware of the history, so just did a little bit of catching up :-), but I confirm that the description you propose matches the actual behavior... > +# use_mi_command selects whether MI or CLI commands are used by the > +# procedure. > +# args is passed to the command used to run the test program, "run" or > +# "-exec-run", depending on the value of use_mi_command. Therefore, if > +# use_mi_command is false, they are effectively args to the test program. > +# If use_mi_command is true, they are simply arguments to -exec-run. > proc mi_run_cmd_full {use_mi_command args} { > global suppress_flag > if { $suppress_flag } { > -- > 2.0.0.rc0 When speaking of the value of a variable, the variable name should be in all caps. For instance: # USE_MI_COMMAND selects whether MI or CLI commands are used by the # procedure. If I may suggest a different approach (anyone is welcome to comment on that too), it'd be useful to succintly say what the function does first, before getting into the details of the parameters. Also, I would group the semantics of each value of use_mi_command together. So, one example: # Send the command to run the test program. # # If USE_MI_COMMAND is true, the "-exec-run" command is used. # Otherwise, the "run" (CLI) command is used. # # ARGS is passed as argument to the command used to run the test program. # Beware that arguments to "-exec-run" do not have the same semantics as # arguments to the "run" command, so USE_MI_COMMAND influences the meaning # of ARGS. If USE_MI_COMMAND is true, they are arguments to -exec-run. # If USE_MI_COMMAND is false, they are effectively arguments passed # to the test program. I hope it helps!
On 14-05-21 06:26 PM, Joel Brobecker wrote: > Hi Simon, > >> 2014-05-21 Simon Marchi <simon.marchi@ericsson.com> >> >> * lib/mi-support.exp (mi_run_cmd_full): Add comments. > > Overall, this looks good to me. I wasn't aware of the history, > so just did a little bit of catching up :-), but I confirm that > the description you propose matches the actual behavior... > >> +# use_mi_command selects whether MI or CLI commands are used by the >> +# procedure. >> +# args is passed to the command used to run the test program, "run" or >> +# "-exec-run", depending on the value of use_mi_command. Therefore, if >> +# use_mi_command is false, they are effectively args to the test program. >> +# If use_mi_command is true, they are simply arguments to -exec-run. >> proc mi_run_cmd_full {use_mi_command args} { >> global suppress_flag >> if { $suppress_flag } { >> -- >> 2.0.0.rc0 > > When speaking of the value of a variable, the variable name should be > in all caps. For instance: > > # USE_MI_COMMAND selects whether MI or CLI commands are used by the > # procedure. Ok. > If I may suggest a different approach (anyone is welcome to comment > on that too), it'd be useful to succintly say what the function does > first, before getting into the details of the parameters. Also, I would > group the semantics of each value of use_mi_command together. So, > one example: > > # Send the command to run the test program. > # > # If USE_MI_COMMAND is true, the "-exec-run" command is used. > # Otherwise, the "run" (CLI) command is used. > # > # ARGS is passed as argument to the command used to run the test program. > # Beware that arguments to "-exec-run" do not have the same semantics as > # arguments to the "run" command, so USE_MI_COMMAND influences the meaning > # of ARGS. If USE_MI_COMMAND is true, they are arguments to -exec-run. > # If USE_MI_COMMAND is false, they are effectively arguments passed > # to the test program. > > I hope it helps! > I think this is very clear. Should I post an updated patch ?
> > If I may suggest a different approach (anyone is welcome to comment > > on that too), it'd be useful to succintly say what the function does > > first, before getting into the details of the parameters. Also, I would > > group the semantics of each value of use_mi_command together. So, > > one example: > > > > # Send the command to run the test program. > > # > > # If USE_MI_COMMAND is true, the "-exec-run" command is used. > > # Otherwise, the "run" (CLI) command is used. > > # > > # ARGS is passed as argument to the command used to run the test program. > > # Beware that arguments to "-exec-run" do not have the same semantics as > > # arguments to the "run" command, so USE_MI_COMMAND influences the meaning > > # of ARGS. If USE_MI_COMMAND is true, they are arguments to -exec-run. > > # If USE_MI_COMMAND is false, they are effectively arguments passed > > # to the test program. > > > > I hope it helps! > > I think this is very clear. Should I post an updated patch ? Sure! :). Watchout for the missing second space after a period in the second to last line.
diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp index 6d011b9..26015dc 100644 --- a/gdb/testsuite/lib/mi-support.exp +++ b/gdb/testsuite/lib/mi-support.exp @@ -806,6 +806,12 @@ proc mi_gdb_test { args } { # In patterns, the newline sequence ``\r\n'' is matched explicitly as # ``.*$'' could swallow up output that we attempt to match elsewhere. +# use_mi_command selects whether MI or CLI commands are used by the +# procedure. +# args is passed to the command used to run the test program, "run" or +# "-exec-run", depending on the value of use_mi_command. Therefore, if +# use_mi_command is false, they are effectively args to the test program. +# If use_mi_command is true, they are simply arguments to -exec-run. proc mi_run_cmd_full {use_mi_command args} { global suppress_flag if { $suppress_flag } {