Correct comment. Correct comment. Much commenting; #ifdeffed-out debug

2008-01-16  Thomas Thurman  <tthurman@gnome.org>

        * src/core/bell.c: Correct comment.
        * src/core/main.c: Correct comment.
        * src/core/theme.c: Much commenting; #ifdeffed-out debug code removed.
        * src/core/theme.h: Much commenting.



svn path=/trunk/; revision=3522

ChangeLog

@@ -1,3 +1,10 @@
+2008-01-16  Thomas Thurman  <tthurman@gnome.org>
+
+	* src/core/bell.c: Correct comment.
+	* src/core/main.c: Correct comment.
+	* src/core/theme.c: Much commenting; #ifdeffed-out debug code removed.
+	* src/core/theme.h: Much commenting.
+
 2008-01-13  Thomas Thurman  <tthurman@gnome.org>
 
 	* src/core/bell.c: Commenting.

src/core/bell.c

@@ -174,8 +174,8 @@ meta_bell_flash_fullscreen (MetaDisplay *display,
 
 /**
  * Makes a frame be not flashed; this is the timeout half of
- * meta_frame_flash(). This is done simply by clearing the flash flag and
+ * meta_bell_flash_window_frame(). This is done simply by clearing the
- * queuing a redraw of the frame.
+ * flash flag and queuing a redraw of the frame.
  *
  * If the configure script found we had no XKB, this does not exist.
  *

src/core/main.c

@@ -558,7 +558,7 @@ meta_restart (void)
  * Should they be?
  *
  * \param pref  Which preference has changed
- * \param pref  Arbitrary data (which we ignore)
+ * \param data  Arbitrary data (which we ignore)
  */
 static void
 prefs_changed_callback (MetaPreference pref,

src/ui/theme.c

@@ -21,6 +21,37 @@
  * 02111-1307, USA.
  */
 
+/**
+ * \file theme.c    Making Metacity look pretty
+ *
+ * The window decorations drawn by Metacity are described by files on disk
+ * known internally as "themes" (externally as "window border themes" on
+ * http://art.gnome.org/themes/metacity/ or "Metacity themes"). This file
+ * contains most of the code necessary to support themes; it does not
+ * contain the XML parser, which is in theme-parser.c.
+ *
+ * \bug This is a big file with lots of different subsystems, which might
+ * be better split out into separate files.
+ */
+
+/**
+ * \defgroup tokenizer   The theme expression tokenizer
+ *
+ * Themes can use a simple expression language to represent the values of
+ * things. This is the tokeniser used for that language.
+ *
+ * \bug We could remove almost all this code by using GScanner instead,
+ * but we would also have to find every expression in every existing theme
+ * we could and make sure the parse trees were the same.
+ */
+
+/**
+ * \defgroup parser  The theme expression parser
+ *
+ * Themes can use a simple expression language to represent the values of
+ * things. This is the parser used for that language.
+ */
+
 #include <config.h>
 #include "theme.h"
 #include "theme-parser.h"
@@ -60,6 +91,9 @@ static void hls_to_rgb			(gdouble	 *h,
 					 gdouble	 *l,
 					 gdouble	 *s);
 
+/**
+ * The current theme. (Themes are singleton.)
+ */
 static MetaTheme *meta_current_theme = NULL;
 
 static GdkPixbuf *
@@ -155,6 +189,11 @@ color_composite (const GdkColor *bg,
   color->blue = color->blue + (((fg->blue - color->blue) * alpha + 0x8000) >> 16);
 }
 
+/**
+ * Sets all the fields of a border to dummy values.
+ *
+ * \param border The border whose fields should be reset.
+ */
 static void
 init_border (GtkBorder *border)
 {
@@ -164,6 +203,12 @@ init_border (GtkBorder *border)
   border->right = -1;
 }
 
+/**
+ * Creates a new, empty MetaFrameLayout. The fields will be set to dummy
+ * values.
+ *
+ * \return The newly created MetaFrameLayout.
+ */
 MetaFrameLayout*
 meta_frame_layout_new  (void)
 {
@@ -198,6 +243,9 @@ meta_frame_layout_new  (void)
   return layout;
 }
 
+/**
+ *
+ */
 static gboolean
 validate_border (const GtkBorder *border,
                  const char     **bad)
@@ -216,6 +264,19 @@ validate_border (const GtkBorder *border,
   return *bad == NULL;
 }
 
+/**
+ * Ensures that the theme supplied a particular dimension. When a
+ * MetaFrameLayout is created, all its integer fields are set to -1
+ * by meta_frame_layout_new(). After an instance of this type
+ * should have been initialised, this function checks that
+ * a given field is not still at -1. It is never called directly, but
+ * rather via the CHECK_GEOMETRY_VALUE and CHECK_GEOMETRY_BORDER
+ * macros.
+ *
+ * \param      val    The value to check
+ * \param      name   The name to use in the error message
+ * \param[out] error  Set to an error if val was not initialised
+ */
 static gboolean
 validate_geometry_value (int         val,
                          const char *name,
@@ -1348,6 +1409,12 @@ meta_color_spec_render (MetaColorSpec *spec,
     }
 }
 
+/**
+ * Represents an operation as a string.
+ *
+ * \param type  an operation, such as addition
+ * \return  a string, such as "+"
+ */
 static const char*
 op_name (PosOperatorType type)
 {
@@ -1374,6 +1441,14 @@ op_name (PosOperatorType type)
   return "<unknown>";
 }
 
+/**
+ * Parses a string and returns an operation.
+ *
+ * \param p  a pointer into a string representing an operation; part of an
+ *           expression somewhere, so not null-terminated
+ * \param len  set to the length of the string found. Set to 0 if none is.
+ * \return  the operation found. If none was, returns POS_OP_NONE.
+ */
 static PosOperatorType
 op_from_string (const char *p,
                 int        *len)
@@ -1422,6 +1497,13 @@ op_from_string (const char *p,
   return POS_OP_NONE;
 }
 
+/**
+ * Frees an array of tokens. All the tokens and their associated memory
+ * will be freed.
+ *
+ * \param tokens  an array of tokens to be freed
+ * \param n_tokens  how many tokens are in the array.
+ */
 static void
 free_tokens (PosToken *tokens,
              int       n_tokens)
@@ -1439,6 +1521,22 @@ free_tokens (PosToken *tokens,
   g_free (tokens);
 }
 
+/**
+ * Tokenises a number in an expression.
+ *
+ * \param p  a pointer into a string representing an operation; part of an
+ *           expression somewhere, so not null-terminated
+ * \param end_return  set to a pointer to the end of the number found; but
+ *                    not updated if no number was found at all
+ * \param next  set to either an integer or a float token
+ * \param[out] err  set to the problem if there was a problem
+ * \return TRUE if a valid number was found, FALSE otherwise (and "err" will
+ *         have been set)
+ *
+ * \bug The "while (*start)..." part: what's wrong with strchr-ish things?
+ * \bug The name is wrong: it doesn't parse anything.
+ * \ingroup tokenizer
+ */
 static gboolean
 parse_number (const char  *p,
               const char **end_return,
@@ -1514,6 +1612,9 @@ parse_number (const char  *p,
   return TRUE;
 }
 
+/**
+ * Whether a variable can validly appear as part of the name of a variable.
+ */
 #define IS_VARIABLE_CHAR(c) (g_ascii_isalpha ((c)) || (c) == '_')
 
 #if 0
@@ -1556,6 +1657,18 @@ debug_print_tokens (PosToken *tokens,
 }
 #endif
 
+/**
+ * Tokenises an expression.
+ *
+ * \param      expr        The expression
+ * \param[out] token_p     The resulting tokens
+ * \param[out] n_tokens_p  The number of resulting tokens
+ * \param[out] err  set to the problem if there was a problem
+ *
+ * \return  True if the expression was successfully tokenised; false otherwise.
+ *
+ * \ingroup tokenizer
+ */
 static gboolean
 pos_tokenize (const char  *expr,
               PosToken   **tokens_p,
@@ -1688,6 +1801,10 @@ typedef enum
   POS_EXPR_OPERATOR
 } PosExprType;
 
+/**
+ *
+ * \bug operator is char; it should really be of PosOperatorType.
+ */
 typedef struct
 {
   PosExprType type;
@@ -1976,14 +2093,37 @@ do_operations (PosExpr *exprs,
   return TRUE;
 }
 
+/**
+ * There is a predefined set of variables which can appear in an expression.
+ * Here we take a token representing a variable, and return the current value
+ * of that variable in a particular environment.
+ * (The value is always an integer.)
+ *
+ * There are supposedly some circumstances in which this function can be
+ * called from outside Metacity, in which case env->theme will be NULL, and
+ * therefore we can't use it to find out quark values, so we do the comparison
+ * using strcmp, which is slower.
+ *
+ * \param t  The token representing a variable
+ * \param[out] result  The value of that variable; not set if the token did
+ *                     not represent a known variable
+ * \param env  The environment within which t should be evaluated
+ * \param[out] err  set to the problem if there was a problem
+ *
+ * \return true if we found the variable asked for, false if we didn't
+ *
+ * \bug shouldn't t be const?
+ * \bug we should perhaps consider some sort of lookup arrangement into an
+ *      array; also, the duplication of code is unlovely; perhaps using glib
+ *      string hashes instead of quarks would fix both problems?
+ * \ingroup parser
+ */
 static gboolean
 pos_eval_get_variable (PosToken                  *t,
                        int                       *result,
                        const MetaPositionExprEnv *env,
                        GError                   **err)
 {
-  /* In certain circumstances (when the theme parser is used outside
-     of metacity) env->theme will be NULL so we run the slow variable search */
   if (env->theme)
     {
       if (t->d.v.name_quark == env->theme->quark_width)
@@ -2070,6 +2210,12 @@ pos_eval_get_variable (PosToken                  *t,
   return TRUE;
 }
 
+/**
+ * foo
+ *
+ * \param tokens
+ * \bug FIXME write this
+ */
 static gboolean
 pos_eval_helper (PosToken                   *tokens,
                  int                         n_tokens,
@@ -2077,7 +2223,7 @@ pos_eval_helper (PosToken                   *tokens,
                  PosExpr                    *result,
                  GError                    **err)
 {
-  /* lazy-ass hardcoded limit on expression size */
+  /* Lazy-ass hardcoded limit on number of terms in expression */
 #define MAX_EXPRS 32
   int paren_level;
   int first_paren;
@@ -2086,10 +2232,6 @@ pos_eval_helper (PosToken                   *tokens,
   int n_exprs;
   int precedence;
   
-#if 0
-  g_print ("Pos eval helper on %d tokens:\n", n_tokens);
-#endif
-
   /* Our first goal is to get a list of PosExpr, essentially
    * substituting variables and handling parentheses.
    */

src/ui/theme.h

@@ -56,6 +56,13 @@ typedef enum
   META_THEME_ERROR_FAILED
 } MetaThemeError;
 
+/**
+ * Whether a button's size is calculated from the area around it (aspect
+ * sizing) or is given as a fixed height and width in pixels (fixed sizing).
+ *
+ * \bug This could be done away with; see the comment at the top of
+ * MetaFrameLayout.
+ */
 typedef enum
 {
   META_BUTTON_SIZING_ASPECT,
@@ -63,65 +70,102 @@ typedef enum
   META_BUTTON_SIZING_LAST
 } MetaButtonSizing;
 
-/* Parameters used to calculate the geometry of the frame */
+/**
+ * Various parameters used to calculate the geometry of a frame.
+ *
+ * \bug button_sizing isn't really necessary, because we could easily say
+ * that if button_aspect is zero, the height and width are fixed values.
+ * This would also mean that MetaButtonSizing didn't need to exist, and
+ * save code.
+ **/
 struct _MetaFrameLayout
 {
+  /** Reference count. */
   int refcount;
   
-  /* Size of left/right/bottom sides */
+  /** Size of left side */
   int left_width;
+  /** Size of right side */
   int right_width;
+  /** Size of bottom side */
   int bottom_height;
   
-  /* Border of blue title region */
+  /** Border of blue title region
+   * \bug (blue?!)
+   **/
   GtkBorder title_border;
 
-  /* Extra height for inside of title region, above the font height */
+  /** Extra height for inside of title region, above the font height */
   int title_vertical_pad;
   
-  /* indent of buttons from edges of frame */
+  /** Right indent of buttons from edges of frame */
   int right_titlebar_edge;
+  /** Left indent of buttons from edges of frame */
   int left_titlebar_edge;
   
-  /* Size of buttons */
+  /**
+   * Sizing rule of buttons, either META_BUTTON_SIZING_ASPECT
+   * (in which case button_aspect will be honoured, and
+   * button_width and button_height set from it), or
+   * META_BUTTON_SIZING_FIXED (in which case we read the width
+   * and height directly).
+   */
   MetaButtonSizing button_sizing;
 
-  double button_aspect; /* height / width */
+  /**
+   * Ratio of height/width. Honoured only if
+   * button_sizing==META_BUTTON_SIZING_ASPECT.
+   * Otherwise we figure out the height from the button_border.
+   */
+  double button_aspect;
   
+  /** Width of a button; set even when we are using aspect sizing */
   int button_width;
+
+  /** Height of a button; set even when we are using aspect sizing */
   int button_height;
 
-  /* Space around buttons */
+  /** Space around buttons */
   GtkBorder button_border;
 
-  /* scale factor for title text */
+  /** scale factor for title text */
   double title_scale;
   
-  /* Whether title text will be displayed */
+  /** Whether title text will be displayed */
   guint has_title : 1;
 
-  /* Whether we should hide the buttons */
+  /** Whether we should hide the buttons */
   guint hide_buttons : 1;
 
-  /* Round corners */
+  /** Radius of the top left-hand corner; 0 if not rounded */
   guint top_left_corner_rounded_radius;
+  /** Radius of the top right-hand corner; 0 if not rounded */
   guint top_right_corner_rounded_radius;
+  /** Radius of the bottom left-hand corner; 0 if not rounded */
   guint bottom_left_corner_rounded_radius;
+  /** Radius of the bottom right-hand corner; 0 if not rounded */
   guint bottom_right_corner_rounded_radius;
 };
 
+/**
+ * The size of a button (FIXME: In general? Or on one frame?).
+ * The reason for two different rectangles here is Fitts' law & maximized
+ * windows; see bug #97703 for more details.
+ *
+ * \bug In general? Or on one frame? FIXME
+ * \bug Bugs should be hyperlinked: talk to doxygen people
+ */
 struct _MetaButtonSpace
 {
-  /* The reason for two different rectangles here is Fitts' law & maximized
+  /** The screen area where the button's image is drawn */
-   * windows; see #97703 for more details.
-   *   visible   - The screen area where the button's image is drawn
-   *   clickable - The screen area where the button can be activated by clicking
-   */
   GdkRectangle visible;
+  /** The screen area where the button can be activated by clicking */
   GdkRectangle clickable;
 };
 
-/* Calculated actual geometry of the frame */
+/**
+ * Calculated actual geometry of the frame
+ */
 struct _MetaFrameGeometry
 {
   int left_width;