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

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

Open
legoktm opened this issue May 21, 2024 · 1 comment
Open

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

legoktm opened this issue May 21, 2024 · 1 comment

Comments

@legoktm
Copy link
Member

legoktm commented May 21, 2024

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 #1582 with the intention that it would be more complex (#1582 (comment)) 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
Copy link
Member

cfm commented May 23, 2024

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. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants