Re: Emacs i18n

[Paul Eggert <eggert@cs.ucla.edu>]

[-- Attachment #1: Type: text/plain, Size: 3615 bytes --]

On 3/7/19 6:46 AM, Eli Zaretskii wrote:

>> From: Richard Stallman <rms@gnu.org> Cc: eggert@cs.ucla.edu,
>> emacs-devel@gnu.org, juri@linkov.net Date: Wed, 06 Mar 2019 22:42:06
>> -0500 > If we do that, how do we deal with strings that are computed
>> by > concatenation or formatting? Feed them in through %s or
>> something like that.
>>
> But then the strings that are formatted via %s will not be translated,
> they will remain in English.
>
Yes, but the scenario you describe should not occur in a properly
internationalized GNU application. We obviously can't assume that
Emacs's translation subroutine acts like Google Translate and can
translate any English-language string to the user's language. All we can
assume is that the translation subroutine converts one of a fixed set of
English-language strings to a string appropriate for the user's language.

This limitation will cause problems with Elisp code that does extensive
parsing or processing of English syntax (doctor.el, say), and that sort
of Elisp code will remain English-only (unless someone takes the time to
i18nize them specially). However most Elisp code does not parse English
or generate idiomatic English on the fly: instead, it uses a fixed,
stilted style that can routinely be converted to calls like (message
FORMAT ARG1 ARG2 ...) where FORMAT is translated and the ARG values are not.

To get a quick feel for this issue, I did a simple grep for the string
'(message (concat' in the Emacs source code. I found 41 instances of
this string. Of these, 8 were erroneous because the result of the
concatenation could contain an unwanted "%" or '`' that could cause
'message' to go awry, and I fixed them by installing the attached patch
(by the way, it's routine for i18n efforts to find trivial bugs like
this). The other 33 instances could easily be reworded to do proper i18n
when 'message' translates just its first argument and only simple,
xgettext-style static analysis is used to find the message strings. For
example, this code in calc-do-embedded:

  (message (concat
            "Embedded Calc mode enabled; "
            (if calc-embedded-quiet
                "Type `C-x * x'"
              "Give this command again")
            " to return to normal"))

can easily be rewritten to this:

   (if calc-embedded-quiet
       "Embedded Calc mode enabled; Type `C-x * x' to return to normal"
     "Embedded Calc mode enabled; Give this command again to return to
normal"))

which is easier for translators to grok, and is arguably clearer even if
we don't want to translate at all.Obviously my '(message (concat'
exercises only a small part of the problem, but the results of this
little sample are encouraging.

So this problem is solvable. Sure, it'll require substantial work, but
the work is routine and this sort of thing has been done for other packages.

The main argument against doing all this is that it's too much work
overall and nobody will have the time to do it all, so let's not even
bother. I have some sympathy for this argument, as i18n is clearly too
much work for any single contributor and the work will distract us from
other things. On the other hand, there's no pressing need to do all the
work quickly, it's a low-level task that can be farmed out to non-expert
volunteers that could conceivably grow the volunteer population, and if
we never even start the work then it will never get done and Emacs will
remain unfriendly to users who don't grok English.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Be-safer-about-in-message-formats.patch --]
[-- Type: text/x-patch; name="0001-Be-safer-about-in-message-formats.patch", Size: 7751 bytes --]

From f15d0d0247ffe7bc3bbd5fbe10271c93b2e2fb1c Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Thu, 7 Mar 2019 09:02:15 -0800
Subject: [PATCH] Be safer about "%" in message formats
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* lisp/calc/calc-store.el (calc-copy-special-constant):
* lisp/net/rcirc.el (rcirc-handler-PART, rcirc-handler-KICK):
* lisp/org/org-agenda.el (org-agenda):
* lisp/org/org-clock.el (org-clock-out, org-clock-display):
* lisp/org/org.el (org-refile):
* lisp/progmodes/ada-xref.el (ada-goto-declaration):
* lisp/progmodes/idlwave.el (idlwave-scan-library-catalogs):
Don’t trust arbitrary strings to not contain "%" or "`" in
(message (concat STRING1 STRING2 ...)).
---
 lisp/calc/calc-store.el    |  4 ++--
 lisp/net/rcirc.el          |  4 ++--
 lisp/org/org-agenda.el     | 13 ++++++-------
 lisp/org/org-clock.el      | 22 ++++++++++++----------
 lisp/org/org.el            |  3 ++-
 lisp/progmodes/ada-xref.el |  3 +--
 lisp/progmodes/idlwave.el  |  7 +++----
 7 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/lisp/calc/calc-store.el b/lisp/calc/calc-store.el
index 589a776c41..3987c129c2 100644
--- a/lisp/calc/calc-store.el
+++ b/lisp/calc/calc-store.el
@@ -405,8 +405,8 @@ calc-copy-special-constant
                                     sconst))))
        (if var
            (let ((msg (calc-store-value var value "")))
-             (message (concat "Special constant \"%s\" copied to \"%s\"" msg)
-                      sconst (calc-var-name var)))))))))
+	     (message "Special constant \"%s\" copied to \"%s\"%s"
+		      sconst (calc-var-name var) msg))))))))
 
 (defun calc-copy-variable (&optional var1 var2)
   (interactive)
diff --git a/lisp/net/rcirc.el b/lisp/net/rcirc.el
index b1a6c1ce8d..9d53cd4436 100644
--- a/lisp/net/rcirc.el
+++ b/lisp/net/rcirc.el
@@ -2685,7 +2685,7 @@ rcirc-handler-PART-or-KICK
 (defun rcirc-handler-PART (process sender args _text)
   (let* ((channel (car args))
 	 (reason (cadr args))
-	 (message (concat channel " " reason)))
+	 (message "%s %s" channel reason))
     (rcirc-print process sender "PART" channel message)
     ;; print in private chat buffer if it exists
     (when (rcirc-get-buffer (rcirc-buffer-process) sender)
@@ -2697,7 +2697,7 @@ rcirc-handler-KICK
   (let* ((channel (car args))
 	 (nick (cadr args))
 	 (reason (nth 2 args))
-	 (message (concat nick " " channel " " reason)))
+	 (message "%s %s %s" nick channel reason))
     (rcirc-print process sender "KICK" channel message t)
     ;; print in private chat buffer if it exists
     (when (rcirc-get-buffer (rcirc-buffer-process) nick)
diff --git a/lisp/org/org-agenda.el b/lisp/org/org-agenda.el
index e416f5f062..23ee8d71e6 100644
--- a/lisp/org/org-agenda.el
+++ b/lisp/org/org-agenda.el
@@ -2882,13 +2882,12 @@ org-agenda
 	     (let* ((m (org-agenda-get-any-marker))
 		    (note (and m (org-entry-get m "THEFLAGGINGNOTE"))))
 	       (when note
-		 (message (concat
-			   "FLAGGING-NOTE ([?] for more info): "
-			   (org-add-props
-			       (replace-regexp-in-string
-				"\\\\n" "//"
-				(copy-sequence note))
-			       nil 'face 'org-warning)))))))
+		 (message "FLAGGING-NOTE ([?] for more info): %s"
+			  (org-add-props
+			   (replace-regexp-in-string
+			    "\\\\n" "//"
+			    (copy-sequence note))
+			   nil 'face 'org-warning))))))
 	 t t))
        ((equal org-keys "#") (call-interactively 'org-agenda-list-stuck-projects))
        ((equal org-keys "/") (call-interactively 'org-occur-in-agenda-files))
diff --git a/lisp/org/org-clock.el b/lisp/org/org-clock.el
index 34b694d487..62c7cd92d1 100644
--- a/lisp/org/org-clock.el
+++ b/lisp/org/org-clock.el
@@ -1622,9 +1622,10 @@ org-clock-out
 						"\\>"))))
 		  (org-todo org-clock-out-switch-to-state))))))
 	  (force-mode-line-update)
-	  (message (concat "Clock stopped at %s after "
-			   (org-duration-from-minutes (+ (* 60 h) m)) "%s")
-		   te (if remove " => LINE REMOVED" ""))
+	  (message (if remove
+		       "Clock stopped at %s after %s => LINE REMOVED"
+		     "Clock stopped at %s after %s")
+		   te (org-duration-from-minutes (+ (* 60 h) m)))
 	  (run-hooks 'org-clock-out-hook)
 	  (unless (org-clocking-p)
 	    (setq org-clock-current-task nil)))))))
@@ -1925,13 +1926,14 @@ org-clock-display
 		    nil 'local))))
     (let* ((h (/ org-clock-file-total-minutes 60))
 	   (m (- org-clock-file-total-minutes (* 60 h))))
-      (message (concat (format "Total file time%s: "
-			       (cond (todayp " for today")
-				     (customp " (custom)")
-				     (t "")))
-		       (org-duration-from-minutes
-			org-clock-file-total-minutes)
-		       " (%d hours and %d minutes)")
+      (message (cond
+		(todayp
+		 "Total file time for today: %s (%d hours and %d minutes)")
+		(customp
+		 "Total file time (custom): %s (%d hours and %d minutes)")
+		(t
+		 "Total file time: %s (%d hours and %d minutes)"))
+	       (org-duration-from-minutes org-clock-file-total-minutes)
 	       h m))))
 
 (defvar-local org-clock-overlays nil)
diff --git a/lisp/org/org.el b/lisp/org/org.el
index 3a434d12df..e3c78ae90d 100644
--- a/lisp/org/org.el
+++ b/lisp/org/org.el
@@ -11878,7 +11878,8 @@ org-refile
 	    (when (featurep 'org-inlinetask)
 	      (org-inlinetask-remove-END-maybe))
 	    (setq org-markers-to-move nil)
-	    (message (concat actionmsg " to \"%s\" in file %s: done") (car it) file)))))))
+	    (message "%s to \"%s\" in file %s: done" actionmsg
+		     (car it) file)))))))
 
 (defun org-refile-goto-last-stored ()
   "Go to the location where the last refile was stored."
diff --git a/lisp/progmodes/ada-xref.el b/lisp/progmodes/ada-xref.el
index 28c52b0653..c9c923e1d6 100644
--- a/lisp/progmodes/ada-xref.el
+++ b/lisp/progmodes/ada-xref.el
@@ -1133,8 +1133,7 @@ ada-goto-declaration
 	(ada-find-in-ali identlist other-frame)
       ;; File not found: print explicit error message
       (ada-error-file-not-found
-       (message (concat (error-message-string err)
-			(nthcdr 1 err))))
+       (message "%s%s" (error-message-string err) (nthcdr 1 err)))
 
       (error
        (let ((ali-file (ada-get-ali-file-name (ada-file-of identlist))))
diff --git a/lisp/progmodes/idlwave.el b/lisp/progmodes/idlwave.el
index 476d935e8a..25bc788ffc 100644
--- a/lisp/progmodes/idlwave.el
+++ b/lisp/progmodes/idlwave.el
@@ -5588,7 +5588,7 @@ idlwave-scan-library-catalogs
 	     (mapcar 'car idlwave-path-alist)))
 	  (old-libname "")
 	  dir-entry dir catalog all-routines)
-      (if message-base (message message-base))
+      (if message-base (message "%s" message-base))
       (while (setq dir (pop dirs))
 	(catch 'continue
 	  (when (file-readable-p
@@ -5603,8 +5603,7 @@ idlwave-scan-library-catalogs
 		     message-base
 		     (not (string= idlwave-library-catalog-libname
 				   old-libname)))
-		(message "%s" (concat message-base
-				      idlwave-library-catalog-libname))
+		(message "%s%s" message-base idlwave-library-catalog-libname)
 		(setq old-libname idlwave-library-catalog-libname))
 	      (when idlwave-library-catalog-routines
 		(setq all-routines
@@ -5618,7 +5617,7 @@ idlwave-scan-library-catalogs
 		       (setq dir-entry (assoc dir idlwave-path-alist)))
 	      (idlwave-path-alist-add-flag dir-entry 'lib)))))
       (unless no-load (setq idlwave-library-catalog-routines all-routines))
-      (if message-base (message (concat message-base "done"))))))
+      (if message-base (message "%sdone" message-base)))))
 
 ;;----- Communicating with the Shell -------------------
 
-- 
2.20.1