Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor HTML rewriter class to make it more open to change and expressive #343

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

benoit74
Copy link
Collaborator

@benoit74 benoit74 commented Jun 28, 2024

Fix #305

Idea: we should be able to define individual rewriting rules with functions and decorators, just like we do when defining endpoints in FastAPI. We have one decorator per kind of modification: drop attribute, rewrite attribute, rewrite data (and maybe others) and we implement one decorated function per kind of modifications expected.

Changes:

  • new HTMLRewritingRules class with decorators drop_attribute (to decide to drop an HTML tag attribute), rewrite_attribute (to rewrite a given HTML tag attribute - name + value), rewrite_tag (to rewrite the whole HTML tag, potentially dropping it completely or changing tag name), rewrite_data (to rewrite HTML tag data)
  • an instance of this class is wired in the html rewriting logic to properly process HTML code
  • new tests are added to unit-test the new class behavior

See #353 for a show case of how this new code structure make it easier to:

  • extend rewriting logic with support for new case
  • test in isolation the extension, without having to deal with whole HTML tree
@benoit74 benoit74 self-assigned this Jun 28, 2024
@benoit74
Copy link
Collaborator Author

@rgaudin this is far from being done, but I would like to have a first feedback on this approach

For now only the simple "drop attribute" case is implemented, but it gives a rough idea of what it will look like if I continue on this approach.

I'm a bit surprise I had to implement the call_func helper function myself, I'm pretty sure it is possible without this custom code and Python stdlib code, but I failed to find how to write it properly. Idea is that I want to pass whatever argument are defined in the decorated function but not other ones.

@benoit74 benoit74 requested a review from rgaudin June 28, 2024 08:49
@benoit74 benoit74 force-pushed the open_html_rewriter branch 2 times, most recently from 33a127e to 14c0c96 Compare July 18, 2024 09:16
@benoit74 benoit74 marked this pull request as ready for review July 18, 2024 11:37
@benoit74
Copy link
Collaborator Author

Refactoring work is now completed, all HTML rewrite operations are isolated in standalone methods.

Create HTMLRewritingRules object supporting various rewrite operations:
- drop attribute
- rewrite attribute
- rewrite whole tag
- rewrite data

Each operation is supported by a decorator which can be used on any
function defining a rewrite operation.

This commit also moves all existing rewriting operations to standalone
functions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
1 participant