emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: package.el strings


From: Noam Postavsky
Subject: Re: package.el strings
Date: Wed, 25 Apr 2018 21:10:05 -0400

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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]