This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] microMIPS support


On Thu, 26 Apr 2012, Eli Zaretskii wrote:

> > > 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).

 OK, but is that a problem?  If a foreign locale uses a different sorting 
order, then it's exactly what the user of that locale expects, isn't it?  
Unless that sorting is broken for some reason, that is, but that's a 
problem to solve in the particular collator implementation, such as the C 
library in question.

 As far as I know writing MIPS in small letters when referred to the name 
of the architecture (as opposed to a keyword, such in GAS's `.set mips0' 
directive) is incorrect.

> > > 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.

 As the platform maintainer I can offer you to review the whole manual and 
adjust all the instances of "MIPS" (plus any variants and any related 
acronyms I'll spot) as a separate change.  I think it will be more 
productive and not really a lot of effort.

> >  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}?

 D'oh, that's what I did -- how did you know?  That however has prompted 
me to check the texinfo manual and it looks like @sc is actually not what 
fits here:

 "As shown here, we recommend using `@acronym' for actual acronyms
(*note acronym::), and reserving `@sc' for special cases where you want
small caps.  The output is not the same (`@acronym' prints in a smaller
text font, not the small caps font), but more importantly it describes
the actual text more accurately."

so I'd rather stick to @acronym (besides, "microMIPS" is supposed to use 
small letters for the first part and not small capitals).

 But then the application of @acronym to anything that is derived from 
"MIPS" is questionable, because "MIPS" is considered a proper name these 
days, not an acronym anymore (it's a trademark as well, and as such the 
word is an adjective, as are any derivatives, such as "MIPS16", etc.).  
Some probably don't even remember or know what the original acronym was 
supposed to expand to.  Of course there is no question about words such as 
ISA or ABI, these are true acronyms.

 So in the end it looks to me "MIPS", etc. should actually use no special 
formatting (except possibly where used as keywords), so while I can still 
go through the manual, I'll only adjust items like ISA or ASE.

> > 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.

 I think the right thing is to fix it all in one go first and only then 
enforce the new rule.  I think holding a useful functional change just 
because it keeps using the established practice in documentation is 
counter-productive.

> > > > +@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.

 It makes no sense to me not to revamp the entire section -- the parent 
section will only have a reference to one of its child subsections only; 
that's missing the point IMHO.

> >  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.

 I'm getting a continuous flow of text in the PDF version with no page 
breaks at section boundaries -- these I only get between whole chapters.

> > 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.

 It doesn't make sense to me to make this whole section flat, the matters 
described are distinct enough, I just can't seem convinced we need all 
these nodes here.

> >  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.

 These are the three subsubsections in the section concerned 
(Architecture-Specific Protocol Details).  With my change in place two 
will be titled "Breakpoint Kinds" so I prepended their containing 
subsection names (ARM and MIPS) to the respective index entries.  
Likewise I can see no sense in having a generic index entry such as 
"Register Packet Format" for a reference to a platform-specific piece of 
text, so I have prepended the subsection (platform) name there too.

> > > OK with those changes.
> > 
> >  Well, as you can see I have troubles to agree, sorry.
> 
> How can I convince you?

 I don't know -- are you suggesting that I start inventing ways to 
convince myself?  I don't think that's going to work as it sort of 
conflicts with the point of view I have.  I am willing to get convinced if 
you give me good arguments of course.

> > > > +  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"?

 OK, that sounds like a plan to me -- how about:

"Set the compressed ISA encoding used by MIPS code."

then?  I think "executable code" is redundant, code is expected to be 
executable.

> > 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

 Yes, I know.

> user will just see
> 
>  set mips compression -- Set the compressed ISA encoding used.
> 
> This looks too vague to me.

 Maybe, maybe not -- the name of the command itself already names the MIPS 
architecture, which is why I thought the description above should be 
enough.  I hope we can agree on the new proposal earlier on though.

  Maciej


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]