unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* package.el strings
@ 2017-05-24  5:08 Jean-Christophe Helary
  2017-05-24 11:00 ` Jean-Christophe Helary
                   ` (2 more replies)
  0 siblings, 3 replies; 41+ messages in thread
From: Jean-Christophe Helary @ 2017-05-24  5:08 UTC (permalink / raw)
  To: emacs-devel

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

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


[-- Attachment #2: package.el_0524.diff --]
[-- Type: application/octet-stream, Size: 30160 bytes --]

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index c0ecb0447f..c493d95fba 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -468,13 +468,13 @@ This is, approximately, the inverse of `version-to-list'.
           (push (int-to-string num) str-list)
           (push "." str-list))
          ((< num -4)
-          (error "Invalid version list `%s'" vlist))
+          (error "Invalid version list `%s'." vlist))
          (t
           ;; pre, or beta, or alpha
           (cond ((equal "." (car str-list))
                  (pop str-list))
                 ((not (string-match "[0-9]+" (car str-list)))
-                 (error "Invalid version list `%s'" vlist)))
+                 (error "Invalid version list `%s'." vlist)))
           (push (cond ((= num -1) "pre")
                       ((= num -2) "beta")
                       ((= num -3) "alpha")
@@ -494,7 +494,7 @@ This is, approximately, the inverse of `version-to-list'.
     (`single ".el")
     (`tar ".tar")
     (`dir "")
-    (kind (error "Unknown package kind: %s" kind))))
+    (kind (error "Unknown package kind: %s." kind))))
 
 (defun package-desc--keywords (pkg-desc)
   (let ((keywords (cdr (assoc :keywords (package-desc-extras pkg-desc)))))
@@ -581,7 +581,7 @@ loaded and/or activated, customize `package-load-list'.")
         (goto-char (point-min))
         (let ((pkg-desc (or (package-process-define-package
                              (read (current-buffer)))
-                            (error "Can't find define-package in %s" pkg-file))))
+                            (error "Can't find define-package in %s." pkg-file))))
           (setf (package-desc-dir pkg-desc) pkg-dir)
           (if (file-exists-p signed-file)
               (setf (package-desc-signed pkg-desc) t))
@@ -635,7 +635,7 @@ Return the max version (as a string) if the package is held at a lower version."
           ((stringp force)              ; held
            (unless (version-list-= version (version-to-list force))
              force))
-          (t (error "Invalid element in `package-load-list'")))))
+          (t (error "Invalid element in `package-load-list'.")))))
 
 (defun package-built-in-p (package &optional min-version)
   "Return true if PACKAGE is built-in to Emacs.
@@ -664,7 +664,7 @@ PKG-DESC is a `package-desc' object."
   (let* ((old-lp load-path)
          (pkg-dir (package-desc-dir pkg-desc))
          (pkg-dir-dir (file-name-as-directory pkg-dir)))
-    (with-demoted-errors "Error loading autoloads: %s"
+    (with-demoted-errors "Error loading autoloads: %s."
       (load (package--autoloads-file-name pkg-desc) nil t))
     (when (and (eq old-lp load-path)
                (not (or (member pkg-dir load-path)
@@ -706,7 +706,7 @@ correspond to previously loaded files (those returned by
   (let* ((name (package-desc-name pkg-desc))
          (pkg-dir (package-desc-dir pkg-desc)))
     (unless pkg-dir
-      (error "Internal error: unable to find directory for `%s'"
+      (error "Internal error: unable to find directory for `%s'."
              (package-desc-full-name pkg-desc)))
     ;; Activate its dependencies recursively.
     ;; FIXME: This doesn't check whether the activated version is the
@@ -714,7 +714,7 @@ correspond to previously loaded files (those returned by
     (when deps
       (dolist (req (package-desc-reqs pkg-desc))
         (unless (package-activate (car req))
-          (error "Unable to activate package `%s'.\nRequired package `%s-%s' is unavailable"
+          (error "Unable to activate package `%s'.\nRequired package `%s-%s' is unavailable."
                  name (car req) (package-version-join (cadr req))))))
     (package--load-files-for-activation pkg-desc reload)
     ;; Add info node.
@@ -819,7 +819,7 @@ untar into a directory named DIR; otherwise, signal an error."
             ;; directories with a trailing slash (Bug#13136).
             (and (string-equal dir name)
                  (eq (tar-header-link-type tar-data) 5))
-            (error "Package does not untar cleanly into directory %s/" dir)))))
+            (error "Package does not untar cleanly into directory %s/." dir)))))
   (tar-untar-buffer))
 
 (defun package--alist-to-plist-args (alist)
@@ -855,13 +855,13 @@ untar into a directory named DIR; otherwise, signal an error."
        (let ((el-file (expand-file-name (format "%s.el" name) pkg-dir)))
          (make-directory pkg-dir t)
          (package--write-file-no-coding el-file)))
-      (kind (error "Unknown package kind: %S" kind)))
+      (kind (error "Unknown package kind: %S." kind)))
     (package--make-autoloads-and-stuff pkg-desc pkg-dir)
     ;; Update package-alist.
     (let ((new-desc (package-load-descriptor pkg-dir)))
       (unless (equal (package-desc-full-name new-desc)
                      (package-desc-full-name pkg-desc))
-        (error "The retrieved package (`%s') doesn't match what the archive offered (`%s')"
+        (error "The retrieved package (`%s') doesn't match what the archive offered (`%s')."
                (package-desc-full-name new-desc) (package-desc-full-name pkg-desc)))
       ;; Activation has to be done before compilation, so that if we're
       ;; upgrading and macros have changed we load the new definitions
@@ -968,7 +968,7 @@ Signal an error if the entire string was not used."
                      t)
             (end-of-file nil))))
     (if more-left
-        (error "Can't read whole string")
+        (error "Can't read whole string.")
       (car read-data))))
 
 (defun package--prepare-dependencies (deps)
@@ -979,12 +979,12 @@ of \"0\" (meaning any version) and an appropriate level of lists
 is wrapped around any parts requiring it."
   (cond
    ((not (listp deps))
-    (error "Invalid requirement specifier: %S" deps))
+    (error "Invalid requirement specifier: %S." deps))
    (t (mapcar (lambda (dep)
                 (cond
                  ((symbolp dep) `(,dep "0"))
                  ((stringp dep)
-                  (error "Invalid requirement specifier: %S" dep))
+                  (error "Invalid requirement specifier: %S." dep))
                  ((and (listp dep) (null (cdr dep)))
                   (list (car dep) "0"))
                  (t dep)))
@@ -1003,12 +1003,12 @@ error.  If there is a package, narrow the buffer to the file's
 boundaries."
   (goto-char (point-min))
   (unless (re-search-forward "^;;; \\([^ ]*\\)\\.el ---[ \t]*\\(.*?\\)[ \t]*\\(-\\*-.*-\\*-[ \t]*\\)?$" nil t)
-    (error "Package lacks a file header"))
+    (error "Package lacks a file header."))
   (let ((file-name (match-string-no-properties 1))
         (desc      (match-string-no-properties 2))
         (start     (line-beginning-position)))
     (unless (search-forward (concat ";;; " file-name ".el ends here"))
-      (error "Package lacks a terminating comment"))
+      (error "Package lacks a terminating comment."))
     ;; Try to include a trailing newline.
     (forward-line)
     (narrow-to-region start (point))
@@ -1023,7 +1023,7 @@ boundaries."
            (homepage (lm-homepage)))
       (unless pkg-version
         (error
-            "Package lacks a \"Version\" or \"Package-Version\" header"))
+            "Package lacks a \"Version\" or \"Package-Version\" header."))
       (package-desc-from-define
        file-name pkg-version desc
        (if requires-str
@@ -1060,11 +1060,11 @@ The return result is a `package-desc'."
          (desc-file (package--description-file dir-name))
          (tar-desc (tar-get-file-descriptor (concat dir-name desc-file))))
     (unless tar-desc
-      (error "No package descriptor file found"))
+      (error "No package descriptor file found."))
     (with-current-buffer (tar--extract tar-desc)
       (unwind-protect
           (or (package--read-pkg-desc 'tar)
-              (error "Can't find define-package in %s"
+              (error "Can't find define-package in %s."
                 (tar-header-name tar-desc)))
         (kill-buffer (current-buffer))))))
 
@@ -1089,7 +1089,7 @@ The return result is a `package-desc'."
               ;; set the 'dir kind,
               (setf (package-desc-kind info) 'dir))))
         (unless info
-          (error "No .el files with package headers in `%s'" default-directory))
+          (error "No .el files with package headers in `%s'." default-directory))
         ;; and return the info.
         info))))
 
@@ -1149,7 +1149,7 @@ buffer is killed afterwards.  Return the last value in BODY."
      (if (string-match-p "\\`https?:" ,location)
          (url-insert-file-contents (concat ,location ,file))
        (unless (file-name-absolute-p ,location)
-         (error "Archive location %s is not an absolute file name"
+         (error "Archive location %s is not an absolute file name."
            ,location))
        (insert-file-contents (expand-file-name ,file ,location)))
      ,@body))
@@ -1191,11 +1191,11 @@ errors signaled by ERROR-FORM or by BODY).
                                                    (require 'url-handlers)
                                                    (unless-error ,body
                                                                  (when-let ((er (plist-get status :error)))
-                                                                   (error "Error retrieving: %s %S" ,url-sym er))
+                                                                   (error "Error retrieving: %s %S." ,url-sym er))
                                                                  (with-current-buffer ,b-sym
                                                                    (goto-char (point-min))
                                                                    (unless (search-forward-regexp "^\r?\n\r?" nil 'noerror)
-                                                                     (error "Error retrieving: %s %S" ,url-sym "incomprehensible buffer")))
+                                                                     (error "Error retrieving: %s %S." ,url-sym "incomprehensible buffer")))
                                                                  (url-insert-buffer-contents ,b-sym ,url-sym)
                                                                  (kill-buffer ,b-sym)
                                                                  (goto-char (point-min)))))
@@ -1205,7 +1205,7 @@ errors signaled by ERROR-FORM or by BODY).
            (unless-error ,body
                          (let ((url (expand-file-name ,file ,url-1)))
                            (unless (file-name-absolute-p url)
-                             (error "Location %s is not a url nor an absolute file name" url))
+                             (error "Location %s is not a url nor an absolute file name." url))
                            (insert-file-contents url))))))))
 
 (define-error 'bad-signature "Failed to verify signature")
@@ -1268,7 +1268,7 @@ else, even if an error is signaled."
                       (funcall callback nil))
                     (when unwind (funcall unwind))
                     (unless allow-unsigned
-                      (error "Unsigned file `%s' at %s" file location)))
+                      (error "Unsigned file `%s' at %s." file location)))
       ;; OTOH, an error here means "bad signature", which we never
       ;; suppress.  (Bug#22089)
       (unwind-protect
@@ -1397,7 +1397,7 @@ Will throw an error if the archive version is too new."
           (insert-file-contents filename))
         (let ((contents (read (current-buffer))))
           (if (> (car contents) package-archive-version)
-              (error "Package archive version %d is higher than %d"
+              (error "Package archive version %d is higher than %d."
                 (car contents) package-archive-version))
           (cdr contents))))))
 
@@ -1510,7 +1510,7 @@ Once it's empty, run `package--post-download-archives-hook'."
     (package--build-compatibility-table)
     ;; We message before running the hook, so the hook can give
     ;; messages as well.
-    (message "Package refresh done")
+    (message "Package refresh done.")
     (run-hooks 'package--post-download-archives-hook)))
 
 (defun package--download-one-archive (archive file &optional async)
@@ -1578,7 +1578,7 @@ downloads in the background."
     (when (and package-check-signature (file-exists-p default-keyring))
       (condition-case-unless-debug error
           (package-import-keyring default-keyring)
-        (error (message "Cannot import default keyring: %S" (cdr error))))))
+        (error (message "Cannot import default keyring: %S." (cdr error))))))
   (package--download-and-read-archives async))
 
 \f
@@ -1621,11 +1621,11 @@ SEEN is used internally to detect infinite recursion."
             ;; we re-add it (along with its dependencies) at an earlier place
             ;; below (bug#16994).
             (if (memq already seen)     ;Avoid inf-loop on dependency cycles.
-                (message "Dependency cycle going through %S"
+                (message "Dependency cycle going through %S."
                          (package-desc-full-name already))
               (setq packages (delq already packages))
               (setq already nil))
-          (error "Need package `%s-%s', but only %s is being installed"
+          (error "Need package `%s-%s', but only %s is being installed."
                  next-pkg (package-version-join next-version)
                  (package-version-join (package-desc-version already)))))
       (cond
@@ -1654,20 +1654,20 @@ SEEN is used internally to detect infinite recursion."
                   (setq problem
                         (if (stringp disabled)
                             (format-message
-                             "Package `%s' held at version %s, but version %s required"
+                             "Package `%s' held at version %s, but version %s required."
                              next-pkg disabled
                              (package-version-join next-version))
-                          (format-message "Required package `%s' is disabled"
+                          (format-message "Required package `%s' is disabled."
                                           next-pkg)))))
                (t (setq found pkg-desc)))))
           (unless found
             (cond
              (problem (error "%s" problem))
              (found-something
-              (error "Need package `%s-%s', but only %s is available"
+              (error "Need package `%s-%s', but only %s is available."
                      next-pkg (package-version-join next-version)
                      found-something))
-             (t (error "Package `%s-%s' is unavailable"
+             (t (error "Package `%s-%s' is unavailable."
                        next-pkg (package-version-join next-version)))))
           (setq packages
                 (package-compute-transaction (cons found packages)
@@ -1806,7 +1806,7 @@ if all the in-between dependencies are also in PACKAGE-LIST."
   "Download and install a tar package."
   ;; This won't happen, unless the archive is doing something wrong.
   (when (eq (package-desc-kind pkg-desc) 'dir)
-    (error "Can't install directory package from archive"))
+    (error "Can't install directory package from archive."))
   (let* ((location (package-archive-base pkg-desc))
          (file (concat (package-desc-full-name pkg-desc)
                        (package-desc-suffix pkg-desc))))
@@ -1971,7 +1971,7 @@ to install it but still mark it as selected."
                                                  (package-desc-reqs pkg)))
                 (package-compute-transaction () (list (list pkg))))))
         (package-download-transaction transaction)
-      (message "`%s' is already installed" name))))
+      (message "`%s' is already installed." name))))
 
 (defun package-strip-rcs-id (str)
   "Strip RCS version ID from the version string STR.
@@ -2047,22 +2047,22 @@ If some packages are not installed propose to install them."
   ;; using here, because the outcome is the same either way (nothing
   ;; gets installed).
   (if (not package-selected-packages)
-      (message "`package-selected-packages' is empty, nothing to install")
+      (message "`package-selected-packages' is empty, nothing to install.")
     (let* ((not-installed (seq-remove #'package-installed-p package-selected-packages))
            (available (seq-filter (lambda (p) (assq p package-archive-contents)) not-installed))
            (difference (- (length not-installed) (length available))))
       (cond
        (available
         (when (y-or-n-p
-               (format "%s packages will be installed:\n%s, proceed?"
+               (format "Number of packages that will be installed: %s.\n%s, proceed?"
                        (length 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. Consider `M-x package-refresh-contents'."
                  difference))
        (t
-        (message "All your packages are already installed"))))))
+        (message "All your packages are already installed."))))))
 
 \f
 ;;; Package Deletion
@@ -2116,13 +2116,13 @@ If NOSAVE is non-nil, the package is not removed from
                                   (expand-file-name package-user-dir))
                                  (expand-file-name dir)))
            ;; Don't delete "system" packages.
-           (error "Package `%s' is a system package, not deleting"
+           (error "Package `%s' is a system package, not deleting."
                   (package-desc-full-name pkg-desc)))
           ((and (null force)
                 (setq pkg-used-elsewhere-by
                       (package--used-elsewhere-p pkg-desc)))
            ;; Don't delete packages used as dependency elsewhere.
-           (error "Package `%s' is used by `%s' as dependency, not deleting"
+           (error "Package `%s' is used by `%s' as dependency, not deleting."
                   (package-desc-full-name pkg-desc)
                   (package-desc-name pkg-used-elsewhere-by)))
           (t
@@ -2171,13 +2171,13 @@ will be deleted."
     (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 that will be deleted: %s.\n%s, proceed? "
                    (length removable)
                    (mapconcat #'symbol-name removable ", ")))
             (mapc (lambda (p)
                     (package-delete (cadr (assq p package-alist)) t))
                   removable))
-        (message "Nothing to autoremove")))))
+        (message "Nothing to autoremove.")))))
 
 \f
 ;;;; Package description buffer.
@@ -2207,7 +2207,7 @@ will be deleted."
                                                         (symbol-name guess)))))
          (list (intern val))))))
   (if (not (or (package-desc-p package) (and package (symbolp package))))
-      (message "No package specified")
+      (message "No packages specified.")
     (help-setup-xref (list #'describe-package package)
                      (called-interactively-p 'interactive))
     (with-help-window (help-buffer)
@@ -2232,6 +2232,7 @@ Otherwise no newline is inserted."
 
 (declare-function lm-commentary "lisp-mnt" (&optional file))
 
+;; TODO -> prin1 / princ
 (defun describe-package-1 (pkg)
   (require 'lisp-mnt)
   (let* ((desc (or
@@ -2265,6 +2266,7 @@ Otherwise no newline is inserted."
     (princ status)
     (princ " package.\n\n")
 
+    ;; TODO -> insert
     (package--print-help-section "Status")
     (cond (built-in
            (insert (propertize (capitalize status)
@@ -2641,12 +2643,12 @@ Installed obsolete packages are always displayed.")
   "Toggle visibility of obsolete available packages."
   (interactive)
   (unless (derived-mode-p 'package-menu-mode)
-    (user-error "The current buffer is not a Package Menu"))
+    (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)
@@ -2947,7 +2949,7 @@ This fetches the contents of each archive specified in
 `package-archives', and then refreshes the package menu."
   (interactive)
   (unless (derived-mode-p 'package-menu-mode)
-    (user-error "The current buffer is not a Package Menu"))
+    (user-error "The current buffer is not a Package Menu."))
   (setq package-menu--old-archive-contents package-archive-contents)
   (setq package-menu--new-package-list nil)
   (package-refresh-contents package-menu-async))
@@ -2969,11 +2971,10 @@ If optional arg BUTTON is non-nil, describe its associated 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 it."
+               (length hidden)
+               (substitute-command-key "`\\[package-menu-toggle-hiding]'")
+               (substitute-command-key "`\\[customize-variable] RET package-hidden-regexps'" )))))
 
 (defun package-menu-describe-package (&optional button)
   "Describe the current package.
@@ -2983,7 +2984,7 @@ If optional arg BUTTON is non-nil, describe its associated package."
                     (tabulated-list-get-id))))
     (if pkg-desc
         (describe-package pkg-desc)
-      (user-error "No package here"))))
+      (user-error "No package here."))))
 
 ;; fixme numeric argument
 (defun package-menu-mark-delete (&optional _num)
@@ -3104,7 +3105,7 @@ consideration."
   "Mark all upgradable packages in the Package Menu.
 Implementation of `package-menu-mark-upgrades'."
   (unless (derived-mode-p 'package-menu-mode)
-    (error "The current buffer is not a Package Menu"))
+    (error "The current buffer is not a Package Menu."))
   (setq package-menu--mark-upgrades-pending nil)
   (let ((upgrades (package-menu--find-upgrades)))
     (if (null upgrades)
@@ -3121,9 +3122,8 @@ Implementation of `package-menu-mark-upgrades'."
                    (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.
@@ -3141,22 +3141,6 @@ immediately."
     (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.
-Formats the returned string to be usable in a minibuffer
-prompt (see `package-menu--prompt-transaction-p')."
-  (cond
-   ;; None
-   ((not packages) "")
-   ;; More than 1
-   ((cdr packages)
-    (format "these %d packages (%s)"
-      (length packages)
-      (mapconcat #'package-desc-full-name packages ", ")))
-   ;; Exactly 1
-   (t (format-message "package `%s'"
-                      (package-desc-full-name (car packages))))))
 
 (defun package-menu--prompt-transaction-p (delete install upgrade)
   "Prompt the user about DELETE, INSTALL, and UPGRADE.
@@ -3164,16 +3148,19 @@ 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)
-    "? ")))
+    (when delete
+    (format-message "Number of packages to delete: %d (%s).\n"
+      (length delete)
+      (mapconcat #'package-desc-full-name delete ", ")))
+    (when install
+    (format-message "Number of packages to install: %d (%s).\n"
+      (length install)
+      (mapconcat #'package-desc-full-name install ", ")))
+    (when upgrade
+    (format-message "Number of packages to upgrade: %d (%s).\n"
+      (length upgrade)
+      (mapconcat #'package-desc-full-name upgrade ", ")))
+    "Do you want to proceed? ")))
 
 (defun package-menu--partition-transaction (install delete)
   "Return an alist describing an INSTALL DELETE transaction.
@@ -3211,7 +3198,7 @@ objects removed."
       (condition-case-unless-debug err
           (let ((inhibit-message package-menu-async))
             (package-delete elt nil 'nosave))
-        (error (message "Error trying to delete `%s': %S"
+        (error (message "Error trying to delete `%s': %S."
                  (package-desc-full-name elt)
                  err))))))
 
@@ -3237,7 +3224,7 @@ packages marked for deletion are removed.
 Optional argument NOQUERY non-nil means do not ask the user to confirm."
   (interactive)
   (unless (derived-mode-p 'package-menu-mode)
-    (error "The current buffer is not in Package Menu mode"))
+    (error "The current buffer is not in Package Menu mode."))
   (let (install-list delete-list cmd pkg-desc)
     (save-excursion
       (goto-char (point-min))
@@ -3252,30 +3239,26 @@ Optional argument NOQUERY non-nil means do not ask the user to confirm."
                  (push pkg-desc install-list))))
         (forward-line)))
     (unless (or delete-list install-list)
-      (user-error "No operations specified"))
+      (user-error "No operations specified."))
     (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")
+          (message "Package menu operation started: [deleting %s; installing %s; upgrading %s]. "
+	       (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"
+                (message "Package menu operation completed. Number of packages no longer needed: %d. Type %s for removal."
                   (length removable)
-                  (substitute-command-keys
-                   "are no longer needed, type `\\[package-autoremove]' to remove them"))
-              (message (replace-regexp-in-string "__" "ed" message-template)
-                "finished"))))))))
+                  (substitute-command-keys "`\\[package-autoremove]'"))
+              (message "Package menu operation completed: [deleted %s; installed %s; upgraded %s]. "
+	       (length .delete)
+	       (length .install)
+	       (length .upgrade))))))))
 
 (defun package-menu--version-predicate (A B)
   (let ((vA (or (aref (cadr A) 1)  '(0)))
@@ -3342,11 +3325,9 @@ Store this list in `package-menu--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.

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* Re: package.el strings
  2017-05-24  5:08 package.el strings Jean-Christophe Helary
@ 2017-05-24 11:00 ` Jean-Christophe Helary
  2017-05-24 12:04 ` Dmitry Gutov
  2017-07-14  2:53 ` Jean-Christophe Helary
  2 siblings, 0 replies; 41+ messages in thread
From: Jean-Christophe Helary @ 2017-05-24 11:00 UTC (permalink / raw)
  To: emacs-devel

It looks like I introduced problems in this patch.

Please ignore.

Jean-Christophe 

> 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>




^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: package.el strings
  2017-05-24  5:08 package.el strings Jean-Christophe Helary
  2017-05-24 11:00 ` Jean-Christophe Helary
@ 2017-05-24 12:04 ` Dmitry Gutov
  2017-05-24 12:07   ` Jean-Christophe Helary
                     ` (2 more replies)
  2017-07-14  2:53 ` Jean-Christophe Helary
  2 siblings, 3 replies; 41+ messages in thread
From: Dmitry Gutov @ 2017-05-24 12:04 UTC (permalink / raw)
  To: Jean-Christophe Helary, emacs-devel

On 5/24/17 8:08 AM, Jean-Christophe Helary wrote:
> -          (error "Invalid version list `%s'" vlist))
> +          (error "Invalid version list `%s'." vlist))
> 
> -                 (error "Invalid version list `%s'" vlist)))
> +                 (error "Invalid version list `%s'." vlist)))

More often than not, we avoid ending messages and errors with a period.

Think article headings and commit summaries, rather than sentences.



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: package.el strings
  2017-05-24 12:04 ` Dmitry Gutov
@ 2017-05-24 12:07   ` Jean-Christophe Helary
  2017-05-24 12:36   ` Yuri Khan
  2017-05-24 14:16   ` Richard Stallman
  2 siblings, 0 replies; 41+ messages in thread
From: Jean-Christophe Helary @ 2017-05-24 12:07 UTC (permalink / raw)
  To: emacs-devel

Thank you Dmitri.

I noticed that *after* I sent the patch. I'm going to work again on this and I'll send something better next week.

Sorry for that.

Jean-Christophe 

> On May 24, 2017, at 21:04, Dmitry Gutov <dgutov@yandex.ru> wrote:
> 
> On 5/24/17 8:08 AM, Jean-Christophe Helary wrote:
>> -          (error "Invalid version list `%s'" vlist))
>> +          (error "Invalid version list `%s'." vlist))
>> -                 (error "Invalid version list `%s'" vlist)))
>> +                 (error "Invalid version list `%s'." vlist)))
> 
> More often than not, we avoid ending messages and errors with a period.
> 
> Think article headings and commit summaries, rather than sentences.




^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: package.el strings
  2017-05-24 12:04 ` Dmitry Gutov
  2017-05-24 12:07   ` Jean-Christophe Helary
@ 2017-05-24 12:36   ` Yuri Khan
  2017-05-24 14:16   ` Richard Stallman
  2 siblings, 0 replies; 41+ messages in thread
From: Yuri Khan @ 2017-05-24 12:36 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Jean-Christophe Helary, emacs-devel

On Wed, May 24, 2017 at 7:04 PM, Dmitry Gutov <dgutov@yandex.ru> wrote:
> More often than not, we avoid ending messages and errors with a period.
>
> Think article headings and commit summaries, rather than sentences.

A commit summary *is* an article heading.



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: package.el strings
  2017-05-24 12:04 ` Dmitry Gutov
  2017-05-24 12:07   ` Jean-Christophe Helary
  2017-05-24 12:36   ` Yuri Khan
@ 2017-05-24 14:16   ` Richard Stallman
  2 siblings, 0 replies; 41+ messages in thread
From: Richard Stallman @ 2017-05-24 14:16 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: jean.christophe.helary, emacs-devel

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > More often than not, we avoid ending messages and errors with a period.

It is a convention in Emacs that they don't end with a period.
If you come across any that do end in a period, please delete the
period.

-- 
Dr Richard Stallman
President, Free Software Foundation (gnu.org, fsf.org)
Internet Hall-of-Famer (internethalloffame.org)
Skype: No way! See stallman.org/skype.html.




^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: package.el strings
  2017-05-24  5:08 package.el strings Jean-Christophe Helary
  2017-05-24 11:00 ` Jean-Christophe Helary
  2017-05-24 12:04 ` Dmitry Gutov
@ 2017-07-14  2:53 ` Jean-Christophe Helary
  2017-07-15 12:52   ` Eli Zaretskii
  2 siblings, 1 reply; 41+ messages in thread
From: Jean-Christophe Helary @ 2017-07-14  2:53 UTC (permalink / raw)
  To: emacs-devel

[-- 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>


^ permalink raw reply related	[flat|nested] 41+ messages in thread

* Re: package.el strings
  2017-07-14  2:53 ` Jean-Christophe Helary
@ 2017-07-15 12:52   ` Eli Zaretskii
  2017-07-15 14:48     ` Jean-Christophe Helary
  0 siblings, 1 reply; 41+ messages in thread
From: Eli Zaretskii @ 2017-07-15 12:52 UTC (permalink / raw)
  To: Jean-Christophe Helary; +Cc: emacs-devel

> From: Jean-Christophe Helary <jean.christophe.helary@gmail.com>
> Date: Fri, 14 Jul 2017 11:53:51 +0900
> 
> 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.

Thanks for working in this.  I have a few comments:

> @@ -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))))

Can you tell why this is needed?  The current code is how we say this
in a lot of places, and I don't think I see why it's bad for l10n.

> -               (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 " ")))

You've removed the newline, so the prompt will wrap at some random
place.  Is it really a good idea?

Also, I'd lose the "Number" part, and use %d for format, so it's clear
to translators that a number will follow.

> -        (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'"

Likewise here: I'd say "Packages not available: %d".  Same issue with
some other replacements you propose.

> -                 (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 " ")))

And here.

> -    (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))

Too wordy for my liking.  How about this:

  (princ (format "Package %S is %s.\n\n" name status))

> -   (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))))

This loses the feature of saying just what's needed, instead of
showing zero.  Can we do better?



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: package.el strings
  2017-07-15 12:52   ` Eli Zaretskii
@ 2017-07-15 14:48     ` Jean-Christophe Helary
  2017-07-16 13:55       ` Jean-Christophe Helary
                         ` (2 more replies)
  0 siblings, 3 replies; 41+ messages in thread
From: Jean-Christophe Helary @ 2017-07-15 14:48 UTC (permalink / raw)
  To: emacs-devel


> On Jul 15, 2017, at 21:52, Eli Zaretskii <eliz@gnu.org> wrote:
>> -    (message "Importing %s...done" (file-name-nondirectory file))))
>> +    (message "Importing %s... Done" (file-name-nondirectory file))))
> 
> Can you tell why this is needed?  The current code is how we say this
> in a lot of places, and I don't think I see why it's bad for l10n.

I thought there was a typo with the lack of space between the ... and the "done" and the lack upper case after a ... punctuation mark. If it is not, I'm fine with the original.

>> -               (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 " ")))
> 
> You've removed the newline, so the prompt will wrap at some random
> place.  Is it really a good idea?

It was not a problem as far as I tested it since the package names are separated by a space (depending on the number of packages the original list itself can wrap in unexpected places).

> Also, I'd lose the "Number" part, and use %d for format, so it's clear
> to translators that a number will follow.

Ok. Like:

>> "Packages to install: %d (%s), proceed? "


?

>> -    (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))
> 
> Too wordy for my liking.  How about this:
> 
>  (princ (format "Package %S is %s.\n\n" name status))

Nice :) It took me a while to figure out how to put all the parts together...

>> -   (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))))
> 
> This loses the feature of saying just what's needed, instead of
> showing zero.  Can we do better?

If you ask, there probably is a way... I'll try to find something better.

Jean-Christophe


^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: package.el strings
  2017-07-15 14:48     ` Jean-Christophe Helary
@ 2017-07-16 13:55       ` Jean-Christophe Helary
  2017-07-16 14:16         ` Eli Zaretskii
  2017-07-16 14:35       ` Eli Zaretskii
  2017-07-17 15:28       ` Jean-Christophe Helary
  2 siblings, 1 reply; 41+ messages in thread
From: Jean-Christophe Helary @ 2017-07-16 13:55 UTC (permalink / raw)
  To: emacs-devel

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


> On Jul 15, 2017, at 23:48, Jean-Christophe Helary <jean.christophe.helary@gmail.com> wrote:
> 
>> Also, I'd lose the "Number" part, and use %d for format, so it's clear
>> to translators that a number will follow.
> 
> Ok. Like:
> 
>>> "Packages to install: %d (%s), proceed? "

After replying, I realized that's actually the kind of string that I thought was weird when %d=1:

Packages to install: 1 (...), proceed ?

Here, "Packages" should be "Package", shouldn't it ?

Whereas

Number of packages to install: 1 (...), proceed ?
is always correct.


Jean-Christophe 

[-- Attachment #2: Type: text/html, Size: 3287 bytes --]

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: package.el strings
  2017-07-16 13:55       ` Jean-Christophe Helary
@ 2017-07-16 14:16         ` Eli Zaretskii
  0 siblings, 0 replies; 41+ messages in thread
From: Eli Zaretskii @ 2017-07-16 14:16 UTC (permalink / raw)
  To: Jean-Christophe Helary; +Cc: emacs-devel

> From: Jean-Christophe Helary <jean.christophe.helary@gmail.com>
> Date: Sun, 16 Jul 2017 22:55:40 +0900
> 
>  "Packages to install: %d (%s), proceed? "
> 
> After replying, I realized that's actually the kind of string that I thought was weird when %d=1:
> 
> Packages to install: 1 (...), proceed ?

I see nothing weird with it.

> Here, "Packages" should be "Package", shouldn't it ?

No, not necessarily.

> Whereas
> 
> Number of packages to install: 1 (...), proceed ?
> is always correct.

To my eyes, both are correct.



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: package.el strings
  2017-07-15 14:48     ` Jean-Christophe Helary
  2017-07-16 13:55       ` Jean-Christophe Helary
@ 2017-07-16 14:35       ` Eli Zaretskii
  2017-07-16 14:37         ` Jean-Christophe Helary
  2017-07-17 15:28       ` Jean-Christophe Helary
  2 siblings, 1 reply; 41+ messages in thread
From: Eli Zaretskii @ 2017-07-16 14:35 UTC (permalink / raw)
  To: Jean-Christophe Helary; +Cc: emacs-devel

> From: Jean-Christophe Helary <jean.christophe.helary@gmail.com>
> Date: Sat, 15 Jul 2017 23:48:57 +0900
> 
> > On Jul 15, 2017, at 21:52, Eli Zaretskii <eliz@gnu.org> wrote:
> >> -    (message "Importing %s...done" (file-name-nondirectory file))))
> >> +    (message "Importing %s... Done" (file-name-nondirectory file))))
> > 
> > Can you tell why this is needed?  The current code is how we say this
> > in a lot of places, and I don't think I see why it's bad for l10n.
> 
> I thought there was a typo with the lack of space between the ... and the "done" and the lack upper case after a ... punctuation mark. If it is not, I'm fine with the original.

It's not a typo.  We hove many dozens of these all over the place.

Thanks.



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: package.el strings
  2017-07-16 14:35       ` Eli Zaretskii
@ 2017-07-16 14:37         ` Jean-Christophe Helary
  0 siblings, 0 replies; 41+ messages in thread
From: Jean-Christophe Helary @ 2017-07-16 14:37 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

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

Ok, I'll amend following your 2 recent confirmations.

Jean-Christophe 

> On Jul 16, 2017, at 23:35, Eli Zaretskii <eliz@gnu.org> wrote:
> 
>> From: Jean-Christophe Helary <jean.christophe.helary@gmail.com>
>> Date: Sat, 15 Jul 2017 23:48:57 +0900
>> 
>>> On Jul 15, 2017, at 21:52, Eli Zaretskii <eliz@gnu.org> wrote:
>>>> -    (message "Importing %s...done" (file-name-nondirectory file))))
>>>> +    (message "Importing %s... Done" (file-name-nondirectory file))))
>>> 
>>> Can you tell why this is needed?  The current code is how we say this
>>> in a lot of places, and I don't think I see why it's bad for l10n.
>> 
>> I thought there was a typo with the lack of space between the ... and the "done" and the lack upper case after a ... punctuation mark. If it is not, I'm fine with the original.
> 
> It's not a typo.  We hove many dozens of these all over the place.
> 
> Thanks.


[-- Attachment #2: Type: text/html, Size: 3665 bytes --]

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: package.el strings
  2017-07-15 14:48     ` Jean-Christophe Helary
  2017-07-16 13:55       ` Jean-Christophe Helary
  2017-07-16 14:35       ` Eli Zaretskii
@ 2017-07-17 15:28       ` Jean-Christophe Helary
  2017-07-22  9:23         ` Eli Zaretskii
  2 siblings, 1 reply; 41+ messages in thread
From: Jean-Christophe Helary @ 2017-07-17 15:28 UTC (permalink / raw)
  To: emacs-devel

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

>>> -   (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))))
>> 
>> This loses the feature of saying just what's needed, instead of
>> showing zero.  Can we do better?
> 
> If you ask, there probably is a way... I'll try to find something better.

Ok, I found something for this part that I also adapted to 2 other parts that I had overly simplified. I also made a few modifications here and there compared to my original file.

Jean-Christophe 


[-- Attachment #2: git_diff_package.el.diff --]
[-- Type: application/octet-stream, Size: 11260 bytes --]

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index 4245294457..8a17f2bce6 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.
@@ -1524,7 +1526,7 @@ package--download-one-archive
     (let* ((location (cdr archive))
            (name (car archive))
            (content (buffer-string))
-           (dir (expand-file-name (format "archives/%s" name) package-user-dir))
+           (dir (expand-file-name (concat "archives/" name) package-user-dir))
            (local-file (expand-file-name file dir)))
       (when (listp (read-from-string content))
         (make-directory dir t)
@@ -2054,12 +2056,12 @@ package-install-selected-packages
       (cond
        (available
         (when (y-or-n-p
-               (format "%s packages will be installed:\n%s, proceed?"
+               (format "Packages to install: %d (%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 "Packages that are not available: %d (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 +2178,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 "Packages to delete: %d (%s), proceed? "
                    (length removable)
-                   (mapconcat #'symbol-name removable ", ")))
+                   (mapconcat #'symbol-name removable " ")))
             (mapc (lambda (p)
                     (package-delete (cadr (assq p package-alist)) t))
                   removable))
@@ -2237,6 +2239,7 @@ package--print-help-section
 
 (declare-function lm-commentary "lisp-mnt" (&optional file))
 
+;; TODO the whole describe-package-1 function must be fixed for strings l10n
 (defun describe-package-1 (pkg)
   (require 'lisp-mnt)
   (let* ((desc (or
@@ -2264,11 +2267,7 @@ 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")
+    (princ (format "Package %S is %s.\n\n" name status))
 
     (package--print-help-section "Status")
     (cond (built-in
@@ -2649,9 +2648,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 +2973,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 "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 +3111,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 +3124,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 "Packages marked for upgrading: %d"
+        (length upgrades)))))
 
 (defun package-menu-mark-upgrades ()
   "Mark all upgradable packages in the Package Menu.
@@ -3146,39 +3143,32 @@ 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.
 Formats the returned string to be usable in a minibuffer
 prompt (see `package-menu--prompt-transaction-p')."
-  (cond
-   ;; None
-   ((not packages) "")
-   ;; More than 1
-   ((cdr packages)
-    (format "these %d packages (%s)"
-      (length packages)
-      (mapconcat #'package-desc-full-name packages ", ")))
-   ;; Exactly 1
-   (t (format-message "package `%s'"
-                      (package-desc-full-name (car packages))))))
+;; The case where `package' is empty is handled in
+;; package-menu--prompt-transation-p below
+  (format "%d (%s)"
+          (length 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 "%s%s%s%s"
+           (if (not delete) ""
+             (format "Packages to delete: %s. " (package-menu--list-to-prompt delete)))
+           (if (not install) ""
+             (format "Packages to install: %s. " (package-menu--list-to-prompt install)))
+           (if (not upgrade) ""
+             (format "Packages to upgrade: %s. " (package-menu--list-to-prompt upgrade)))
+           "Proceed? ")))
+
 
 (defun package-menu--partition-transaction (install delete)
   "Return an alist describing an INSTALL DELETE transaction.
@@ -3262,25 +3252,23 @@ package-menu-execute
       (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")
+               (format "[ %s%s%s]"
+                       (if (not .delete) ""
+                         (format "Delete %d " (length .delete)))
+                       (if (not .install) ""
+                         (format "Install %d " (length .install)))
+                       (if (not .upgrade) ""
+                         (format "Upgrade %d " (length .upgrade))))))
+          (message "Operation %s started" message-template)
           ;; 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. Packages that are no longer needed: %d. Type `%s' to remove them"
+                         (length removable)
+                         (substitute-command-keys "\\[package-autoremove]"))
+              (message "Operation %s finished" message-template))))))))
 
 (defun package-menu--version-predicate (A B)
   (let ((vA (or (aref (cadr A) 1)  '(0)))
@@ -3347,11 +3335,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 "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.

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* Re: package.el strings
  2017-07-17 15:28       ` Jean-Christophe Helary
@ 2017-07-22  9:23         ` Eli Zaretskii
  2018-04-18  6:38           ` Jean-Christophe Helary
  0 siblings, 1 reply; 41+ messages in thread
From: Eli Zaretskii @ 2017-07-22  9:23 UTC (permalink / raw)
  To: Jean-Christophe Helary; +Cc: emacs-devel

> From: Jean-Christophe Helary <jean.christophe.helary@gmail.com>
> Date: Tue, 18 Jul 2017 00:28:59 +0900
> 
> >>> -   (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))))
> >> 
> >> This loses the feature of saying just what's needed, instead of
> >> showing zero.  Can we do better?
> > 
> > If you ask, there probably is a way... I'll try to find something better.
> 
> Ok, I found something for this part that I also adapted to 2 other parts that I had overly simplified. I also made a few modifications here and there compared to my original file.

Thanks, this looks OK.  Could someone who uses/makes changes in
package.el please review these and, if OK, push to master?



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: package.el strings
  2017-07-22  9:23         ` Eli Zaretskii
@ 2018-04-18  6:38           ` Jean-Christophe Helary
  2018-04-26  1:10             ` Noam Postavsky
  0 siblings, 1 reply; 41+ messages in thread
From: Jean-Christophe Helary @ 2018-04-18  6:38 UTC (permalink / raw)
  To: Emacs developers

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

Almost 9 months and nothing in master. Was there anything wrong with the code?

Jean-Christophe 

> On Jul 22, 2017, at 18:23, Eli Zaretskii <eliz@gnu.org> wrote:
> 
>> From: Jean-Christophe Helary <jean.christophe.helary@gmail.com>
>> Date: Tue, 18 Jul 2017 00:28:59 +0900
>> 
>>>>> -   (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))))
>>>> 
>>>> This loses the feature of saying just what's needed, instead of
>>>> showing zero.  Can we do better?
>>> 
>>> If you ask, there probably is a way... I'll try to find something better.
>> 
>> Ok, I found something for this part that I also adapted to 2 other parts that I had overly simplified. I also made a few modifications here and there compared to my original file.
> 
> Thanks, this looks OK.  Could someone who uses/makes changes in
> package.el please review these and, if OK, push to master?

Jean-Christophe Helary
-----------------------------------------------
http://mac4translators.blogspot.com @brandelune



[-- Attachment #2: Type: text/html, Size: 3967 bytes --]

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: package.el strings
  2018-04-18  6:38           ` Jean-Christophe Helary
@ 2018-04-26  1:10             ` Noam Postavsky
  2018-04-26  6:31               ` Jean-Christophe Helary
  2018-04-26 13:40               ` Jean-Christophe Helary
  0 siblings, 2 replies; 41+ messages in thread
From: Noam Postavsky @ 2018-04-26  1:10 UTC (permalink / raw)
  To: Jean-Christophe Helary; +Cc: Emacs developers


> Almost 9 months and nothing in master. Was there anything wrong with
> the code?

I think the problem is more a lack of people who "make changes in
package.el".  Although there's enough that your patch no longer applies
cleanly.

    error: patch failed: lisp/emacs-lisp/package.el:3262
    error: lisp/emacs-lisp/package.el: patch does not apply

Some minor comments below.

> +;; 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"))

Should that be a FIXME or TODO?

> +      (message "Packages to hide: %s. Type `%s' to toggle or `%s' to customize"
                                        ^
                                        Missing double space
  
>  (defun package-menu--list-to-prompt (packages)
> +;; The case where `package' is empty is handled in
> +;; package-menu--prompt-transation-p below
                                 ^
                           transaction      
  
>  (defun package-menu--prompt-transaction-p (delete install upgrade)

> +   (format "%s%s%s%s"

This kind of format call is that same as concat, right?

> +           (if (not delete) ""
> +             (format "Packages to delete: %s. " (package-menu--list-to-prompt delete)))
> +           (if (not install) ""
> +             (format "Packages to install: %s. " (package-menu--list-to-prompt install)))
> +           (if (not upgrade) ""
> +             (format "Packages to upgrade: %s. " (package-menu--list-to-prompt upgrade)))
> +           "Proceed? ")))
> +
>  

> @@ -3262,25 +3252,23 @@ package-menu-execute

> +               (format "[ %s%s%s]"
> +                       (if (not .delete) ""
> +                         (format "Delete %d " (length .delete)))
> +                       (if (not .install) ""
> +                         (format "Install %d " (length .install)))
> +                       (if (not .upgrade) ""
> +                         (format "Upgrade %d " (length .upgrade))))))

Perhaps this one too? (although in this case it would mean splitting up
the brackets)

> +                (message "Operation finished. Packages that are no longer needed: %d. Type `%s' to remove them"
                                                ^                                       ^
                                                Double spacing                          Double spacing

This line is getting pretty long, perhaps it should be broken up?




^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: package.el strings
  2018-04-26  1:10             ` Noam Postavsky
@ 2018-04-26  6:31               ` Jean-Christophe Helary
  2018-04-26 11:28                 ` Noam Postavsky
  2018-04-26 13:40               ` Jean-Christophe Helary
  1 sibling, 1 reply; 41+ messages in thread
From: Jean-Christophe Helary @ 2018-04-26  6:31 UTC (permalink / raw)
  To: Emacs developers

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



> On Apr 26, 2018, at 10:10, Noam Postavsky <npostavs@gmail.com> wrote:
> 
> 
>> Almost 9 months and nothing in master. Was there anything wrong with
>> the code?
> 
> I think the problem is more a lack of people who "make changes in
> package.el".  Although there's enough that your patch no longer applies
> cleanly.

I thought so, but thank you for getting back to me.

>    error: patch failed: lisp/emacs-lisp/package.el:3262
>    error: lisp/emacs-lisp/package.el: patch does not apply

I'll fix the issues and will resubmit.

> Some minor comments below.
> 
>> +;; 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"))
> 
> Should that be a FIXME or TODO?

If we consider l10n and the function of this string, there is no reason this string should be in English (or in any native language for that matter). I don't know what that makes it. I'd say FIXME.

>> (defun package-menu--prompt-transaction-p (delete install upgrade)
> 
>> +   (format "%s%s%s%s"
> 
> This kind of format call is that same as concat, right?

I'm not sure I understand the question. This format call produces messages that are semantically equivalent to the original concat calls but the concat calls were not localizable so I changed them to something that we'll eventually be able to properly extract and localize.

There are probably ways to use concat to produce localizable strings though.

>> +                (message "Operation finished. Packages that are no longer needed: %d. Type `%s' to remove them"
>                                                ^                                       ^
>                                                Double spacing                          Double spacing
> 
> This line is getting pretty long, perhaps it should be broken up?

I guess you mean the line of code?


Jean-Christophe Helary
-----------------------------------------------
http://mac4translators.blogspot.com @brandelune



[-- Attachment #2: Type: text/html, Size: 5620 bytes --]

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: package.el strings
  2018-04-26  6:31               ` Jean-Christophe Helary
@ 2018-04-26 11:28                 ` Noam Postavsky
  2018-04-26 13:32                   ` Jean-Christophe Helary
  0 siblings, 1 reply; 41+ messages in thread
From: Noam Postavsky @ 2018-04-26 11:28 UTC (permalink / raw)
  To: Jean-Christophe Helary; +Cc: Emacs developers


> I'm not sure I understand the question. This format call produces
> messages that are semantically equivalent to the original concat calls
> but the concat calls were not localizable so I changed them to something
> that we'll eventually be able to properly extract and localize.

I meant just the outer (format "%s%s%s%s" ...) vs (concat ...).  Is that
also needed to make strings localizable?  If yes, I think adding a
comment would be helpful, because it looks a bit unnatural to me.
 
>>> +                (message "Operation finished. Packages that are no longer needed: %d. Type `%s' to remove them"

>> This line is getting pretty long, perhaps it should be broken up?
> 
> I guess you mean the line of code?

Yes.



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: package.el strings
  2018-04-26 11:28                 ` Noam Postavsky
@ 2018-04-26 13:32                   ` Jean-Christophe Helary
  2018-04-28 22:11                     ` Noam Postavsky
  0 siblings, 1 reply; 41+ messages in thread
From: Jean-Christophe Helary @ 2018-04-26 13:32 UTC (permalink / raw)
  To: Emacs developers

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



> On Apr 26, 2018, at 20:28, Noam Postavsky <npostavs@gmail.com> wrote:
> 
> 
>> I'm not sure I understand the question. This format call produces
>> messages that are semantically equivalent to the original concat calls
>> but the concat calls were not localizable so I changed them to something
>> that we'll eventually be able to properly extract and localize.
> 
> I meant just the outer (format "%s%s%s%s" ...) vs (concat ...).  Is that
> also needed to make strings localizable?  If yes, I think adding a
> comment would be helpful, because it looks a bit unnatural to me.

Oh, you mean (format "%s%s%s%s" ...) vs an eventual (concat "%s%s%s%s" ...) ?

I checked the lisp ref to find something that would make it a clear choice but did not find anything really conclusive.

What I did is I try to keep concat for simple strings. And anything that involved fixing complex native strings was either moved to format or something else.

There is nothing that makes concat of format etc more or less appropriate for l10n in my understanding. I guess I was influenced by the way the original authors abused concat to put native strings together...

>>>> +                (message "Operation finished. Packages that are no longer needed: %d. Type `%s' to remove them"
> 
>>> This line is getting pretty long, perhaps it should be broken up?
>> 
>> I guess you mean the line of code?
> 
> Yes.

It there a length limit in emacs code ?

Jean-Christophe Helary
-----------------------------------------------
http://mac4translators.blogspot.com @brandelune


[-- Attachment #2: Type: text/html, Size: 3787 bytes --]

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: package.el strings
  2018-04-26  1:10             ` Noam Postavsky
  2018-04-26  6:31               ` Jean-Christophe Helary
@ 2018-04-26 13:40               ` Jean-Christophe Helary
  1 sibling, 0 replies; 41+ messages in thread
From: Jean-Christophe Helary @ 2018-04-26 13:40 UTC (permalink / raw)
  To: Emacs developers

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



> On Apr 26, 2018, at 10:10, Noam Postavsky <npostavs@gmail.com> wrote:
> 
> 
>> Almost 9 months and nothing in master. Was there anything wrong with
>> the code?
> 
> I think the problem is more a lack of people who "make changes in
> package.el".  Although there's enough that your patch no longer applies
> cleanly.
> 
>    error: patch failed: lisp/emacs-lisp/package.el:3262
>    error: lisp/emacs-lisp/package.el: patch does not apply

Here is the updated patch with fixes for the comments you made.

Jean-Christophe 


> Some minor comments below.
> 
>> +;; 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"))
> 
> Should that be a FIXME or TODO?
> 
>> +      (message "Packages to hide: %s. Type `%s' to toggle or `%s' to customize"
>                                        ^
>                                        Missing double space
> 
>> (defun package-menu--list-to-prompt (packages)
>> +;; The case where `package' is empty is handled in
>> +;; package-menu--prompt-transation-p below
>                                 ^
>                           transaction      
> 
>> (defun package-menu--prompt-transaction-p (delete install upgrade)
> 
>> +   (format "%s%s%s%s"
> 
> This kind of format call is that same as concat, right?
> 
>> +           (if (not delete) ""
>> +             (format "Packages to delete: %s. " (package-menu--list-to-prompt delete)))
>> +           (if (not install) ""
>> +             (format "Packages to install: %s. " (package-menu--list-to-prompt install)))
>> +           (if (not upgrade) ""
>> +             (format "Packages to upgrade: %s. " (package-menu--list-to-prompt upgrade)))
>> +           "Proceed? ")))
>> +
>> 
> 
>> @@ -3262,25 +3252,23 @@ package-menu-execute
> 
>> +               (format "[ %s%s%s]"
>> +                       (if (not .delete) ""
>> +                         (format "Delete %d " (length .delete)))
>> +                       (if (not .install) ""
>> +                         (format "Install %d " (length .install)))
>> +                       (if (not .upgrade) ""
>> +                         (format "Upgrade %d " (length .upgrade))))))
> 
> Perhaps this one too? (although in this case it would mean splitting up
> the brackets)
> 
>> +                (message "Operation finished. Packages that are no longer needed: %d. Type `%s' to remove them"
>                                                ^                                       ^
>                                                Double spacing                          Double spacing
> 
> This line is getting pretty long, perhaps it should be broken up?
> 

Jean-Christophe Helary
-----------------------------------------------
http://mac4translators.blogspot.com @brandelune



[-- Attachment #2.1: Type: text/html, Size: 1247 bytes --]

[-- Attachment #2.2: package.el_0426.diff --]
[-- Type: application/octet-stream, Size: 10898 bytes --]

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index 94d98178c4..3546fa0fb1 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -200,8 +200,10 @@ 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.
 
@@ -1015,6 +1017,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.
@@ -1552,7 +1555,7 @@ package--download-one-archive
     (let* ((location (cdr archive))
            (name (car archive))
            (content (buffer-string))
-           (dir (expand-file-name (format "archives/%s" name) package-user-dir))
+           (dir (expand-file-name (concat "archives/" name) package-user-dir))
            (local-file (expand-file-name file dir)))
       (when (listp (read-from-string content))
         (make-directory dir t)
@@ -2034,12 +2037,12 @@ package-install-selected-packages
       (cond
        (available
         (when (y-or-n-p
-               (format "%s packages will be installed:\n%s, proceed?"
+              (format "Packages to install: %d (%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 "Packages that are not available: %d (the rest is already installed), maybe you need to `M-x package-refresh-contents'"
                  difference))
        (t
         (message "All your packages are already installed"))))))
@@ -2158,9 +2161,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 "Packages to delete: %d (%s), proceed? "
                    (length removable)
-                   (mapconcat #'symbol-name removable ", ")))
+                   (mapconcat #'symbol-name removable " ")))
             (mapc (lambda (p)
                     (package-delete (cadr (assq p package-alist)) t))
                   removable))
@@ -2247,12 +2250,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")
-
+    (princ (format "Package %S is %s.\n\n" name status))
+    
     (package--print-help-section "Status")
     (cond (built-in
            (insert (propertize (capitalize status)
@@ -2634,9 +2633,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)
@@ -2960,11 +2959,11 @@ 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 "Packages to hide: %d.  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.
@@ -3099,7 +3098,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))
@@ -3112,9 +3111,9 @@ 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 "Packages marked for upgrading: %d"
+               (length upgrades)))))
+
 
 (defun package-menu-mark-upgrades ()
   "Mark all upgradable packages in the Package Menu.
@@ -3137,34 +3136,27 @@ package-menu--list-to-prompt
 PACKAGES is a list of `package-desc' objects.
 Formats the returned string to be usable in a minibuffer
 prompt (see `package-menu--prompt-transaction-p')."
-  (cond
-   ;; None
-   ((not packages) "")
-   ;; More than 1
-   ((cdr packages)
-    (format "these %d packages (%s)"
-      (length packages)
-      (mapconcat #'package-desc-full-name packages ", ")))
-   ;; Exactly 1
-   (t (format-message "package `%s'"
-                      (package-desc-full-name (car packages))))))
+  ;; The case where `package' is empty is handled in
+  ;; package-menu--prompt-transaction-p below
+  (format "%d (%s)"
+          (length 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 "%s%s%s%s"
+           (if (not delete) ""
+             (format "Packages to delete: %s. " (package-menu--list-to-prompt delete)))
+           (if (not install) ""
+             (format "Packages to install: %s. " (package-menu--list-to-prompt install)))
+           (if (not upgrade) ""
+             (format "Packages to upgrade: %s. " (package-menu--list-to-prompt upgrade)))
+           "Proceed? ")))
+
 
 (defun package-menu--partition-transaction (install delete)
   "Return an alist describing an INSTALL DELETE transaction.
@@ -3248,25 +3240,23 @@ package-menu-execute
       (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")
+               (format "[ %s%s%s]"
+                       (if (not .delete) ""
+                         (format "Delete %d " (length .delete)))
+                       (if (not .install) ""
+                         (format "Install %d " (length .install)))
+                       (if (not .upgrade) ""
+                         (format "Upgrade %d " (length .upgrade))))))
+          (message "Operation %s started" message-template)
           ;; 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.  Packages that are no longer needed: %d.  Type `%s' to remove them"
+                         (length removable)
+                         (substitute-command-keys "\\[package-autoremove]"))
+              (message "Operation %s finished" message-template))))))))
 
 (defun package-menu--version-predicate (A B)
   (let ((vA (or (aref (cadr A) 1)  '(0)))
@@ -3333,11 +3323,11 @@ 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."
-      (length upgrades)
-      (if (= (length upgrades) 1) "" "s")
-      (substitute-command-keys "\\[package-menu-mark-upgrades]")
-      (if (= (length upgrades) 1) "it" "them"))))
+    (message "Packages that can be upgraded: %d; type `%s' to mark for upgrading."
+             (length upgrades)
+             (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 #2.3: Type: text/html, Size: 7436 bytes --]

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* Re: package.el strings
  2018-04-26 13:32                   ` Jean-Christophe Helary
@ 2018-04-28 22:11                     ` Noam Postavsky
  2018-04-28 23:46                       ` Jean-Christophe Helary
  2018-04-29 19:43                       ` Stefan Monnier
  0 siblings, 2 replies; 41+ messages in thread
From: Noam Postavsky @ 2018-04-28 22:11 UTC (permalink / raw)
  To: Jean-Christophe Helary; +Cc: Emacs developers

>> I meant just the outer (format "%s%s%s%s" ...) vs (concat ...).  Is that
>> also needed to make strings localizable?  If yes, I think adding a
>> comment would be helpful, because it looks a bit unnatural to me.
> 
> Oh, you mean (format "%s%s%s%s" ...) vs an eventual (concat "%s%s%s%s" ...) ?

Sorry, I was trying to elide the the less relevant parts, but I see it's
making things less understandable.  Let me just spell it out in full:

Instead of this:

   (format "%s%s%s%s"
           (if (not delete) ""
             (format "Packages to delete: %s. " (package-menu--list-to-prompt delete)))
           (if (not install) ""
             (format "Packages to install: %s. " (package-menu--list-to-prompt install)))
           (if (not upgrade) ""
             (format "Packages to upgrade: %s. " (package-menu--list-to-prompt upgrade)))
           "Proceed? ")

I think this would be a bit clearer:

   (concat
    (when delete
      (format "Packages to delete: %s. " (package-menu--list-to-prompt delete)))
    (when install
      (format "Packages to install: %s. " (package-menu--list-to-prompt install)))
    (when upgrade
      (format "Packages to upgrade: %s. " (package-menu--list-to-prompt upgrade)))
    "Proceed? ")

> It there a length limit in emacs code ?

Um, I thought there was a convention of generally trying to keep things
within 80 columns, but now I can't find anything saying that.  I may
have just been thinking of other non-Emacs projects.
 



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: package.el strings
  2018-04-28 22:11                     ` Noam Postavsky
@ 2018-04-28 23:46                       ` Jean-Christophe Helary
  2018-05-29 22:38                         ` Noam Postavsky
  2018-04-29 19:43                       ` Stefan Monnier
  1 sibling, 1 reply; 41+ messages in thread
From: Jean-Christophe Helary @ 2018-04-28 23:46 UTC (permalink / raw)
  To: Emacs developers

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



> On Apr 29, 2018, at 7:11, Noam Postavsky <npostavs@gmail.com> wrote:
> 
>>> I meant just the outer (format "%s%s%s%s" ...) vs (concat ...).  Is that
>>> also needed to make strings localizable?  If yes, I think adding a
>>> comment would be helpful, because it looks a bit unnatural to me.
>> 
>> Oh, you mean (format "%s%s%s%s" ...) vs an eventual (concat "%s%s%s%s" ...) ?
> 
> Sorry, I was trying to elide the the less relevant parts, but I see it's
> making things less understandable.  Let me just spell it out in full:

Thank you. I see what you mean and that makes perfect sense. Give me a few days.

>> It there a length limit in emacs code ?
> 
> Um, I thought there was a convention of generally trying to keep things
> within 80 columns, but now I can't find anything saying that.  I may
> have just been thinking of other non-Emacs projects.

But I seem to remember something similar too. Wasn't it in the documentation strings ? Or the commit messages ?


Jean-Christophe Helary
-----------------------------------------------
http://mac4translators.blogspot.com @brandelune



[-- Attachment #2: Type: text/html, Size: 3050 bytes --]

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: package.el strings
  2018-04-28 22:11                     ` Noam Postavsky
  2018-04-28 23:46                       ` Jean-Christophe Helary
@ 2018-04-29 19:43                       ` Stefan Monnier
  1 sibling, 0 replies; 41+ messages in thread
From: Stefan Monnier @ 2018-04-29 19:43 UTC (permalink / raw)
  To: emacs-devel

> Um, I thought there was a convention of generally trying to keep things
> within 80 columns,

Yes, please,


        Stefan




^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: package.el strings
  2018-04-28 23:46                       ` Jean-Christophe Helary
@ 2018-05-29 22:38                         ` Noam Postavsky
  2018-05-29 22:46                           ` Jean-Christophe Helary
  2018-06-17 14:02                           ` Jean-Christophe Helary
  0 siblings, 2 replies; 41+ messages in thread
From: Noam Postavsky @ 2018-05-29 22:38 UTC (permalink / raw)
  To: Jean-Christophe Helary; +Cc: Emacs developers

On 28 April 2018 at 19:46, Jean-Christophe Helary
<jean.christophe.helary@gmail.com> wrote:

> Thank you. I see what you mean and that makes perfect sense. Give me a few
> days.

Ping? Did you intend to make any other changes? If it's just about
that one concat vs format thing I can just do it on my end before
pushing.



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: package.el strings
  2018-05-29 22:38                         ` Noam Postavsky
@ 2018-05-29 22:46                           ` Jean-Christophe Helary
  2018-06-17 14:02                           ` Jean-Christophe Helary
  1 sibling, 0 replies; 41+ messages in thread
From: Jean-Christophe Helary @ 2018-05-29 22:46 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: Emacs developers

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

Noam, thank you for the reminder. I intend to do it, I'm just busy with work right now. I'll have more time next week.

Also, I had a few issues with other strings but because they are "decorated" I felt the need to discuss that and kind of put that part on the side.

JC

> On May 30, 2018, at 7:38, Noam Postavsky <npostavs@gmail.com> wrote:
> 
> On 28 April 2018 at 19:46, Jean-Christophe Helary
> <jean.christophe.helary@gmail.com> wrote:
> 
>> Thank you. I see what you mean and that makes perfect sense. Give me a few
>> days.
> 
> Ping? Did you intend to make any other changes? If it's just about
> that one concat vs format thing I can just do it on my end before
> pushing.

Jean-Christophe Helary
-----------------------------------------------
http://mac4translators.blogspot.com @brandelune



[-- Attachment #2: Type: text/html, Size: 2583 bytes --]

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: package.el strings
  2018-05-29 22:38                         ` Noam Postavsky
  2018-05-29 22:46                           ` Jean-Christophe Helary
@ 2018-06-17 14:02                           ` Jean-Christophe Helary
  2018-06-17 14:33                             ` Eli Zaretskii
  1 sibling, 1 reply; 41+ messages in thread
From: Jean-Christophe Helary @ 2018-06-17 14:02 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: Emacs developers

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

Noam,

There is indeed another change I want to make but that can wait. It involves removing some of the text decorations to be able to straighten some strings.

If there is an agreement to proceed I can do that later. In the meanwhile it would be nice if you could first commit the biggest part of the modifications.

Here would be the log message:

===========================
* lisp/emacs-lisp/package.el: reformat message strings for future l10n
===========================


[-- Attachment #2: package.el_0617.diff --]
[-- Type: application/octet-stream, Size: 10845 bytes --]

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index 94d98178c4..9e49b6964e 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -200,8 +200,10 @@ 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.
 
@@ -1015,6 +1017,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.
@@ -1552,7 +1555,7 @@ package--download-one-archive
     (let* ((location (cdr archive))
            (name (car archive))
            (content (buffer-string))
-           (dir (expand-file-name (format "archives/%s" name) package-user-dir))
+           (dir (expand-file-name (concat "archives/" name) package-user-dir))
            (local-file (expand-file-name file dir)))
       (when (listp (read-from-string content))
         (make-directory dir t)
@@ -2034,12 +2037,12 @@ package-install-selected-packages
       (cond
        (available
         (when (y-or-n-p
-               (format "%s packages will be installed:\n%s, proceed?"
+              (format "Packages to install: %d (%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 "Packages that are not available: %d (the rest is already installed), maybe you need to `M-x package-refresh-contents'"
                  difference))
        (t
         (message "All your packages are already installed"))))))
@@ -2158,9 +2161,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 "Packages to delete: %d (%s), proceed? "
                    (length removable)
-                   (mapconcat #'symbol-name removable ", ")))
+                   (mapconcat #'symbol-name removable " ")))
             (mapc (lambda (p)
                     (package-delete (cadr (assq p package-alist)) t))
                   removable))
@@ -2247,12 +2250,9 @@ 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")
+    (princ (format "Package %S is %s.\n\n" name status))
 
+    ;; TODO: remove the string decorations and reformat the strings for future l10n
     (package--print-help-section "Status")
     (cond (built-in
            (insert (propertize (capitalize status)
@@ -2634,9 +2634,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)
@@ -2960,11 +2960,11 @@ 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 "Packages to hide: %d.  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.
@@ -3099,7 +3099,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))
@@ -3112,9 +3112,9 @@ 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 "Packages marked for upgrading: %d"
+               (length upgrades)))))
+
 
 (defun package-menu-mark-upgrades ()
   "Mark all upgradable packages in the Package Menu.
@@ -3137,17 +3137,12 @@ package-menu--list-to-prompt
 PACKAGES is a list of `package-desc' objects.
 Formats the returned string to be usable in a minibuffer
 prompt (see `package-menu--prompt-transaction-p')."
-  (cond
-   ;; None
-   ((not packages) "")
-   ;; More than 1
-   ((cdr packages)
-    (format "these %d packages (%s)"
-      (length packages)
-      (mapconcat #'package-desc-full-name packages ", ")))
-   ;; Exactly 1
-   (t (format-message "package `%s'"
-                      (package-desc-full-name (car packages))))))
+  ;; The case where `package' is empty is handled in
+  ;; package-menu--prompt-transaction-p below
+  (format "%d (%s)"
+          (length packages)
+          (mapconcat #'package-desc-full-name packages " ")))
+
 
 (defun package-menu--prompt-transaction-p (delete install upgrade)
   "Prompt the user about DELETE, INSTALL, and UPGRADE.
@@ -3155,16 +3150,14 @@ package-menu--prompt-transaction-p
 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)
-    "? ")))
+    (when delete
+      (format "Packages to delete: %s. " (package-menu--list-to-prompt delete)))
+    (when install
+      (format "Packages to install: %s. " (package-menu--list-to-prompt install)))
+    (when upgrade
+      (format "Packages to upgrade: %s. " (package-menu--list-to-prompt upgrade)))
+    "Proceed? ")))
+
 
 (defun package-menu--partition-transaction (install delete)
   "Return an alist describing an INSTALL DELETE transaction.
@@ -3248,25 +3241,24 @@ package-menu-execute
       (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)))
+               (concat "[ "
+                       (when .delete
+                         (format "Delete %d " (length .delete)))
+                       (when .install
+                         (format "Install %d " (length .install)))
+                       (when .upgrade
+                         (format "Upgrade %d " (length .upgrade)))
                        "]")))
-          (message (replace-regexp-in-string "__" "ing" message-template) "started")
+          (message "Operation %s started" message-template)
           ;; 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.  Packages that are no longer needed: %d.  Type `%s' to remove them"
+                         (length removable)
+                         (substitute-command-keys "\\[package-autoremove]"))
+              (message "Operation %s finished" message-template))))))))
 
 (defun package-menu--version-predicate (A B)
   (let ((vA (or (aref (cadr A) 1)  '(0)))
@@ -3333,11 +3325,11 @@ 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."
-      (length upgrades)
-      (if (= (length upgrades) 1) "" "s")
-      (substitute-command-keys "\\[package-menu-mark-upgrades]")
-      (if (= (length upgrades) 1) "it" "them"))))
+    (message "Packages that can be upgraded: %d; type `%s' to mark for upgrading."
+             (length upgrades)
+             (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: 554 bytes --]



Jean-Christophe 


> On May 30, 2018, at 7:38, Noam Postavsky <npostavs@gmail.com> wrote:
> 
> On 28 April 2018 at 19:46, Jean-Christophe Helary
> <jean.christophe.helary@gmail.com> wrote:
> 
>> Thank you. I see what you mean and that makes perfect sense. Give me a few
>> days.
> 
> Ping? Did you intend to make any other changes? If it's just about
> that one concat vs format thing I can just do it on my end before
> pushing.

Jean-Christophe Helary
-----------------------------------------------
http://mac4translators.blogspot.com @brandelune



^ permalink raw reply related	[flat|nested] 41+ messages in thread

* Re: package.el strings
  2018-06-17 14:02                           ` Jean-Christophe Helary
@ 2018-06-17 14:33                             ` Eli Zaretskii
  2018-06-17 14:45                               ` Jean-Christophe Helary
  0 siblings, 1 reply; 41+ messages in thread
From: Eli Zaretskii @ 2018-06-17 14:33 UTC (permalink / raw)
  To: Jean-Christophe Helary; +Cc: npostavs, emacs-devel

> From: Jean-Christophe Helary <jean.christophe.helary@gmail.com>
> Date: Sun, 17 Jun 2018 23:02:08 +0900
> Cc: Emacs developers <emacs-devel@gnu.org>
> 
> There is indeed another change I want to make but that can wait. It involves removing some of the text decorations to be able to straighten some strings.
> 
> If there is an agreement to proceed I can do that later. In the meanwhile it would be nice if you could first commit the biggest part of the modifications.

Thanks, but there are a couple of hunks I don't understand:

>  (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))))
> +

Why did you need this change?

> -           (dir (expand-file-name (format "archives/%s" name) package-user-dir))
> +           (dir (expand-file-name (concat "archives/" name) package-user-dir))

Same question here.
> -        (message "No packages to upgrade.")
> +        (message "No packages to upgrade")

I think our style is not to include a period in messages.



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: package.el strings
  2018-06-17 14:33                             ` Eli Zaretskii
@ 2018-06-17 14:45                               ` Jean-Christophe Helary
  2018-06-17 14:50                                 ` Eli Zaretskii
  0 siblings, 1 reply; 41+ messages in thread
From: Jean-Christophe Helary @ 2018-06-17 14:45 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: npostavs, emacs-devel

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

Eli,

Thank you for the quick check.

> On Jun 17, 2018, at 23:33, Eli Zaretskii <eliz@gnu.org> wrote:
> 
>> From: Jean-Christophe Helary <jean.christophe.helary@gmail.com>
>> Date: Sun, 17 Jun 2018 23:02:08 +0900
>> Cc: Emacs developers <emacs-devel@gnu.org>
>> 
>> There is indeed another change I want to make but that can wait. It involves removing some of the text decorations to be able to straighten some strings.
>> 
>> If there is an agreement to proceed I can do that later. In the meanwhile it would be nice if you could first commit the biggest part of the modifications.
> 
> Thanks, but there are a couple of hunks I don't understand:
> 
>> (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))))
>> +
> 
> Why did you need this change?

That's something we discussed last year:

> On Jul 1, 2017, at 10:56, Jean-Christophe Helary <jean.christophe.helary@gmail.com> wrote:
> 
> 
>> On Jun 22, 2017, at 10:57, Richard Stallman <rms@gnu.org> wrote:
>> 
>>>> (format "http%s://elpa.gnu.org/packages/"
>>>>                                       (if (gnutls-available-p) "s" ""))
>> 
>>> Yes, it is. But obviously I think that, since I wrote it.
>> 
>> I agree it is acceptable, but this looks like something that might be
>> needed in a number of places, so perhaps we should make a nicer
>> interface to do it.
> 
> There is only one instance of such a query in package.el and no other package in the emacs distribution seems to use gnutls-available-p to add an "s" to http.
> 
> Paul suggested that it would be a maintenance hassle to keep 2 almost identical urls if we spelled them out as Yuri suggested but I checked other source packages and for ex auth-source-pass-tests.el explicitly spells out all the urls without resorting to smart formatting to save a few characters.
> 
> Also, considering the way %s is abused in other places package.el, for ex in:
> 
> (message "%d package%s marked for upgrading."                                                               
>        (length upgrades)                                                                                          
>        (if (= (length upgrades) 1) "" "s"))))) 
> 
> I don't think it is good to keep the above code because it gives bad incentives to authors especially if work on i18n/l10n proceeds (even though http/https is not related to l10n).
> 
> So, I'm going to spell out the urls as I proceed with untangling code and translatable strings in package.el. I'll send a diff here when I'm done for evaluation.
> 
> Jean-Christophe 


>> -           (dir (expand-file-name (format "archives/%s" name) package-user-dir))
>> +           (dir (expand-file-name (concat "archives/" name) package-user-dir))
> 
> Same question here.

I don't remember. I wrote this part last year. I guess I was trying to separate format and concat use: format for user visible strings and concat for internal strings. But you can also read that as a reaction to the format abuse in package.el... concat makes the string shorter without compromise.

>> -        (message "No packages to upgrade.")
>> +        (message "No packages to upgrade")
> 
> I think our style is not to include a period in messages.

I think I removed a period here. What are you referring to?


Jean-Christophe Helary
-----------------------------------------------
http://mac4translators.blogspot.com @brandelune



[-- Attachment #2: Type: text/html, Size: 9465 bytes --]

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: package.el strings
  2018-06-17 14:45                               ` Jean-Christophe Helary
@ 2018-06-17 14:50                                 ` Eli Zaretskii
  2018-06-17 14:58                                   ` Jean-Christophe Helary
  0 siblings, 1 reply; 41+ messages in thread
From: Eli Zaretskii @ 2018-06-17 14:50 UTC (permalink / raw)
  To: Jean-Christophe Helary; +Cc: npostavs, emacs-devel

> From: Jean-Christophe Helary <jean.christophe.helary@gmail.com>
> Date: Sun, 17 Jun 2018 23:45:46 +0900
> Cc: npostavs@gmail.com,
>  emacs-devel@gnu.org
> 
>  (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))))
>  +
> 
>  Why did you need this change?
> 
> That's something we discussed last year:
> 
>  On Jul 1, 2017, at 10:56, Jean-Christophe Helary <jean.christophe.helary@gmail.com> wrote:
> 
>  On Jun 22, 2017, at 10:57, Richard Stallman <rms@gnu.org> wrote:
> 
>  (format "http%s://elpa.gnu.org/packages/"
>                                        (if (gnutls-available-p) "s" ""))
> 
>  Yes, it is. But obviously I think that, since I wrote it.
> 
>  I agree it is acceptable, but this looks like something that might be
>  needed in a number of places, so perhaps we should make a nicer
>  interface to do it.
> 
>  There is only one instance of such a query in package.el and no other package in the emacs distribution
>  seems to use gnutls-available-p to add an "s" to http.
> 
>  Paul suggested that it would be a maintenance hassle to keep 2 almost identical urls if we spelled them
>  out as Yuri suggested but I checked other source packages and for ex auth-source-pass-tests.el
>  explicitly spells out all the urls without resorting to smart formatting to save a few characters.

I don't understand this discussion.  To make things clear, I don't see
why appending "s" to "http" in a format call would be bad in this
case, since "https" is not a plural of "http".

>  -        (message "No packages to upgrade.")
>  +        (message "No packages to upgrade")
> 
>  I think our style is not to include a period in messages.
> 
> I think I removed a period here. What are you referring to?

Oops.



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: package.el strings
  2018-06-17 14:50                                 ` Eli Zaretskii
@ 2018-06-17 14:58                                   ` Jean-Christophe Helary
  2018-06-20 13:53                                     ` Jean-Christophe Helary
  0 siblings, 1 reply; 41+ messages in thread
From: Jean-Christophe Helary @ 2018-06-17 14:58 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: npostavs, emacs-devel



> On Jun 17, 2018, at 23:50, Eli Zaretskii <eliz@gnu.org> wrote:
> 
> I don't understand this discussion.  To make things clear, I don't see
> why appending "s" to "http" in a format call would be bad in this
> case, since "https" is not a plural of "http".

As far as I checked, it is not the current practice in the rest of the emacs code.
Similar URLs are spelled out.

Jean-Christophe Helary
-----------------------------------------------
http://mac4translators.blogspot.com @brandelune





^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: package.el strings
  2018-06-17 14:58                                   ` Jean-Christophe Helary
@ 2018-06-20 13:53                                     ` Jean-Christophe Helary
  2018-06-20 15:54                                       ` Eli Zaretskii
  0 siblings, 1 reply; 41+ messages in thread
From: Jean-Christophe Helary @ 2018-06-20 13:53 UTC (permalink / raw)
  To: Emacs developers; +Cc: Eli Zaretskii, Noam Postavsky

Eli,

Do you prefer that I remove this hunk ?

JC

> On Jun 17, 2018, at 23:58, Jean-Christophe Helary <jean.christophe.helary@gmail.com> wrote:
> 
> 
> 
>> On Jun 17, 2018, at 23:50, Eli Zaretskii <eliz@gnu.org> wrote:
>> 
>> I don't understand this discussion.  To make things clear, I don't see
>> why appending "s" to "http" in a format call would be bad in this
>> case, since "https" is not a plural of "http".
> 
> As far as I checked, it is not the current practice in the rest of the emacs code.
> Similar URLs are spelled out.




^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: package.el strings
  2018-06-20 13:53                                     ` Jean-Christophe Helary
@ 2018-06-20 15:54                                       ` Eli Zaretskii
  2018-06-20 16:19                                         ` Jean-Christophe Helary
  0 siblings, 1 reply; 41+ messages in thread
From: Eli Zaretskii @ 2018-06-20 15:54 UTC (permalink / raw)
  To: Jean-Christophe Helary; +Cc: npostavs, emacs-devel

> From: Jean-Christophe Helary <brandelune@gmail.com>
> Date: Wed, 20 Jun 2018 22:53:56 +0900
> Cc: Eli Zaretskii <eliz@gnu.org>,
>  Noam Postavsky <npostavs@gmail.com>
> 
> Do you prefer that I remove this hunk ?

Yes.



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: package.el strings
  2018-06-20 15:54                                       ` Eli Zaretskii
@ 2018-06-20 16:19                                         ` Jean-Christophe Helary
  2018-06-24 15:37                                           ` Noam Postavsky
  0 siblings, 1 reply; 41+ messages in thread
From: Jean-Christophe Helary @ 2018-06-20 16:19 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: npostavs, emacs-devel

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

Et voilà !

===========================
* lisp/emacs-lisp/package.el: reformat message strings for future l10n
===========================



Jean-Christophe 

> On Jun 21, 2018, at 0:54, Eli Zaretskii <eliz@gnu.org> wrote:
> 
>> From: Jean-Christophe Helary <brandelune@gmail.com>
>> Date: Wed, 20 Jun 2018 22:53:56 +0900
>> Cc: Eli Zaretskii <eliz@gnu.org>,
>> Noam Postavsky <npostavs@gmail.com>
>> 
>> Do you prefer that I remove this hunk ?
> 
> Yes.

Jean-Christophe Helary
-----------------------------------------------
http://mac4translators.blogspot.com @brandelune



[-- Attachment #2.1: Type: text/html, Size: 849 bytes --]

[-- Attachment #2.2: package.el_0621.diff --]
[-- Type: application/octet-stream, Size: 10420 bytes --]

On branch l10n
Your branch is up to date with 'origin/master'.

Changes to be committed:
  (use "git reset HEAD <file>..." to unstage)

	modified:   lisp/emacs-lisp/package.el

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index 94d98178c4..8219b62d83 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -1015,6 +1015,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.
@@ -1552,7 +1553,7 @@ package--download-one-archive
     (let* ((location (cdr archive))
            (name (car archive))
            (content (buffer-string))
-           (dir (expand-file-name (format "archives/%s" name) package-user-dir))
+           (dir (expand-file-name (concat "archives/" name) package-user-dir))
            (local-file (expand-file-name file dir)))
       (when (listp (read-from-string content))
         (make-directory dir t)
@@ -2034,12 +2035,12 @@ package-install-selected-packages
       (cond
        (available
         (when (y-or-n-p
-               (format "%s packages will be installed:\n%s, proceed?"
+              (format "Packages to install: %d (%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 "Packages that are not available: %d (the rest is already installed), maybe you need to `M-x package-refresh-contents'"
                  difference))
        (t
         (message "All your packages are already installed"))))))
@@ -2158,9 +2159,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 "Packages to delete: %d (%s), proceed? "
                    (length removable)
-                   (mapconcat #'symbol-name removable ", ")))
+                   (mapconcat #'symbol-name removable " ")))
             (mapc (lambda (p)
                     (package-delete (cadr (assq p package-alist)) t))
                   removable))
@@ -2247,12 +2248,9 @@ 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")
+    (princ (format "Package %S is %s.\n\n" name status))
 
+    ;; TODO: remove the string decorations and reformat the strings for future l10n
     (package--print-help-section "Status")
     (cond (built-in
            (insert (propertize (capitalize status)
@@ -2634,9 +2632,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)
@@ -2960,11 +2958,11 @@ 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 "Packages to hide: %d.  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.
@@ -3099,7 +3097,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))
@@ -3112,9 +3110,9 @@ 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 "Packages marked for upgrading: %d"
+               (length upgrades)))))
+
 
 (defun package-menu-mark-upgrades ()
   "Mark all upgradable packages in the Package Menu.
@@ -3137,17 +3135,12 @@ package-menu--list-to-prompt
 PACKAGES is a list of `package-desc' objects.
 Formats the returned string to be usable in a minibuffer
 prompt (see `package-menu--prompt-transaction-p')."
-  (cond
-   ;; None
-   ((not packages) "")
-   ;; More than 1
-   ((cdr packages)
-    (format "these %d packages (%s)"
-      (length packages)
-      (mapconcat #'package-desc-full-name packages ", ")))
-   ;; Exactly 1
-   (t (format-message "package `%s'"
-                      (package-desc-full-name (car packages))))))
+  ;; The case where `package' is empty is handled in
+  ;; package-menu--prompt-transaction-p below
+  (format "%d (%s)"
+          (length packages)
+          (mapconcat #'package-desc-full-name packages " ")))
+
 
 (defun package-menu--prompt-transaction-p (delete install upgrade)
   "Prompt the user about DELETE, INSTALL, and UPGRADE.
@@ -3155,16 +3148,14 @@ package-menu--prompt-transaction-p
 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)
-    "? ")))
+    (when delete
+      (format "Packages to delete: %s. " (package-menu--list-to-prompt delete)))
+    (when install
+      (format "Packages to install: %s. " (package-menu--list-to-prompt install)))
+    (when upgrade
+      (format "Packages to upgrade: %s. " (package-menu--list-to-prompt upgrade)))
+    "Proceed? ")))
+
 
 (defun package-menu--partition-transaction (install delete)
   "Return an alist describing an INSTALL DELETE transaction.
@@ -3248,25 +3239,24 @@ package-menu-execute
       (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)))
+               (concat "[ "
+                       (when .delete
+                         (format "Delete %d " (length .delete)))
+                       (when .install
+                         (format "Install %d " (length .install)))
+                       (when .upgrade
+                         (format "Upgrade %d " (length .upgrade)))
                        "]")))
-          (message (replace-regexp-in-string "__" "ing" message-template) "started")
+          (message "Operation %s started" message-template)
           ;; 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.  Packages that are no longer needed: %d.  Type `%s' to remove them"
+                         (length removable)
+                         (substitute-command-keys "\\[package-autoremove]"))
+              (message "Operation %s finished" message-template))))))))
 
 (defun package-menu--version-predicate (A B)
   (let ((vA (or (aref (cadr A) 1)  '(0)))
@@ -3333,11 +3323,11 @@ 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."
-      (length upgrades)
-      (if (= (length upgrades) 1) "" "s")
-      (substitute-command-keys "\\[package-menu-mark-upgrades]")
-      (if (= (length upgrades) 1) "it" "them"))))
+    (message "Packages that can be upgraded: %d; type `%s' to mark for upgrading."
+             (length upgrades)
+             (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 #2.3: Type: text/html, Size: 2240 bytes --]

^ permalink raw reply related	[flat|nested] 41+ messages in thread

* Re: package.el strings
  2018-06-20 16:19                                         ` Jean-Christophe Helary
@ 2018-06-24 15:37                                           ` Noam Postavsky
  2018-06-24 21:57                                             ` Jean-Christophe Helary
  0 siblings, 1 reply; 41+ messages in thread
From: Noam Postavsky @ 2018-06-24 15:37 UTC (permalink / raw)
  To: Jean-Christophe Helary; +Cc: Eli Zaretskii, Emacs developers

On 20 June 2018 at 12:19, Jean-Christophe Helary <brandelune@gmail.com> wrote:
> Et voilà !
>
> ===========================
> * lisp/emacs-lisp/package.el: reformat message strings for future l10n
> ===========================

Just one more question

> @@ -1015,6 +1017,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"))

That comment is basically aspirational, right? That is, currently the
terminating comment can't be a generic string. So I would rephrase it
as

    ;; The terminating comment format could be extended to accept a
    ;; generic string that is not in English.

If you agree, I'll make that change and push.



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: package.el strings
  2018-06-24 15:37                                           ` Noam Postavsky
@ 2018-06-24 21:57                                             ` Jean-Christophe Helary
  2018-06-24 23:09                                               ` Noam Postavsky
  0 siblings, 1 reply; 41+ messages in thread
From: Jean-Christophe Helary @ 2018-06-24 21:57 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: Eli Zaretskii, Emacs developers

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



> On Jun 25, 2018, at 0:37, Noam Postavsky <npostavs@gmail.com> wrote:
> 
> On 20 June 2018 at 12:19, Jean-Christophe Helary <brandelune@gmail.com> wrote:
>> Et voilà !
>> 
>> ===========================
>> * lisp/emacs-lisp/package.el: reformat message strings for future l10n
>> ===========================
> 
> Just one more question
> 
>> @@ -1015,6 +1017,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"))
> 
> That comment is basically aspirational, right? That is, currently the
> terminating comment can't be a generic string. So I would rephrase it
> as
> 
>    ;; The terminating comment format could be extended to accept a
>    ;; generic string that is not in English.
> 
> If you agree, I'll make that change and push.


I don't understand what you mean by "can't be a generic string", for ex can't it be (concat ";;; " file-name ".el <<<<<<<") ?
But regarding the comment itself, I'm OK with anything that you think is better. :)

Jean-Christophe Helary
-----------------------------------------------
http://mac4translators.blogspot.com @brandelune



[-- Attachment #2: Type: text/html, Size: 3583 bytes --]

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: package.el strings
  2018-06-24 21:57                                             ` Jean-Christophe Helary
@ 2018-06-24 23:09                                               ` Noam Postavsky
  2018-06-25  1:39                                                 ` Jean-Christophe Helary
  0 siblings, 1 reply; 41+ messages in thread
From: Noam Postavsky @ 2018-06-24 23:09 UTC (permalink / raw)
  To: Jean-Christophe Helary; +Cc: Eli Zaretskii, Emacs developers

On 24 June 2018 at 17:57, Jean-Christophe Helary <brandelune@gmail.com> wrote:

> I don't understand what you mean by "can't be a generic string", for ex
> can't it be (concat ";;; " file-name ".el <<<<<<<") ?

I mean, a package author can't end their package with a string like
that right now; Emacs would signal an error.



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: package.el strings
  2018-06-24 23:09                                               ` Noam Postavsky
@ 2018-06-25  1:39                                                 ` Jean-Christophe Helary
  2018-06-25 23:22                                                   ` Noam Postavsky
  0 siblings, 1 reply; 41+ messages in thread
From: Jean-Christophe Helary @ 2018-06-25  1:39 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: Eli Zaretskii, Emacs developers

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



> On Jun 25, 2018, at 8:09, Noam Postavsky <npostavs@gmail.com> wrote:
> 
> On 24 June 2018 at 17:57, Jean-Christophe Helary <brandelune@gmail.com> wrote:
> 
>> I don't understand what you mean by "can't be a generic string", for ex
>> can't it be (concat ";;; " file-name ".el <<<<<<<") ?
> 
> I mean, a package author can't end their package with a string like
> that right now; Emacs would signal an error.

Yes, I understand that :) Thank you.


Jean-Christophe Helary
-----------------------------------------------
http://mac4translators.blogspot.com @brandelune



[-- Attachment #2: Type: text/html, Size: 2280 bytes --]

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: package.el strings
  2018-06-25  1:39                                                 ` Jean-Christophe Helary
@ 2018-06-25 23:22                                                   ` Noam Postavsky
  2018-06-27  4:21                                                     ` Jean-Christophe Helary
  0 siblings, 1 reply; 41+ messages in thread
From: Noam Postavsky @ 2018-06-25 23:22 UTC (permalink / raw)
  To: Jean-Christophe Helary; +Cc: Eli Zaretskii, Emacs developers

I've pushed to master.

[1: 61f73703c7]: 2018-06-25 19:18:55 -0400
  Reformat package.el message strings for future l10n
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=61f73703c74756e6963cc622f03bcc6938ab71b2



^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: package.el strings
  2018-06-25 23:22                                                   ` Noam Postavsky
@ 2018-06-27  4:21                                                     ` Jean-Christophe Helary
  2018-06-27 18:32                                                       ` Noam Postavsky
  0 siblings, 1 reply; 41+ messages in thread
From: Jean-Christophe Helary @ 2018-06-27  4:21 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: Eli Zaretskii, Emacs developers

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

Noam,

This patch was also related to bug#27033, since it's a wrong plural form that triggered the rewriting of the strings.

I don't think I have the ability to close the bug as fixed, what is the procedure for that?

> On Jun 26, 2018, at 8:22, Noam Postavsky <npostavs@gmail.com> wrote:
> 
> I've pushed to master.
> 
> [1: 61f73703c7]: 2018-06-25 19:18:55 -0400
>  Reformat package.el message strings for future l10n
>  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=61f73703c74756e6963cc622f03bcc6938ab71b2

Jean-Christophe Helary
-----------------------------------------------
http://mac4translators.blogspot.com @brandelune



[-- Attachment #2: Type: text/html, Size: 2395 bytes --]

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: package.el strings
  2018-06-27  4:21                                                     ` Jean-Christophe Helary
@ 2018-06-27 18:32                                                       ` Noam Postavsky
  0 siblings, 0 replies; 41+ messages in thread
From: Noam Postavsky @ 2018-06-27 18:32 UTC (permalink / raw)
  To: Jean-Christophe Helary; +Cc: Eli Zaretskii, Emacs developers

On 27 June 2018 at 00:21, Jean-Christophe Helary <brandelune@gmail.com> wrote:
> Noam,
>
> This patch was also related to bug#27033, since it's a wrong plural form
> that triggered the rewriting of the strings.
>
> I don't think I have the ability to close the bug as fixed, what is the
> procedure for that?

You (or anyone) can close it by sending a message to 27033-done@debbugs.gnu.org
If the message starts with

    Version: 27.1

then it will be marked as fixed in 27.1 as well. See
https://debbugs.gnu.org/Developer.html for details.

You can also change bug attributes by sending commands to
control@debbugs.gnu.org, as described in
https://debbugs.gnu.org/server-control.html.
This is what the debbugs.el GNU ELPA package does.
https://elpa.gnu.org/packages/debbugs.html



^ permalink raw reply	[flat|nested] 41+ messages in thread

end of thread, other threads:[~2018-06-27 18:32 UTC | newest]

Thread overview: 41+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-24  5:08 package.el strings Jean-Christophe Helary
2017-05-24 11:00 ` Jean-Christophe Helary
2017-05-24 12:04 ` Dmitry Gutov
2017-05-24 12:07   ` Jean-Christophe Helary
2017-05-24 12:36   ` Yuri Khan
2017-05-24 14:16   ` Richard Stallman
2017-07-14  2:53 ` Jean-Christophe Helary
2017-07-15 12:52   ` Eli Zaretskii
2017-07-15 14:48     ` Jean-Christophe Helary
2017-07-16 13:55       ` Jean-Christophe Helary
2017-07-16 14:16         ` Eli Zaretskii
2017-07-16 14:35       ` Eli Zaretskii
2017-07-16 14:37         ` Jean-Christophe Helary
2017-07-17 15:28       ` Jean-Christophe Helary
2017-07-22  9:23         ` Eli Zaretskii
2018-04-18  6:38           ` Jean-Christophe Helary
2018-04-26  1:10             ` Noam Postavsky
2018-04-26  6:31               ` Jean-Christophe Helary
2018-04-26 11:28                 ` Noam Postavsky
2018-04-26 13:32                   ` Jean-Christophe Helary
2018-04-28 22:11                     ` Noam Postavsky
2018-04-28 23:46                       ` Jean-Christophe Helary
2018-05-29 22:38                         ` Noam Postavsky
2018-05-29 22:46                           ` Jean-Christophe Helary
2018-06-17 14:02                           ` Jean-Christophe Helary
2018-06-17 14:33                             ` Eli Zaretskii
2018-06-17 14:45                               ` Jean-Christophe Helary
2018-06-17 14:50                                 ` Eli Zaretskii
2018-06-17 14:58                                   ` Jean-Christophe Helary
2018-06-20 13:53                                     ` Jean-Christophe Helary
2018-06-20 15:54                                       ` Eli Zaretskii
2018-06-20 16:19                                         ` Jean-Christophe Helary
2018-06-24 15:37                                           ` Noam Postavsky
2018-06-24 21:57                                             ` Jean-Christophe Helary
2018-06-24 23:09                                               ` Noam Postavsky
2018-06-25  1:39                                                 ` Jean-Christophe Helary
2018-06-25 23:22                                                   ` Noam Postavsky
2018-06-27  4:21                                                     ` Jean-Christophe Helary
2018-06-27 18:32                                                       ` Noam Postavsky
2018-04-29 19:43                       ` Stefan Monnier
2018-04-26 13:40               ` Jean-Christophe Helary

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).