[m-rev.] for post-commit review: colouring column groups
Paul Bone
pbone at csse.unimelb.edu.au
Mon Aug 11 13:53:44 AEST 2008
On Wed, Aug 06, 2008 at 06:10:52PM +1000, Zoltan Somogyi wrote:
> For post-commit review by Paul.
>
> Zoltan.
>
> Implement the colouring of column groups.
>
> -:- type table_header_cell
> - ---> table_header_cell(
> +:- type table_header_group
> + ---> table_header_group(
> % The table contents.
> - thc_contents :: table_data,
> + thg_titles :: table_header_group_columns,
>
> % The class may be used by a layout to make decisions
> % about how to paint this column.
> - thc_class :: table_col_class
> - )
> - ; table_header_group(
> - thg_title :: string,
> - thg_subtitles :: list(table_data),
> + thg_class :: table_column_class,
>
> - % The class may be used by a layout to make decisions
> - % about how to paint this column.
> - thg_class :: table_col_class
> + thg_set_style :: table_set_style
> ).
>
> +:- type table_header_group_columns
> + ---> table_header_group_single(
> + % The header of the single column in the group.
> + thsc_title :: table_data
> + )
> + ; table_header_group_multi(
> + % The spanning header, which applies to all columns
> + % in the group.
> + thmc_title :: string,
> +
> + % The headers of the individual columns in the group.
> + thmc_subtitles :: list(table_data)
> + ).
> +
> +:- type table_set_style
> + ---> table_set_style
> + ; table_do_not_set_style.
> +
Should this be named table_column_set_style? Please place a comment
here, or in the table_header_group type about what this does, for the
benefit of someone implementing htmlize_display or creating table data.
>
> -htmlize_display(Deep, display(MaybeSubTitle, Items)) = HTML :-
> +htmlize_display(Deep, Prefs, Display) = HTML :-
> + Display = display(MaybeTitle, Items),
> MainTitle = str_to_html("Mercury Deep Profile for ") ++
> str_to_html(Deep ^ data_file_name),
> (
> - MaybeSubTitle = no,
> - Title = MainTitle,
> + MaybeTitle = no,
> + HeadTitle = MainTitle,
> HeadingHTML = empty_html
> ;
> - MaybeSubTitle = yes(Subtitle),
> - SubTitleHTML = str_to_html(Subtitle),
> - Title = MainTitle ++ str_to_html(" - ") ++ SubTitleHTML,
> - HeadingHTML = wrap_tags("<h3>", "</h3>\n", SubTitleHTML)
> + MaybeTitle = yes(Title),
> + TitleHTML = str_to_html(Title),
> + HeadTitle = MainTitle ++ str_to_html(" - ") ++ TitleHTML,
> + HeadingHTML = wrap_tags("<h3>", "</h3>\n", TitleHTML)
> ),
> - TitleHTML = wrap_tags("<title>", "</title>\n", Title),
> + HeadTitleHTML = wrap_tags("<title>", "</title>\n", HeadTitle),
> +
> deep_to_http_context(Deep, HTTPContext),
> + StyleControlMap0 = default_style_control_map,
> map_join_html(item_to_html("<div>\n", "</div>\n", HTTPContext),
> - Items, ItemsHTML),
> + StyleControlMap0, StyleControlMap1, Items, ItemsHTML),
> +
> + ColourScheme = Prefs ^ pref_colour,
> + (
> + ColourScheme = colour_column_groups,
> + StyleControlMap = StyleControlMap1
> + ;
> + ColourScheme = colour_none,
> + % Ignore the updates in StyleControlMap1. This works as long as
> + % all such updates implement the colouring of column groups.
I'm concerned about this, And I don't understand why the style is
created from the content. The style should be chosen based on the
user's preferences.
> + StyleControlMap = default_style_control_map
> + ),
> +
> + StyleHTML = css_style_html(StyleControlMap),
> +
> HTML = doc_type_html ++
> wrap_tags("<html>\n", "</html>\n",
> - wrap_tags("<head>\n", "</head>\n", TitleHTML ++ css_style_html) ++
> + wrap_tags("<head>\n", "</head>\n", HeadTitleHTML ++ StyleHTML) ++
> wrap_tags("<body>\n", "</body>\n", HeadingHTML ++ ItemsHTML)
> ).
>
> @@ -222,83 +241,45 @@
> "<!DOCTYPE HTML PUBLIC \"-//W3C//DTD HTML 4.01//EN\"\n" ++
> "\"http://www.w3.org/TR/html4/strict.dtd\">\n").
>
> -:- func css_style_html = html.
> +%-----------------------------------------------------------------------------%
> +
> +:- func css_style_html(style_control_map) = html.
>
> -css_style_html =
> +css_style_html(StyleControlMap) = HTML :-
> % XXX This ignores colour_column_groups. We should respect it,
> % and process it either here, or when converting table columns to HTML.
>
> - str_to_html("
> - <style type=\"text/css\">
> - td.allocations
> - {
> - text-align: right;
> - }
> - td.callseqs
> - {
> - text-align: right;
> - }
> - td.memory
> - {
> - text-align: right;
> - }
> - td.number
> - {
> - text-align: right;
> - }
> - td.ordinal_rank
> - {
> - text-align: right;
> - }
> - td.port_counts
> - {
> - text-align: right;
> - }
> - td.proc
> - {
> - text-align: left;
> - }
> - td.ticks_and_times
> - {
> - text-align: right;
> - }
> - a.control
> - {
> - margin: 5px;
> - text-decoration: none;
> - }
> - table.plain
> - {
> - border-style: none;
> - }
> - table.boxed
> - {
> - border-width: 1px 1px 1px 1px;
> - border-spacing: 2px;
> - border-style: outset outset outset outset;
> - }
> - table.boxed th
> - {
> - border-width: 1px 1px 1px 1px;
> - padding: 3px 3px 3px 3px;
> - border-style: inset inset inset inset;
> - }
> - table.boxed td
> - {
> - border-width: 1px 1px 1px 1px;
> - padding: 3px 3px 3px 3px;
> - border-style: inset inset inset inset;
> - }
> - </style>
> -").
> + map.to_assoc_list(StyleControlMap, StyleControls),
> + ControlHTMLs = list.map(style_control_to_html, StyleControls),
> + ControlsHTML = append_htmls(ControlHTMLs),
> + HTML = wrap_tags("<style type=\"text/css\">\n", "</style>\n",
> + ControlsHTML).
This could probably be done using map.foldl, this will eliminate the
need to create the intermediate list.
> +
> +:- func style_control_to_html(pair(style_control, style_element_map)) = html.
> +
> +style_control_to_html(Control - StyleElementMap) = HTML :-
> + Control = style_control(ControlName),
> + StyleElements = map.to_assoc_list(StyleElementMap),
> + ElementHTMLs = list.map(style_element_to_html, StyleElements),
> + ElementsHTML = append_htmls(ElementHTMLs),
> + StartFragment = string.format("\t%s\n\t{\n", [s(ControlName)]),
> + EndFragment = "\t}\n",
> + HTML = wrap_tags(StartFragment, EndFragment, ElementsHTML).
> +
wrap_tags isn't an appropriate name for this use of this function. I
suggest renaming wrap_tags warp_string. And creating a new wrap_tags
using wrap_string.
> %-----------------------------------------------------------------------------%
>
> - % Determine how many rows the table header requires, and a map between
> - % column numbers and classes. This should be used with foldl3 and
> - % takes a number of accumulator values.
> + % Determine how many rows the table header requires, and set up a map
> + % from column numbers to classes. Update the style control map for
> + % table header groups that specify table_set_style.
> %
> -:- pred table_header_num_rows_and_classmap(table_header_cell::in,
> + % This should be used with list.foldl5.
> + %
> +:- pred table_header_num_rows_and_classmap(table_header_group::in,
> table_header_rows::in, table_header_rows::out,
> - int::in, int::out, col_class_map::in, col_class_map::out) is det.
> + int::in, int::out, column_class_map::in, column_class_map::out,
> + int::in, int::out, style_control_map::in, style_control_map::out) is det.
>
> -table_header_num_rows_and_classmap(Cell, !NumRows, !ColNum, !ClassMap) :-
> - (
> - Cell = table_header_cell(_, Class),
> - table_col_class_to_string(Class, ClassStr),
> - svmap.det_insert(!.ColNum, ClassStr, !ClassMap),
> - NumSubCols = 1
> +table_header_num_rows_and_classmap(HeaderGroup, !NumRows, !ColumnNumber,
> + !ClassMap, !HeaderGroupNumber, !StyleControlMap) :-
> + HeaderGroup = table_header_group(ColumnTitles, ColumnClass, SetStyle),
> + ColumnClassStr = table_column_class_to_string(ColumnClass),
> + (
> + ColumnTitles = table_header_group_single(_),
> + NumSubCols = 1,
> + svmap.det_insert(!.ColumnNumber, ColumnClassStr, !ClassMap)
> ;
> - Cell = table_header_group(_, Subtitles, Class),
> - length(Subtitles, NumSubCols),
> + ColumnTitles = table_header_group_multi(_, SubTitles),
> + list.length(SubTitles, NumSubCols),
> !:NumRows = two_header_rows,
> - table_col_class_to_string(Class, ClassStr),
> % fold_up is inclusive of the higher number.
> - fold_up(insert_col_classmap(ClassStr),
> - !.ColNum, !.ColNum + NumSubCols - 1, !ClassMap)
> + int.fold_up(insert_column_into_classmap(ColumnClassStr),
> + !.ColumnNumber, !.ColumnNumber + NumSubCols - 1, !ClassMap)
> ),
> - !:ColNum = !.ColNum + NumSubCols.
> -
> -%-----------------------------------------------------------------------------%
> + (
> + SetStyle = table_do_not_set_style
> + ;
> + SetStyle = table_set_style,
> + update_style_control_map(ColumnClass, !.HeaderGroupNumber,
> + !StyleControlMap),
> + !:HeaderGroupNumber = !.HeaderGroupNumber + 1
> + ),
> + !:ColumnNumber = !.ColumnNumber + NumSubCols.
>
> -:- pred insert_col_classmap(string::in, int::in,
> - col_class_map::in, col_class_map::out) is det.
> +:- pred insert_column_into_classmap(string::in, int::in,
> + column_class_map::in, column_class_map::out) is det.
>
> -insert_col_classmap(Value, Key, !Map) :-
> +insert_column_into_classmap(Value, Key, !Map) :-
> svmap.det_insert(Key, Value, !Map).
>
> +:- pred update_style_control_map(table_column_class::in, int::in,
> + style_control_map::in, style_control_map::out) is det.
> +
> +update_style_control_map(ColumnClass, HeaderGroupNumber, !StyleControlMap) :-
> + ColumnClassStr = table_column_class_to_string(ColumnClass),
> + StyleControl = style_control("td." ++ ColumnClassStr),
> + StyleElement = style_element("background"),
> + ( HeaderGroupNumber /\ 1 = 0 ->
> + Colour = "LightGrey"
> + ;
> + Colour = "White"
> + ),
Rather than set the colour attribute in the style, this should set the
style. This will increase the number of td styles, by creating a _odd and
a _even style for each existing style.`
> + ( map.search(!.StyleControlMap, StyleControl, StyleElementMap0) ->
> + map.set(StyleElementMap0, StyleElement, Colour, StyleElementMap),
> + svmap.det_update(StyleControl, StyleElementMap, !StyleControlMap)
> + ;
> + map.det_insert(map.init, StyleElement, Colour, StyleElementMap),
> + svmap.det_insert(StyleControl, StyleElementMap, !StyleControlMap)
> + ).
> +
This code adds the restriction that the same style cannot be used for
two column groups.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mercurylang.org/archives/reviews/attachments/20080811/0fffb1a6/attachment.sig>
More information about the reviews
mailing list