Notify OSD

Merge lp:~macslow/notify-osd/fix-396736 into lp:notify-osd/karmic

  1. fix-396736
  2. Merge into karmic
Proposed by Mirco Müller
Status: Merged
Merged at revision: not available
Proposed branch: lp:~macslow/notify-osd/fix-396736
Merge into: lp:notify-osd/karmic
Diff against target: 508 lines
7 files modified
NEWS (+10/-0)
configure.in (+1/-1)
src/bubble.c (+19/-23)
src/defaults.c (+15/-45)
src/util.c (+103/-0)
src/util.h (+9/-1)
tests/test-text-filtering.c (+69/-2)
To merge this branch: bzr merge lp:~macslow/notify-osd/fix-396736
Reviewer Review Type Date Requested Status
David Barth (community) Approve
Mirco Müller (community) Abstain
Review via email: mp+12954@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Mirco Müller (macslow) wrote :

Replaced GScanner with GRegex to parse the font-name/style/size obtained from gconf. This corrects parsing the full string. Before a font like "Courier New Bold 10" was only interpreted as "Courier" with size 10. Rendering title- and body-text has also been adjusted to take advantage of this. Now that the full name and style are correctly parsed LP: 396736 is fixed too.

Revision history for this message
David Barth (dbarth) wrote :

Needs information or fixing, in that I'd like to see a unit test that shows that the regexp is equivalent to the gscanner thing, and also that it doesn't enter an infinite loop if the font name is weird (unset, or whatever).

The rest looks fine.

review: Needs Information
lp:~macslow/notify-osd/fix-396736 updated
409. By Mirco Müller

made unit-test-able functions out of the reg.-exp. for extracting font-face and point-size, added unit-test for those, also reverted the newline text-filter unit-tests done in rev405, which were forgotton to be reverted in rev407... caused unit-test to fail

Revision history for this message
Mirco Müller (macslow) wrote :

Made extraction of font-face and point-size separate functions in order to make them unit-testable. Added unit-tests for those. Also fixed/reverted unit-test from regression in rev405, which only partily was reveret in rev407.

review: Abstain
Revision history for this message
David Barth (dbarth) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'NEWS'
--- NEWS 2009-04-23 14:26:48 +0000
+++ NEWS 2009-10-19 05:00:34 +0000
@@ -8,3 +8,13 @@
8oper guidelines availabled under:8oper guidelines availabled under:
99
10 https://wiki.ubuntu.com/NotificationDevelopmentGuidelines10 https://wiki.ubuntu.com/NotificationDevelopmentGuidelines
11
12Noteworthy changes since last release (0.9.22):
13
14 * updating NEWS file now, fixes LP: #432486
15 * no long using hard-coded -Werror, fixes LP: #361788
16 * solved regression regarding throbbing-animation for
17 value-indicator, fixes LP: #435116
18 * looking up gravity-gconf-key upon startup, fixes LP: #431200
19 * convert newline characters to spaces, fixes LP: #402247
20
1121
=== modified file 'configure.in'
--- configure.in 2009-09-29 15:48:51 +0000
+++ configure.in 2009-10-19 05:00:34 +0000
@@ -1,4 +1,4 @@
1AC_INIT(notify-osd, 0.9.22, dx-team@canonical.com)1AC_INIT(notify-osd, 0.9.23, dx-team@canonical.com)
22
3AC_CONFIG_SRCDIR(src/main.c)3AC_CONFIG_SRCDIR(src/main.c)
4AC_CONFIG_HEADERS(config.h)4AC_CONFIG_HEADERS(config.h)
55
=== modified file 'src/bubble.c'
--- src/bubble.c 2009-10-15 13:33:14 +0000
+++ src/bubble.c 2009-10-19 05:00:35 +0000
@@ -965,21 +965,19 @@
965965
966 // create pango desc/layout966 // create pango desc/layout
967 layout = pango_cairo_create_layout (cr);967 layout = pango_cairo_create_layout (cr);
968 desc = pango_font_description_new ();968 text_font_face = defaults_get_text_font_face (d);
969 desc = pango_font_description_from_string (text_font_face);
970 g_free ((gpointer) text_font_face);
969971
970 pango_font_description_set_size (desc,972 pango_font_description_set_size (desc,
971 defaults_get_system_font_size (d) *973 defaults_get_system_font_size (d) *
972 defaults_get_text_title_size (d) *974 defaults_get_text_title_size (d) *
973 PANGO_SCALE);975 PANGO_SCALE);
974976
975 text_font_face = defaults_get_text_font_face (d);
976 pango_font_description_set_family_static (desc, text_font_face);
977 pango_font_description_set_weight (desc,977 pango_font_description_set_weight (desc,
978 defaults_get_text_title_weight (d));978 defaults_get_text_title_weight (d));
979 pango_font_description_set_style (desc, PANGO_STYLE_NORMAL);
980 pango_layout_set_font_description (layout, desc);979 pango_layout_set_font_description (layout, desc);
981 pango_font_description_free (desc);980 pango_font_description_free (desc);
982 g_free ((gpointer) text_font_face);
983981
984 pango_layout_set_wrap (layout, PANGO_WRAP_WORD_CHAR);982 pango_layout_set_wrap (layout, PANGO_WRAP_WORD_CHAR);
985 pango_layout_set_ellipsize (layout, PANGO_ELLIPSIZE_END);983 pango_layout_set_ellipsize (layout, PANGO_ELLIPSIZE_END);
@@ -1071,21 +1069,19 @@
10711069
1072 // create pango desc/layout1070 // create pango desc/layout
1073 layout = pango_cairo_create_layout (cr);1071 layout = pango_cairo_create_layout (cr);
1074 desc = pango_font_description_new ();1072 text_font_face = defaults_get_text_font_face (d);
1073 desc = pango_font_description_from_string (text_font_face);
1074 g_free ((gpointer) text_font_face);
10751075
1076 pango_font_description_set_size (desc,1076 pango_font_description_set_size (desc,
1077 defaults_get_system_font_size (d) *1077 defaults_get_system_font_size (d) *
1078 defaults_get_text_body_size (d) *1078 defaults_get_text_body_size (d) *
1079 PANGO_SCALE);1079 PANGO_SCALE);
10801080
1081 text_font_face = defaults_get_text_font_face (d);
1082 pango_font_description_set_family_static (desc, text_font_face);
1083 pango_font_description_set_weight (desc,1081 pango_font_description_set_weight (desc,
1084 defaults_get_text_body_weight (d));1082 defaults_get_text_body_weight (d));
1085 pango_font_description_set_style (desc, PANGO_STYLE_NORMAL);
1086 pango_layout_set_font_description (layout, desc);1083 pango_layout_set_font_description (layout, desc);
1087 pango_font_description_free (desc);1084 pango_font_description_free (desc);
1088 g_free ((gpointer) text_font_face);
10891085
1090 pango_layout_set_wrap (layout, PANGO_WRAP_WORD_CHAR);1086 pango_layout_set_wrap (layout, PANGO_WRAP_WORD_CHAR);
1091 pango_layout_set_ellipsize (layout, PANGO_ELLIPSIZE_END);1087 pango_layout_set_ellipsize (layout, PANGO_ELLIPSIZE_END);
@@ -2306,6 +2302,7 @@
2306bubble_set_title (Bubble* self,2302bubble_set_title (Bubble* self,
2307 const gchar* title)2303 const gchar* title)
2308{2304{
2305 gchar* text;
2309 BubblePrivate* priv;2306 BubblePrivate* priv;
23102307
2311 if (!self || !IS_BUBBLE (self))2308 if (!self || !IS_BUBBLE (self))
@@ -2316,11 +2313,16 @@
2316 if (priv->title)2313 if (priv->title)
2317 g_string_free (priv->title, TRUE);2314 g_string_free (priv->title, TRUE);
23182315
2319 priv->title = g_string_new (title);2316 // convert any newline to space
2317 text = newline_to_space (title);
2318
2319 priv->title = g_string_new (text);
23202320
2321 g_object_notify (2321 g_object_notify (
2322 G_OBJECT (gtk_widget_get_accessible (GET_PRIVATE(self)->widget)), 2322 G_OBJECT (gtk_widget_get_accessible (GET_PRIVATE(self)->widget)),
2323 "accessible-name");2323 "accessible-name");
2324
2325 g_free (text);
2324}2326}
23252327
2326const gchar*2328const gchar*
@@ -3105,7 +3107,9 @@
3105 }3107 }
31063108
3107 layout = pango_cairo_create_layout (cr);3109 layout = pango_cairo_create_layout (cr);
3108 desc = pango_font_description_new ();3110 text_font_face = defaults_get_text_font_face (d);
3111 desc = pango_font_description_from_string (text_font_face);
3112 g_free ((gpointer) text_font_face);
31093113
3110 // make sure system-wide font-options like hinting, antialiasing etc.3114 // make sure system-wide font-options like hinting, antialiasing etc.
3111 // are taken into account3115 // are taken into account
@@ -3122,17 +3126,12 @@
3122 defaults_get_text_title_size (d) *3126 defaults_get_text_title_size (d) *
3123 PANGO_SCALE);3127 PANGO_SCALE);
31243128
3125 text_font_face = defaults_get_text_font_face (d);
3126 pango_font_description_set_family_static (desc, text_font_face);
3127
3128 pango_font_description_set_weight (3129 pango_font_description_set_weight (
3129 desc,3130 desc,
3130 defaults_get_text_title_weight (d));3131 defaults_get_text_title_weight (d));
31313132
3132 pango_font_description_set_style (desc, PANGO_STYLE_NORMAL);
3133 pango_layout_set_font_description (layout, desc);3133 pango_layout_set_font_description (layout, desc);
3134 pango_font_description_free (desc);3134 pango_font_description_free (desc);
3135 g_free ((gpointer) text_font_face);
31363135
3137 pango_layout_set_ellipsize (layout, PANGO_ELLIPSIZE_END);3136 pango_layout_set_ellipsize (layout, PANGO_ELLIPSIZE_END);
3138 pango_layout_set_wrap (layout, PANGO_WRAP_WORD_CHAR);3137 pango_layout_set_wrap (layout, PANGO_WRAP_WORD_CHAR);
@@ -3176,7 +3175,9 @@
3176 }3175 }
31773176
3178 layout = pango_cairo_create_layout (cr);3177 layout = pango_cairo_create_layout (cr);
3179 desc = pango_font_description_new ();3178 text_font_face = defaults_get_text_font_face (d);
3179 desc = pango_font_description_from_string (text_font_face);
3180 g_free ((gpointer) text_font_face);
31803181
3181 // make sure system-wide font-options like hinting, antialiasing etc.3182 // make sure system-wide font-options like hinting, antialiasing etc.
3182 // are taken into account3183 // are taken into account
@@ -3193,14 +3194,10 @@
3193 defaults_get_text_body_size (d) *3194 defaults_get_text_body_size (d) *
3194 PANGO_SCALE);3195 PANGO_SCALE);
31953196
3196 text_font_face = defaults_get_text_font_face (d);
3197 pango_font_description_set_family_static (desc, text_font_face);
3198
3199 pango_font_description_set_weight (3197 pango_font_description_set_weight (
3200 desc,3198 desc,
3201 defaults_get_text_body_weight (d));3199 defaults_get_text_body_weight (d));
32023200
3203 pango_font_description_set_style (desc, PANGO_STYLE_NORMAL);
3204 pango_layout_set_font_description (layout, desc);3201 pango_layout_set_font_description (layout, desc);
32053202
3206 pango_layout_set_wrap (layout, PANGO_WRAP_WORD_CHAR);3203 pango_layout_set_wrap (layout, PANGO_WRAP_WORD_CHAR);
@@ -3255,7 +3252,6 @@
3255 body_height = PANGO_PIXELS (log_rect.height);3252 body_height = PANGO_PIXELS (log_rect.height);
32563253
3257 pango_font_description_free (desc);3254 pango_font_description_free (desc);
3258 g_free ((gpointer) text_font_face);
3259 g_object_unref (layout);3255 g_object_unref (layout);
3260 cairo_destroy (cr);3256 cairo_destroy (cr);
32613257
32623258
=== modified file 'src/defaults.c'
--- src/defaults.c 2009-09-29 09:53:59 +0000
+++ src/defaults.c 2009-10-19 05:00:35 +0000
@@ -41,6 +41,7 @@
41#include <libwnck/workspace.h>41#include <libwnck/workspace.h>
4242
43#include "defaults.h"43#include "defaults.h"
44#include "util.h"
4445
45G_DEFINE_TYPE (Defaults, defaults, G_TYPE_OBJECT);46G_DEFINE_TYPE (Defaults, defaults, G_TYPE_OBJECT);
4647
@@ -171,15 +172,13 @@
171static void172static void
172_get_font_size_dpi (Defaults* self)173_get_font_size_dpi (Defaults* self)
173{174{
174 GString* string = NULL;175 GString* string = NULL;
175 GError* error = NULL;176 GError* error = NULL;
176 GScanner* scanner = NULL;177 guint points = 0;
177 GTokenType token = G_TOKEN_NONE;178 GString* font_face = NULL;
178 gint points = 0;179 gdouble dpi = 0.0f;
179 GString* font_face = NULL;180 gdouble pixels_per_em = 0;
180 gdouble dpi = 0.0f;181 gchar* font_name = NULL;
181 gdouble pixels_per_em = 0;
182 gchar* font_name = NULL;
183182
184 if (!IS_DEFAULTS (self))183 if (!IS_DEFAULTS (self))
185 return;184 return;
@@ -192,7 +191,7 @@
192 string = g_string_new (font_name);191 string = g_string_new (font_name);
193 if (error)192 if (error)
194 {193 {
195 /* if something went wrong, assume "Sans 10" and continue */194 // if something went wrong, assume "Sans 10" and continue
196 string = g_string_assign (string, "Sans 10");195 string = g_string_assign (string, "Sans 10");
197196
198 g_warning ("_get_font_size_dpi(): Got error \"%s\"\n",197 g_warning ("_get_font_size_dpi(): Got error \"%s\"\n",
@@ -201,41 +200,12 @@
201 }200 }
202 g_free ((gpointer) font_name);201 g_free ((gpointer) font_name);
203202
204 /* extract font-family-name and font-size */203 // extract text point-size
205 scanner = g_scanner_new (NULL);204 points = extract_point_size (string->str);
206 if (scanner)205
207 {206 // extract font-face-name/style
208 g_scanner_input_text (scanner, string->str, string->len);207 font_face = extract_font_face (string->str);
209 for (token = g_scanner_get_next_token (scanner);208
210 token != G_TOKEN_EOF;
211 token = g_scanner_get_next_token (scanner))
212 {
213 switch (token)
214 {
215 case G_TOKEN_INT:
216 points = (gint) scanner->value.v_int;
217 break;
218
219 case G_TOKEN_IDENTIFIER:
220 if (!font_face)
221 font_face = g_string_new (scanner->value.v_string);
222 else
223 {
224 g_string_append (font_face,
225 " ");
226 g_string_append (font_face,
227 scanner->value.v_string);
228 }
229 break;
230
231 default:
232 break;
233 }
234 }
235 g_scanner_destroy (scanner);
236 }
237
238 /* clean up */
239 if (string != NULL)209 if (string != NULL)
240 g_string_free (string, TRUE);210 g_string_free (string, TRUE);
241211
242212
=== modified file 'src/util.c'
--- src/util.c 2009-09-27 20:48:03 +0000
+++ src/util.c 2009-10-19 05:00:35 +0000
@@ -103,6 +103,28 @@
103 { CHARACTER_GT_REGEX, ">" },103 { CHARACTER_GT_REGEX, ">" },
104 { CHARACTER_APOS_REGEX, "'" },104 { CHARACTER_APOS_REGEX, "'" },
105 { CHARACTER_QUOT_REGEX, "\"" },105 { CHARACTER_QUOT_REGEX, "\"" },
106 { CHARACTER_NEWLINE_REGEX, "\n" }
107 };
108
109 ReplaceMarkupData* ptr = data;
110 ReplaceMarkupData* end = data + sizeof(data) / sizeof(ReplaceMarkupData);
111 for (; ptr != end; ++ptr) {
112 gchar* tmp = replace_markup (text1, ptr->regex, ptr->replacement);
113 g_free (text1);
114 text1 = tmp;
115 }
116
117 return text1;
118}
119
120gchar*
121newline_to_space (const gchar *text)
122{
123 gchar *text1;
124
125 text1 = strip_html (text, TAG_MATCH_REGEX, TAG_REPLACE_REGEX);
126
127 static ReplaceMarkupData data[] = {
106 { CHARACTER_NEWLINE_REGEX, " " }128 { CHARACTER_NEWLINE_REGEX, " " }
107 };129 };
108130
@@ -235,3 +257,84 @@
235257
236 return (gchar*) buffer;258 return (gchar*) buffer;
237}259}
260
261guint
262extract_point_size (const gchar* string)
263{
264 guint point_size = 0;
265 GRegex* regex = NULL;
266 GMatchInfo* match_info = NULL;
267
268 // sanity check
269 if (!string)
270 return 0;
271
272 // setup regular expression to extract an integer from the end of string
273 regex = g_regex_new ("\\d+$", 0, 0, NULL);
274 if (!regex)
275 return 0;
276
277 // walk the string
278 g_regex_match (regex, string, 0, &match_info);
279 while (g_match_info_matches (match_info))
280 {
281 gchar* word = NULL;
282
283 word = g_match_info_fetch (match_info, 0);
284 if (word)
285 {
286 sscanf (word, "%d", &point_size);
287 g_free (word);
288 }
289
290 g_match_info_next (match_info, NULL);
291 }
292
293 // clean up
294 g_match_info_free (match_info);
295 g_regex_unref (regex);
296
297 return point_size;
298}
299
300GString*
301extract_font_face (const gchar* string)
302{
303 GRegex* regex = NULL;
304 GMatchInfo* match_info = NULL;
305 GString* font_face = NULL;
306
307 // sanity check
308 if (!string)
309 return NULL;
310
311 // extract font-face-name/style
312 font_face = g_string_new ("");
313 if (!font_face)
314 return NULL;
315
316 // setup regular expression to extract leading text before trailing int
317 regex = g_regex_new ("([A-Z a-z])+", 0, 0, NULL);
318
319 // walk the string
320 g_regex_match (regex, string, 0, &match_info);
321 while (g_match_info_matches (match_info))
322 {
323 gchar* word = NULL;
324
325 word = g_match_info_fetch (match_info, 0);
326 if (word)
327 {
328 g_string_append (font_face, word);
329 g_free (word);
330 }
331
332 g_match_info_next (match_info, NULL);
333 }
334
335 // clean up
336 g_match_info_free (match_info);
337 g_regex_unref (regex);
338
339 return font_face;
340}
238341
=== modified file 'src/util.h'
--- src/util.h 2009-07-31 13:11:46 +0000
+++ src/util.h 2009-10-19 05:00:35 +0000
@@ -38,7 +38,10 @@
38#define WM_NAME_XMONAD "xmonad"38#define WM_NAME_XMONAD "xmonad"
3939
40gchar*40gchar*
41filter_text (const gchar* app_name);41filter_text (const gchar* text);
42
43gchar*
44newline_to_space (const gchar* text);
4245
43cairo_surface_t*46cairo_surface_t*
44copy_surface (cairo_surface_t* orig);47copy_surface (cairo_surface_t* orig);
@@ -49,3 +52,8 @@
49gchar*52gchar*
50get_wm_name (Display* dpy);53get_wm_name (Display* dpy);
5154
55guint
56extract_point_size (const gchar* string);
57
58GString*
59extract_font_face (const gchar* string);
5260
=== modified file 'tests/test-text-filtering.c'
--- tests/test-text-filtering.c 2009-08-10 09:36:02 +0000
+++ tests/test-text-filtering.c 2009-10-19 05:00:35 +0000
@@ -34,6 +34,11 @@
34 const gchar *expected;34 const gchar *expected;
35} TextComparisons;35} TextComparisons;
3636
37typedef struct {
38 const gchar* before;
39 guint expected;
40} IntegerExtraction;
41
37static void42static void
38test_text_filter ()43test_text_filter ()
39{44{
@@ -69,8 +74,8 @@
69 { "<tt>Testing tag</tt>", "Testing tag" },74 { "<tt>Testing tag</tt>", "Testing tag" },
70 { "<html>Surrounded by html</html>", "Surrounded by html" },75 { "<html>Surrounded by html</html>", "Surrounded by html" },
71 { "<qt>Surrounded by qt</qt>", "Surrounded by qt" },76 { "<qt>Surrounded by qt</qt>", "Surrounded by qt" },
72 { "First line <br dumb> \r \n Second line", "First line\nSecond line" },77 { "First line <br dumb> \r \n Second line", "First line\nSecond line" },
73 { "First line\n<br /> <br>\n2nd line\r\n3rd line", "First line\n2nd line\n3rd line" },78 { "First line\n<br /> <br>\n2nd line\r\n3rd line", "First line\n2nd line\n3rd line" },
74 { NULL, NULL }79 { NULL, NULL }
75 };80 };
7681
@@ -81,6 +86,65 @@
81 }86 }
82}87}
8388
89static void
90test_newline_to_space ()
91{
92 static const TextComparisons tests[] = {
93 { "one\ntwo\nthree\nfour\nfive\nsix", "one two three four five six" },
94 { "1\n2\n3\n4\n5\n6", "1 2 3 4 5 6" },
95 { NULL, NULL }
96 };
97
98 for (int i = 0; tests[i].before != NULL; i++) {
99 char *filtered = newline_to_space (tests[i].before);
100 g_assert_cmpstr (filtered, ==, tests[i].expected);
101 g_free (filtered);
102 }
103}
104
105static void
106test_extract_point_size ()
107{
108 static const IntegerExtraction tests[] = {
109 { "", 0 },
110 { "foobar", 0 },
111 { "Bla Fasel -12.0", 0 },
112 { "Sans 10", 10 },
113 { "Candara 9", 9 },
114 { "Bitstream Vera Serif Italic 1", 1 },
115 { "Calibri Italic 100", 100 },
116 { "Century Schoolbook L Italic 42", 42 },
117 { NULL, 0 }
118 };
119
120 for (int i = 0; tests[i].before != NULL; i++)
121 {
122 guint extracted = extract_point_size (tests[i].before);
123 g_assert_cmpuint (extracted, ==, tests[i].expected);
124 }
125}
126
127static void
128test_extract_font_face ()
129{
130 static const TextComparisons tests[] = {
131 { "", "" },
132 { "Sans 10", "Sans " },
133 { "Candara 9", "Candara " },
134 { "Bitstream Vera Serif Italic 1", "Bitstream Vera Serif Italic " },
135 { "Calibri Italic 100", "Calibri Italic " },
136 { "Century Schoolbook L Italic 10", "Century Schoolbook L Italic " },
137 { NULL, NULL }
138 };
139
140 for (int i = 0; tests[i].before != NULL; i++)
141 {
142 GString* filtered = extract_font_face (tests[i].before);
143 g_assert_cmpstr (filtered->str, ==, tests[i].expected);
144 g_string_free (filtered, TRUE);
145 }
146}
147
84GTestSuite *148GTestSuite *
85test_filtering_create_test_suite (void)149test_filtering_create_test_suite (void)
86{150{
@@ -91,6 +155,9 @@
91#define TC(x) g_test_create_case(#x, 0, NULL, NULL, x, NULL)155#define TC(x) g_test_create_case(#x, 0, NULL, NULL, x, NULL)
92156
93 g_test_suite_add(ts, TC(test_text_filter));157 g_test_suite_add(ts, TC(test_text_filter));
158 g_test_suite_add(ts, TC(test_newline_to_space));
159 g_test_suite_add(ts, TC(test_extract_point_size));
160 g_test_suite_add(ts, TC(test_extract_font_face));
94161
95 return ts;162 return ts;
96}163}

Subscribers

People subscribed via source and target branches

to all changes: