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
1=== modified file 'NEWS'
2--- NEWS 2009-04-23 14:26:48 +0000
3+++ NEWS 2009-10-19 05:00:34 +0000
4@@ -8,3 +8,13 @@
5 oper guidelines availabled under:
6
7 https://wiki.ubuntu.com/NotificationDevelopmentGuidelines
8+
9+Noteworthy changes since last release (0.9.22):
10+
11+ * updating NEWS file now, fixes LP: #432486
12+ * no long using hard-coded -Werror, fixes LP: #361788
13+ * solved regression regarding throbbing-animation for
14+ value-indicator, fixes LP: #435116
15+ * looking up gravity-gconf-key upon startup, fixes LP: #431200
16+ * convert newline characters to spaces, fixes LP: #402247
17+
18
19=== modified file 'configure.in'
20--- configure.in 2009-09-29 15:48:51 +0000
21+++ configure.in 2009-10-19 05:00:34 +0000
22@@ -1,4 +1,4 @@
23-AC_INIT(notify-osd, 0.9.22, dx-team@canonical.com)
24+AC_INIT(notify-osd, 0.9.23, dx-team@canonical.com)
25
26 AC_CONFIG_SRCDIR(src/main.c)
27 AC_CONFIG_HEADERS(config.h)
28
29=== modified file 'src/bubble.c'
30--- src/bubble.c 2009-10-15 13:33:14 +0000
31+++ src/bubble.c 2009-10-19 05:00:35 +0000
32@@ -965,21 +965,19 @@
33
34 // create pango desc/layout
35 layout = pango_cairo_create_layout (cr);
36- desc = pango_font_description_new ();
37+ text_font_face = defaults_get_text_font_face (d);
38+ desc = pango_font_description_from_string (text_font_face);
39+ g_free ((gpointer) text_font_face);
40
41 pango_font_description_set_size (desc,
42 defaults_get_system_font_size (d) *
43 defaults_get_text_title_size (d) *
44 PANGO_SCALE);
45
46- text_font_face = defaults_get_text_font_face (d);
47- pango_font_description_set_family_static (desc, text_font_face);
48 pango_font_description_set_weight (desc,
49 defaults_get_text_title_weight (d));
50- pango_font_description_set_style (desc, PANGO_STYLE_NORMAL);
51 pango_layout_set_font_description (layout, desc);
52 pango_font_description_free (desc);
53- g_free ((gpointer) text_font_face);
54
55 pango_layout_set_wrap (layout, PANGO_WRAP_WORD_CHAR);
56 pango_layout_set_ellipsize (layout, PANGO_ELLIPSIZE_END);
57@@ -1071,21 +1069,19 @@
58
59 // create pango desc/layout
60 layout = pango_cairo_create_layout (cr);
61- desc = pango_font_description_new ();
62+ text_font_face = defaults_get_text_font_face (d);
63+ desc = pango_font_description_from_string (text_font_face);
64+ g_free ((gpointer) text_font_face);
65
66 pango_font_description_set_size (desc,
67 defaults_get_system_font_size (d) *
68 defaults_get_text_body_size (d) *
69 PANGO_SCALE);
70
71- text_font_face = defaults_get_text_font_face (d);
72- pango_font_description_set_family_static (desc, text_font_face);
73 pango_font_description_set_weight (desc,
74 defaults_get_text_body_weight (d));
75- pango_font_description_set_style (desc, PANGO_STYLE_NORMAL);
76 pango_layout_set_font_description (layout, desc);
77 pango_font_description_free (desc);
78- g_free ((gpointer) text_font_face);
79
80 pango_layout_set_wrap (layout, PANGO_WRAP_WORD_CHAR);
81 pango_layout_set_ellipsize (layout, PANGO_ELLIPSIZE_END);
82@@ -2306,6 +2302,7 @@
83 bubble_set_title (Bubble* self,
84 const gchar* title)
85 {
86+ gchar* text;
87 BubblePrivate* priv;
88
89 if (!self || !IS_BUBBLE (self))
90@@ -2316,11 +2313,16 @@
91 if (priv->title)
92 g_string_free (priv->title, TRUE);
93
94- priv->title = g_string_new (title);
95+ // convert any newline to space
96+ text = newline_to_space (title);
97+
98+ priv->title = g_string_new (text);
99
100 g_object_notify (
101 G_OBJECT (gtk_widget_get_accessible (GET_PRIVATE(self)->widget)),
102 "accessible-name");
103+
104+ g_free (text);
105 }
106
107 const gchar*
108@@ -3105,7 +3107,9 @@
109 }
110
111 layout = pango_cairo_create_layout (cr);
112- desc = pango_font_description_new ();
113+ text_font_face = defaults_get_text_font_face (d);
114+ desc = pango_font_description_from_string (text_font_face);
115+ g_free ((gpointer) text_font_face);
116
117 // make sure system-wide font-options like hinting, antialiasing etc.
118 // are taken into account
119@@ -3122,17 +3126,12 @@
120 defaults_get_text_title_size (d) *
121 PANGO_SCALE);
122
123- text_font_face = defaults_get_text_font_face (d);
124- pango_font_description_set_family_static (desc, text_font_face);
125-
126 pango_font_description_set_weight (
127 desc,
128 defaults_get_text_title_weight (d));
129
130- pango_font_description_set_style (desc, PANGO_STYLE_NORMAL);
131 pango_layout_set_font_description (layout, desc);
132 pango_font_description_free (desc);
133- g_free ((gpointer) text_font_face);
134
135 pango_layout_set_ellipsize (layout, PANGO_ELLIPSIZE_END);
136 pango_layout_set_wrap (layout, PANGO_WRAP_WORD_CHAR);
137@@ -3176,7 +3175,9 @@
138 }
139
140 layout = pango_cairo_create_layout (cr);
141- desc = pango_font_description_new ();
142+ text_font_face = defaults_get_text_font_face (d);
143+ desc = pango_font_description_from_string (text_font_face);
144+ g_free ((gpointer) text_font_face);
145
146 // make sure system-wide font-options like hinting, antialiasing etc.
147 // are taken into account
148@@ -3193,14 +3194,10 @@
149 defaults_get_text_body_size (d) *
150 PANGO_SCALE);
151
152- text_font_face = defaults_get_text_font_face (d);
153- pango_font_description_set_family_static (desc, text_font_face);
154-
155 pango_font_description_set_weight (
156 desc,
157 defaults_get_text_body_weight (d));
158
159- pango_font_description_set_style (desc, PANGO_STYLE_NORMAL);
160 pango_layout_set_font_description (layout, desc);
161
162 pango_layout_set_wrap (layout, PANGO_WRAP_WORD_CHAR);
163@@ -3255,7 +3252,6 @@
164 body_height = PANGO_PIXELS (log_rect.height);
165
166 pango_font_description_free (desc);
167- g_free ((gpointer) text_font_face);
168 g_object_unref (layout);
169 cairo_destroy (cr);
170
171
172=== modified file 'src/defaults.c'
173--- src/defaults.c 2009-09-29 09:53:59 +0000
174+++ src/defaults.c 2009-10-19 05:00:35 +0000
175@@ -41,6 +41,7 @@
176 #include <libwnck/workspace.h>
177
178 #include "defaults.h"
179+#include "util.h"
180
181 G_DEFINE_TYPE (Defaults, defaults, G_TYPE_OBJECT);
182
183@@ -171,15 +172,13 @@
184 static void
185 _get_font_size_dpi (Defaults* self)
186 {
187- GString* string = NULL;
188- GError* error = NULL;
189- GScanner* scanner = NULL;
190- GTokenType token = G_TOKEN_NONE;
191- gint points = 0;
192- GString* font_face = NULL;
193- gdouble dpi = 0.0f;
194- gdouble pixels_per_em = 0;
195- gchar* font_name = NULL;
196+ GString* string = NULL;
197+ GError* error = NULL;
198+ guint points = 0;
199+ GString* font_face = NULL;
200+ gdouble dpi = 0.0f;
201+ gdouble pixels_per_em = 0;
202+ gchar* font_name = NULL;
203
204 if (!IS_DEFAULTS (self))
205 return;
206@@ -192,7 +191,7 @@
207 string = g_string_new (font_name);
208 if (error)
209 {
210- /* if something went wrong, assume "Sans 10" and continue */
211+ // if something went wrong, assume "Sans 10" and continue
212 string = g_string_assign (string, "Sans 10");
213
214 g_warning ("_get_font_size_dpi(): Got error \"%s\"\n",
215@@ -201,41 +200,12 @@
216 }
217 g_free ((gpointer) font_name);
218
219- /* extract font-family-name and font-size */
220- scanner = g_scanner_new (NULL);
221- if (scanner)
222- {
223- g_scanner_input_text (scanner, string->str, string->len);
224- for (token = g_scanner_get_next_token (scanner);
225- token != G_TOKEN_EOF;
226- token = g_scanner_get_next_token (scanner))
227- {
228- switch (token)
229- {
230- case G_TOKEN_INT:
231- points = (gint) scanner->value.v_int;
232- break;
233-
234- case G_TOKEN_IDENTIFIER:
235- if (!font_face)
236- font_face = g_string_new (scanner->value.v_string);
237- else
238- {
239- g_string_append (font_face,
240- " ");
241- g_string_append (font_face,
242- scanner->value.v_string);
243- }
244- break;
245-
246- default:
247- break;
248- }
249- }
250- g_scanner_destroy (scanner);
251- }
252-
253- /* clean up */
254+ // extract text point-size
255+ points = extract_point_size (string->str);
256+
257+ // extract font-face-name/style
258+ font_face = extract_font_face (string->str);
259+
260 if (string != NULL)
261 g_string_free (string, TRUE);
262
263
264=== modified file 'src/util.c'
265--- src/util.c 2009-09-27 20:48:03 +0000
266+++ src/util.c 2009-10-19 05:00:35 +0000
267@@ -103,6 +103,28 @@
268 { CHARACTER_GT_REGEX, ">" },
269 { CHARACTER_APOS_REGEX, "'" },
270 { CHARACTER_QUOT_REGEX, "\"" },
271+ { CHARACTER_NEWLINE_REGEX, "\n" }
272+ };
273+
274+ ReplaceMarkupData* ptr = data;
275+ ReplaceMarkupData* end = data + sizeof(data) / sizeof(ReplaceMarkupData);
276+ for (; ptr != end; ++ptr) {
277+ gchar* tmp = replace_markup (text1, ptr->regex, ptr->replacement);
278+ g_free (text1);
279+ text1 = tmp;
280+ }
281+
282+ return text1;
283+}
284+
285+gchar*
286+newline_to_space (const gchar *text)
287+{
288+ gchar *text1;
289+
290+ text1 = strip_html (text, TAG_MATCH_REGEX, TAG_REPLACE_REGEX);
291+
292+ static ReplaceMarkupData data[] = {
293 { CHARACTER_NEWLINE_REGEX, " " }
294 };
295
296@@ -235,3 +257,84 @@
297
298 return (gchar*) buffer;
299 }
300+
301+guint
302+extract_point_size (const gchar* string)
303+{
304+ guint point_size = 0;
305+ GRegex* regex = NULL;
306+ GMatchInfo* match_info = NULL;
307+
308+ // sanity check
309+ if (!string)
310+ return 0;
311+
312+ // setup regular expression to extract an integer from the end of string
313+ regex = g_regex_new ("\\d+$", 0, 0, NULL);
314+ if (!regex)
315+ return 0;
316+
317+ // walk the string
318+ g_regex_match (regex, string, 0, &match_info);
319+ while (g_match_info_matches (match_info))
320+ {
321+ gchar* word = NULL;
322+
323+ word = g_match_info_fetch (match_info, 0);
324+ if (word)
325+ {
326+ sscanf (word, "%d", &point_size);
327+ g_free (word);
328+ }
329+
330+ g_match_info_next (match_info, NULL);
331+ }
332+
333+ // clean up
334+ g_match_info_free (match_info);
335+ g_regex_unref (regex);
336+
337+ return point_size;
338+}
339+
340+GString*
341+extract_font_face (const gchar* string)
342+{
343+ GRegex* regex = NULL;
344+ GMatchInfo* match_info = NULL;
345+ GString* font_face = NULL;
346+
347+ // sanity check
348+ if (!string)
349+ return NULL;
350+
351+ // extract font-face-name/style
352+ font_face = g_string_new ("");
353+ if (!font_face)
354+ return NULL;
355+
356+ // setup regular expression to extract leading text before trailing int
357+ regex = g_regex_new ("([A-Z a-z])+", 0, 0, NULL);
358+
359+ // walk the string
360+ g_regex_match (regex, string, 0, &match_info);
361+ while (g_match_info_matches (match_info))
362+ {
363+ gchar* word = NULL;
364+
365+ word = g_match_info_fetch (match_info, 0);
366+ if (word)
367+ {
368+ g_string_append (font_face, word);
369+ g_free (word);
370+ }
371+
372+ g_match_info_next (match_info, NULL);
373+ }
374+
375+ // clean up
376+ g_match_info_free (match_info);
377+ g_regex_unref (regex);
378+
379+ return font_face;
380+}
381
382=== modified file 'src/util.h'
383--- src/util.h 2009-07-31 13:11:46 +0000
384+++ src/util.h 2009-10-19 05:00:35 +0000
385@@ -38,7 +38,10 @@
386 #define WM_NAME_XMONAD "xmonad"
387
388 gchar*
389-filter_text (const gchar* app_name);
390+filter_text (const gchar* text);
391+
392+gchar*
393+newline_to_space (const gchar* text);
394
395 cairo_surface_t*
396 copy_surface (cairo_surface_t* orig);
397@@ -49,3 +52,8 @@
398 gchar*
399 get_wm_name (Display* dpy);
400
401+guint
402+extract_point_size (const gchar* string);
403+
404+GString*
405+extract_font_face (const gchar* string);
406
407=== modified file 'tests/test-text-filtering.c'
408--- tests/test-text-filtering.c 2009-08-10 09:36:02 +0000
409+++ tests/test-text-filtering.c 2009-10-19 05:00:35 +0000
410@@ -34,6 +34,11 @@
411 const gchar *expected;
412 } TextComparisons;
413
414+typedef struct {
415+ const gchar* before;
416+ guint expected;
417+} IntegerExtraction;
418+
419 static void
420 test_text_filter ()
421 {
422@@ -69,8 +74,8 @@
423 { "<tt>Testing tag</tt>", "Testing tag" },
424 { "<html>Surrounded by html</html>", "Surrounded by html" },
425 { "<qt>Surrounded by qt</qt>", "Surrounded by qt" },
426- { "First line <br dumb> \r \n Second line", "First line\nSecond line" },
427- { "First line\n<br /> <br>\n2nd line\r\n3rd line", "First line\n2nd line\n3rd line" },
428+ { "First line <br dumb> \r \n Second line", "First line\nSecond line" },
429+ { "First line\n<br /> <br>\n2nd line\r\n3rd line", "First line\n2nd line\n3rd line" },
430 { NULL, NULL }
431 };
432
433@@ -81,6 +86,65 @@
434 }
435 }
436
437+static void
438+test_newline_to_space ()
439+{
440+ static const TextComparisons tests[] = {
441+ { "one\ntwo\nthree\nfour\nfive\nsix", "one two three four five six" },
442+ { "1\n2\n3\n4\n5\n6", "1 2 3 4 5 6" },
443+ { NULL, NULL }
444+ };
445+
446+ for (int i = 0; tests[i].before != NULL; i++) {
447+ char *filtered = newline_to_space (tests[i].before);
448+ g_assert_cmpstr (filtered, ==, tests[i].expected);
449+ g_free (filtered);
450+ }
451+}
452+
453+static void
454+test_extract_point_size ()
455+{
456+ static const IntegerExtraction tests[] = {
457+ { "", 0 },
458+ { "foobar", 0 },
459+ { "Bla Fasel -12.0", 0 },
460+ { "Sans 10", 10 },
461+ { "Candara 9", 9 },
462+ { "Bitstream Vera Serif Italic 1", 1 },
463+ { "Calibri Italic 100", 100 },
464+ { "Century Schoolbook L Italic 42", 42 },
465+ { NULL, 0 }
466+ };
467+
468+ for (int i = 0; tests[i].before != NULL; i++)
469+ {
470+ guint extracted = extract_point_size (tests[i].before);
471+ g_assert_cmpuint (extracted, ==, tests[i].expected);
472+ }
473+}
474+
475+static void
476+test_extract_font_face ()
477+{
478+ static const TextComparisons tests[] = {
479+ { "", "" },
480+ { "Sans 10", "Sans " },
481+ { "Candara 9", "Candara " },
482+ { "Bitstream Vera Serif Italic 1", "Bitstream Vera Serif Italic " },
483+ { "Calibri Italic 100", "Calibri Italic " },
484+ { "Century Schoolbook L Italic 10", "Century Schoolbook L Italic " },
485+ { NULL, NULL }
486+ };
487+
488+ for (int i = 0; tests[i].before != NULL; i++)
489+ {
490+ GString* filtered = extract_font_face (tests[i].before);
491+ g_assert_cmpstr (filtered->str, ==, tests[i].expected);
492+ g_string_free (filtered, TRUE);
493+ }
494+}
495+
496 GTestSuite *
497 test_filtering_create_test_suite (void)
498 {
499@@ -91,6 +155,9 @@
500 #define TC(x) g_test_create_case(#x, 0, NULL, NULL, x, NULL)
501
502 g_test_suite_add(ts, TC(test_text_filter));
503+ g_test_suite_add(ts, TC(test_newline_to_space));
504+ g_test_suite_add(ts, TC(test_extract_point_size));
505+ g_test_suite_add(ts, TC(test_extract_font_face));
506
507 return ts;
508 }

Subscribers

People subscribed via source and target branches

to all changes: