Refactor element declarations.

[bulletmark]

1. The void_elements were identically defined in two different files. This PR moves them to 1 file.

2. Better to define the element names in a table and loop over that table to create the elements (exploiting globals()) rather than use explicit code per element.

[bulletmark]

Sorry but can somebody please explain why that pre-commit is failing? It says ruff "Found 1 error" but it does not say what the error is?!

Also, the mypy/pyright errors seem to be something else?

[bulletmark]

Ran pre-commit, then ruff, locally and found error was:

elements.py:23:43: RUF100 [*] Unused `noqa` directive (unused: `E501`)
   |
21 | # the inspector with:
22 | # Array.from($0.querySelectorAll('li')).filter(x=>!x.querySelector('.icon-deprecated')).map(x
23 | # => x.querySelector('code').textContent) # noqa: E501
   |                                           ^^^^^^^^^^^^ RUF100
24 | elements = (
25 |     "a",
   |
   = help: Remove unused `noqa` directive

Found 1 error.

I copied that comment from the original __init__.py so not sure why that was complaining but I removed it anyhow.

Later edit: OK, E501 inhibits the line length check which was not applicable to me new lines because I wrapped them. Have force-pushed to fix this. Would be good if somebody fixed the CI though so users can see the error messages.

[bulletmark]

The mypy and pyright tests are failing because they can not determine VoidElement types in the global namespace now that I am dynamically assigning them. So e.g. this test:

def test_void_element() -> None:
    element = input(name="foo")
    assert_type(element, VoidElement)
    assert isinstance(element, VoidElement)

mypy and pyright fail on the static assert_type check but pytest passes the following runtime assert isinstance() so the code does actually work fine. It appears the type checkers are seeing the global __getattr__(name: str) -> Element which is actually a fall-through at runtime. but they assume input() type is set by that fall-through. Thus I'm thinking those assert_type() would have to be removed from that test (and similar other changes) but that is of course up to the maintainers to decide.

[pelme]

Hey @bulletmark , thanks for the PR!

assert_type is used to ensure that the static types as seen by mypy/pyright aligns with the run-time types. That is why both checks are performed. Static type checkers cannot understand globals() assignment, that is why they just fall back on the __getitem__ definition which returns Element. Other than adding type definitions like area: VoidElement, base: VoidElement, ... back, I do not see any other straightforward way of telling static type checkers about these types. I also find it nice that all of "core" htpy is in a single file since it is still a reasonably small file.

To avoid the duplication of the _void_elements in html2htpy, something like this could be used:

_void_elements = {
    element._name for element in htpy.__dict__.values() if isinstance(element, htpy.VoidElement)
}

I am skeptical of that too though since it makes it harder to read the code for a small win in DRYness. The HTML elements does not change that often that this is a big burden to update the list in two places.

Are there other ways these changes improves the code?

[bulletmark]

@pelme , note the type checkers are not a problem here. Both mypy and pyright run across all source code in this PR without reporting any error. Also, pytest runs across the tests fine as well. The problem is that your test cases assume something for type checking that is no longer valid with this PR so technically I should update the test cases also, but that would be to delete those assert_type() etc. That is why I asked the question here but it seems you want to keep those tests as they are so I am perfectly happy for you to reject this PR.

Seems a shame though that keeping that constraint means you have to duplicate that void_element data structure across two files and that you have all those explicitly coded repetitive lines instead of a simple data table. Type checking is good, but not when it goes so far to compromise code quality. That is just my opinion though.

[pelme]

You PR does make it more DRY and avoids some duplication. I have however put quite some effort into getting htpy to play nicely with types, that is a big reason for its existence, so I do not want to compromise on the types in this case. The difference between VoidElement and Element is that VoidElement does not implement getitem but it is still a difference that I want to keep.

In 2f3ca97, I removed the hard coded void element names, so they are not duplicated anymore and there is no possibility of forgetting to updated them in two places whenever there is a new HTML void element.

Thanks for your PR and desire to improve htpy but I will close this PR for now. 🙏

[bulletmark]

@pelme regarding your statement "I removed the hard coded void element names, so they are not duplicated anymore and there is no possibility of forgetting to updated them in two places whenever there is a new HTML void element" - note that both changes I did here were simply about making the code look better. I don't consider it likely at all that somebody would forget to add a new element in both places. It is just that replicated data structures, and multi-replicated code lines in the other change, are quite ugly. Code is not just about function, but also about style and aesthetics.

That change you did looks much better though and I am completely fine that you chose to close this PR.