freedomofpress / securedrop-client

a Qt-based GUI for SecureDrop journalists 📰🗞️
GNU Affero General Public License v3.0
39 stars 37 forks source link

Re-evaluate jinja2 dependency in the short-term #2023

Open legoktm opened 1 month ago

legoktm commented 1 month ago

Description

We currently have one jinja2 plain text template (https://github.com/freedomofpress/securedrop-client/blob/43cd2450c8d8722c03b5f3598d4d984217f521e2/client/securedrop_client/conversation/transcript/templates/transcript.txt.jinja), that was added in https://github.com/freedomofpress/securedrop-client/pull/1582 with the intention that it would be more complex (https://github.com/freedomofpress/securedrop-client/pull/1582#discussion_r1061086969) or be a building block for other things - but that hasn't happened yet.

The current template should be rather trivial to reimplement in plain Python, an untested port by ChatGPT 4o gave me ~20 lines:

from gettext import gettext as _

def render_messages(items):
    if not items:
        return _("No messages.")

    output = []
    last_sender = None

    for index, item in enumerate(items):
        if item['type'] == "message":
            if item['sender'] != last_sender:
                output.append(_("{sender} wrote:").format(sender=item['sender']))
                last_sender = item['sender']
            output.append(item['content'])
        elif item['type'] == "file":
            output.append(_("{sender} sent:").format(sender=item['sender']))
            output.append(_("File: {filename}").format(filename=item['filename']))

        if index < len(items) - 1:
            output.append("------")

    return "\n".join(output)

Versus the jinja2 dependency we're currently carrying:

$ tokei Jinja2-3.1.3/src/ MarkupSafe-2.0.1/src/
===============================================================================
 Language            Files        Lines         Code     Comments       Blanks
===============================================================================
 C                       1          339          296            6           37
 Python                 27        14619        11753          569         2297
 Plain Text              8          149            0          146            3
===============================================================================
 Total                  36        15107        12049          721         2337
===============================================================================

My current thinking is that we remove the jinja2 dependency in the short term and re-introduce it whenever we're ready to use more of its functionality.

cfm commented 1 month ago

Summarizing my conversation with @legoktm this morning: I'm reluctant to turn a UI (of a sort) that's currently in declarative form into imperative form, when indeed I'd like to see the rest of the Client's UIs go in the opposite direction (like the Server's). In #1582 our thinking was that since we already use Jinja2 in the Server, we don't add security burden (diff reviews), just operational burden (updates, potential releases), by using it here too.

However, I certainly won't block if push comes to shove and we decide to go this route. :-)