From: Nick Roberts <nickrob@snap.net.nz>
To: Daniel Jacobowitz <drow@false.org>
Cc: Eli Zaretskii <eliz@gnu.org>, gdb-patches@sources.redhat.com
Subject: Re: [PATCH] -stack-info-frames
Date: Sun, 19 Jun 2005 22:31:00 -0000 [thread overview]
Message-ID: <17077.61587.164352.664225@farnswood.snap.net.nz> (raw)
In-Reply-To: <20050619145612.GA8219@nevyn.them.org>
> > OK. I've committed the -stack-info-frame part of the change that posted
> > (Sat, 18 Jun 2005 10:52:09 +1200). Perhaps that doesn't follow the
> > letter of the law but I hope it follows the spirit. In any case, I find
> > it easier to make further changes to the repository than juggle patches
> > (as demonstrated shown with my earlier mangling).
>
> No, Nick. You don't get to make up rules as you go along, no matter
> how much the current ones irk you. That patch was never reviewed and
> never approved. Don't do that again; if you won't wait for approval,
> we'll remove you from write-after-approval.
I wasn't trying to make up the rules, just interpret them. I posted a very
similar patch earlier which was reviewed. As no branch/release is imminent,
it seemed a safe thing to do. I know not to apply judgement again. Sorry.
> If you have trouble juggling patches, have a complete checkout for each
> independent project you are working on. That's not hard to do.
I'll have to work out a new routine. Contributing to Emacs works differently.
> +enum mi_cmd_result
> +mi_cmd_stack_info_frame (char *command, char **argv, int argc)
> +{
> + if (argc > 0)
> + error (_("mi_cmd_stack_info_frame: No arguments required"));
> +
> + print_frame_info (get_selected_frame (NULL), 1, LOC_AND_ADDRESS, 0);
> + return MI_CMD_DONE;
> +}
>
> "No arguments required" doesn't make much sense as an error message; it
> suggests that no arguments are necessary, but not that any arguments
> are invalid. But I see there are two uses of it already, and none of
> any other format for functions which take no arguements. So the code
> parts of the patch are belatedly OK...
Where possible, I just copy what is already there.
> > This commit is slightly different in two respects:
> >
> > 1) mi_cmd_stack_info_frame uses print_frame_info instead of
> > print_stack_frame. This follows mi_cmd_stack_list_frames and means
> > that the argument values aren't printed.
> >
> > 2) The documentation for -stack-info-frame previously said (before I
> > removed it) "Get info on the current frame.". I've corrected this to
> > "Get info on the selected frame." I've also removed the argument
> > values from the example as explained in 1).
>
> Despite the fact that you made it up as you went along. Why did you
> decide that this change was a better idea?
Which change?
> The documentation is up to Eli, but I can say with some confidence that
> it is NOT ok, since you didn't really remove argument values from the
> examples. I still see one:
>
> > + @smallexample
> > + (@value{GDBP})
> > + -stack-info-frame
> > + ^done,frame=@{level="1",addr="0x0001076c",func="callee3",
> > + args=[@{name="strarg",value="0x11940 \"A string argument.\""@}],
> > + file="../../../devo/gdb/testsuite/gdb.mi/basics.c",
> > + fullname="/home/foo/bar/devo/gdb/testsuite/gdb.mi/basics.c",line="17"@}
> > + (@value{GDBP})
> > + @end smallexample
> > +
The patch was OK but the diff wasn't. I picked up the backup copy by mistake:
*** /home/nick/src/gdb/doc/gdb.texinfo.~1.269~ 2005-06-19...
--- /home/nick/src/gdb/doc/gdb.texinfo~ 2005-06-19...
***************
^^^
Nick
next prev parent reply other threads:[~2005-06-19 22:31 UTC|newest]
Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top
2005-06-17 22:51 Nick Roberts
2005-06-17 23:01 ` Daniel Jacobowitz
2005-06-17 23:14 ` Daniel Jacobowitz
2005-06-18 1:28 ` Nick Roberts
2005-06-18 1:58 ` Daniel Jacobowitz
2005-06-18 3:16 ` Nick Roberts
2005-06-18 8:25 ` Eli Zaretskii
2005-06-18 8:51 ` Nick Roberts
2005-06-18 10:20 ` Eli Zaretskii
2005-06-18 15:51 ` Daniel Jacobowitz
2005-06-18 22:48 ` Nick Roberts
2005-06-18 15:57 ` Daniel Jacobowitz
2005-06-18 22:48 ` Nick Roberts
2005-06-18 23:20 ` Daniel Jacobowitz
2005-06-19 3:39 ` Nick Roberts
2005-06-19 14:56 ` Daniel Jacobowitz
2005-06-19 18:33 ` Eli Zaretskii
2005-06-19 22:31 ` Nick Roberts
2005-06-20 3:41 ` Eli Zaretskii
2005-06-19 22:31 ` Nick Roberts [this message]
2005-06-20 0:01 ` Daniel Jacobowitz
2005-06-20 1:24 ` Nick Roberts
2005-06-20 1:31 ` Daniel Jacobowitz
2005-06-20 3:43 ` Eli Zaretskii
2005-06-20 5:03 ` Nick Roberts
2005-06-20 13:51 ` Daniel Jacobowitz
2005-06-20 21:48 ` [PATCH] MI error messages Nick Roberts
2005-06-20 22:10 ` Andreas Schwab
2005-06-21 3:40 ` Eli Zaretskii
2005-06-21 8:50 ` Andreas Schwab
2005-06-21 9:40 ` Nick Roberts
2005-06-21 19:51 ` Eli Zaretskii
2005-06-21 21:43 ` Nick Roberts
2005-06-21 21:59 ` Jason Molenda
2005-06-22 3:32 ` Daniel Jacobowitz
2005-06-22 3:41 ` Jason Molenda
2005-06-22 3:46 ` Daniel Jacobowitz
2005-06-22 7:27 ` Nick Roberts
2005-06-23 3:48 ` Daniel Jacobowitz
2005-06-22 13:04 ` Bob Rossi
2005-06-21 22:01 ` Bob Rossi
2005-06-22 11:06 ` Nick Roberts
2005-06-22 11:24 ` Andreas Schwab
2005-06-22 19:19 ` Jason Molenda
2005-06-22 21:55 ` Nick Roberts
2005-06-19 21:55 ` [PATCH] -stack-info-frames Jason Molenda
2005-06-19 22:12 ` Daniel Jacobowitz
2005-06-17 23:11 ` Jason Molenda
2005-06-17 23:31 ` Nick Roberts
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=17077.61587.164352.664225@farnswood.snap.net.nz \
--to=nickrob@snap.net.nz \
--cc=drow@false.org \
--cc=eliz@gnu.org \
--cc=gdb-patches@sources.redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox