mmontone / djula

Common Lisp port of the Django templating language
http://mmontone.github.io/djula/djula
MIT License
152 stars 21 forks source link

Feedback: possible issue with string-downcase in backend-translate for gettext #85

Closed vindarel closed 1 year ago

vindarel commented 1 year ago

Currently:

(defmethod backend-translate ((backend (eql :gettext)) string language &rest args)
  (apply #'format-translation
         (gettext:gettext* string *gettext-domain* nil (string-downcase (string language)))
         args))

this string-downcase made us spend some more debugging time (all credit to @fstamour).

Shall we remove it?

Or, maybe, there could be a set-locale interface between us and djula:*current-language* that does the right thing (a downcase too?). fstamour came up with these helpers in our i18n.lisp:

(defun list-loaded-locales ()
  "Get the list of locales loaded in gettext's cache."
  (remove-duplicates
   (mapcar #'first
           (alexandria:hash-table-keysfunction
            gettext::*catalog-cache*))
   :test #'string=))

(defun set-locale (locale)
  "Setf gettext:*current-locale* and djula:*current-language* if LOCALE seems valid."
  ;; It is valid to set the locale to nil.
  (when (and locale
             (not (member locale (list-loaded-locales)
                          :test 'string=)))
    (error "Locale not valid or not available: ~s" locale))
  (setf *current-locale* locale
        djula:*current-language* locale))
mmontone commented 1 year ago

Ouch. I'm not sure why string-downcase is there. I'll remove it.

fstamour commented 1 year ago

Thanks!