Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH] Doxygenate defs.h
@ 2014-02-17 21:57 Stan Shebs
  2014-02-17 22:17 ` Mark Kettenis
  2014-02-26  0:02 ` Stan Shebs
  0 siblings, 2 replies; 9+ messages in thread
From: Stan Shebs @ 2014-02-17 21:57 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 645 bytes --]

This is a first patch that modifies source code to be more useful with
Doxygen.  It does little more than add an extra "*" to comment blocks
that document the source construct immediately following.

In keeping with our usual practice, I have not changed anything outside
comments, and the comments themselves are only minimally tweaked,
despite the great temptation to expand on some of the more cryptic. :-)

I'll push this in a couple days if people are willing to live with this
format for comments.  Next up, minsyms.h.

Stan
stan@codesourcery.com

2014-02-17  Stan Shebs  <stan@codesourcery.com>

	* defs.h: Annotate comments for Doxygen.

[-- Attachment #2: defs-patch-1 --]
[-- Type: text/plain, Size: 13855 bytes --]

diff --git a/gdb/defs.h b/gdb/defs.h
index a9669cf..0bb1f7f 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -103,13 +103,13 @@
 
 #include "bfd.h"
 
-/* A byte from the program being debugged.  */
+/* * A byte from the program being debugged.  */
 typedef bfd_byte gdb_byte;
 
-/* An address in the program being debugged.  Host byte order.  */
+/* * An address in the program being debugged.  Host byte order.  */
 typedef bfd_vma CORE_ADDR;
 
-/* The largest CORE_ADDR value.  */
+/* * The largest CORE_ADDR value.  */
 #define CORE_ADDR_MAX (~ (CORE_ADDR) 0)
 
 /* This is to make sure that LONGEST is at least as big as CORE_ADDR.  */
@@ -135,23 +135,23 @@ typedef bfd_vma CORE_ADDR;
 
 #include "ptid.h"
 
-/* Enable xdb commands if set.  */
+/* * Enable xdb commands if set.  */
 extern int xdb_commands;
 
-/* Enable dbx commands if set.  */
+/* * Enable dbx commands if set.  */
 extern int dbx_commands;
 
-/* System root path, used to find libraries etc.  */
+/* * System root path, used to find libraries etc.  */
 extern char *gdb_sysroot;
 
-/* GDB datadir, used to store data files.  */
+/* * GDB datadir, used to store data files.  */
 extern char *gdb_datadir;
 
-/* If non-NULL, the possibly relocated path to python's "lib" directory
+/* * If non-NULL, the possibly relocated path to python's "lib" directory
    specified with --with-python.  */
 extern char *python_libdir;
 
-/* Search path for separate debug files.  */
+/* * Search path for separate debug files.  */
 extern char *debug_file_directory;
 
 /* GDB has two methods for handling SIGINT.  When immediate_quit is
@@ -163,12 +163,12 @@ extern char *debug_file_directory;
    These functions use the extension_language_ops API to allow extension
    language(s) and GDB SIGINT handling to coexist seamlessly.  */
 
-/* Clear the quit flag.  */
+/* * Clear the quit flag.  */
 extern void clear_quit_flag (void);
-/* Evaluate to non-zero if the quit flag is set, zero otherwise.  This
+/* * Evaluate to non-zero if the quit flag is set, zero otherwise.  This
    will clear the quit flag as a side effect.  */
 extern int check_quit_flag (void);
-/* Set the quit flag.  */
+/* * Set the quit flag.  */
 extern void set_quit_flag (void);
 
 extern int immediate_quit;
@@ -187,7 +187,7 @@ extern void quit (void);
   if (deprecated_interactive_hook) deprecated_interactive_hook (); \
 }
 
-/* Languages represented in the symbol table and elsewhere.
+/* * Languages represented in the symbol table and elsewhere.
    This should probably be in language.h, but since enum's can't
    be forward declared to satisfy opaque references before their
    actual definition, needs to be here.  */
@@ -219,7 +219,8 @@ enum precision_type
     unspecified_precision
   };
 
-/* A generic, not quite boolean, enumeration.  */
+/* * A generic, not quite boolean, enumeration.  This is used for
+   set/show commands in which the options are on/off/automatic.  */
 enum auto_boolean
 {
   AUTO_BOOLEAN_TRUE,
@@ -227,26 +228,28 @@ enum auto_boolean
   AUTO_BOOLEAN_AUTO
 };
 
-/* Potential ways that a function can return a value of a given type.  */
+/* * Potential ways that a function can return a value of a given
+   type.  */
+
 enum return_value_convention
 {
-  /* Where the return value has been squeezed into one or more
+  /* * Where the return value has been squeezed into one or more
      registers.  */
   RETURN_VALUE_REGISTER_CONVENTION,
-  /* Commonly known as the "struct return convention".  The caller
+  /* * Commonly known as the "struct return convention".  The caller
      passes an additional hidden first parameter to the caller.  That
      parameter contains the address at which the value being returned
      should be stored.  While typically, and historically, used for
      large structs, this is convention is applied to values of many
      different types.  */
   RETURN_VALUE_STRUCT_CONVENTION,
-  /* Like the "struct return convention" above, but where the ABI
+  /* * Like the "struct return convention" above, but where the ABI
      guarantees that the called function stores the address at which
      the value being returned is stored in a well-defined location,
      such as a register or memory slot in the stack frame.  Don't use
      this if the ABI doesn't explicitly guarantees this.  */
   RETURN_VALUE_ABI_RETURNS_ADDRESS,
-  /* Like the "struct return convention" above, but where the ABI
+  /* * Like the "struct return convention" above, but where the ABI
      guarantees that the address at which the value being returned is
      stored will be available in a well-defined location, such as a
      register or memory slot in the stack frame.  Don't use this if
@@ -284,16 +287,16 @@ extern char *re_comp (const char *);
 
 extern void symbol_file_command (char *, int);
 
-/* Remote targets may wish to use this as their load function.  */
+/* * Remote targets may wish to use this as their load function.  */
 extern void generic_load (char *name, int from_tty);
 
-/* Report on STREAM the performance of memory transfer operation,
+/* * Report on STREAM the performance of memory transfer operation,
    such as 'load'.
-   DATA_COUNT is the number of bytes transferred.
-   WRITE_COUNT is the number of separate write operations, or 0,
+   @param DATA_COUNT is the number of bytes transferred.
+   @param WRITE_COUNT is the number of separate write operations, or 0,
    if that information is not available.
-   START_TIME is the time at which an operation was started.
-   END_TIME is the time at which an operation ended.  */
+   @param START_TIME is the time at which an operation was started.
+   @param END_TIME is the time at which an operation ended.  */
 struct timeval;
 extern void print_transfer_performance (struct ui_file *stream,
 					unsigned long data_count,
@@ -359,40 +362,41 @@ extern void init_source_path (void);
 
 /* From exec.c */
 
-/* Process memory area starting at ADDR with length SIZE.  Area is readable iff
-   READ is non-zero, writable if WRITE is non-zero, executable if EXEC is
-   non-zero.  Area is possibly changed against its original file based copy if
-   MODIFIED is non-zero.  DATA is passed without changes from a caller.  */
+/* * Process memory area starting at ADDR with length SIZE.  Area is
+   readable iff READ is non-zero, writable if WRITE is non-zero,
+   executable if EXEC is non-zero.  Area is possibly changed against
+   its original file based copy if MODIFIED is non-zero.  DATA is
+   passed without changes from a caller.  */
 
 typedef int (*find_memory_region_ftype) (CORE_ADDR addr, unsigned long size,
 					 int read, int write, int exec,
 					 int modified, void *data);
 
-/* Take over the 'find_mapped_memory' vector from exec.c.  */
+/* * Take over the 'find_mapped_memory' vector from exec.c.  */
 extern void exec_set_find_memory_regions
   (int (*func) (find_memory_region_ftype func, void *data));
 
-/* Possible lvalue types.  Like enum language, this should be in
+/* * Possible lvalue types.  Like enum language, this should be in
    value.h, but needs to be here for the same reason.  */
 
 enum lval_type
   {
-    /* Not an lval.  */
+    /* * Not an lval.  */
     not_lval,
-    /* In memory.  */
+    /* * In memory.  */
     lval_memory,
-    /* In a register.  Registers are relative to a frame.  */
+    /* * In a register.  Registers are relative to a frame.  */
     lval_register,
-    /* In a gdb internal variable.  */
+    /* * In a gdb internal variable.  */
     lval_internalvar,
-    /* Part of a gdb internal variable (structure field).  */
+    /* * Part of a gdb internal variable (structure field).  */
     lval_internalvar_component,
-    /* Value's bits are fetched and stored using functions provided by
-       its creator.  */
+    /* * Value's bits are fetched and stored using functions provided
+       by its creator.  */
     lval_computed
   };
 
-/* Control types for commands */
+/* * Control types for commands.  */
 
 enum misc_command_type
   {
@@ -416,17 +420,17 @@ enum command_control_type
     invalid_control
   };
 
-/* Structure for saved commands lines
-   (for breakpoints, defined commands, etc).  */
+/* * Structure for saved commands lines (for breakpoints, defined
+   commands, etc).  */
 
 struct command_line
   {
     struct command_line *next;
     char *line;
     enum command_control_type control_type;
-    /* The number of elements in body_list.  */
+    /* * The number of elements in body_list.  */
     int body_count;
-    /* For composite commands, the nested lists of commands.  For
+    /* * For composite commands, the nested lists of commands.  For
        example, for "if" command this will contain the then branch and
        the else branch, if that is available.  */
     struct command_line **body_list;
@@ -441,44 +445,44 @@ extern struct command_line *read_command_lines_1 (char * (*) (void), int,
 
 extern void free_command_lines (struct command_line **);
 
-/* Parameters of the "info proc" command.  */
+/* * Parameters of the "info proc" command.  */
 
 enum info_proc_what
   {
-    /* Display the default cmdline, cwd and exe outputs.  */
+    /* * Display the default cmdline, cwd and exe outputs.  */
     IP_MINIMAL,
 
-    /* Display `info proc mappings'.  */
+    /* * Display `info proc mappings'.  */
     IP_MAPPINGS,
 
-    /* Display `info proc status'.  */
+    /* * Display `info proc status'.  */
     IP_STATUS,
 
-    /* Display `info proc stat'.  */
+    /* * Display `info proc stat'.  */
     IP_STAT,
 
-    /* Display `info proc cmdline'.  */
+    /* * Display `info proc cmdline'.  */
     IP_CMDLINE,
 
-    /* Display `info proc exe'.  */
+    /* * Display `info proc exe'.  */
     IP_EXE,
 
-    /* Display `info proc cwd'.  */
+    /* * Display `info proc cwd'.  */
     IP_CWD,
 
-    /* Display all of the above.  */
+    /* * Display all of the above.  */
     IP_ALL
   };
 
-/* String containing the current directory (what getwd would return).  */
+/* * String containing the current directory (what getwd would return).  */
 
 extern char *current_directory;
 
-/* Default radixes for input and output.  Only some values supported.  */
+/* * Default radixes for input and output.  Only some values supported.  */
 extern unsigned input_radix;
 extern unsigned output_radix;
 
-/* Possibilities for prettyformat parameters to routines which print
+/* * Possibilities for prettyformat parameters to routines which print
    things.  Like enum language, this should be in value.h, but needs
    to be here for the same reason.  FIXME:  If we can eliminate this
    as an arg to LA_VAL_PRINT, then we can probably move it back to
@@ -488,11 +492,11 @@ enum val_prettyformat
   {
     Val_no_prettyformat = 0,
     Val_prettyformat,
-    /* Use the default setting which the user has specified.  */
+    /* * Use the default setting which the user has specified.  */
     Val_prettyformat_default
   };
 
-/* Optional native machine support.  Non-native (and possibly pure
+/* * Optional native machine support.  Non-native (and possibly pure
    multi-arch) targets do not need a "nm.h" file.  This will be a
    symlink to one of the nm-*.h files, built by the `configure'
    script.  */
@@ -544,7 +548,7 @@ enum val_prettyformat
 #define	LONGEST_MAX ((LONGEST)(ULONGEST_MAX >> 1))
 #endif
 
-/* Convert a LONGEST to an int.  This is used in contexts (e.g. number of
+/* * Convert a LONGEST to an int.  This is used in contexts (e.g. number of
    arguments to a function, number in a value history, register number, etc.)
    where the value must not be larger than can fit in an int.  */
 
@@ -552,7 +556,7 @@ extern int longest_to_int (LONGEST);
 
 #include "common-utils.h"
 
-/* List of known OS ABIs.  If you change this, make sure to update the
+/* * List of known OS ABIs.  If you change this, make sure to update the
    table in osabi.c.  */
 enum gdb_osabi
 {
@@ -625,20 +629,20 @@ extern void *alloca ();
 /* Dynamic target-system-dependent parameters for GDB.  */
 #include "gdbarch.h"
 
-/* Maximum size of a register.  Something small, but large enough for
+/* * Maximum size of a register.  Something small, but large enough for
    all known ISAs.  If it turns out to be too small, make it bigger.  */
 
 enum { MAX_REGISTER_SIZE = 64 };
 
 /* Static target-system-dependent parameters for GDB.  */
 
-/* Number of bits in a char or unsigned char for the target machine.
+/* * Number of bits in a char or unsigned char for the target machine.
    Just like CHAR_BIT in <limits.h> but describes the target machine.  */
 #if !defined (TARGET_CHAR_BIT)
 #define TARGET_CHAR_BIT 8
 #endif
 
-/* If we picked up a copy of CHAR_BIT from a configuration file
+/* * If we picked up a copy of CHAR_BIT from a configuration file
    (which may get it by including <limits.h>) then use it to set
    the number of bits in a host char.  If not, use the same size
    as the target.  */
@@ -679,7 +683,7 @@ extern int watchdog;
 
 /* Hooks for alternate command interfaces.  */
 
-/* The name of the interpreter if specified on the command line.  */
+/* * The name of the interpreter if specified on the command line.  */
 extern char *interpreter_p;
 
 /* If a given interpreter matches INTERPRETER_P then it should update
@@ -732,7 +736,7 @@ extern int (*deprecated_ui_load_progress_hook) (const char *section,
 #define ISATTY(FP)	(isatty (fileno (FP)))
 #endif
 
-/* A width that can achieve a better legibility for GDB MI mode.  */
+/* * A width that can achieve a better legibility for GDB MI mode.  */
 #define GDB_MI_MSG_WIDTH  80
 
 /* From progspace.c */
@@ -740,7 +744,7 @@ extern int (*deprecated_ui_load_progress_hook) (const char *section,
 extern void initialize_progspace (void);
 extern void initialize_inferiors (void);
 
-/* Special block numbers */
+/* * Special block numbers */
 
 enum block_enum
 {

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Doxygenate defs.h
  2014-02-17 21:57 [PATCH] Doxygenate defs.h Stan Shebs
@ 2014-02-17 22:17 ` Mark Kettenis
  2014-02-18 11:37   ` Jose E. Marchesi
  2014-02-18 19:52   ` Stan Shebs
  2014-02-26  0:02 ` Stan Shebs
  1 sibling, 2 replies; 9+ messages in thread
From: Mark Kettenis @ 2014-02-17 22:17 UTC (permalink / raw)
  To: stanshebs; +Cc: gdb-patches

> Date: Mon, 17 Feb 2014 13:57:16 -0800
> From: Stan Shebs <stanshebs@earthlink.net>
> 
> This is a first patch that modifies source code to be more useful with
> Doxygen.  It does little more than add an extra "*" to comment blocks
> that document the source construct immediately following.
> 
> In keeping with our usual practice, I have not changed anything outside
> comments, and the comments themselves are only minimally tweaked,
> despite the great temptation to expand on some of the more cryptic. :-)
> 
> I'll push this in a couple days if people are willing to live with this
> format for comments.  Next up, minsyms.h.

Sorry, no, I'm not willing to live with this.  It's making the
comments significantly harder to read.  And what benefit does the
documentation have over just reading the header file? There really is
only one thing that the old internals documentation tried to provide
that the comments in the source code aren't very good at: explaining
how the interfaces work together.  And that's not something Doxygen is
going to provide.

BTW, you realize this all violates the GNU coding standards.

> 2014-02-17  Stan Shebs  <stan@codesourcery.com>
> 
> 	* defs.h: Annotate comments for Doxygen.
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Doxygenate defs.h
  2014-02-17 22:17 ` Mark Kettenis
@ 2014-02-18 11:37   ` Jose E. Marchesi
  2014-02-18 14:14     ` Simon Marchi
  2014-02-19  1:47     ` Yao Qi
  2014-02-18 19:52   ` Stan Shebs
  1 sibling, 2 replies; 9+ messages in thread
From: Jose E. Marchesi @ 2014-02-18 11:37 UTC (permalink / raw)
  To: Mark Kettenis; +Cc: stanshebs, gdb-patches


    > This is a first patch that modifies source code to be more useful with
    > Doxygen.  It does little more than add an extra "*" to comment blocks
    > that document the source construct immediately following.
    > 
    > In keeping with our usual practice, I have not changed anything outside
    > comments, and the comments themselves are only minimally tweaked,
    > despite the great temptation to expand on some of the more cryptic. :-)
    > 
    > I'll push this in a couple days if people are willing to live with this
    > format for comments.  Next up, minsyms.h.
    
    Sorry, no, I'm not willing to live with this.  It's making the
    comments significantly harder to read.  And what benefit does the
    documentation have over just reading the header file? There really is
    only one thing that the old internals documentation tried to provide
    that the comments in the source code aren't very good at: explaining
    how the interfaces work together.  And that's not something Doxygen is
    going to provide.

I am of the same opinion.  Usually only managers ever "use"
Doxygen-generated "documentation" of C programs, and mostly only because
it is required as a deliverable by contractual reasons.

Most developers will just open the header files and read them, using
some indexing tool (ctags, CEDET/Emacs, whatever Eclipse uses..) for
jumping through references.  IMO polluting the comments like this,
restating the obvious with marks like @param, will only make them more
difficult to read with no practical benefit: what gdb hacker will ever
fire up a Firefox or similar to find out what the parameters to some
function are?

If the decision to use Doxygen has been already taken, would it be
possible to at least avoid these @param marks?


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Doxygenate defs.h
  2014-02-18 11:37   ` Jose E. Marchesi
@ 2014-02-18 14:14     ` Simon Marchi
  2014-02-19  1:47     ` Yao Qi
  1 sibling, 0 replies; 9+ messages in thread
From: Simon Marchi @ 2014-02-18 14:14 UTC (permalink / raw)
  To: gdb-patches

On 14-02-18 06:40 AM, Jose E. Marchesi wrote:
> 
>     > This is a first patch that modifies source code to be more useful with
>     > Doxygen.  It does little more than add an extra "*" to comment blocks
>     > that document the source construct immediately following.
>     > 
>     > In keeping with our usual practice, I have not changed anything outside
>     > comments, and the comments themselves are only minimally tweaked,
>     > despite the great temptation to expand on some of the more cryptic. :-)
>     > 
>     > I'll push this in a couple days if people are willing to live with this
>     > format for comments.  Next up, minsyms.h.
>     
>     Sorry, no, I'm not willing to live with this.  It's making the
>     comments significantly harder to read.  And what benefit does the
>     documentation have over just reading the header file? There really is
>     only one thing that the old internals documentation tried to provide
>     that the comments in the source code aren't very good at: explaining
>     how the interfaces work together.  And that's not something Doxygen is
>     going to provide.
> 
> I am of the same opinion.  Usually only managers ever "use"
> Doxygen-generated "documentation" of C programs, and mostly only because
> it is required as a deliverable by contractual reasons.
> 
> Most developers will just open the header files and read them, using
> some indexing tool (ctags, CEDET/Emacs, whatever Eclipse uses..) for
> jumping through references.  IMO polluting the comments like this,
> restating the obvious with marks like @param, will only make them more
> difficult to read with no practical benefit: what gdb hacker will ever
> fire up a Firefox or similar to find out what the parameters to some
> function are?
> 
> If the decision to use Doxygen has been already taken, would it be
> possible to at least avoid these @param marks?
> 

I agree that the @param adds some weight to the code, but it doesn't really bother me.

What I like of it is that it adds some form of standardization, both for the form and the content. Currently, function headers (when they exist) are usually one paragraph that sometimes describe some of the arguments. This format makes sure you have at least one statement describing the function and then one line per argument. If the structure of the comments is always the same, it might actually be easier to read, even though it's more text.

Simon

/////////////////////////////////////////////////
/// at least we're not using comments like this... oh god why.
/////////////////////////////////////////////////


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Doxygenate defs.h
  2014-02-17 22:17 ` Mark Kettenis
  2014-02-18 11:37   ` Jose E. Marchesi
@ 2014-02-18 19:52   ` Stan Shebs
  2014-02-18 23:38     ` Doug Evans
  1 sibling, 1 reply; 9+ messages in thread
From: Stan Shebs @ 2014-02-18 19:52 UTC (permalink / raw)
  To: gdb-patches

On 2/17/14 2:17 PM, Mark Kettenis wrote:
>> Date: Mon, 17 Feb 2014 13:57:16 -0800
>> From: Stan Shebs <stanshebs@earthlink.net>
>>
>> This is a first patch that modifies source code to be more useful with
>> Doxygen.  It does little more than add an extra "*" to comment blocks
>> that document the source construct immediately following.
>>
>> In keeping with our usual practice, I have not changed anything outside
>> comments, and the comments themselves are only minimally tweaked,
>> despite the great temptation to expand on some of the more cryptic. :-)
>>
>> I'll push this in a couple days if people are willing to live with this
>> format for comments.  Next up, minsyms.h.
> 
> Sorry, no, I'm not willing to live with this.  It's making the
> comments significantly harder to read.

Really?  We have a half-million lines of C, the language whose syntax is
one step above line noise, and it's an extra asterisk in comment blocks
that makes it significantly harder to read? :-)

> And what benefit does the
> documentation have over just reading the header file?

Cross-links and formatting, to start with.  For instance, clicking on
the name of a struct in a function signature takes you to its
definition.  If reading the header file suffices for you, that's great,
but I personally spend a lot of time grepping around and then trying to
make sense of the spew.

> There really is
> only one thing that the old internals documentation tried to provide
> that the comments in the source code aren't very good at: explaining
> how the interfaces work together.  And that's not something Doxygen is
> going to provide.

Doxygen actually has sufficient machinery to build a version of the
internals manual from comment blocks in the code; I didn't lead with
that because the individual construct documentation is useful to
people, and a simpler starting place.  But I can start with that if you
like.

> BTW, you realize this all violates the GNU coding standards.

Really?  I'd be interested in the specific passages that you think
are being violated.  I note that the the standards have very few
hard rules that individual projects cannot decide to supersede,
mostly having to do with the copyleft.

I also note that libstdc++, GNU radio, and other GNU projects have been
using Doxygen for some time, so it's not like GDB is even the first.

Stan
stan@codesourcery.com


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Doxygenate defs.h
  2014-02-18 19:52   ` Stan Shebs
@ 2014-02-18 23:38     ` Doug Evans
  2014-02-19  0:08       ` Stan Shebs
  0 siblings, 1 reply; 9+ messages in thread
From: Doug Evans @ 2014-02-18 23:38 UTC (permalink / raw)
  To: Stan Shebs; +Cc: gdb-patches

On Tue, Feb 18, 2014 at 11:52 AM, Stan Shebs <stanshebs@earthlink.net> wrote:
> On 2/17/14 2:17 PM, Mark Kettenis wrote:
>>> Date: Mon, 17 Feb 2014 13:57:16 -0800
>>> From: Stan Shebs <stanshebs@earthlink.net>
>>>
>>> This is a first patch that modifies source code to be more useful with
>>> Doxygen.  It does little more than add an extra "*" to comment blocks
>>> that document the source construct immediately following.
>>>
>>> In keeping with our usual practice, I have not changed anything outside
>>> comments, and the comments themselves are only minimally tweaked,
>>> despite the great temptation to expand on some of the more cryptic. :-)
>>>
>>> I'll push this in a couple days if people are willing to live with this
>>> format for comments.  Next up, minsyms.h.
>>
>> Sorry, no, I'm not willing to live with this.  It's making the
>> comments significantly harder to read.
>
> Really?  We have a half-million lines of C, the language whose syntax is
> one step above line noise, and it's an extra asterisk in comment blocks
> that makes it significantly harder to read? :-)

I don't find the new defs.h significantly harder to read at all.
I wonder though, having seen it in action so to speak, if "/* * "
could be replaced with "/** " (same as now with the space between the
* * deleted).

>> And what benefit does the
>> documentation have over just reading the header file?

One thing I like about Doxygen is the improved S/N ratio when trying
to understand what the API of any particular module provides.  Headers
help a bit, but not completely, but it's now worse because when I want
to edit a function I now have two files to potentially deal with.
Plus I *have* to write a silly little one liner at the function
definition site that says "go see the .h".   Blech!  If I were allowed
to disapprove of the move of function documentation to headers I
would.

> Cross-links and formatting, to start with.  For instance, clicking on
> the name of a struct in a function signature takes you to its
> definition.  If reading the header file suffices for you, that's great,
> but I personally spend a lot of time grepping around and then trying to
> make sense of the spew.
>
>> There really is
>> only one thing that the old internals documentation tried to provide
>> that the comments in the source code aren't very good at: explaining
>> how the interfaces work together.  And that's not something Doxygen is
>> going to provide.
>
> Doxygen actually has sufficient machinery to build a version of the
> internals manual from comment blocks in the code; I didn't lead with
> that because the individual construct documentation is useful to
> people, and a simpler starting place.  But I can start with that if you
> like.

I like the benefit of an internals manual coming from the code.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Doxygenate defs.h
  2014-02-18 23:38     ` Doug Evans
@ 2014-02-19  0:08       ` Stan Shebs
  0 siblings, 0 replies; 9+ messages in thread
From: Stan Shebs @ 2014-02-19  0:08 UTC (permalink / raw)
  To: gdb-patches

On 2/18/14 3:38 PM, Doug Evans wrote:
> On Tue, Feb 18, 2014 at 11:52 AM, Stan Shebs <stanshebs@earthlink.net> wrote:
>> On 2/17/14 2:17 PM, Mark Kettenis wrote:
>>>> Date: Mon, 17 Feb 2014 13:57:16 -0800
>>>> From: Stan Shebs <stanshebs@earthlink.net>
>>>>
>>>> This is a first patch that modifies source code to be more useful with
>>>> Doxygen.  It does little more than add an extra "*" to comment blocks
>>>> that document the source construct immediately following.
>>>>
>>>> In keeping with our usual practice, I have not changed anything outside
>>>> comments, and the comments themselves are only minimally tweaked,
>>>> despite the great temptation to expand on some of the more cryptic. :-)
>>>>
>>>> I'll push this in a couple days if people are willing to live with this
>>>> format for comments.  Next up, minsyms.h.
>>>
>>> Sorry, no, I'm not willing to live with this.  It's making the
>>> comments significantly harder to read.
>>
>> Really?  We have a half-million lines of C, the language whose syntax is
>> one step above line noise, and it's an extra asterisk in comment blocks
>> that makes it significantly harder to read? :-)
> 
> I don't find the new defs.h significantly harder to read at all.
> I wonder though, having seen it in action so to speak, if "/* * "
> could be replaced with "/** " (same as now with the space between the
> * * deleted).

If the "/** " is on the same line as text, Emacs will want to indent the
rest of the block by 4, instead of the usual 3.  The other option is to
put the "/**" on a line by itself, which is plausible but wastes a line
of screen space.  "/* * " is the least-disruptive option I could find.

The filter turns "/* * " into "/** " so that Doxygen recognizes it as
special for its processing.  We could use just about any other string
that is distinctive, but alternatives like "/* ! " don't seem like much
of an improvement.

Stan
stan@codesourcery.com


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Doxygenate defs.h
  2014-02-18 11:37   ` Jose E. Marchesi
  2014-02-18 14:14     ` Simon Marchi
@ 2014-02-19  1:47     ` Yao Qi
  1 sibling, 0 replies; 9+ messages in thread
From: Yao Qi @ 2014-02-19  1:47 UTC (permalink / raw)
  To: Jose E. Marchesi; +Cc: Mark Kettenis, stanshebs, gdb-patches

On 02/18/2014 07:40 PM, Jose E. Marchesi wrote:
> Most developers will just open the header files and read them, using
> some indexing tool (ctags, CEDET/Emacs, whatever Eclipse uses..) for
> jumping through references.  IMO polluting the comments like this,
> restating the obvious with marks like @param, will only make them more
> difficult to read with no practical benefit: what gdb hacker will ever
> fire up a Firefox or similar to find out what the parameters to some
> function are?

I feel @param is useful here.  "@param arg" is equivalent to "ARG" we
are using in comments nowadays, but more descriptive, IMO.  It should
not confuse any GDB newbie.  Some refactor tools can update @param when
argument is renamed, to keep comments consistent with code.

We use doxygen to generate internal documentation from source code, not
only function comments, but also general overview of each 'module'.  It
isn't hard to understand each function comments but hard to get a big
picture of each 'module'.  Doxygen helps!

-- 
Yao (齐尧)


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Doxygenate defs.h
  2014-02-17 21:57 [PATCH] Doxygenate defs.h Stan Shebs
  2014-02-17 22:17 ` Mark Kettenis
@ 2014-02-26  0:02 ` Stan Shebs
  1 sibling, 0 replies; 9+ messages in thread
From: Stan Shebs @ 2014-02-26  0:02 UTC (permalink / raw)
  To: gdb-patches

On 2/17/14 1:57 PM, Stan Shebs wrote:
> This is a first patch that modifies source code to be more useful with
> Doxygen.  It does little more than add an extra "*" to comment blocks
> that document the source construct immediately following.
> 
> In keeping with our usual practice, I have not changed anything outside
> comments, and the comments themselves are only minimally tweaked,
> despite the great temptation to expand on some of the more cryptic. :-)
> 
> I'll push this in a couple days if people are willing to live with this
> format for comments.  Next up, minsyms.h.
> 
> Stan
> stan@codesourcery.com
> 
> 2014-02-17  Stan Shebs  <stan@codesourcery.com>
> 
> 	* defs.h: Annotate comments for Doxygen.
> 

This is now pushed.  While I recognize that this direction is not
unanimously popular, I think this is something we need to try;
as always, we can back it all out a year from now if we are not finding
it helpful.

Stan
stan@codesourcery.com


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2014-02-26  0:02 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-17 21:57 [PATCH] Doxygenate defs.h Stan Shebs
2014-02-17 22:17 ` Mark Kettenis
2014-02-18 11:37   ` Jose E. Marchesi
2014-02-18 14:14     ` Simon Marchi
2014-02-19  1:47     ` Yao Qi
2014-02-18 19:52   ` Stan Shebs
2014-02-18 23:38     ` Doug Evans
2014-02-19  0:08       ` Stan Shebs
2014-02-26  0:02 ` Stan Shebs

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox