Closed not-my-profile closed 1 year ago
I have an idea how to make this new API even nicer, but before I do, we should resolve #135.
I updated the PR. Whereas previously attributes were either a dictionary or a string or None
, now they are just dictionary or an empty tuple. The elegant part about this is that dict.update(())
doesn't change the dictionary (whereas with None
this would require an extra if
statement everywhere).
And this also addresses your concern that title
should be overridable ... now all attributes can be overridden.
I updated the PR. Whereas previously attributes were either a dictionary or a string or
None
, now they are just dictionary or an empty tuple. The elegant part about this is thatdict.update(())
doesn't change the dictionary (whereas withNone
this would require an extraif
statement everywhere).And this also addresses your concern that
title
should be overridable ... now all attributes can be overridden.
OK, this looks quite clean, yet maybe a bit hacky with the empty tuple trick (unless this is an idiomatic way in Python?). If there is no dramatic performance penalty from this solution, I would get along with it. :)
Anyway, please see my remarks and also if you would like to add some unit tests (e.g. to cover the possibility of attributes overriding), that would be great. Thank you.
Note: Don't hesitate to possibly just create a new commit this time.
Hello, I recently had the need to include html attributes onto a rendered HTML nodes.
I would like to propose an alternative solution that will not alter existing render
methods.
I opened a PR#170 that demonstrates the idea.
Primary difference is how/where html attributes
are stored and accessed.
@pbodnar @not-my-profile
This PR introduces optional
attr
parameters to some methods of theHTMLRenderer
, allowing API users to easily add their own attributes without having to reimplement the logic of the renderer methods. (An example for that is provided in the docstring and I also added two test cases).