Re: package.el strings

[Jean-Christophe Helary]

On Apr 26, 2018, at 10:10, Noam Postavsky <address@hidden> 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 

package.el_0426.diff
Description: Binary data

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