Opened 4 years ago

Closed 5 months ago

#19567 closed New feature (fixed)

Make javascript i18n view as CBV and more extensible.

Reported by: niwi Owned by: claudep
Component: Internationalization Version: master
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 (12)

comment:1 Changed 4 years ago by russellm

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to needsinfo
  • Status changed from new to closed

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 Changed 4 years ago by niwi

  • Resolution needsinfo deleted
  • Status changed from closed to new

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 Changed 4 years ago by aaugustin

  • Component changed from Uncategorized to Internationalization
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Uncategorized to New feature

comment:5 Changed 3 years ago by KJ

  • Needs documentation set
  • Needs tests set
  • Patch needs improvement set

I’ve made a comment in the pull request.

comment:6 Changed 2 years ago by alejandro

  • Owner changed from nobody to alejandro
  • Status changed from new to assigned

comment:7 Changed 6 months ago by claudep

  • Needs tests unset
  • Owner changed from alejandro to claudep

I have a revised patch with work in progress.

comment:8 Changed 5 months ago by claudep

  • Needs documentation unset
  • Patch needs improvement unset

comment:9 Changed 5 months ago by timgraham

  • Patch needs improvement set

Left some review comments.

comment:10 Changed 5 months ago by claudep

  • Patch needs improvement unset

comment:11 Changed 5 months ago by timgraham

  • Triage Stage changed from Accepted to Ready for checkin

comment:12 Changed 5 months ago by Claude Paroz <claude@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In de40cfbe:

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

Thanks Cristiano Coelho and Tim Graham for the reviews.

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