Re: package.el strings

[Jean-Christophe Helary <jean.christophe.helary@gmail.com>]

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

It took me a lot longer than I thought but here is a diff for your comments. There is a dozen modifications in the file and a few comments.

There is 1 modification that is not related to (future) l10n, it was discussed in a different thread (using format to create https vs http). The rest is about native strings.

I have checked:

* DONE %
* DONE (... :tag "..."
* DONE (cl-defstruct
* DONE (completing-read
* DONE (completing-read-multiple
* DONE (concat -> problems
* DONE (defconst
* DONE (defcustom
* DONE (defface
* DONE (define-error
* DONE (defun
* DONE (defvar
* DONE (easy-menu-define
* DONE (error
* DONE (format
* DONE (format-message
* DONE (insert -> problems
* DONE (interactive -> prompt should be defined separately
* DONE (interactive-only
* DONE (mapconcat
* DONE (message
* DONE (prin1
* DONE (princ
* DONE (read-string
* DONE (setq
* DONE (user-error
* DONE (with-demoted-errors

There are string issues that I could not solve with some concat and some insert and as I wrote in May (describe-package-1) seems to use a lot of magic to create strings. I've fixed the prin1/princ block but there are other places where I'm not sure how to proceed.

Here is the "git diff master" file.

Jean-Christophe 


[-- Attachment #2: l10n_package.diff --]
[-- Type: application/octet-stream, Size: 11149 bytes --]

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index 4245294457..8ef8db22e9 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -195,8 +195,9 @@ package-load-list
   :version "24.1")
 
 (defcustom package-archives `(("gnu" .
-                               ,(format "http%s://elpa.gnu.org/packages/"
-                                        (if (gnutls-available-p) "s" ""))))
+                               ,(let ((https "https://elpa.gnu.org/packages/")
+                                      (http   "http://elpa.gnu.org/packages/"))
+                                  (if (gnutls-available-p) https http))))
   "An alist of archives from which to fetch.
 The default value points to the GNU Emacs package repository.
 
@@ -1007,6 +1008,7 @@ package-buffer-info
   (let ((file-name (match-string-no-properties 1))
         (desc      (match-string-no-properties 2))
         (start     (line-beginning-position)))
+;; The terminating comment could be a generic string that is not in English
     (unless (search-forward (concat ";;; " file-name ".el ends here"))
       (error "Package lacks a terminating comment"))
     ;; Try to include a trailing newline.
@@ -1490,7 +1492,7 @@ package-import-keyring
       (setf (epg-context-home-directory context) package-gnupghome-dir))
     (message "Importing %s..." (file-name-nondirectory file))
     (epg-import-keys-from-file context file)
-    (message "Importing %s...done" (file-name-nondirectory file))))
+    (message "Importing %s... Done" (file-name-nondirectory file))))
 
 (defvar package--post-download-archives-hook nil
   "Hook run after the archive contents are downloaded.
@@ -1524,6 +1526,7 @@ package--download-one-archive
     (let* ((location (cdr archive))
            (name (car archive))
            (content (buffer-string))
+           ;; Shouldn't concat be used instead of format ?
            (dir (expand-file-name (format "archives/%s" name) package-user-dir))
            (local-file (expand-file-name file dir)))
       (when (listp (read-from-string content))
@@ -2054,12 +2057,12 @@ package-install-selected-packages
       (cond
        (available
         (when (y-or-n-p
-               (format "%s packages will be installed:\n%s, proceed?"
+               (format "Number of packages to install: %s (%s), proceed? "
                        (length available)
-                       (mapconcat #'symbol-name available ", ")))
+                       (mapconcat #'symbol-name available " ")))
           (mapc (lambda (p) (package-install p 'dont-select)) available)))
        ((> difference 0)
-        (message "%s packages are not available (the rest already installed), maybe you need to `M-x package-refresh-contents'"
+        (message "Number of packages that are not available: %s (the rest is already installed), maybe you need to `M-x package-refresh-contents'"
                  difference))
        (t
         (message "All your packages are already installed"))))))
@@ -2176,9 +2179,9 @@ package-autoremove
     (let ((removable (package--removable-packages)))
       (if removable
           (when (y-or-n-p
-                 (format "%s packages will be deleted:\n%s, proceed? "
+                 (format "Number of packages to delete: %s (%s), proceed? "
                    (length removable)
-                   (mapconcat #'symbol-name removable ", ")))
+                   (mapconcat #'symbol-name removable " ")))
             (mapc (lambda (p)
                     (package-delete (cadr (assq p package-alist)) t))
                   removable))
@@ -2264,11 +2267,8 @@ describe-package-1
       (setq status "available obsolete"))
     (when incompatible-reason
       (setq status "incompatible"))
-    (prin1 name)
-    (princ " is ")
-    (princ (if (memq (aref status 0) '(?a ?e ?i ?o ?u)) "an " "a "))
-    (princ status)
-    (princ " package.\n\n")
+    (let ((sentence (format "The status of package %S is `%s'.\n\n" name status)))
+    (princ sentence))
 
     (package--print-help-section "Status")
     (cond (built-in
@@ -2649,9 +2649,9 @@ package-menu-toggle-hiding
     (user-error "The current buffer is not a Package Menu"))
   (setq package-menu--hide-packages
         (not package-menu--hide-packages))
-  (message "%s packages" (if package-menu--hide-packages
-                             "Hiding obsolete or unwanted"
-                           "Displaying all"))
+  (if package-menu--hide-packages
+      (message "Hiding obsolete or unwanted packages")
+    (message "Displaying all packages"))
   (revert-buffer nil 'no-confirm))
 
 (defun package--remove-hidden (pkg-list)
@@ -2974,11 +2974,10 @@ package-menu-hide-package
     (let ((hidden
            (cl-remove-if-not (lambda (e) (string-match re (symbol-name (car e))))
                              package-archive-contents)))
-      (message (substitute-command-keys
-                (concat "Hiding %s packages, type `\\[package-menu-toggle-hiding]'"
-                        " to toggle or `\\[customize-variable] RET package-hidden-regexps'"
-                        " to customize it"))
-        (length hidden)))))
+      (message "Number of packages to hide: %s. Type `%s' to toggle or `%s' to customize"
+               (length hidden)
+               (substitute-command-keys "\\[package-menu-toggle-hidding]")
+               (substitute-command-keys "\\[customize-variable] RET package-hidden-regexps")))))
 
 (defun package-menu-describe-package (&optional button)
   "Describe the current package.
@@ -3113,7 +3112,7 @@ package-menu--mark-upgrades-1
   (setq package-menu--mark-upgrades-pending nil)
   (let ((upgrades (package-menu--find-upgrades)))
     (if (null upgrades)
-        (message "No packages to upgrade.")
+        (message "No packages to upgrade")
       (widen)
       (save-excursion
         (goto-char (point-min))
@@ -3126,9 +3125,8 @@ package-menu--mark-upgrades-1
                    (package-menu-mark-install))
                   (t
                    (package-menu-mark-delete))))))
-      (message "%d package%s marked for upgrading."
-        (length upgrades)
-        (if (= (length upgrades) 1) "" "s")))))
+      (message "Number of packages marked for upgrading: %d"
+        (length upgrades)))))
 
 (defun package-menu-mark-upgrades ()
   "Mark all upgradable packages in the Package Menu.
@@ -3146,6 +3144,7 @@ package-menu-mark-upgrades
     (setq package-menu--mark-upgrades-pending t)
     (message "Waiting for refresh to finish...")))
 
+
 (defun package-menu--list-to-prompt (packages)
   "Return a string listing PACKAGES that's usable in a prompt.
 PACKAGES is a list of `package-desc' objects.
@@ -3153,32 +3152,23 @@ package-menu--list-to-prompt
 prompt (see `package-menu--prompt-transaction-p')."
   (cond
    ;; None
-   ((not packages) "")
-   ;; More than 1
-   ((cdr packages)
-    (format "these %d packages (%s)"
+   ((not packages) "0")
+   ;; 1 and more
+   ((car packages)
+    (format "%d (%s)"
       (length packages)
-      (mapconcat #'package-desc-full-name packages ", ")))
-   ;; Exactly 1
-   (t (format-message "package `%s'"
-                      (package-desc-full-name (car packages))))))
+      (mapconcat #'package-desc-full-name packages " ")))))
 
 (defun package-menu--prompt-transaction-p (delete install upgrade)
   "Prompt the user about DELETE, INSTALL, and UPGRADE.
 DELETE, INSTALL, and UPGRADE are lists of `package-desc' objects.
 Either may be nil, but not all."
   (y-or-n-p
-   (concat
-    (when delete "Delete ")
-    (package-menu--list-to-prompt delete)
-    (when (and delete install)
-      (if upgrade "; " "; and "))
-    (when install "Install ")
-    (package-menu--list-to-prompt install)
-    (when (and upgrade (or install delete)) "; and ")
-    (when upgrade "Upgrade ")
-    (package-menu--list-to-prompt upgrade)
-    "? ")))
+   (format "Number of packages to delete: %s / install: %s / upgrade: %s, proceed? "
+	   (package-menu--list-to-prompt delete)
+	   (package-menu--list-to-prompt install)
+	   (package-menu--list-to-prompt upgrade))))
+
 
 (defun package-menu--partition-transaction (install delete)
   "Return an alist describing an INSTALL DELETE transaction.
@@ -3261,26 +3251,19 @@ package-menu-execute
     (let-alist (package-menu--partition-transaction install-list delete-list)
       (when (or noquery
                 (package-menu--prompt-transaction-p .delete .install .upgrade))
-        (let ((message-template
-               (concat "Package menu: Operation %s ["
-                       (when .delete  (format "Delet__ %s" (length .delete)))
-                       (when (and .delete .install) "; ")
-                       (when .install (format "Install__ %s" (length .install)))
-                       (when (and .upgrade (or .install .delete)) "; ")
-                       (when .upgrade (format "Upgrad__ %s" (length .upgrade)))
-                       "]")))
-          (message (replace-regexp-in-string "__" "ing" message-template) "started")
+        (progn
+          (message "Operation [Delete %d; Install %d; Upgrade %d] started"
+	           (length .delete) (length .install) (length .upgrade))
           ;; Packages being upgraded are not marked as selected.
           (package--update-selected-packages .install .delete)
           (package-menu--perform-transaction install-list delete-list)
           (when package-selected-packages
             (if-let ((removable (package--removable-packages)))
-                (message "Package menu: Operation finished.  %d packages %s"
-                  (length removable)
-                  (substitute-command-keys
-                   "are no longer needed, type `\\[package-autoremove]' to remove them"))
-              (message (replace-regexp-in-string "__" "ed" message-template)
-                "finished"))))))))
+                (message "Operation finished. Number of packages that are no longer needed: %d. Type `%s' to remove them"
+                         (length removable)
+                         (substitute-command-keys "\\[package-autoremove]"))
+              (message "Operation [Delete %d; Install %d; Upgrade %d] finished"
+	               (length .delete) (length .install) (length .upgrade)))))))))
 
 (defun package-menu--version-predicate (A B)
   (let ((vA (or (aref (cadr A) 1)  '(0)))
@@ -3347,11 +3330,10 @@ package-menu--populate-new-package-list
 (defun package-menu--find-and-notify-upgrades ()
   "Notify the user of upgradable packages."
   (when-let ((upgrades (package-menu--find-upgrades)))
-    (message "%d package%s can be upgraded; type `%s' to mark %s for upgrading."
+    (message "Number of packages that can be upgraded: %d; type `%s' to mark for upgrading."
       (length upgrades)
-      (if (= (length upgrades) 1) "" "s")
-      (substitute-command-keys "\\[package-menu-mark-upgrades]")
-      (if (= (length upgrades) 1) "it" "them"))))
+      (substitute-command-keys "\\[package-menu-mark-upgrades]"))
+    ))
 
 (defun package-menu--post-refresh ()
   "If there's a *Packages* buffer, revert it and check for new packages and upgrades.

[-- Attachment #3: Type: text/plain, Size: 726 bytes --]


> On May 24, 2017, at 14:08, Jean-Christophe Helary <jean.christophe.helary@gmail.com> wrote:
> 
> I've tried to straighten up the strings so that they stand on their own and don't rely of complex concatenations and variable substitutions. That makes for much less "smart" code but the resulting code/strings are more readable.
> 
> There is one big chunk left though (I put 2 TODOs there). It is the (describe-package-1) function. The function itself is about 200 lines long and there are 2 places I'm not sure yet what to do with, one uses prin1/princ and the other uses insert to generate strings. I'll check that part later.
> 
> Let me know what you think.
> 
> Jean-Christophe
> 
> <package.el_0524.diff>