class attribute should be more flexible

[sirex]

Thanks for the great library!

I have one feature request. In HTML, class attribute is very often used and in fact, value of class attribute is set of strings. So my suggestion would be to replace klass with classes and make classes a set(). This would allow developers to work with CSS classes in a much improved way.

Actually classes could be a special set() object with added functionality, something like this:

doc.classes.toggle('active')

I have implemented something similar here: https://github.com/python-dirbtuves/akl.lt/blob/master/akllt/common/tests/test_htmlutils.py#L31-L48

[leforestier]

Excellent idea! I just added the feature. You get the add_class, remove_class and toggle_class to work with html classes (example of use at https://github.com/leforestier/yattag/blob/master/tests/tests_simpledoc.py ) Internally, the class attribute remains a string, because I didn't want to slow down the code of people who don't use the feature.

[sirex]

Thanks, added functionality simplifies work with class attribute, but still having set internally would make it more powerful, because you can do a lot of interesting things with sets. And for the performance, I did a test (see code below) and it turned out that it takes 0.000004 seconds to sort and join a set.

And for the toggle functionality, I was talking about well known jQuery functionality with state as a second argument.

Usually you want to toggle a class together with a state, for example:

doc.classes.toggle('active', is_active)

Without state you would have to do:

if is_active:
    doc.classes.add('active')
elif 'active' in doc.classes:
    doc.classes.remove('active')

And here is the code for using sets internally, together with performance tests:

import timeit
import unittest

class ClassValue(set):
    def __str__(self):
        return ' '.join(sorted(self))

    def toggle(self, name, state=None):
        if state is None:
            self ^= set([name])
        elif state:
            self.add(name)
        elif name in self:
            self.remove(name)

class ClassValueTests(unittest.TestCase):
    def test_toggle(self):
        classes = ClassValue('bc')

        classes.toggle('a')
        self.assertEqual(str(classes), 'a b c')

        classes.toggle('a')
        self.assertEqual(str(classes), 'b c')

        classes.toggle('a', True)
        self.assertEqual(str(classes), 'a b c')

        classes.toggle('a', True)
        self.assertEqual(str(classes), 'a b c')

        classes.toggle('a', False)
        self.assertEqual(str(classes), 'b c')

        classes.toggle('a', False)
        self.assertEqual(str(classes), 'b c')

    def test_speed(self):
        classes = ClassValue('abcd')
        n = 1000000
        time = timeit.timeit(lambda: str(classes), number=n)
        print('%f total time.' % (time,))
        print('%d times per second.' % ((1/time)*n))
        print('%f seconds single time.' % (time/n))

        # Output:
        # 3.583808 total time.
        # 279032 times per second.
        # 0.000004 seconds single time.

If you don't mind, I can update you current implementation with this one?

[leforestier]

I think that's too much change introduced for a feature that is only a small shortcut. For now at least, let's just stick with the current representation that, among other things, already works with all the html form features in doc.py.

I think adding and removing a html class pretty much covers up what's needed, but if someone really needs to do something tricky with sets, she can always use a set and use doc.attr(klass = ' '.join(myset)).

As for the toggle_class method, you're totally right, it would make a lot more sense if it had the behavior you describe (the same as jQuery). I would accept a pull request about that.

[sirex]

I think that's too much change introduced for a feature that is only a small shortcut.

I tried to implement it in pull request, and as I expected, implementation was quite simple and backwards compatible.

Here is diffstat output:

tests/tests_simpledoc.py | 35 +++++++++++++++++++++++++++++------
yattag/doc.py            | 20 +++++++-------------
yattag/simpledoc.py      | 86 +++++++++++++++++++++++++++++++-------------------------------------------------------
3 files changed, 67 insertions(+), 74 deletions(-)

if someone really needs to do something tricky with sets, she can always use a set and use doc.attr(klass = ' '.join(myset)).

It is not that simple, because first you would have to get current class value:

myset = set(doc.current_tag.attrs.get('class', '').split())
# do something with myset
doc.attr(klass=''.join(myset))