widelands-dev team mailing list archive

[kaputtnik <somal@xxxxxxxx>]

Did now some proof reading, see inline comments.

What i stumbled over is a mixed use of uppercase/lowercase after hyphen/colon. Eg. for the two text composition functions we have:

In join_sentence() its lowercase:
:arg sentence1: text of the first sentence

In localize lists its uppercase:
:arg items:              An array of strings



Diff comments:

> 
> === modified file 'data/scripting/richtext.lua'
> --- data/scripting/richtext.lua	2018-02-12 10:17:22 +0000
> +++ data/scripting/richtext.lua	2018-02-24 11:18:33 +0000
> @@ -232,24 +247,55 @@
>  --    Closes a paragraph.
>  --
>  --    :returns: The closing tags for a paragraph
> +
>  function close_p(t)
>     return vspace(6) .. "</font>" .. vspace(6)  .. "</p>"
>  end
>  
> +
> +-- RST
> +-- .. function:: p_font(text_or_attributes[, text = nil])

The description of arguments does not match the args below.

p_font(p_or_font_attributes[, text_or_font_attributes]) ?

> +--
> +--    Returns one paragraph with text followed by a small vertical gap. Options
> +--    can be given as first argument similar to :func:`rt`.
> +--
> +--    :arg p_or_font_attributes: Optional paragraphs or font attributes.

Optional paragraph or ... ?

> +--    :type p_or_font_attributes: :class:`string`
> +--
> +--    :arg text_or_font_attributes: Optional font attributes or the text itself.
> +--    :type text_or_font_attributes: :class:`string`
> +--
> +--    See the :ref:`p tag's documentation <rt_tags_p>` for a list of paragraph
> +--    attributes and the :ref:`font tag's documentation <rt_tags_font>` for a
> +--    list of font attributes.
> +--
> +--    :returns: The text wrapped in <p attributes><font attributes>text</font></p>
> +
> +function p_font(p_or_font_attributes, text_or_font_attributes, text)
> +   if text then
> +      return ("<p %s>"):format(p_or_font_attributes) .. "<font " .. text_or_font_attributes .. ">" .. text .. close_p()
> +   else
> +      return "<p><font " .. p_or_font_attributes .. ">" .. text_or_font_attributes .. close_p()
> +   end
> +end
> +
> +-- RST
> +-- :ref:`Return to index<richtext.lua>`
> +--
> +-- Text Formatting
> +-- ^^^^^^^^^^^^^^^
> +--
> +-- This section covers convenience functions for text formatting.
> +
> +
>  -- RST
>  -- .. function:: font(attributes, text)
>  --
> ---    Wraps the text in font tags.
> +--    Wraps the text in font tags. See also :any:`p_font`.
>  --
> ---    Allowed attributes are:
> ---       size:      the font size in pt
> ---       face:      sans, serif or condensed
> ---       color:     a hex color
> ---       bold:      if you add bold=1, the text will be bold
> ---       italic:    if you add italic=1, the text will be italic
> ---       underline: if you add underline=1, the text will be underlined
> ---       shadow:    if you add shadow=1, the text will have a shadow
> ---       ref:       TODO(GunChleoc): I don't know what this does.
> +--    :arg attributes: see the :ref:`font tag's documentation <rt_tags_font>`
> +--                     for a list of attributes and their descriptions.
> +--    :type attributes: :class:`string`
>  --
>  --    :returns: The text wrapped in font tags with the given attributes
>  
> @@ -322,18 +396,20 @@
>  --    :arg text: the text of the list item
>  --
>  --    :returns: li("→", text)
> +
>  function li_arrow(text)
>     -- TODO(GunChleoc): Reverse arrow for rtl languages.
>     return li("→", text)
>  end
>  
> +
>  -- RST
> --- .. function li_image(imagepath, text)
> +-- .. function:: li_image(imagepath, text)

Arguments doesn't match args below. text_width_percent is missing.

>  --
>  --    Places a paragraph of text to the right of an image
> -
> +--
>  --    :arg imagepath: the full path to the image file

"to the image file as a string" or 
:type imagepath: :class:`string`

> ---    :arg text_width_percent: the percentatge of space that the text will occupy
> +--    :arg text_width_percent: the percentage of space that the text will occupy

Is the percentage value a string? Needs it a percentage sign?

>  --    :arg text: the text to be placed next to the image
>  --
>  --    :returns: the text wrapped in a paragraph and placed next to the image, the outer tag is a div.
> @@ -452,9 +496,92 @@
>  --    :arg sentence1: text of the first sentence
>  --    :arg sentence2: text of the second sentence
>  --    :returns: two concatenated sentences with a localized sentence joiner.
> ---
> +
>  function join_sentences(sentence1, sentence2)
>     -- TRANSLATORS: Put 2 sentences one after the other.
>     -- TRANSLATORS: Languages using Chinese script probably want to lose the blank space here.
>     return pgettext("sentence_separator", "%s %s"):bformat(sentence1, sentence2)
>  end
> +
> +
> +-- RST
> +-- .. function:: localize_list(items, listtype, former_textdomain)
> +--
> +--    Turns an array of string items into a localized string list with
> +--    appropriate concatenation.
> +--
> +--    e.g. localize_list({"foo", "bar", baz"}, "or", "widelands") will return
> +--    _"foo, bar or baz"

, "baz"

> +--
> +--    :arg items:              An array of strings
> +--    :arg listtype:           The type of concatenation to use.
> +--                             Legal values are "&", "and", "or", and ";"
> +--    :arg former_textdomain:  The textdomain to restore after running this function.
> +--    :returns: The concatenated list string, using localized concatenation operators.
> +--
> +-- Same algorithm as in src/base/i18n
> +
> +function localize_list(items, listtype, former_textdomain)
> +   set_textdomain("widelands")
> +   local result = ""
> +   for i, item in pairs(items) do
> +      if (i == 1) then
> +         result = item
> +      elseif (i == #items) then
> +         if (listtype == "&") then
> +            result = _"%1$s & %2$s":bformat(result, item)
> +         elseif (listtype == "or") then
> +            result = _"%1$s or %2$s":bformat(result, item)
> +         elseif (listtype == ",") then
> +            result = _"%1$s, %2$s":bformat(result, item)
> +         else
> +            result = _"%1$s and %2$s":bformat(result, item)
> +      end
> +      else
> +         result = _"%1$s, %2$s":bformat(result, item)
> +      end
> +   end
> +   set_textdomain(former_textdomain)
> +   return result
> +end
> +
> +
> +-- RST
> +-- :ref:`Return to index<richtext.lua>`
> +--
> +-- :ref: .. _lua_formatting_example:
> +--
> +-- Code Example
> +-- ^^^^^^^^^^^^
> +-- Here's an example on how these functions and their attributes can be used. The double dot
> +-- (``..``) is the LUA string concatenation operator. Note that this example
> +-- also includes translation markup (the ``_[[Some text]]`` or ``_"Some text"`` function):
> +--
> +-- .. code-block:: lua
> +--
> +--    include "scripting/richtext.lua"
> +--
> +--    title = "Text Formatting",
> +--    body = h1(_[[Normal header]]) ..
> +--           h1("004aff", _[[Colored header]]) ..
> +--           p(_[[Normal paragraph, just with a bit more text to show how it looks like.]]) ..
> +--           p("align=center", _[[A centered paragraph]]) ..
> +--           li_image("images/wui/menus/menu_toggle_menu.png", _[[An image with right aligned text. This is just text to show automatic line breaks and behavior in regard with images]]) ..
> +--           li(_[[A list item]]) ..
> +--           li(font("color=004aff bold=1", _[[Blue and bold]])) ..
> +--           li_arrow(_[[A list item with an arrow]]) ..
> +--           p(_[[A more complicated paragraph with ]] ..
> +--              font("color=ffffff", _[[white text ]]) ..
> +--              _[[and ]] ..
> +--              font("italic=1 bold=1", _[[bold italic formatted text.]])
> +--           ),
> +--
> +--
> +-- This results in the following for a campaign message box:
> +--
> +-- .. image:: images/wlrichtext.png
> +--    :scale: 100
> +--    :alt: sample rendering
> +--    :align: center
> +--
> +-- :ref:`Return to index<richtext.lua>`
> 
> === modified file 'src/graphic/text/rt_parse.cc'
> --- src/graphic/text/rt_parse.cc	2017-12-09 06:29:26 +0000
> +++ src/graphic/text/rt_parse.cc	2018-02-24 11:18:33 +0000
> @@ -194,30 +247,92 @@
>  		tc.has_closing_tag = true;
>  		tag_constraints_["rt"] = tc;
>  	}
> -	{  // br tag
> +	{
> +		/* RST
> +.. _rt_tags_div:
> +
> +Division -- <div>
> +-----------------
> +
> +This tag defines a rectangle and an be used as a layout control.

and can be used

> +
> +Attributes
> +^^^^^^^^^^
> +
> +The same attributes as :ref:`rt_tags_rt`, plus the following:
> +
> +* **margin**: Shrink all contents to leave a margin towards the outer edge of this tag's rectangle
> +* **float**: To be implemented. Allowed values are ``left``,  ``right``
> +* **valign**: Align the contents vertically. Allowed values are ``top`` (default), ``center`` = ``middle``, ``bottom``.

``center`` or ``middle`` ?
There should only be be one valid value for this, imho.

> +* **width**: The width of this element, as a pixel amount, or as a percentage.
> +  The last ``div`` in a row can be expanded automatically by using ``*``.
> +
> +Sub-tags
> +^^^^^^^^
> +
> +* :ref:`rt_tags_div`
> +* :ref:`rt_tags_font`
> +* :ref:`rt_tags_p`
> +* :ref:`rt_tags_vspace`
> +
> +:ref:`Return to tag index<rt_tags>`
> +		*/
> +		TagConstraint tc;
> +		tc.allowed_attrs.insert("padding");
> +		tc.allowed_attrs.insert("padding_r");
> +		tc.allowed_attrs.insert("padding_l");
> +		tc.allowed_attrs.insert("padding_b");
> +		tc.allowed_attrs.insert("padding_t");
> +		tc.allowed_attrs.insert("margin");
> +		tc.allowed_attrs.insert("float");
> +		tc.allowed_attrs.insert("valign");
> +		tc.allowed_attrs.insert("background");
> +		tc.allowed_attrs.insert("width");
> +
> +		tc.allowed_children.insert("p");
> +		tc.allowed_children.insert("vspace");
> +		tc.allowed_children.insert("font");
> +		tc.allowed_children.insert("div");
> +
> +		tc.text_allowed = false;
> +		tc.has_closing_tag = true;
> +		tag_constraints_["div"] = tc;
> +	}
> +	{
> +		/* RST
> +.. _rt_tags_br:
> +
> +Line Break -- <br>
> +------------------
> +
> +A single line break. Use sparingly for things like poetry stanzas.
> +If you are starting a new paragraph, use :ref:`rt_tags_p` instead.
> +
> +:ref:`Return to tag index<rt_tags>`
> +		*/
>  		TagConstraint tc;
>  		tc.text_allowed = false;
>  		tc.has_closing_tag = false;
>  		tag_constraints_["br"] = tc;
>  	}
> -	{  // img tag
> -		TagConstraint tc;
> -		tc.allowed_attrs.insert("src");
> -		tc.allowed_attrs.insert("ref");
> -		tc.allowed_attrs.insert("color");
> -		tc.allowed_attrs.insert("width");
> -		tc.text_allowed = false;
> -		tc.has_closing_tag = false;
> -		tag_constraints_["img"] = tc;
> -	}
> -	{  // vspace tag
> -		TagConstraint tc;
> -		tc.allowed_attrs.insert("gap");
> -		tc.text_allowed = false;
> -		tc.has_closing_tag = false;
> -		tag_constraints_["vspace"] = tc;
> -	}
> -	{  // space tag
> +	{
> +		/* RST
> +.. _rt_tags_space:
> +
> +Horizontal Space -- <space>
> +---------------------------
> +
> +Inserts a horizontal gap between the previous and the following text.
> +This space can be filled with a character of your choice.
> +
> +Attributes
> +^^^^^^^^^^
> +
> +* **gap**: The size of the gap as a pixel amount
> +* **fill**: A character to fill the gap with
> +
> +:ref:`Return to tag index<rt_tags>`
> +		*/
>  		TagConstraint tc;
>  		tc.allowed_attrs.insert("gap");
>  		tc.allowed_attrs.insert("fill");
> @@ -225,29 +340,58 @@
>  		tc.has_closing_tag = false;
>  		tag_constraints_["space"] = tc;
>  	}
> -	{  // div tag
> +	{
> +		/* RST
> +.. _rt_tags_vspace:
> +
> +Vertical Space -- <vspace>
> +--------------------------
> +
> +Inserts a vertical gap between the previous and the following text.
> +
> +Attributes
> +^^^^^^^^^^
> +
> +* **gap**: The size of the gap as a pixel amount
> +
> +:ref:`Return to tag index<rt_tags>`
> +		*/
>  		TagConstraint tc;
> -		tc.allowed_attrs.insert("padding");
> -		tc.allowed_attrs.insert("padding_r");
> -		tc.allowed_attrs.insert("padding_l");
> -		tc.allowed_attrs.insert("padding_b");
> -		tc.allowed_attrs.insert("padding_t");
> -		tc.allowed_attrs.insert("margin");
> -		tc.allowed_attrs.insert("float");
> -		tc.allowed_attrs.insert("valign");
> -		tc.allowed_attrs.insert("background");
> -		tc.allowed_attrs.insert("width");
> -
> -		tc.allowed_children.insert("p");
> -		tc.allowed_children.insert("vspace");
> -		tc.allowed_children.insert("font");
> -		tc.allowed_children.insert("div");
> -
> +		tc.allowed_attrs.insert("gap");
>  		tc.text_allowed = false;
> -		tc.has_closing_tag = true;
> -		tag_constraints_["div"] = tc;
> +		tc.has_closing_tag = false;
> +		tag_constraints_["vspace"] = tc;
>  	}
> -	{  // p tag
> +	{
> +		/* RST
> +.. _rt_tags_p:
> +
> +Paragraph -- <p>
> +----------------
> +
> +This tag encloses a text paragraph.
> +
> +Attributes
> +^^^^^^^^^^
> +
> +* **indent**: Adds an indent to the first line of the paragraph
> +* **align**: The horizontal alignment for the paragraph's text.
> +  Allowed values are ``left`` (default), ``center`` = ``middle``, ``right``.

center or middle ?

> +* **valign**: See :ref:`rt_tags_div`
> +* **spacing**: Vertical line spacing as a pixel value
> +
> +Sub-tags
> +^^^^^^^^
> +
> +* :ref:`rt_tags_br`
> +* :ref:`rt_tags_div`
> +* :ref:`rt_tags_font`
> +* :ref:`rt_tags_img`
> +* :ref:`rt_tags_space`
> +* :ref:`rt_tags_vspace`
> +
> +:ref:`Return to tag index<rt_tags>`
> +		*/
>  		TagConstraint tc;
>  		tc.allowed_attrs.insert("indent");
>  		tc.allowed_attrs.insert("align");


-- 
https://code.launchpad.net/~widelands-dev/widelands/fh1-documentation/+merge/337545
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/fh1-documentation.