[PATCH 0/2] convert display text attributes to HTML

[PATCH 0/2] convert display text attributes to HTML

[Kristen Carlson Accardi]

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

Kristen Carlson Accardi (2):
  stkutil: convert text attributes to html
  test-stkutil: add html formatted text tests

 src/stkutil.c       |  165 +++++++++++++++++++++++++++++++++
 src/stkutil.h       |   22 +++++
 unit/test-stkutil.c |  257 +++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 444 insertions(+), 0 deletions(-)

[PATCH 1/2] stkutil: convert text attributes to html

[Kristen Carlson Accardi]

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

---
 src/stkutil.c |  165 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/stkutil.h |   22 ++++++++
 2 files changed, 187 insertions(+), 0 deletions(-)

diff --git a/src/stkutil.c b/src/stkutil.c
index 642081e..1c8bac9 100644
--- a/src/stkutil.c
+++ b/src/stkutil.c
@@ -26,6 +26,7 @@
 #include <string.h>
 #include <stdlib.h>
 #include <stdint.h>
+#include <stdio.h>
 
 #include <glib.h>
 
@@ -5978,3 +5979,167 @@ const unsigned char *stk_pdu_from_envelope(const struct stk_envelope *envelope,
 
 	return pdu;
 }
+
+static const char *html_colors[] = {
+	"#000000", /* Black */
+	"#808080", /* Dark Grey */
+	"#C11B17", /* Dark Red */
+	"#FBB117", /* Dark Yellow */
+	"#347235", /* Dark Green */
+	"#307D7E", /* Dark Cyan */
+	"#0000A0", /* Dark Blue */
+	"#C031C7", /* Dark Magenta */
+	"#C0C0C0", /* Grey */
+	"#FFFFFF", /* White */
+	"#FF0000", /* Bright Red */
+	"#FFFF00", /* Bright Yellow */
+	"#00FF00", /* Bright Green */
+	"#00FFFF", /* Bright Cyan */
+	"#0000FF", /* Bright Blue */
+	"#FF00FF", /* Bright Magenta */
+};
+
+static void end_format(GString *string, guint8 code)
+{
+	g_string_append(string, "</span>");
+
+	if ((code & STK_TEXT_FORMAT_ALIGN_MASK) != STK_TEXT_FORMAT_NO_ALIGN)
+		g_string_append(string, "</div>");
+}
+
+static void start_format(GString *string, guint8 code, guint8 color)
+{
+	guint8 align = code & STK_TEXT_FORMAT_ALIGN_MASK;
+	guint8 font = code & STK_TEXT_FORMAT_FONT_MASK;
+	guint8 style = code & STK_TEXT_FORMAT_STYLE_MASK;
+	int fg = color & 0x0f;
+	int bg = (color >> 4) & 0x0f;
+
+	/* align formatting applies to a block of test */
+	if (align != STK_TEXT_FORMAT_NO_ALIGN)
+		g_string_append(string, "<div style=\"");
+
+	if (align == STK_TEXT_FORMAT_RIGHT_ALIGN)
+		g_string_append(string, "text-align: right;\">");
+	else if (align == STK_TEXT_FORMAT_CENTER_ALIGN)
+		g_string_append(string, "text-align: center;\">");
+	else if (align == STK_TEXT_FORMAT_LEFT_ALIGN)
+		g_string_append(string, "text-align: left;\">");
+
+	/* font, style, and color are inline */
+	g_string_append(string, "<span style=\"");
+
+	if (font == STK_TEXT_FORMAT_FONT_SIZE_LARGE)
+		g_string_append(string, "font-size: big;");
+	else if (font == STK_TEXT_FORMAT_FONT_SIZE_SMALL)
+		g_string_append(string, "font-size: small;");
+
+	if (style == STK_TEXT_FORMAT_STYLE_BOLD)
+		g_string_append(string, "font-weight: bold;");
+	else if (style == STK_TEXT_FORMAT_STYLE_ITALIC)
+		g_string_append(string, "font-style: italic;");
+	else if (style == STK_TEXT_FORMAT_STYLE_UNDERLINED)
+		g_string_append(string, "text-decoration: underline;");
+	else if (style == STK_TEXT_FORMAT_STYLE_STRIKETHROUGH)
+		g_string_append(string, "text-decoration: line-through;");
+
+	/* add any color */
+	if (fg)
+		g_string_append_printf(string, "color: %s;", html_colors[fg]);
+	if (bg)
+		g_string_append_printf(string, "background-color: %s;",
+						html_colors[bg]);
+	g_string_append(string, "\">");
+}
+
+char *stk_text_to_html(char *text, int text_len,
+				const unsigned char *attrs, int attrs_len)
+{
+	GString *string = g_string_sized_new(text_len + 1);
+	GQueue formats[241];  /* maximum number of chars in text + 1 */
+	int pos = 0, attr, i;
+	guint8 start, end, code, color, len;
+	guint8 align = STK_DEFAULT_TEXT_ALIGNMENT; /* hardcoded to left */
+
+	/* we may need formatting at the position beyond the last char */
+	for (i = 0; i <= text_len; i++)
+		g_queue_init(&formats[i]);
+
+	i = 0;
+
+	/*
+	 * each position in the text may have multiple attributes.
+	 * store each attribute in a queue for that position in the
+	 * order in which it was sent.  This will cause the last
+	 * attribute sent for that position to take priority.
+	 */
+	while (i < attrs_len) {
+		/* TBD - check for bad values */
+		start = attrs[i++];
+		len = attrs[i++];
+		code = attrs[i++];
+
+		/*
+		 * if the alignment is the same as either the default
+		 * or the last alignment used, don't set any alignment
+		 * value.
+		 */
+		if ((code & STK_TEXT_FORMAT_ALIGN_MASK) == align)
+			code |= 0x3;
+		else
+			align = code & STK_TEXT_FORMAT_ALIGN_MASK;
+
+		if (i < attrs_len)
+			color = attrs[i++];
+		else
+			color = 0;
+
+		if (len == 0)
+			end = text_len;
+		else
+			end = start + len;
+
+		attr = start | (code << 16) | (color << 24);
+
+		g_queue_push_tail(&formats[start], GINT_TO_POINTER(attr));
+		g_queue_push_tail(&formats[end], GINT_TO_POINTER(attr));
+	}
+
+	while (pos <= text_len) {
+		GQueue *q = &formats[pos];
+
+		/* there may be multiple formats per position */
+		while ((attr = GPOINTER_TO_INT(g_queue_pop_head(q)))) {
+			start = attr & 0xff;
+			code = attr >> 16 & 0xff;
+			color = attr >> 24 & 0xff;
+
+			if (pos == start)
+				start_format(string, code, color);
+			else
+				end_format(string, code);
+		}
+
+		if (pos == text_len)
+			break;
+
+		if (text[pos] == '\n')
+			g_string_append(string, "<br/>");
+		else if (text[pos] == '\r') {
+			g_string_append(string, "<br/>");
+			if ((pos + 1 < text_len) && (text[pos + 1] == '\n'))
+				pos++;
+		} else if (text[pos] == '<')
+			g_string_append(string, "&lt;");
+		else if (text[pos] == '>')
+			g_string_append(string, "&gt;");
+		else if (text[pos] == '&')
+			g_string_append(string, "&amp;");
+		else
+			g_string_append_c(string, text[pos]);
+		pos++;
+	}
+
+	/* return characters from string. Caller must free char data */
+	return g_string_free(string, FALSE);
+}
diff --git a/src/stkutil.h b/src/stkutil.h
index ea0f77a..50e594d 100644
--- a/src/stkutil.h
+++ b/src/stkutil.h
@@ -557,6 +557,26 @@ enum stk_me_status {
 	STK_ME_STATUS_NOT_IDLE = 	0x01
 };
 
+#define STK_TEXT_FORMAT_ALIGN_MASK 0x03
+#define STK_TEXT_FORMAT_FONT_MASK 0x0C
+#define STK_TEXT_FORMAT_STYLE_MASK 0xF0
+#define STK_DEFAULT_TEXT_ALIGNMENT 0x00
+
+/* Defined in ETSI 123 40 9.2.3.24.10.1.1 */
+enum stk_text_format_code {
+	STK_TEXT_FORMAT_LEFT_ALIGN = 0x00,
+	STK_TEXT_FORMAT_CENTER_ALIGN = 0x01,
+	STK_TEXT_FORMAT_RIGHT_ALIGN = 0x02,
+	STK_TEXT_FORMAT_NO_ALIGN = 0x03,
+	STK_TEXT_FORMAT_FONT_SIZE_LARGE = 0x04,
+	STK_TEXT_FORMAT_FONT_SIZE_SMALL = 0x08,
+	STK_TEXT_FORMAT_FONT_SIZE_RESERVED = 0x0c,
+	STK_TEXT_FORMAT_STYLE_BOLD = 0x10,
+	STK_TEXT_FORMAT_STYLE_ITALIC = 0x20,
+	STK_TEXT_FORMAT_STYLE_UNDERLINED = 0x40,
+	STK_TEXT_FORMAT_STYLE_STRIKETHROUGH = 0x80,
+};
+
 /* For data object that only has a byte array with undetermined length */
 struct stk_common_byte_array {
 	unsigned char *array;
@@ -1635,3 +1655,5 @@ const unsigned char *stk_pdu_from_response(const struct stk_response *response,
 						unsigned int *out_length);
 const unsigned char *stk_pdu_from_envelope(const struct stk_envelope *envelope,
 						unsigned int *out_length);
+char *stk_text_to_html(char *text, int text_len,
+				const unsigned char *attrs, int attrs_len);
-- 
1.6.6.1

Re: [PATCH 1/2] stkutil: convert text attributes to html

[andrzej zaborowski]

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

Hi,

On 21 June 2010 15:06, Kristen Carlson Accardi <kristen@linux.intel.com> wrote:
> ---
>  src/stkutil.c |  165 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/stkutil.h |   22 ++++++++
>  2 files changed, 187 insertions(+), 0 deletions(-)
>
> diff --git a/src/stkutil.c b/src/stkutil.c
> index 642081e..1c8bac9 100644
> --- a/src/stkutil.c
> +++ b/src/stkutil.c
> @@ -26,6 +26,7 @@
>  #include <string.h>
>  #include <stdlib.h>
>  #include <stdint.h>
> +#include <stdio.h>
>
>  #include <glib.h>
>
> @@ -5978,3 +5979,167 @@ const unsigned char *stk_pdu_from_envelope(const struct stk_envelope *envelope,
>
>        return pdu;
>  }
> +
> +static const char *html_colors[] = {
> +       "#000000", /* Black */
> +       "#808080", /* Dark Grey */
> +       "#C11B17", /* Dark Red */
> +       "#FBB117", /* Dark Yellow */
> +       "#347235", /* Dark Green */
> +       "#307D7E", /* Dark Cyan */
> +       "#0000A0", /* Dark Blue */
> +       "#C031C7", /* Dark Magenta */
> +       "#C0C0C0", /* Grey */
> +       "#FFFFFF", /* White */
> +       "#FF0000", /* Bright Red */
> +       "#FFFF00", /* Bright Yellow */
> +       "#00FF00", /* Bright Green */
> +       "#00FFFF", /* Bright Cyan */
> +       "#0000FF", /* Bright Blue */
> +       "#FF00FF", /* Bright Magenta */
> +};
> +
> +static void end_format(GString *string, guint8 code)
> +{
> +       g_string_append(string, "</span>");
> +
> +       if ((code & STK_TEXT_FORMAT_ALIGN_MASK) != STK_TEXT_FORMAT_NO_ALIGN)
> +               g_string_append(string, "</div>");
> +}
> +
> +static void start_format(GString *string, guint8 code, guint8 color)
> +{
> +       guint8 align = code & STK_TEXT_FORMAT_ALIGN_MASK;
> +       guint8 font = code & STK_TEXT_FORMAT_FONT_MASK;
> +       guint8 style = code & STK_TEXT_FORMAT_STYLE_MASK;
> +       int fg = color & 0x0f;
> +       int bg = (color >> 4) & 0x0f;
> +
> +       /* align formatting applies to a block of test */
> +       if (align != STK_TEXT_FORMAT_NO_ALIGN)
> +               g_string_append(string, "<div style=\"");
> +
> +       if (align == STK_TEXT_FORMAT_RIGHT_ALIGN)
> +               g_string_append(string, "text-align: right;\">");
> +       else if (align == STK_TEXT_FORMAT_CENTER_ALIGN)
> +               g_string_append(string, "text-align: center;\">");
> +       else if (align == STK_TEXT_FORMAT_LEFT_ALIGN)
> +               g_string_append(string, "text-align: left;\">");
> +
> +       /* font, style, and color are inline */
> +       g_string_append(string, "<span style=\"");
> +
> +       if (font == STK_TEXT_FORMAT_FONT_SIZE_LARGE)
> +               g_string_append(string, "font-size: big;");
> +       else if (font == STK_TEXT_FORMAT_FONT_SIZE_SMALL)
> +               g_string_append(string, "font-size: small;");
> +
> +       if (style == STK_TEXT_FORMAT_STYLE_BOLD)
> +               g_string_append(string, "font-weight: bold;");
> +       else if (style == STK_TEXT_FORMAT_STYLE_ITALIC)
> +               g_string_append(string, "font-style: italic;");
> +       else if (style == STK_TEXT_FORMAT_STYLE_UNDERLINED)
> +               g_string_append(string, "text-decoration: underline;");
> +       else if (style == STK_TEXT_FORMAT_STYLE_STRIKETHROUGH)
> +               g_string_append(string, "text-decoration: line-through;");
> +
> +       /* add any color */
> +       if (fg)
> +               g_string_append_printf(string, "color: %s;", html_colors[fg]);
> +       if (bg)
> +               g_string_append_printf(string, "background-color: %s;",
> +                                               html_colors[bg]);
> +       g_string_append(string, "\">");
> +}
> +
> +char *stk_text_to_html(char *text, int text_len,
> +                               const unsigned char *attrs, int attrs_len)
> +{
> +       GString *string = g_string_sized_new(text_len + 1);
> +       GQueue formats[241];  /* maximum number of chars in text + 1 */
> +       int pos = 0, attr, i;
> +       guint8 start, end, code, color, len;
> +       guint8 align = STK_DEFAULT_TEXT_ALIGNMENT; /* hardcoded to left */
> +
> +       /* we may need formatting at the position beyond the last char */
> +       for (i = 0; i <= text_len; i++)
> +               g_queue_init(&formats[i]);
> +
> +       i = 0;
> +
> +       /*
> +        * each position in the text may have multiple attributes.
> +        * store each attribute in a queue for that position in the
> +        * order in which it was sent.  This will cause the last
> +        * attribute sent for that position to take priority.
> +        */
> +       while (i < attrs_len) {
> +               /* TBD - check for bad values */
> +               start = attrs[i++];
> +               len = attrs[i++];
> +               code = attrs[i++];
> +
> +               /*
> +                * if the alignment is the same as either the default
> +                * or the last alignment used, don't set any alignment
> +                * value.
> +                */
> +               if ((code & STK_TEXT_FORMAT_ALIGN_MASK) == align)
> +                       code |= 0x3;
> +               else
> +                       align = code & STK_TEXT_FORMAT_ALIGN_MASK;
> +
> +               if (i < attrs_len)
> +                       color = attrs[i++];
> +               else
> +                       color = 0;
> +
> +               if (len == 0)
> +                       end = text_len;
> +               else
> +                       end = start + len;
> +
> +               attr = start | (code << 16) | (color << 24);
> +
> +               g_queue_push_tail(&formats[start], GINT_TO_POINTER(attr));
> +               g_queue_push_tail(&formats[end], GINT_TO_POINTER(attr));
> +       }
> +
> +       while (pos <= text_len) {
> +               GQueue *q = &formats[pos];
> +
> +               /* there may be multiple formats per position */
> +               while ((attr = GPOINTER_TO_INT(g_queue_pop_head(q)))) {
> +                       start = attr & 0xff;
> +                       code = attr >> 16 & 0xff;
> +                       color = attr >> 24 & 0xff;
> +
> +                       if (pos == start)
> +                               start_format(string, code, color);
> +                       else
> +                               end_format(string, code);
> +               }

Can we always end_format() and start_format() when the formatting
changes, and just once instead of in a loop?  Here's a test case that
I'm worried about:

text is "abc"
attribute 1 (red text) starts at 0, len 2
attribute 2 (blue text) starts at 1, len 2

the generated html, by my reading will look like this:
<span style="color: red;">a<span style="color: blue;">b</span>c</span>

The end result is that c is red, while it should be blue.  But the
real problem is if one of them was a div, then you would have badly
balanced xml and it wouldn't parse. (I mentioned this on IRC)

> +
> +               if (pos == text_len)
> +                       break;
> +
> +               if (text[pos] == '\n')
> +                       g_string_append(string, "<br/>");
> +               else if (text[pos] == '\r') {
> +                       g_string_append(string, "<br/>");
> +                       if ((pos + 1 < text_len) && (text[pos + 1] == '\n'))
> +                               pos++;
> +               } else if (text[pos] == '<')
> +                       g_string_append(string, "&lt;");
> +               else if (text[pos] == '>')
> +                       g_string_append(string, "&gt;");
> +               else if (text[pos] == '&')
> +                       g_string_append(string, "&amp;");
> +               else
> +                       g_string_append_c(string, text[pos]);
> +               pos++;
> +       }
> +
> +       /* return characters from string. Caller must free char data */
> +       return g_string_free(string, FALSE);
> +}

This approach with having so many queues seems a little heavy.  On irc
I proposed that probably we don't need to implement this behaviour
fully and maybe could make some assumptions to simplify it.  But if
you want a full implementation here's what I'd do:

Make a list of all starts and ends of attributes, and sort it by
position in the text, for example for the abc case above, that list
would contain:
position 0, pointer to attribute 1
position 1, pointer to attribute 2
position 2, pointer to attribute 1
position 3, pointer to attribute 2

Each attribute has a .start, a .end (.start + .len) and a rank which
is just its position in the original array.

Then I'd loop through the text like this:

while (pos <= text_len) {
    if anything to do on current position {
        close previous <span>;

        foreach (attributes that start here) {
            if attribute->start >= attribute.end
                continue
            if attribute->rank > current_attribute->rank {
                current_attribute->start = attribute->end;
                current_attribute = attribute;
            } else
                attribute->start = current_attribute->end;
        }

        open new span(current_attribute);
    }

    append chunk of text up to next interesting position;
}

I'm not sure if this is enough (would have to try it) but it seems to
work, provided the list is maintained sorted.  That way whenever one
attibute ends, another is going to start at the same position, so
maybe it's unnecessary to keep the end positions in that list.

Regards

Re: [PATCH 1/2] stkutil: convert text attributes to html

[Kristen Carlson Accardi]

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

On Tue, 22 Jun 2010 00:45:39 +0200
andrzej zaborowski <balrogg@gmail.com> wrote:

> This approach with having so many queues seems a little heavy.  On irc
> I proposed that probably we don't need to implement this behaviour
> fully and maybe could make some assumptions to simplify it.  

I'm not sure what you mean when you say "heavy".  Are you saying
that you feel it has too much overhead?  I wonder if that is really
an issue for this functionality.  To me this code seems to be
easier to read than what you propose.

Re: [PATCH 1/2] stkutil: convert text attributes to html

[Kristen Carlson Accardi]

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

On Tue, 22 Jun 2010 00:45:39 +0200
andrzej zaborowski <balrogg@gmail.com> wrote:

> Can we always end_format() and start_format() when the formatting
> changes, and just once instead of in a loop?  Here's a test case that
> I'm worried about:
> 
> text is "abc"
> attribute 1 (red text) starts at 0, len 2
> attribute 2 (blue text) starts at 1, len 2
> 
> the generated html, by my reading will look like this:
> <span style="color: red;">a<span style="color: blue;">b</span>c</span>

I ran this test just to make sure, the generated html is not as
you fear and is correct (I used different colors though)

<html><body><span style="color: #347235;background-color: #FFFFFF;">a<span style="color: #0000A0;background-color: #FFFFFF;">b</span></span>c</body></html>

I can add this exact test as a unit test if you would like.

Re: [PATCH 1/2] stkutil: convert text attributes to html

[Kristen Carlson Accardi]

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

On Mon, 21 Jun 2010 19:51:06 -0700
Kristen Carlson Accardi <kristen@linux.intel.com> wrote:

> On Tue, 22 Jun 2010 00:45:39 +0200
> andrzej zaborowski <balrogg@gmail.com> wrote:
> 
> > Can we always end_format() and start_format() when the formatting
> > changes, and just once instead of in a loop?  Here's a test case that
> > I'm worried about:
> > 
> > text is "abc"
> > attribute 1 (red text) starts at 0, len 2
> > attribute 2 (blue text) starts at 1, len 2
> > 
> > the generated html, by my reading will look like this:
> > <span style="color: red;">a<span style="color: blue;">b</span>c</span>
> 
> I ran this test just to make sure, the generated html is not as
> you fear and is correct (I used different colors though)
> 
> <html><body><span style="color: #347235;background-color: #FFFFFF;">a<span style="color: #0000A0;background-color: #FFFFFF;">b</span></span>c</body></html>

Hum, ok, this is not correct.... I see your concern and will fix.

RE: [PATCH 1/2] stkutil: convert text attributes to html

[Gu, Yang]

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

Hi, 



>-----Original Message-----
>From: ofono-bounces(a)ofono.org [mailto:ofono-bounces(a)ofono.org] On Behalf Of
>Kristen Carlson Accardi
>Sent: Tuesday, June 22, 2010 11:17 AM
>To: ofono(a)ofono.org
>Subject: Re: [PATCH 1/2] stkutil: convert text attributes to html
>
>On Mon, 21 Jun 2010 19:51:06 -0700
>Kristen Carlson Accardi <kristen@linux.intel.com> wrote:
>
>> On Tue, 22 Jun 2010 00:45:39 +0200
>> andrzej zaborowski <balrogg@gmail.com> wrote:
>>
>> > Can we always end_format() and start_format() when the formatting
>> > changes, and just once instead of in a loop?  Here's a test case that
>> > I'm worried about:
>> >
>> > text is "abc"
>> > attribute 1 (red text) starts at 0, len 2
>> > attribute 2 (blue text) starts at 1, len 2
>> >
>> > the generated html, by my reading will look like this:
>> > <span style="color: red;">a<span style="color: blue;">b</span>c</span>
>>
>> I ran this test just to make sure, the generated html is not as
>> you fear and is correct (I used different colors though)
>>
>> <html><body><span style="color: #347235;background-color:
>#FFFFFF;">a<span style="color: #0000A0;background-color:
>#FFFFFF;">b</span></span>c</body></html>
>
>Hum, ok, this is not correct.... I see your concern and will fix.

When you fix your current algorithm, can we think about these optimizations at the same time?
1. The attribute overlapping may split the text into undesirable pieces. That is, we may find opportunities to merge the adjacent ones together at last. 
2. Some embedded attributes may be omitted. Taking your above case as example, the second "background-color:#FFFFFF" can be omitted.
3. Part of attributes, not the whole, can be merged together. For example: <red bold>a</red bold><red italic>b</red italic> "may" (not always bring benefit) be optimized as <red><bold>a</bold><italic>b</italic></red>. 

The target of these optimizations is to make to generated html file as small as possible. I guess this is a NP-complete problem, and not easy to handle :) 


Regards,
-Yang

Re: [PATCH 1/2] stkutil: convert text attributes to html

[Kristen Carlson Accardi]

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

On Tue, 22 Jun 2010 14:00:44 +0800
"Gu, Yang" <yang.gu@intel.com> wrote:

> 
> When you fix your current algorithm, can we think about these optimizations at the same time?
> 1. The attribute overlapping may split the text into undesirable pieces. That is, we may find opportunities to merge the adjacent ones together at last. 
> 2. Some embedded attributes may be omitted. Taking your above case as example, the second "background-color:#FFFFFF" can be omitted.
> 3. Part of attributes, not the whole, can be merged together. For example: <red bold>a</red bold><red italic>b</red italic> "may" (not always bring benefit) be optimized as <red><bold>a</bold><italic>b</italic></red>. 
> 
> The target of these optimizations is to make to generated html file as small as possible. I guess this is a NP-complete problem, and not easy to handle :) 

My feeling is that we should just worry about getting the 
conversion done in the least complicated and most expedient way.  
We could look at optimizations later if it becomes apparent that
they are needed.

[PATCH 2/2] test-stkutil: add html formatted text tests

[Kristen Carlson Accardi]

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

---
 unit/test-stkutil.c |  257 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 257 insertions(+), 0 deletions(-)

diff --git a/unit/test-stkutil.c b/unit/test-stkutil.c
index c586a7b..21fd02d 100644
--- a/unit/test-stkutil.c
+++ b/unit/test-stkutil.c
@@ -28,6 +28,9 @@
 #include <stdlib.h>
 #include <string.h>
 #include <stdint.h>
+#include <fcntl.h>
+
+#include <sys/stat.h>
 
 #include <glib.h>
 #include <glib/gprintf.h>
@@ -508,6 +511,7 @@ struct display_text_test {
 	struct stk_duration duration;
 	struct stk_text_attribute text_attr;
 	struct stk_frame_id frame_id;
+	const char *test_name;
 };
 
 unsigned char display_text_111[] = { 0xD0, 0x1A, 0x81, 0x03, 0x01, 0x21, 0x80,
@@ -603,6 +607,69 @@ unsigned char display_text_711[] = { 0xD0, 0x19, 0x81, 0x03, 0x01, 0x21, 0x80,
 					0x63, 0x6F, 0x6E, 0x64, 0x84, 0x02,
 					0x01, 0x0A };
 
+unsigned char display_text_811[] = { 0xD0, 0x22, 0x81, 0x03, 0x01, 0x21, 0x80,
+					0x82, 0x02, 0x81, 0x02, 0x8D, 0x11,
+					0x04, 0x54, 0x65, 0x78, 0x74, 0x20,
+					0x41, 0x74, 0x74, 0x72, 0x69, 0x62,
+					0x75, 0x74, 0x65, 0x20, 0x31, 0xD0,
+					0x04, 0x00, 0x10, 0x00, 0xB4 };
+
+unsigned char display_text_821[] = { 0xD0, 0x22, 0x81, 0x03, 0x01, 0x21, 0x80,
+					0x82, 0x02, 0x81, 0x02, 0x8D, 0x11,
+					0x04, 0x54, 0x65, 0x78, 0x74, 0x20,
+					0x41, 0x74, 0x74, 0x72, 0x69, 0x62,
+					0x75, 0x74, 0x65, 0x20, 0x31, 0xD0,
+					0x04, 0x00, 0x10, 0x01, 0xB4 };
+
+unsigned char display_text_831[] = { 0xD0, 0x22, 0x81, 0x03, 0x01, 0x21, 0x80,
+					0x82, 0x02, 0x81, 0x02, 0x8D, 0x11,
+					0x04, 0x54, 0x65, 0x78, 0x74, 0x20,
+					0x41, 0x74, 0x74, 0x72, 0x69, 0x62,
+					0x75, 0x74, 0x65, 0x20, 0x31, 0xD0,
+					0x04, 0x00, 0x10, 0x02, 0xB4 };
+
+unsigned char display_text_841[] = { 0xD0, 0x22, 0x81, 0x03, 0x01, 0x21, 0x80,
+					0x82, 0x02, 0x81, 0x02, 0x8D, 0x11,
+					0x04, 0x54, 0x65, 0x78, 0x74, 0x20,
+					0x41, 0x74, 0x74, 0x72, 0x69, 0x62,
+					0x75, 0x74, 0x65, 0x20, 0x31, 0xD0,
+					0x04, 0x00, 0x10, 0x04, 0xB4 };
+
+unsigned char display_text_851[] = { 0xD0, 0x22, 0x81, 0x03, 0x01, 0x21, 0x80,
+					0x82, 0x02, 0x81, 0x02, 0x8D, 0x11,
+					0x04, 0x54, 0x65, 0x78, 0x74, 0x20,
+					0x41, 0x74, 0x74, 0x72, 0x69, 0x62,
+					0x75, 0x74, 0x65, 0x20, 0x31, 0xD0,
+					0x04, 0x00, 0x10, 0x08, 0xB4 };
+
+unsigned char display_text_861[] = { 0xD0, 0x22, 0x81, 0x03, 0x01, 0x21, 0x80,
+					0x82, 0x02, 0x81, 0x02, 0x8D, 0x11,
+					0x04, 0x54, 0x65, 0x78, 0x74, 0x20,
+					0x41, 0x74, 0x74, 0x72, 0x69, 0x62,
+					0x75, 0x74, 0x65, 0x20, 0x31, 0xD0,
+					0x04, 0x00, 0x10, 0x10, 0xB4 };
+
+unsigned char display_text_871[] = { 0xD0, 0x22, 0x81, 0x03, 0x01, 0x21, 0x80,
+					0x82, 0x02, 0x81, 0x02, 0x8D, 0x11,
+					0x04, 0x54, 0x65, 0x78, 0x74, 0x20,
+					0x41, 0x74, 0x74, 0x72, 0x69, 0x62,
+					0x75, 0x74, 0x65, 0x20, 0x31, 0xD0,
+					0x04, 0x00, 0x10, 0x20, 0xB4 };
+
+unsigned char display_text_881[] = { 0xD0, 0x22, 0x81, 0x03, 0x01, 0x21, 0x80,
+					0x82, 0x02, 0x81, 0x02, 0x8D, 0x11,
+					0x04, 0x54, 0x65, 0x78, 0x74, 0x20,
+					0x41, 0x74, 0x74, 0x72, 0x69, 0x62,
+					0x75, 0x74, 0x65, 0x20, 0x31, 0xD0,
+					0x04, 0x00, 0x10, 0x40, 0xB4 };
+
+unsigned char display_text_891[] = { 0xD0, 0x22, 0x81, 0x03, 0x01, 0x21, 0x80,
+					0x82, 0x02, 0x81, 0x02, 0x8D, 0x11,
+					0x04, 0x54, 0x65, 0x78, 0x74, 0x20,
+					0x41, 0x74, 0x74, 0x72, 0x69, 0x62,
+					0x75, 0x74, 0x65, 0x20, 0x31, 0xD0,
+					0x04, 0x00, 0x10, 0x80, 0xB4 };
+
 unsigned char display_text_911[] = { 0xD0, 0x10, 0x81, 0x03, 0x01, 0x21, 0x80,
 					0x82, 0x02, 0x81, 0x02, 0x8D, 0x05,
 					0x08, 0x4F, 0x60, 0x59, 0x7D };
@@ -708,6 +775,114 @@ static struct display_text_test display_text_data_711 = {
 	}
 };
 
+static struct display_text_test display_text_data_811 = {
+	.pdu = display_text_811,
+	.pdu_len = sizeof(display_text_811),
+	.qualifier = 0x80,
+	.text = "Text Attribute 1",
+	.text_attr = {
+		.len = 4,
+		.attributes = { 0x00, 0x10, 0x00, 0xB4 },
+	},
+	.test_name = "811.html",
+};
+
+static struct display_text_test display_text_data_821 = {
+	.pdu = display_text_821,
+	.pdu_len = sizeof(display_text_821),
+	.qualifier = 0x80,
+	.text = "Text Attribute 1",
+	.text_attr = {
+		.len = 4,
+		.attributes = { 0x00, 0x10, 0x01, 0xB4 },
+	},
+	.test_name = "821.html",
+};
+
+static struct display_text_test display_text_data_831 = {
+	.pdu = display_text_831,
+	.pdu_len = sizeof(display_text_831),
+	.qualifier = 0x80,
+	.text = "Text Attribute 1",
+	.text_attr = {
+		.len = 4,
+		.attributes = { 0x00, 0x10, 0x02, 0xB4 },
+	},
+	.test_name = "831.html",
+};
+
+static struct display_text_test display_text_data_841 = {
+	.pdu = display_text_841,
+	.pdu_len = sizeof(display_text_841),
+	.qualifier = 0x80,
+	.text = "Text Attribute 1",
+	.text_attr = {
+		.len = 4,
+		.attributes = { 0x00, 0x10, 0x04, 0xB4 },
+	},
+	.test_name = "841.html",
+};
+
+static struct display_text_test display_text_data_851 = {
+	.pdu = display_text_851,
+	.pdu_len = sizeof(display_text_851),
+	.qualifier = 0x80,
+	.text = "Text Attribute 1",
+	.text_attr = {
+		.len = 4,
+		.attributes = { 0x00, 0x10, 0x08, 0xB4 },
+	},
+	.test_name = "851.html",
+};
+
+static struct display_text_test display_text_data_861 = {
+	.pdu = display_text_861,
+	.pdu_len = sizeof(display_text_861),
+	.qualifier = 0x80,
+	.text = "Text Attribute 1",
+	.text_attr = {
+		.len = 4,
+		.attributes = { 0x00, 0x10, 0x10, 0xB4 },
+	},
+	.test_name = "861.html",
+};
+
+static struct display_text_test display_text_data_871 = {
+	.pdu = display_text_871,
+	.pdu_len = sizeof(display_text_871),
+	.qualifier = 0x80,
+	.text = "Text Attribute 1",
+	.text_attr = {
+		.len = 4,
+		.attributes = { 0x00, 0x10, 0x20, 0xB4 },
+	},
+	.test_name = "871.html",
+};
+
+static struct display_text_test display_text_data_881 = {
+	.pdu = display_text_881,
+	.pdu_len = sizeof(display_text_881),
+	.qualifier = 0x80,
+	.text = "Text Attribute 1",
+	.text_attr = {
+		.len = 4,
+		.attributes = { 0x00, 0x10, 0x40, 0xB4 },
+	},
+	.test_name = "881.html",
+};
+
+static struct display_text_test display_text_data_891 = {
+	.pdu = display_text_891,
+	.pdu_len = sizeof(display_text_891),
+	.qualifier = 0x80,
+	.text = "Text Attribute 1",
+	.text_attr = {
+		.len = 4,
+		.attributes = { 0x00, 0x10, 0x80, 0xB4 },
+	},
+	.test_name = "891.html",
+};
+
 static struct display_text_test display_text_data_911 = {
 	.pdu = display_text_911,
 	.pdu_len = sizeof(display_text_911),
@@ -722,6 +897,58 @@ static struct display_text_test display_text_data_1011 = {
 	.text = "80ル"
 };
 
+static void do_html_test(char *text, int text_len, const unsigned char *attrs,
+				int attrs_len, const char *filename)
+{
+	int fd;
+	char html_header[] = "<html><body>";
+	char html_close[] = "</body></html>";
+	char *html;
+	ssize_t bytes_written;
+
+	if (filename == NULL)
+		return;
+
+	/* open output file */
+	fd = open(filename, O_WRONLY | O_CREAT, S_IRUSR | S_IWUSR |
+				S_IRGRP | S_IROTH);
+	if (fd < 0) {
+		g_print("unable to open %s\n", filename);
+		return;
+	}
+
+	bytes_written = write(fd, html_header, strlen(html_header));
+	html = stk_text_to_html(text, text_len, attrs, attrs_len);
+	bytes_written = write(fd, html, strlen(html));
+	bytes_written = write(fd, html_close, strlen(html_close));
+	close(fd);
+	g_free(html);
+}
+
+static void test_html1(gconstpointer data)
+{
+	char html_test[] = "EMS messages can contain italic, bold,"
+				" large, small and colored text";
+	const unsigned char attrs[] = {	0x19, 0x06, 0x20, 0x00,
+					0x21, 0x04, 0x10, 0x00,
+					0x27, 0x05, 0x04, 0x00,
+					0x2E, 0x05, 0x08, 0x00,
+					0x38, 0x07, 0x00, 0x2B };
+	const char filename[] = "html1.html";
+
+	do_html_test(html_test, strlen(html_test), attrs, 20, filename);
+}
+
+static void test_html2(gconstpointer data)
+{
+	char html_test[] = "Blue green green green";
+	const unsigned char attrs[] = {	0x00, 0x00, 0x00, 0x94,
+					0x00, 0x04, 0x00, 0x96 };
+	const char filename[] = "html2.html";
+
+	do_html_test(html_test, strlen(html_test), attrs, 8, filename);
+}
+
 /* Defined in TS 102.384 Section 27.22.4.1 */
 static void test_display_text(gconstpointer data)
 {
@@ -747,6 +974,14 @@ static void test_display_text(gconstpointer data)
 	check_duration(&command->display_text.duration, &test->duration);
 	check_text_attr(&command->display_text.text_attr,
 						&test->text_attr);
+
+	if (command->display_text.text_attr.len > 0)
+		do_html_test(command->display_text.text,
+				strlen(command->display_text.text),
+				command->display_text.text_attr.attributes,
+				command->display_text.text_attr.len,
+				test->test_name);
+
 	check_frame_id(&command->display_text.frame_id, &test->frame_id);
 
 	stk_command_free(command);
@@ -21913,6 +22148,28 @@ int main(int argc, char **argv)
 				&display_text_data_611, test_display_text);
 	g_test_add_data_func("/teststk/Display Text 7.1.1",
 				&display_text_data_711, test_display_text);
+	g_test_add_data_func("/teststk/HTML test 1",
+				NULL, test_html1);
+	g_test_add_data_func("/teststk/HTML test 2",
+				NULL, test_html2);
+	g_test_add_data_func("/teststk/Display Text 8.1.1",
+				&display_text_data_811, test_display_text);
+	g_test_add_data_func("/teststk/Display Text 8.2.1",
+				&display_text_data_821, test_display_text);
+	g_test_add_data_func("/teststk/Display Text 8.3.1",
+				&display_text_data_831, test_display_text);
+	g_test_add_data_func("/teststk/Display Text 8.4.1",
+				&display_text_data_841, test_display_text);
+	g_test_add_data_func("/teststk/Display Text 8.5.1",
+				&display_text_data_851, test_display_text);
+	g_test_add_data_func("/teststk/Display Text 8.6.1",
+				&display_text_data_861, test_display_text);
+	g_test_add_data_func("/teststk/Display Text 8.7.1",
+				&display_text_data_871, test_display_text);
+	g_test_add_data_func("/teststk/Display Text 8.8.1",
+				&display_text_data_881, test_display_text);
+	g_test_add_data_func("/teststk/Display Text 8.9.1",
+				&display_text_data_891, test_display_text);
 	g_test_add_data_func("/teststk/Display Text 9.1.1",
 				&display_text_data_911, test_display_text);
 	g_test_add_data_func("/teststk/Display Text 10.1.1",
-- 
1.6.6.1

Re: [PATCH 2/2] test-stkutil: add html formatted text tests

[Andrzej Zaborowski]

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

On 21 June 2010 15:06, Kristen Carlson Accardi <kristen@linux.intel.com> wrote:
> diff --git a/unit/test-stkutil.c b/unit/test-stkutil.c
> index c586a7b..21fd02d 100644
> --- a/unit/test-stkutil.c
> +++ b/unit/test-stkutil.c
> @@ -28,6 +28,9 @@
>  #include <stdlib.h>
>  #include <string.h>
>  #include <stdint.h>
> +#include <fcntl.h>
> +
> +#include <sys/stat.h>
>
>  #include <glib.h>
>  #include <glib/gprintf.h>
> @@ -508,6 +511,7 @@ struct display_text_test {
>        struct stk_duration duration;
>        struct stk_text_attribute text_attr;
>        struct stk_frame_id frame_id;
> +       const char *test_name;
>  };
>
>  unsigned char display_text_111[] = { 0xD0, 0x1A, 0x81, 0x03, 0x01, 0x21, 0x80,
> @@ -603,6 +607,69 @@ unsigned char display_text_711[] = { 0xD0, 0x19, 0x81, 0x03, 0x01, 0x21, 0x80,
>                                        0x63, 0x6F, 0x6E, 0x64, 0x84, 0x02,
>                                        0x01, 0x0A };
>
> +unsigned char display_text_811[] = { 0xD0, 0x22, 0x81, 0x03, 0x01, 0x21, 0x80,
> +                                       0x82, 0x02, 0x81, 0x02, 0x8D, 0x11,
> +                                       0x04, 0x54, 0x65, 0x78, 0x74, 0x20,
> +                                       0x41, 0x74, 0x74, 0x72, 0x69, 0x62,
> +                                       0x75, 0x74, 0x65, 0x20, 0x31, 0xD0,
> +                                       0x04, 0x00, 0x10, 0x00, 0xB4 };
> +
> +unsigned char display_text_821[] = { 0xD0, 0x22, 0x81, 0x03, 0x01, 0x21, 0x80,
> +                                       0x82, 0x02, 0x81, 0x02, 0x8D, 0x11,
> +                                       0x04, 0x54, 0x65, 0x78, 0x74, 0x20,
> +                                       0x41, 0x74, 0x74, 0x72, 0x69, 0x62,
> +                                       0x75, 0x74, 0x65, 0x20, 0x31, 0xD0,
> +                                       0x04, 0x00, 0x10, 0x01, 0xB4 };
> +
> +unsigned char display_text_831[] = { 0xD0, 0x22, 0x81, 0x03, 0x01, 0x21, 0x80,
> +                                       0x82, 0x02, 0x81, 0x02, 0x8D, 0x11,
> +                                       0x04, 0x54, 0x65, 0x78, 0x74, 0x20,
> +                                       0x41, 0x74, 0x74, 0x72, 0x69, 0x62,
> +                                       0x75, 0x74, 0x65, 0x20, 0x31, 0xD0,
> +                                       0x04, 0x00, 0x10, 0x02, 0xB4 };
> +
> +unsigned char display_text_841[] = { 0xD0, 0x22, 0x81, 0x03, 0x01, 0x21, 0x80,
> +                                       0x82, 0x02, 0x81, 0x02, 0x8D, 0x11,
> +                                       0x04, 0x54, 0x65, 0x78, 0x74, 0x20,
> +                                       0x41, 0x74, 0x74, 0x72, 0x69, 0x62,
> +                                       0x75, 0x74, 0x65, 0x20, 0x31, 0xD0,
> +                                       0x04, 0x00, 0x10, 0x04, 0xB4 };
> +
> +unsigned char display_text_851[] = { 0xD0, 0x22, 0x81, 0x03, 0x01, 0x21, 0x80,
> +                                       0x82, 0x02, 0x81, 0x02, 0x8D, 0x11,
> +                                       0x04, 0x54, 0x65, 0x78, 0x74, 0x20,
> +                                       0x41, 0x74, 0x74, 0x72, 0x69, 0x62,
> +                                       0x75, 0x74, 0x65, 0x20, 0x31, 0xD0,
> +                                       0x04, 0x00, 0x10, 0x08, 0xB4 };
> +
> +unsigned char display_text_861[] = { 0xD0, 0x22, 0x81, 0x03, 0x01, 0x21, 0x80,
> +                                       0x82, 0x02, 0x81, 0x02, 0x8D, 0x11,
> +                                       0x04, 0x54, 0x65, 0x78, 0x74, 0x20,
> +                                       0x41, 0x74, 0x74, 0x72, 0x69, 0x62,
> +                                       0x75, 0x74, 0x65, 0x20, 0x31, 0xD0,
> +                                       0x04, 0x00, 0x10, 0x10, 0xB4 };
> +
> +unsigned char display_text_871[] = { 0xD0, 0x22, 0x81, 0x03, 0x01, 0x21, 0x80,
> +                                       0x82, 0x02, 0x81, 0x02, 0x8D, 0x11,
> +                                       0x04, 0x54, 0x65, 0x78, 0x74, 0x20,
> +                                       0x41, 0x74, 0x74, 0x72, 0x69, 0x62,
> +                                       0x75, 0x74, 0x65, 0x20, 0x31, 0xD0,
> +                                       0x04, 0x00, 0x10, 0x20, 0xB4 };
> +
> +unsigned char display_text_881[] = { 0xD0, 0x22, 0x81, 0x03, 0x01, 0x21, 0x80,
> +                                       0x82, 0x02, 0x81, 0x02, 0x8D, 0x11,
> +                                       0x04, 0x54, 0x65, 0x78, 0x74, 0x20,
> +                                       0x41, 0x74, 0x74, 0x72, 0x69, 0x62,
> +                                       0x75, 0x74, 0x65, 0x20, 0x31, 0xD0,
> +                                       0x04, 0x00, 0x10, 0x40, 0xB4 };
> +
> +unsigned char display_text_891[] = { 0xD0, 0x22, 0x81, 0x03, 0x01, 0x21, 0x80,
> +                                       0x82, 0x02, 0x81, 0x02, 0x8D, 0x11,
> +                                       0x04, 0x54, 0x65, 0x78, 0x74, 0x20,
> +                                       0x41, 0x74, 0x74, 0x72, 0x69, 0x62,
> +                                       0x75, 0x74, 0x65, 0x20, 0x31, 0xD0,
> +                                       0x04, 0x00, 0x10, 0x80, 0xB4 };
> +
>  unsigned char display_text_911[] = { 0xD0, 0x10, 0x81, 0x03, 0x01, 0x21, 0x80,
>                                        0x82, 0x02, 0x81, 0x02, 0x8D, 0x05,
>                                        0x08, 0x4F, 0x60, 0x59, 0x7D };
> @@ -708,6 +775,114 @@ static struct display_text_test display_text_data_711 = {
>        }
>  };
>
> +static struct display_text_test display_text_data_811 = {
> +       .pdu = display_text_811,
> +       .pdu_len = sizeof(display_text_811),
> +       .qualifier = 0x80,
> +       .text = "Text Attribute 1",
> +       .text_attr = {
> +               .len = 4,
> +               .attributes = { 0x00, 0x10, 0x00, 0xB4 },
> +       },
> +       .test_name = "811.html",
> +};
> +
> +static struct display_text_test display_text_data_821 = {
> +       .pdu = display_text_821,
> +       .pdu_len = sizeof(display_text_821),
> +       .qualifier = 0x80,
> +       .text = "Text Attribute 1",
> +       .text_attr = {
> +               .len = 4,
> +               .attributes = { 0x00, 0x10, 0x01, 0xB4 },
> +       },
> +       .test_name = "821.html",
> +};
> +
> +static struct display_text_test display_text_data_831 = {
> +       .pdu = display_text_831,
> +       .pdu_len = sizeof(display_text_831),
> +       .qualifier = 0x80,
> +       .text = "Text Attribute 1",
> +       .text_attr = {
> +               .len = 4,
> +               .attributes = { 0x00, 0x10, 0x02, 0xB4 },
> +       },
> +       .test_name = "831.html",
> +};
> +
> +static struct display_text_test display_text_data_841 = {
> +       .pdu = display_text_841,
> +       .pdu_len = sizeof(display_text_841),
> +       .qualifier = 0x80,
> +       .text = "Text Attribute 1",
> +       .text_attr = {
> +               .len = 4,
> +               .attributes = { 0x00, 0x10, 0x04, 0xB4 },
> +       },
> +       .test_name = "841.html",
> +};
> +
> +static struct display_text_test display_text_data_851 = {
> +       .pdu = display_text_851,
> +       .pdu_len = sizeof(display_text_851),
> +       .qualifier = 0x80,
> +       .text = "Text Attribute 1",
> +       .text_attr = {
> +               .len = 4,
> +               .attributes = { 0x00, 0x10, 0x08, 0xB4 },
> +       },
> +       .test_name = "851.html",
> +};
> +
> +static struct display_text_test display_text_data_861 = {
> +       .pdu = display_text_861,
> +       .pdu_len = sizeof(display_text_861),
> +       .qualifier = 0x80,
> +       .text = "Text Attribute 1",
> +       .text_attr = {
> +               .len = 4,
> +               .attributes = { 0x00, 0x10, 0x10, 0xB4 },
> +       },
> +       .test_name = "861.html",
> +};
> +
> +static struct display_text_test display_text_data_871 = {
> +       .pdu = display_text_871,
> +       .pdu_len = sizeof(display_text_871),
> +       .qualifier = 0x80,
> +       .text = "Text Attribute 1",
> +       .text_attr = {
> +               .len = 4,
> +               .attributes = { 0x00, 0x10, 0x20, 0xB4 },
> +       },
> +       .test_name = "871.html",
> +};
> +
> +static struct display_text_test display_text_data_881 = {
> +       .pdu = display_text_881,
> +       .pdu_len = sizeof(display_text_881),
> +       .qualifier = 0x80,
> +       .text = "Text Attribute 1",
> +       .text_attr = {
> +               .len = 4,
> +               .attributes = { 0x00, 0x10, 0x40, 0xB4 },
> +       },
> +       .test_name = "881.html",
> +};
> +
> +static struct display_text_test display_text_data_891 = {
> +       .pdu = display_text_891,
> +       .pdu_len = sizeof(display_text_891),
> +       .qualifier = 0x80,
> +       .text = "Text Attribute 1",
> +       .text_attr = {
> +               .len = 4,
> +               .attributes = { 0x00, 0x10, 0x80, 0xB4 },
> +       },
> +       .test_name = "891.html",
> +};
> +
>  static struct display_text_test display_text_data_911 = {
>        .pdu = display_text_911,
>        .pdu_len = sizeof(display_text_911),
> @@ -722,6 +897,58 @@ static struct display_text_test display_text_data_1011 = {
>        .text = "80ル"
>  };
>
> +static void do_html_test(char *text, int text_len, const unsigned char *attrs,
> +                               int attrs_len, const char *filename)
> +{
> +       int fd;
> +       char html_header[] = "<html><body>";
> +       char html_close[] = "</body></html>";
> +       char *html;
> +       ssize_t bytes_written;
> +
> +       if (filename == NULL)
> +               return;
> +
> +       /* open output file */
> +       fd = open(filename, O_WRONLY | O_CREAT, S_IRUSR | S_IWUSR |
> +                               S_IRGRP | S_IROTH);
> +       if (fd < 0) {
> +               g_print("unable to open %s\n", filename);
> +               return;
> +       }
> +
> +       bytes_written = write(fd, html_header, strlen(html_header));
> +       html = stk_text_to_html(text, text_len, attrs, attrs_len);
> +       bytes_written = write(fd, html, strlen(html));
> +       bytes_written = write(fd, html_close, strlen(html_close));
> +       close(fd);
> +       g_free(html);
> +}
> +
> +static void test_html1(gconstpointer data)
> +{
> +       char html_test[] = "EMS messages can contain italic, bold,"
> +                               " large, small and colored text";
> +       const unsigned char attrs[] = { 0x19, 0x06, 0x20, 0x00,
> +                                       0x21, 0x04, 0x10, 0x00,
> +                                       0x27, 0x05, 0x04, 0x00,
> +                                       0x2E, 0x05, 0x08, 0x00,
> +                                       0x38, 0x07, 0x00, 0x2B };
> +       const char filename[] = "html1.html";
> +
> +       do_html_test(html_test, strlen(html_test), attrs, 20, filename);
> +}
> +
> +static void test_html2(gconstpointer data)
> +{
> +       char html_test[] = "Blue green green green";
> +       const unsigned char attrs[] = { 0x00, 0x00, 0x00, 0x94,
> +                                       0x00, 0x04, 0x00, 0x96 };
> +       const char filename[] = "html2.html";
> +
> +       do_html_test(html_test, strlen(html_test), attrs, 8, filename);
> +}
> +
>  /* Defined in TS 102.384 Section 27.22.4.1 */
>  static void test_display_text(gconstpointer data)
>  {
> @@ -747,6 +974,14 @@ static void test_display_text(gconstpointer data)
>        check_duration(&command->display_text.duration, &test->duration);
>        check_text_attr(&command->display_text.text_attr,
>                                                &test->text_attr);
> +
> +       if (command->display_text.text_attr.len > 0)
> +               do_html_test(command->display_text.text,
> +                               strlen(command->display_text.text),
> +                               command->display_text.text_attr.attributes,
> +                               command->display_text.text_attr.len,
> +                               test->test_name);
> +
>        check_frame_id(&command->display_text.frame_id, &test->frame_id);
>
>        stk_command_free(command);
> @@ -21913,6 +22148,28 @@ int main(int argc, char **argv)
>                                &display_text_data_611, test_display_text);
>        g_test_add_data_func("/teststk/Display Text 7.1.1",
>                                &display_text_data_711, test_display_text);
> +       g_test_add_data_func("/teststk/HTML test 1",
> +                               NULL, test_html1);
> +       g_test_add_data_func("/teststk/HTML test 2",
> +                               NULL, test_html2);
> +       g_test_add_data_func("/teststk/Display Text 8.1.1",
> +                               &display_text_data_811, test_display_text);
> +       g_test_add_data_func("/teststk/Display Text 8.2.1",
> +                               &display_text_data_821, test_display_text);
> +       g_test_add_data_func("/teststk/Display Text 8.3.1",
> +                               &display_text_data_831, test_display_text);
> +       g_test_add_data_func("/teststk/Display Text 8.4.1",
> +                               &display_text_data_841, test_display_text);
> +       g_test_add_data_func("/teststk/Display Text 8.5.1",
> +                               &display_text_data_851, test_display_text);
> +       g_test_add_data_func("/teststk/Display Text 8.6.1",
> +                               &display_text_data_861, test_display_text);
> +       g_test_add_data_func("/teststk/Display Text 8.7.1",
> +                               &display_text_data_871, test_display_text);
> +       g_test_add_data_func("/teststk/Display Text 8.8.1",
> +                               &display_text_data_881, test_display_text);
> +       g_test_add_data_func("/teststk/Display Text 8.9.1",
> +                               &display_text_data_891, test_display_text);

The unit tests can probably do whatever they want, but in my opinion
they should fail if anything is wrong with the tested function, while
this code never calls g_assert.  You should probably include the
expected html with each test and do strcmp (I agree this is not ideal
because there are many valid results, but it'll at least force someone
who sends a patch to run and update the tests).

Regards

Re: [PATCH 2/2] test-stkutil: add html formatted text tests

[Denis Kenzior]

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

Hi Kristen,

> +static void do_html_test(char *text, int text_len, const unsigned char
>  *attrs, +				int attrs_len, const char *filename)
> +{
> +	int fd;
> +	char html_header[] = "<html><body>";
> +	char html_close[] = "</body></html>";
> +	char *html;
> +	ssize_t bytes_written;
> +
> +	if (filename == NULL)
> +		return;
> +
> +	/* open output file */
> +	fd = open(filename, O_WRONLY | O_CREAT, S_IRUSR | S_IWUSR |
> +				S_IRGRP | S_IROTH);
> +	if (fd < 0) {
> +		g_print("unable to open %s\n", filename);
> +		return;
> +	}
> +
> +	bytes_written = write(fd, html_header, strlen(html_header));
> +	html = stk_text_to_html(text, text_len, attrs, attrs_len);
> +	bytes_written = write(fd, html, strlen(html));
> +	bytes_written = write(fd, html_close, strlen(html_close));
> +	close(fd);
> +	g_free(html);
> +}
> +

Please note that unit tests should not require any manual intervention for the 
user to detect whether the code being tested is working or not.  E.g. if you 
don't g_assert at least once, it is not a unit test.

For most of these cases it is quite easy to know the output.  So my suggestion 
here is to set the expected html output as a string in the test structure and 
do a strcmp / g_assert at the end of the test.

Regards,
-Denis