From 704dff9b08bc7abff42dd433012b263123ab5cfc Mon Sep 17 00:00:00 2001 From: Carson Date: Wed, 2 Feb 2022 17:02:21 -0600 Subject: [PATCH 1/5] Close #10: render dictionaries in the positional argument of a tag as tag attributes --- htmltools/core.py | 59 +++++++++++++++++++++++++++++++++++++--------- tests/test_tags.py | 14 +++++++++-- 2 files changed, 60 insertions(+), 13 deletions(-) diff --git a/htmltools/core.py b/htmltools/core.py index 84bd007..27c51f9 100644 --- a/htmltools/core.py +++ b/htmltools/core.py @@ -70,6 +70,9 @@ class MetadataNode: TagT = TypeVar("TagT", bound="Tag") +# Types that can be passed in as attributes to tag functions. +TagAttrArg = Union[str, float, bool, None] + # Types of objects that can be a child of a tag. TagChild = Union["Tagifiable", "Tag", MetadataNode, str] @@ -79,13 +82,11 @@ class MetadataNode: "TagList", float, None, + Dict[str, TagAttrArg], # i.e., tag attrbutes (e.g., {"id": "foo"}) List["TagChildArg"], Tuple["TagChildArg", ...], ] -# Types that can be passed in as attributes to tag functions. -TagAttrArg = Union[str, int, float, bool, None] - # Objects with tagify() methods are considered Tagifiable. Note that an object returns a # TagList, the children of the TagList must also be tagified. @@ -245,9 +246,11 @@ def _repr_html_(self) -> str: # TagAttrs class # ============================================================================= class TagAttrs(Dict[str, str]): - def __init__(self, **kwargs: TagAttrArg) -> None: + def __init__( + self, attrs: Mapping[str, TagAttrArg] = {}, **kwargs: TagAttrArg + ) -> None: super().__init__() - self.update(**kwargs) + self.update(attrs, **kwargs) def __setitem__(self, name: str, value: TagAttrArg) -> None: val = self._normalize_attr_value(value) @@ -265,11 +268,32 @@ def update( # type: ignore def _update(self, __m: Mapping[str, TagAttrArg]) -> None: attrs: Dict[str, str] = {} - for key, val in __m.items(): - val_ = self._normalize_attr_value(val) - if val_ is None: + + for k, v in __m.items(): + val = self._normalize_attr_value(v) + if val is None: continue - attrs[self._normalize_attr_name(key)] = val_ + nm = self._normalize_attr_name(k) + + sep = " " + if isinstance(val, HTML): + sep = HTML(sep) + + final_val = val + attr_val = attrs.get(nm) + if attr_val: + final_val = attr_val + sep + final_val + + attrs[nm] = final_val + + for k, v in attrs.items(): + self_v = self.get(k) + if self_v: + sep = " " + if isinstance(v, HTML): + sep = HTML(sep) + attrs[k] = self_v + sep + v + super().update(**attrs) @staticmethod @@ -339,10 +363,23 @@ def __init__( **kwargs: TagAttrArg, ) -> None: self.name: str = _name - self.attrs: TagAttrs = TagAttrs(**kwargs) + self.attrs: TagAttrs = TagAttrs() + + # As a workaround for Python not allowing for numerous keyword + # arguments of the same name, we treat any dictionaries in the + # positional arguments as attributes. + child_args: List[TagChildArg] = [] + for arg in _flatten(args): + if isinstance(arg, dict): + self.attrs.update(arg) + else: + child_args.append(arg) + + self.attrs.update(kwargs) + self.children: TagList = TagList() - self.children.extend(args) + self.children.extend(child_args) if children: self.children.extend(children) diff --git a/tests/test_tags.py b/tests/test_tags.py index ce25760..280987b 100644 --- a/tests/test_tags.py +++ b/tests/test_tags.py @@ -77,6 +77,17 @@ def test_tag_attrs_update(): assert x.attrs == {"a": "1", "b": "2", "c": "C"} +def test_tag_multiple_repeated_attrs(): + x = div({"class": "foo", "class_": "bar"}, class_="baz") + y = div({"class": "foo"}, {"class_": "bar"}, class_="baz") + z = div({"class": "foo"}, {"class": "bar"}, class_="baz") + assert x.attrs == {"class": "foo bar baz"} + assert y.attrs == {"class": "foo bar baz"} + assert z.attrs == {"class": "foo bar baz"} + x.attrs.update({"class": "bap", "class_": "bas"}, class_="bat") + assert x.attrs == {"class": "foo bar baz bap bas bat"} + + def test_tag_shallow_copy(): dep = HTMLDependency( "a", "1.1", source={"package": None, "subdir": "foo"}, script={"src": "a1.js"} @@ -363,9 +374,8 @@ def test_attr_vals(): def test_tag_normalize_attr(): - # Note that x_ maps to x, and it gets replaced by the latter. x = div(class_="class_", x__="x__", x_="x_", x="x") - assert x.attrs == {"class": "class_", "x-": "x__", "x": "x"} + assert x.attrs == {"class": "class_", "x-": "x__", "x": "x_ x"} x = div(foo_bar="baz") assert x.attrs == {"foo-bar": "baz"} From 3b605785dc833e7a7ea3d4d338ed6adbeb67988b Mon Sep 17 00:00:00 2001 From: Carson Date: Thu, 3 Feb 2022 10:43:03 -0600 Subject: [PATCH 2/5] Keep the behavior of the TagAttrs().update method still overwritting existing attributes --- htmltools/core.py | 80 ++++++++++++++++++++++------------------------ tests/test_tags.py | 2 +- 2 files changed, 39 insertions(+), 43 deletions(-) diff --git a/htmltools/core.py b/htmltools/core.py index 27c51f9..0f71b78 100644 --- a/htmltools/core.py +++ b/htmltools/core.py @@ -246,11 +246,9 @@ def _repr_html_(self) -> str: # TagAttrs class # ============================================================================= class TagAttrs(Dict[str, str]): - def __init__( - self, attrs: Mapping[str, TagAttrArg] = {}, **kwargs: TagAttrArg - ) -> None: + def __init__(self, *args: Mapping[str, TagAttrArg], **kwargs: TagAttrArg) -> None: super().__init__() - self.update(attrs, **kwargs) + self.update(*args, **kwargs) def __setitem__(self, name: str, value: TagAttrArg) -> None: val = self._normalize_attr_value(value) @@ -261,40 +259,38 @@ def __setitem__(self, name: str, value: TagAttrArg) -> None: # Note: typing is ignored because the type checker thinks this is an incompatible # override. It's possible that we could find a way to override so that it's happy. def update( # type: ignore - self, __m: Mapping[str, TagAttrArg] = {}, **kwargs: TagAttrArg + self, *args: Mapping[str, TagAttrArg], **kwargs: TagAttrArg ) -> None: - self._update(__m) - self._update(kwargs) - def _update(self, __m: Mapping[str, TagAttrArg]) -> None: - attrs: Dict[str, str] = {} + attrz: Dict[str, Union[str, HTML]] = {} + for arg in args: + for k, v in arg.items(): + val = self._normalize_attr_value(v) + if val is None: + continue + nm = self._normalize_attr_name(k) + + # Preserve the HTML() when combining two HTML() attributes + sep = HTML(" ") + if nm in attrz: + val = attrz[nm] + sep + val - for k, v in __m.items(): + attrz[nm] = val + + for k, v in kwargs.items(): val = self._normalize_attr_value(v) if val is None: continue nm = self._normalize_attr_name(k) - sep = " " - if isinstance(val, HTML): - sep = HTML(sep) - - final_val = val - attr_val = attrs.get(nm) - if attr_val: - final_val = attr_val + sep + final_val + # Preserve the HTML() when combining two HTML() attributes + sep = HTML(" ") + if nm in attrz: + val = attrz[nm] + sep + val - attrs[nm] = final_val + attrz[nm] = val - for k, v in attrs.items(): - self_v = self.get(k) - if self_v: - sep = " " - if isinstance(v, HTML): - sep = HTML(sep) - attrs[k] = self_v + sep + v - - super().update(**attrs) + super().update(attrz) @staticmethod def _normalize_attr_name(x: str) -> str: @@ -363,25 +359,25 @@ def __init__( **kwargs: TagAttrArg, ) -> None: self.name: str = _name - self.attrs: TagAttrs = TagAttrs() # As a workaround for Python not allowing for numerous keyword - # arguments of the same name, we treat any dictionaries in the - # positional arguments as attributes. - child_args: List[TagChildArg] = [] - for arg in _flatten(args): - if isinstance(arg, dict): - self.attrs.update(arg) - else: - child_args.append(arg) + # arguments of the same name, we treat any dictionaries that appear + # within children as attributes (i.e., treat them like kwargs). + arguments = _flatten(args) + if children: + arguments.extend(_flatten(children)) - self.attrs.update(kwargs) + attrs: List[Dict[str, TagAttrArg]] = [] + kids: List[TagChildArg] = [] + for x in arguments: + if isinstance(x, dict): + attrs.append(x) + else: + kids.append(x) - self.children: TagList = TagList() + self.attrs: TagAttrs = TagAttrs(*attrs, **kwargs) - self.children.extend(child_args) - if children: - self.children.extend(children) + self.children: TagList = TagList(*kids) def __call__(self, *args: TagChildArg, **kwargs: TagAttrArg) -> "Tag": self.children.extend(args) diff --git a/tests/test_tags.py b/tests/test_tags.py index 280987b..f8f76e3 100644 --- a/tests/test_tags.py +++ b/tests/test_tags.py @@ -85,7 +85,7 @@ def test_tag_multiple_repeated_attrs(): assert y.attrs == {"class": "foo bar baz"} assert z.attrs == {"class": "foo bar baz"} x.attrs.update({"class": "bap", "class_": "bas"}, class_="bat") - assert x.attrs == {"class": "foo bar baz bap bas bat"} + assert x.attrs == {"class": "bap bas bat"} def test_tag_shallow_copy(): From 6a34413d61edc5e61fb96764ad38c13b4d2b63f5 Mon Sep 17 00:00:00 2001 From: Carson Date: Thu, 3 Feb 2022 10:45:37 -0600 Subject: [PATCH 3/5] Add a test case for the children kwargs holding a dictionary --- tests/test_tags.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_tags.py b/tests/test_tags.py index f8f76e3..e500da5 100644 --- a/tests/test_tags.py +++ b/tests/test_tags.py @@ -81,9 +81,11 @@ def test_tag_multiple_repeated_attrs(): x = div({"class": "foo", "class_": "bar"}, class_="baz") y = div({"class": "foo"}, {"class_": "bar"}, class_="baz") z = div({"class": "foo"}, {"class": "bar"}, class_="baz") + w = div({"class": "foo"}, children=[{"class_": "bar"}], class_="baz") assert x.attrs == {"class": "foo bar baz"} assert y.attrs == {"class": "foo bar baz"} assert z.attrs == {"class": "foo bar baz"} + assert w.attrs == {"class": "foo bar baz"} x.attrs.update({"class": "bap", "class_": "bas"}, class_="bat") assert x.attrs == {"class": "bap bas bat"} From 6231b71855dfe3834c6efe7abac13b5c86ac9fb1 Mon Sep 17 00:00:00 2001 From: Carson Date: Thu, 3 Feb 2022 17:25:03 -0600 Subject: [PATCH 4/5] code feedback --- htmltools/core.py | 28 +++++----------------------- tests/test_tags.py | 2 ++ 2 files changed, 7 insertions(+), 23 deletions(-) diff --git a/htmltools/core.py b/htmltools/core.py index 0f71b78..119566c 100644 --- a/htmltools/core.py +++ b/htmltools/core.py @@ -261,6 +261,8 @@ def __setitem__(self, name: str, value: TagAttrArg) -> None: def update( # type: ignore self, *args: Mapping[str, TagAttrArg], **kwargs: TagAttrArg ) -> None: + if kwargs: + args = args + (kwargs,) attrz: Dict[str, Union[str, HTML]] = {} for arg in args: @@ -271,25 +273,11 @@ def update( # type: ignore nm = self._normalize_attr_name(k) # Preserve the HTML() when combining two HTML() attributes - sep = HTML(" ") if nm in attrz: - val = attrz[nm] + sep + val + val = attrz[nm] + HTML(" ") + val attrz[nm] = val - for k, v in kwargs.items(): - val = self._normalize_attr_value(v) - if val is None: - continue - nm = self._normalize_attr_name(k) - - # Preserve the HTML() when combining two HTML() attributes - sep = HTML(" ") - if nm in attrz: - val = attrz[nm] + sep + val - - attrz[nm] = val - super().update(attrz) @staticmethod @@ -367,16 +355,10 @@ def __init__( if children: arguments.extend(_flatten(children)) - attrs: List[Dict[str, TagAttrArg]] = [] - kids: List[TagChildArg] = [] - for x in arguments: - if isinstance(x, dict): - attrs.append(x) - else: - kids.append(x) - + attrs = [x for x in arguments if isinstance(x, dict)] self.attrs: TagAttrs = TagAttrs(*attrs, **kwargs) + kids = [x for x in arguments if not isinstance(x, dict)] self.children: TagList = TagList(*kids) def __call__(self, *args: TagChildArg, **kwargs: TagAttrArg) -> "Tag": diff --git a/tests/test_tags.py b/tests/test_tags.py index e500da5..e5579dc 100644 --- a/tests/test_tags.py +++ b/tests/test_tags.py @@ -88,6 +88,8 @@ def test_tag_multiple_repeated_attrs(): assert w.attrs == {"class": "foo bar baz"} x.attrs.update({"class": "bap", "class_": "bas"}, class_="bat") assert x.attrs == {"class": "bap bas bat"} + x.attrs.update({"class": HTML("&")}, class_=HTML("<")) + assert x.attrs == {"class": "& <"} def test_tag_shallow_copy(): From eb6ad70653415f5e32b7fa557cd9e838d7ea52a5 Mon Sep 17 00:00:00 2001 From: Carson Date: Thu, 3 Feb 2022 18:17:10 -0600 Subject: [PATCH 5/5] add a test for add non-HTML to HTML --- tests/test_tags.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/test_tags.py b/tests/test_tags.py index e5579dc..535774d 100644 --- a/tests/test_tags.py +++ b/tests/test_tags.py @@ -89,7 +89,12 @@ def test_tag_multiple_repeated_attrs(): x.attrs.update({"class": "bap", "class_": "bas"}, class_="bat") assert x.attrs == {"class": "bap bas bat"} x.attrs.update({"class": HTML("&")}, class_=HTML("<")) - assert x.attrs == {"class": "& <"} + assert str(x) == '
' + x.attrs.update({"class": HTML("&")}, class_="<") + # Combining HTML() with non-HTML() currently forces everything to be escaped, + # but it'd be good to change this behavior if we can manage to change it + # in general https://github.com/rstudio/py-htmltools/issues/15 + assert str(x) == '
' def test_tag_shallow_copy():