Opened 11 years ago

Closed 8 years ago

Last modified 7 years ago

#19567 closed New feature (fixed)

Make javascript i18n view as CBV and more extensible.

Reported by: Andrei Antoukh Owned by: Claude Paroz
Component: Internationalization Version: dev
Severity: Normal Keywords: i18n javascript
Cc: niwi@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The current view populates a global namespace (see https://code.djangoproject.com/ticket/18231) but also is very monolitic.

My proposal is create a CBV. I don't understand the use of templates for rendering a view (related ticket).
Now, the initial code is available on this file: https://github.com/niwibe/django-jsgettext/blob/master/djsgettext/views.py

If the proposal is accepted, I will create a patch.

Change History (13)

comment:1 by Russell Keith-Magee, 11 years ago

Resolution: needsinfo
Status: newclosed

I'm not sure I see what you're proposing here, or why you're proposing it.

#18231 describes a problem with the javascript catalog; that is an accepted bug that needs to be solved.

I'm not sure I see what moving to a CBV approach gains us. You say the current implementation is 'monolithic' - what problem are you trying to solve that the current implementation prevents?

Before you get to committed to implementing a solution, we need to be clear on what problem you're solving. "It's not a CBV" isn't a problem unless a class-based approach will give flexibility that is actually required, and based on your sample implementation, I'm not sure I see what flexibility we will gain by making the view class-based.

Feel free to reopen with more details on the advantages of going down a class-based approach to the i18n catalog.

comment:2 by Andrei Antoukh, 11 years ago

Resolution: needsinfo
Status: closednew

I think the advantages are obvious but I'll try to explain:

  • Allow users to change certain behaviors without completely rewriting the view
  • Allow expose more than one gettext domain (and not force to use only a "djangojs" domain)

It is very useful to expose different domain that "djangojs". A real case: javascript templates.

The current makemessages command not parse well html files with javascript templates (with domain "djangojs"). A clean solution is to create another command which parses only js templates and place it contents in another po file distinct of "djangojs.po".

The current view is monolithic and if you want to change any behavior, the solution is full rewrite. I think what the framework must give some facility for doing this.

https://github.com/niwibe/django-jsgettext this plugin allows collect messages from htmls what contains javascript templates in other domain distinct from djangojs and currentlly no way to expose these messages to the js.

comment:3 by Aymeric Augustin, 11 years ago

Component: UncategorizedInternationalization
Triage Stage: UnreviewedAccepted
Type: UncategorizedNew feature

comment:5 by Krzysztof Jurewicz, 11 years ago

Needs documentation: set
Needs tests: set
Patch needs improvement: set

I’ve made a comment in the pull request.

comment:6 by alejandro, 10 years ago

Owner: changed from nobody to alejandro
Status: newassigned

comment:7 by Claude Paroz, 8 years ago

Needs tests: unset
Owner: changed from alejandro to Claude Paroz

I have a revised patch with work in progress.

comment:8 by Claude Paroz, 8 years ago

Needs documentation: unset
Patch needs improvement: unset

comment:9 by Tim Graham, 8 years ago

Patch needs improvement: set

Left some review comments.

comment:10 by Claude Paroz, 8 years ago

Patch needs improvement: unset

comment:11 by Tim Graham, 8 years ago

Triage Stage: AcceptedReady for checkin

comment:12 by Claude Paroz <claude@…>, 8 years ago

Resolution: fixed
Status: assignedclosed

In de40cfbe:

Fixed #19567 -- Added JavaScriptCatalog and JSONCatalog class-based views

Thanks Cristiano Coelho and Tim Graham for the reviews.

comment:13 by Tim Graham <timograham@…>, 7 years ago

In 2b20e41:

Refs #19567 -- Removed deprecated javascript_catalog() and json_catalog() views.

Note: See TracTickets for help on using tickets.
Back to Top