From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2839 invoked by alias); 26 Apr 2012 14:04:53 -0000 Received: (qmail 2831 invoked by uid 22791); 26 Apr 2012 14:04:52 -0000 X-SWARE-Spam-Status: No, hits=-2.2 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED,RCVD_IN_DNSWL_NONE,RCVD_IN_HOSTKARMA_NO,RCVD_IN_NIX_SPAM,SPF_SOFTFAIL X-Spam-Check-By: sourceware.org Received: from mtaout22.012.net.il (HELO mtaout22.012.net.il) (80.179.55.172) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 26 Apr 2012 14:04:33 +0000 Received: from conversion-daemon.a-mtaout22.012.net.il by a-mtaout22.012.net.il (HyperSendmail v2007.08) id <0M3300D00B20TN00@a-mtaout22.012.net.il> for gdb-patches@sourceware.org; Thu, 26 Apr 2012 17:03:53 +0300 (IDT) Received: from HOME-C4E4A596F7 ([84.229.249.186]) by a-mtaout22.012.net.il (HyperSendmail v2007.08) with ESMTPA id <0M3300DT5B2HPA30@a-mtaout22.012.net.il>; Thu, 26 Apr 2012 17:03:53 +0300 (IDT) Date: Thu, 26 Apr 2012 14:14:00 -0000 From: Eli Zaretskii Subject: Re: [PATCH] microMIPS support In-reply-to: To: "Maciej W. Rozycki" Cc: gdb-patches@sourceware.org Reply-to: Eli Zaretskii Message-id: <83haw6io8k.fsf@gnu.org> References: <83k414fjmp.fsf@gnu.org> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2012-04/txt/msg00906.txt.bz2 > Date: Thu, 26 Apr 2012 14:34:12 +0100 > From: "Maciej W. Rozycki" > CC: > > On Wed, 25 Apr 2012, Eli Zaretskii wrote: > > > > +@item set mips compression @var{arg} > > > +@kindex set mips compression > > > +@cindex MIPS code compression > > > > Please use lower-case "mips" in the @cindex entry as well. > > It is a proper name, why? All the other places capitalise it correctly, > e.g.: > > @kindex set mips mask-address > @cindex MIPS addresses, masking > > and likewise for other proper names, e.g. AIX. Index entries should all start with lower-case letters, otherwise the index sorting is unpredictable (in non-US locales). > > I would suggest to use either @acronym{ISA} or @sc{isa} (and the same > > with "MIPS16" and "MIPS"), since they will look better in print. Try > > both, produce the PDF version of the manual, and keep the one you like > > best. > > Hmm, there is no existing place that uses any formatting whatever for > "MIPS" or any derivatives (see e.g. "MIPS ABI" just above), except from > @code{MIPS32} used in a single place to refer to a packet format. The > same stands for other processor architectures or names of instruction > sets. I am not trying to say that I disagree with your suggestion as is, > only asking why you think I should be making these changes to my patch? Why not? It's TRT to do. > FWIW, I can't tell the difference between text rendered with no > formatting and the @sc macro In the PDF and DVI output, @sc{isa} should yield a smaller font. Maybe you used @sc{ISA}? > As it stands I'd prefer to keep the formatting consistent through the > manual. There's no reason to consistently do the wrong thing. Every 1000-mile journey begins with the first step. > > > +@subsubsection Breakpoint Kinds > > > + > > > +These breakpoint kinds are defined for the @samp{Z0} and @samp{Z1} packets. > > > > I'd prefer to have a @node here; subsections without a node are > > possible, but are harder to find. > > > > Also, a @cindex entry here would be good; think about someone who'd > > like to find this information quickly by searching the index. > > I copied the layout from the "Register Packet Format" subsubsection that > immediately precedes; actually the whole section lacks any subnodes. I > now recall a similar discussion a while ago about another patch; this > change originally predates that discussion. Sorry for missing that. > > However in this case the whole section would have to be updated > throughout, it does not make sense to have a subnode just for one of the > subsubsections, and revamping the whole section does not belong to this > change. I didn't ask you to revamp the entire section. Even if the rest of the section will never be fixed, it still makes sense to not increase the amount of node-less subsections. > And last but not least I am not even sure if splitting this particular > section into subpages makes sense in the first place. As long as we keep the subsection, the printed version of the manual will layout it separately anyway, even if the Info version doesn't. > Nesting too deep can be frustrating too, and here you'd have to have > two intermediate pages with node lists only for just a couple lines' > worth of nodes. I can go with removing the subsection altogether, if you don't feel strongly with keeping it. But if we keep the subsection, I'd like to have a node there. > I think index references would better be added separately too -- these > would be "ARM Breakpoint Kinds," "MIPS Register Packet Format" and "MIPS > Breakpoint Kinds," respectively. Sorry, I don't follow. Please elaborate. > > OK with those changes. > > Well, as you can see I have troubles to agree, sorry. How can I convince you? > > > + add_setshow_enum_cmd ("compression", class_obscure, mips_compression_strings, > > > + &mips_compression_string, _("\ > > > +Set the compressed ISA encoding used."), _("\ > > > +Show the compressed ISA encoding used."), _("\ > > > > I'd mention microMIPS in the doc strings, as in > > > > Set the compressed ISA encoding used by microMIPS binaries. > > Err, that's not the point of the setting in both aspects. First, it's > not specific to microMIPS code as its very purpose is to switch between > MIPS16 and microMIPS encoding that's mutually exclusive. OK, then how about "MIPS" instead of "microMIPS"? > Second, it's > only used where no binary is available (usually selected with "file" or > suchlike), e.g. ROM contents: > > (gdb) target remote foo > (gdb) x/i $pc > (gdb) stepi > > etc. Then how about "executables" or "executable code" instead of "binaries"? > I agree the description is a bit lacking, but I have also decided > the verbose output from: > > (gdb) help set mips compression > (gdb) help show mips compression > > (which is the same as I pasted in NEWS) is good enough. Do you think it > is not? The thing is, commands that display only the first line of the doc string, such as "apropos", will not show the rest of the text. So the user will just see set mips compression -- Set the compressed ISA encoding used. This looks too vague to me.