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

Models must have a __format__ method #1789

Closed
jace opened this issue Jul 11, 2023 · 5 comments
Closed

Models must have a __format__ method #1789

jace opened this issue Jul 11, 2023 · 5 comments

Comments

@jace
Copy link
Member

jace commented Jul 11, 2023

We have constructs like this in various places: _("Something related to {entity}").format(entity=entity.title)

This is error prone because we may accidentally pass the object instead of the attribute, requiring careful audit. This can be avoided if the model implements __format__ that reflexively returns self.title, or something else as per the format spec.

Bonus: This will also make TemplateVarMixin for SMS templates obsolete, as the template can instead use something like "{project:sms} has an update: {project:shorturl}" and the __format__ method can return a truncated title or short URL based on the format spec.

@jace
Copy link
Member Author

jace commented Jul 11, 2023

Python has an elaborate format spec mini-language, so we shouldn't be adding arbitrary keywords like this. Safer to start with just the standard self.title type return value, deferring to str.__format__ for any other format spec.

@jace
Copy link
Member Author

jace commented Jul 11, 2023

The mini language suggests <align><width> as a valid syntax, and the existing alignment characters are <, >, = and ^. We can add an additional option with . or (Unicode) to indicate truncation to the given width. Syntax:

  1. {var:.30} truncates to 30 using three dots ... as suffix (30 inclusive, so 27 from the var)
  2. {var:…30} truncates to 30 using ellipsis as suffix (30 inclusive, so 29 from the var)

The problem is that SMS template strings will now have that number 30 appearing everywhere, instead of reading it once from self.var_max_length. The __format__ method of the embedded object (say Project or Update) gets no context for where it's being called from, so it can't assume 30 chars.

{var:…{var_max_length}} is an option, but given the verbosity it can only be used in templates shared across transports with only length differences (eg: SMS DLT and WhatsApp). This is unlikely.

@jace
Copy link
Member Author

jace commented Jul 11, 2023

Argument against using format_spec: i18n strings introduce a risk of translator error. We are less likely to have a typo in translating {project} than in {project:.} or {project:…}, so it's better for the code to compensate by calling .format(project=project.title) explicitly.

@jace
Copy link
Member Author

jace commented Jul 11, 2023

The base classes got default __format__ implementations in hasgeek/coaster#396.

@jace
Copy link
Member Author

jace commented Nov 15, 2023

Custom __format__ methods were added in #1697, #1793, #1812 and #1842.

@jace jace closed this as completed Nov 15, 2023
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

1 participant