Some fixes #81
Some fixes #81
Conversation
sayanarijit
commented
Feb 9, 2024
•
sayanarijit
commented
- html.escape the all attributes
- Match link domain more precisely.
- Image height or width can be individually specified
- urllib.quote_plus the url attributes - Match link domain more precisely. - Image height or width can be individually specified
|
Ref: #82 |
tiptapy/templates/codeBlock.html
Outdated
| @@ -1,6 +1,6 @@ | |||
| {%- set language = node.attrs.language|default("") -%} | |||
| {%- if node.content -%} | |||
| {%- set text = escape(node.content[0].text) -%} | |||
| {%- set text node.content[0].text -%} | |||
There was a problem hiding this comment.
Not idea. But it helps visually.
There was a problem hiding this comment.
i think it is needed, without = it will fail
There was a problem hiding this comment.
Ah I thought i added = will fix.
tests/test_transform.py
Outdated
| return store['json'], store['html'] | ||
| store[data_type][file.split(f".{data_type}")[0]] = data | ||
|
|
||
| ## Use this to (re)generate the html files |
There was a problem hiding this comment.
not a fan of commented code, please see if this is really needed
There was a problem hiding this comment.
It’s handy when the change affects a lot of html files. We can move it to a script or separate function, but this comment is in tests, not in lib. So maybe we can keep it?
There was a problem hiding this comment.
seperate function sounds good
There was a problem hiding this comment.
Added an optional argument rebuilt_html=False, to avoid duplication.
tiptapy/__init__.py
Outdated
| @@ -39,22 +43,37 @@ def _get_abs_template_path(path_str): | |||
| return os.path.join(pkg_dir, path_str) | |||
|
|
|||
|
|
|||
| class BaseDoc: | |||
| def escape_recursive(node): | |||
There was a problem hiding this comment.
naming suggestion-> escape_json
There was a problem hiding this comment.
minor refactor suggestion:
def escape_json(data):
""" Escape strings, or recursively escape lists and dictionaries.
skip escape for dictionary keys listed in skip_keys
"""
skip_keys = ["html"]
...
There was a problem hiding this comment.
On naming: it’s not escaping json. It’s taking dict (block/node) as input and recursively html escaping all the values.
on skip_keys, it’s currently only one attribute we need to skip. Not sure if there will ever be another key to skip. So making a separate list for it seems overkill for now.
There was a problem hiding this comment.
yeah any name works, escape_recursive is also ok, in my mind escape_value or escape_json sounds better..
"html" in if condition is without any context and is confusing, if not list, can be assigned to a variable at the least.
and a comment can be added
There was a problem hiding this comment.
Renamed to escape_values_recursive. Also added skip_key = "html" # Skip escaping html values in embeds.
|
Thanks, LGTM |
