From 860429f9c9e08b1f27a01c74691f5e7bccd09018 Mon Sep 17 00:00:00 2001 From: infirit Date: Sat, 25 Oct 2014 13:47:29 +0200 Subject: Clean up code to find themes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Simplify code to find the right theme to load and loading it by moving all the loading code into a load_theme() helper function, and making meta_load_theme() use that as it searches through the directories. Look for old-version themes even when loading relative to the working in debug mode. Don't unnecessarily duplicate and then free info->theme_file and info->theme_dir. http://bugzilla.gnome.org/show_bug.cgi?id=592503 NOTE: Patch copied from mutter and adapted for metacity. Based on metacity commit: bc45c9b68849687d456ec339588d15a63c391046 From: Alberts Muktupāvels --- src/ui/theme-parser.c | 290 ++++++++++++++++++++------------------------------ 1 file changed, 118 insertions(+), 172 deletions(-) diff --git a/src/ui/theme-parser.c b/src/ui/theme-parser.c index e79396b2..91b565db 100644 --- a/src/ui/theme-parser.c +++ b/src/ui/theme-parser.c @@ -91,8 +91,8 @@ typedef struct GSList *states; const char *theme_name; /* name of theme (directory it's in) */ - char *theme_file; /* theme filename */ - char *theme_dir; /* dir the theme is inside */ + const char *theme_file; /* theme filename */ + const char *theme_dir; /* dir the theme is inside */ MetaTheme *theme; /* theme being parsed */ guint format_version; /* version of format of theme file */ char *name; /* name of named thing being parsed */ @@ -296,9 +296,6 @@ parse_info_init (ParseInfo *info) static void parse_info_free (ParseInfo *info) { - g_free (info->theme_file); - g_free (info->theme_dir); - g_slist_free (info->states); if (info->theme) @@ -3989,66 +3986,121 @@ text_handler (GMarkupParseContext *context, #define MARCO_THEME_FILENAME_FORMAT "metacity-theme-%d.xml" -MetaTheme* -meta_theme_load (const char *theme_name, - GError **err) +/* If the theme is not-corrupt, keep looking for alternate versions + * in other locations we might be compatible with + */ +static gboolean +theme_error_is_fatal (GError *error) +{ + return error->domain != G_FILE_ERROR; +} + +static MetaTheme* +load_theme (const char *theme_dir, + const char *theme_name, + guint major_version, + GError **error) { GMarkupParseContext *context; - GError *error; ParseInfo info; char *text; gsize length; + char *theme_filename; char *theme_file; - char *theme_dir; MetaTheme *retval; - guint version; - const gchar* const* xdg_data_dirs; - int i; + + g_return_val_if_fail (error && *error == NULL, NULL); text = NULL; - length = 0; retval = NULL; context = NULL; - theme_dir = NULL; - theme_file = NULL; + theme_filename = g_strdup_printf (MARCO_THEME_FILENAME_FORMAT, major_version); + theme_file = g_build_filename (theme_dir, theme_filename, NULL); - if (meta_is_debugging ()) + if (!g_file_get_contents (theme_file, &text, &length, error)) + goto out; + + meta_topic (META_DEBUG_THEMES, "Parsing theme file %s\n", theme_file); + + parse_info_init (&info); + + info.theme_name = theme_name; + info.theme_file = theme_file; + info.theme_dir = theme_dir; + + info.format_version = major_version; + + context = g_markup_parse_context_new (&marco_theme_parser, 0, &info, NULL); + + if (!g_markup_parse_context_parse (context, text, length, error)) + goto out; + + if (!g_markup_parse_context_end_parse (context, error)) + goto out; + + retval = info.theme; + info.theme = NULL; + + out: + if (*error && !theme_error_is_fatal (*error)) + meta_topic (META_DEBUG_THEMES, "Failed to read theme from file %s: %s\n", + theme_file, (*error)->message); + + g_free (theme_filename); + g_free (theme_file); + g_free (text); + + if (context) + { + g_markup_parse_context_free (context); + parse_info_free (&info); + } + + return retval; +} + +static gboolean +keep_trying (GError **error) +{ + if (*error && !theme_error_is_fatal (*error)) { - gchar *theme_filename = g_strdup_printf (MARCO_THEME_FILENAME_FORMAT, - THEME_VERSION); + g_clear_error (error); + return TRUE; + } + + return FALSE; +} - /* Try in themes in our source tree */ - theme_dir = g_build_filename ("./themes", theme_name, NULL); +MetaTheme* +meta_theme_load (const char *theme_name, + GError **err) +{ + GError *error = NULL; + char *theme_dir; + MetaTheme *retval; + const gchar* const* xdg_data_dirs; + int version; + int i; - theme_file = g_build_filename (theme_dir, - theme_filename, - NULL); + retval = NULL; - error = NULL; - if (!g_file_get_contents (theme_file, - &text, - &length, - &error)) + if (meta_is_debugging ()) + { + /* We try all supported major versions from current to oldest */ + for (version = THEME_VERSION; (version > 0); version--) { - meta_topic (META_DEBUG_THEMES, "Failed to read theme from file %s: %s\n", - theme_file, error->message); - g_error_free (error); - g_free (theme_dir); - g_free (theme_file); - theme_file = NULL; - } - version = THEME_VERSION; + theme_dir = g_build_filename ("./themes", theme_name, NULL); + retval = load_theme (theme_dir, theme_name, version, &error); - g_free (theme_filename); + if (!keep_trying (&error)) + goto out; + } } - /* We try all supported versions from current to oldest */ - for (version = THEME_VERSION; (version > 0) && (text == NULL); version--) + /* We try all supported major versions from current to oldest */ + for (version = THEME_VERSION; (version > 0); version--) { - gchar *theme_filename = g_strdup_printf (MARCO_THEME_FILENAME_FORMAT, - version); - /* We try first in home dir, XDG_DATA_DIRS, then system dir for themes */ /* Try home dir for themes */ @@ -4058,155 +4110,49 @@ meta_theme_load (const char *theme_name, THEME_SUBDIR, NULL); - theme_file = g_build_filename (theme_dir, - theme_filename, - NULL); - - error = NULL; - if (!g_file_get_contents (theme_file, - &text, - &length, - &error)) - { - meta_topic (META_DEBUG_THEMES, "Failed to read theme from file %s: %s\n", - theme_file, error->message); - g_error_free (error); - g_free (theme_dir); - g_free (theme_file); - theme_file = NULL; - } + retval = load_theme (theme_dir, theme_name, version, &error); + g_free (theme_dir); + if (!keep_trying (&error)) + goto out; /* Try each XDG_DATA_DIRS for theme */ xdg_data_dirs = g_get_system_data_dirs(); for(i = 0; xdg_data_dirs[i] != NULL; i++) { - if (text == NULL) - { - theme_dir = g_build_filename (xdg_data_dirs[i], - "themes", - theme_name, - THEME_SUBDIR, - NULL); - - theme_file = g_build_filename (theme_dir, - theme_filename, - NULL); - - error = NULL; - if (!g_file_get_contents (theme_file, - &text, - &length, - &error)) - { - meta_topic (META_DEBUG_THEMES, "Failed to read theme from file %s: %s\n", - theme_file, error->message); - g_error_free (error); - g_free (theme_dir); - g_free (theme_file); - theme_file = NULL; - } - else - { - break; - } - } - } - - /* Look for themes in MARCO_DATADIR */ - if (text == NULL) - { - theme_dir = g_build_filename (MARCO_DATADIR, + theme_dir = g_build_filename (xdg_data_dirs[i], "themes", theme_name, THEME_SUBDIR, NULL); - theme_file = g_build_filename (theme_dir, - theme_filename, - NULL); - - error = NULL; - if (!g_file_get_contents (theme_file, - &text, - &length, - &error)) - { - meta_topic (META_DEBUG_THEMES, "Failed to read theme from file %s: %s\n", - theme_file, error->message); - g_error_free (error); - g_free (theme_dir); - g_free (theme_file); - theme_file = NULL; - } + retval = load_theme (theme_dir, theme_name, version, &error); + g_free (theme_dir); + if (!keep_trying (&error)) + goto out; } - - g_free (theme_filename); - } - - if (text == NULL) - { - g_set_error (err, META_THEME_ERROR, META_THEME_ERROR_FAILED, - _("Failed to find a valid file for theme %s\n"), - theme_name); - - return NULL; /* all fallbacks failed */ + /* Look for themes in MARCO_DATADIR */ + theme_dir = g_build_filename (MARCO_DATADIR, + "themes", + theme_name, + THEME_SUBDIR, + NULL); + retval = load_theme (theme_dir, theme_name, version, &error); + g_free (theme_dir); + if (!keep_trying (&error)) + goto out; } - meta_topic (META_DEBUG_THEMES, "Parsing theme file %s\n", theme_file); - - - parse_info_init (&info); - info.theme_name = theme_name; - - /* pass ownership to info so we free it with the info */ - info.theme_file = theme_file; - info.theme_dir = theme_dir; - - info.format_version = version + 1; - - context = g_markup_parse_context_new (&marco_theme_parser, - 0, &info, NULL); - - error = NULL; - if (!g_markup_parse_context_parse (context, - text, - length, - &error)) - goto out; - - error = NULL; - if (!g_markup_parse_context_end_parse (context, &error)) - goto out; - - goto out; - out: - if (context) - g_markup_parse_context_free (context); - g_free (text); - - if (info.theme) - info.theme->format_version = info.format_version; + if (!error && !retval) + g_set_error (&error, META_THEME_ERROR, META_THEME_ERROR_FAILED, + _("Failed to find a valid file for theme %s\n"), + theme_name); if (error) { g_propagate_error (err, error); } - else if (info.theme) - { - /* Steal theme from info */ - retval = info.theme; - info.theme = NULL; - } - else - { - g_set_error (err, G_MARKUP_ERROR, G_MARKUP_ERROR_PARSE, - _("Theme file %s did not contain a root element"), - info.theme_file); - } - - parse_info_free (&info); return retval; } -- cgit v1.2.1