[nongnu] elpa/opam-switch-mode 22b852a8a4 1/4: Minor tweaks and cleanups

[ELPA Syncer]

branch: elpa/opam-switch-mode
commit 22b852a8a4427a4999f67a96b20a101e1f459894
Author: Stefan Monnier <monnier@iro.umontreal.ca>
Commit: Stefan Monnier <monnier@iro.umontreal.ca>

    Minor tweaks and cleanups
    
    * .gitignore: New file.
    
    * README.md: Add link to NonGNU and insist less on the use
    of `use-package`.
    
    * opam-switch-mode.el: Remove redundant `:group` arguments.
    (opam-switch-change-opam-switch-hook): Fix `:type`.
    (opam-switch--get-root): Avoid unneeded `setq`.
    (opam-switch--root): Include the error info in the error message.
    (opam-switch--get-switches): Change regexp under the assumption that
    switch names don't include newlines nor TABs, and throw away the `.*$`
    tail which just wastes time.  Preserve the order of the switches.
    (opam-switch-set-switch): Comment out broken autoload cookie.
    (opam-switch-mode-map): Rename from `opam-switch--mode-keymap` to
    follow the convention.
    (opam-switch--setup-opam-switch-mode): Adjust accordingly.
    (opam-switch--menu-items): Let backquote worry about `vconcat`.
    (opam-switch-mode): Remove redundant keywords.
    Try and reset the environment when turning the mode off.
    (opam-switch--get-root, opam-switch-mode): Fix incorrect markup.
---
 .gitignore          |  3 +++
 README.md           | 15 +++++++++++---
 opam-switch-mode.el | 60 ++++++++++++++++++++++++++---------------------------
 3 files changed, 44 insertions(+), 34 deletions(-)

diff --git a/.gitignore b/.gitignore
new file mode 100644
index 0000000000..18c633ef0c
--- /dev/null
+++ b/.gitignore
@@ -0,0 +1,3 @@
+/opam-switch-mode-autoloads.el
+/opam-switch-mode-pkg.el
+*.elc
diff --git a/README.md b/README.md
index 05c5e410a8..8369676913 100644
--- a/README.md
+++ b/README.md
@@ -1,5 +1,6 @@
 # opam-switch-mode
 
+[![NonGNU 
ELPA](https://elpa.nongnu.org/nongnu/opam-switch-mode-badge.svg)](https://elpa.nongnu.org/nongnu/opam-switch-mode.html)
 
[![MELPA](https://melpa.org/packages/opam-switch-mode-badge.svg)](https://melpa.org/#/opam-switch-mode)
 
 Provide a command `opam-switch-set-switch` to change the opam switch
@@ -13,9 +14,13 @@ entry "reset" to reset the environment to the state when 
Emacs was started.
 
 ## Installing `opam-switch-mode`
 
-The recommended way to install this mode relies on the
-[MELPA](https://melpa.org/) repository of Emacs packages, along with the
-[`use-package`](https://github.com/jwiegley/use-package) macro.
+We recommend to install this mode from either the 
+[NonGNU ELPA](https://elpa.nongnu.org/) or the
+[MELPA](https://melpa.org/) repository of Emacs packages.
+
+If you use the
+[`use-package`](https://github.com/jwiegley/use-package) macro, the
+recommended configuration is:
 Assuming you have already set up those in your `.emacs`, just write:
 
 ```elisp
@@ -25,6 +30,10 @@ Assuming you have already set up those in your `.emacs`, 
just write:
   (coq-mode . opam-switch-mode))
 ```
 
+If you don't use `use-package`, do the following instead:
+
+    (add-hook 'coq-mode-hook #'opam-switch-mode)
+
 so that the minor mode is automatically enabled when `coq-mode` is on,
 see also [`opam-switch-mode` aware modes](#opam-switch-mode-aware-modes).
 
diff --git a/opam-switch-mode.el b/opam-switch-mode.el
index 7b7ef88ab4..8fbbb3c6f3 100644
--- a/opam-switch-mode.el
+++ b/opam-switch-mode.el
@@ -60,14 +60,12 @@
 
 (defcustom opam-switch-program-name "opam"
   "Name or path of the opam binary."
-  :group 'opam-switch
   :type 'string)
 
 (defcustom opam-switch-common-options ()
   "Options to be supplied to every opam invocation.
 This must be a list of strings, each member string an option
 accepted by opam."
-  :group 'opam-switch
   :type '(repeat string))
 
 (defcustom opam-switch-common-environment
@@ -81,21 +79,19 @@ element should have the form of ENVVARNAME=VALUE.
 The process environment must ensure that output is plain ascii
 without color, non-ascii arrow symbols and that it is in English.
 Otherwise parsing the output of opam commands won't work."
-  :group 'opam-switch
   :type '(repeat string))
 
 (defcustom opam-switch-change-opam-switch-hook nil
   "Hook run when the opam switch changes.
 This is used, for instance, to let Proof General kill the coq
 background process when the opam switch changes."
-  :group 'opam-switch
-  :type '(repeat function))
+  :type 'hook)
 
 ;;; Code:
 
 (defun opam-switch--run-command-without-stderr (sub-cmd
-                                        &optional switch sexp
-                                        &rest args)
+                                                &optional switch sexp
+                                                &rest args)
   "Run opam SUB-CMD, without capturing error output.
 Run opam SUB-CMD with additional arguments and insert the output
 in the current buffer at point.  Error output (stderr) is
@@ -117,7 +113,7 @@ therfore respect file-name handlers specified via
       (push "--sexp" options))
     ;; (message "run %s %s %s" opam-switch-program-name sub-cmd options)
     (apply #'process-file opam-switch-program-name
-               nil '(t nil) nil sub-cmd options)))
+           nil '(t nil) nil sub-cmd options)))
 
 (defun opam-switch--command-as-string (sub-cmd &optional switch sexp &rest 
args)
   "Run opam SUB-CMD, with additional arguments, without capturing stderr.
@@ -138,14 +134,14 @@ This function  `opam-switch--run-command-without-stderr'."
 
 (defun opam-switch--get-root ()
   "Get the opam root directory.
-This function gets the opam variable 'root'.
+This function gets the opam variable `root'.
 This function should not be called directly; see `opam-switch--root'."
   (let ((root (opam-switch--command-as-string "var" nil nil "root")))
     (unless root
       (error "Command 'opam var root' failed"))
-    (when (eq (aref root (1- (length root))) ?\n)
-      (setq root (substring root 0 -1)))
-    root))
+    (if (eq (aref root (1- (length root))) ?\n)
+        (substring root 0 -1)
+      root)))
 
 (defvar opam-switch--root nil
   "The opam root directory.")
@@ -154,9 +150,9 @@ This function should not be called directly; see 
`opam-switch--root'."
   "Set variable `opam-switch--root' once, if possible, and return it."
   (or opam-switch--root
       (let ((result
-             (condition-case _sig
+             (condition-case err
                  (opam-switch--get-root)
-               (file-missing (error "Cannot run opam") nil))))
+               (file-missing (error "Cannot run opam: %S" err) nil))))
         (when result
           (setq opam-switch--root result)))))
 
@@ -184,10 +180,10 @@ This function should not be called directly; see 
`opam-switch--root'."
         ;; opam exit status different from 0 -- some error occured
         (error "Command 'opam switch' failed"))
       (goto-char (point-min))
-      (forward-line)
-      (while (re-search-forward "^.. *\\([^ ]*\\).*$" nil t)
+      (forward-line)                    ;Skip first (header) line.
+      (while (re-search-forward "^.. *\\([^ \n\t]+\\)" nil t)
         (push (match-string 1) opam-switches))
-      opam-switches)))
+      (nreverse opam-switches))))
 
 (defvar opam-switch--switch-history nil
   "Minibuffer history list for `opam-switch--set-switch'.")
@@ -301,12 +297,15 @@ not any other shells outside Emacs."
       (opam-switch--set-env opam-env)))
   (run-hooks 'opam-switch-change-opam-switch-hook))
 
-;;;###autoload
+;; FIXME: This autoload didn't work.  If we want to autoload the command,
+;; then we need to place the autoload on the command itself (and arguably
+;; rename it without the "--").
+;; ;;;###autoload
 (defalias 'opam-switch-set-switch #'opam-switch--set-switch)
 
 ;;; minor mode, keymap and menu
 
-(defvar opam-switch--mode-keymap (make-sparse-keymap)
+(defvar opam-switch-mode-map (make-sparse-keymap)
   "Keymap for `opam-switch-mode'.")
 
 (defun opam-switch--menu-items ()
@@ -321,11 +320,10 @@ not any other shells outside Emacs."
    ;; then the list with all the real opam switches
    (mapcar
     (lambda (switch)
-      (vconcat
-       `(,switch
-         (opam-switch--set-switch ,switch)
-         :active t
-         :help ,(concat "select opam switch \"" switch "\""))))
+      `[,switch
+        (opam-switch--set-switch ,switch)
+        :active t
+        :help ,(concat "select opam switch \"" switch "\"")])
     (opam-switch--get-switches))
    ;; now reset as last element
    '(
@@ -342,26 +340,26 @@ Note that the code for setting up the keymap and running 
the hook
 is automatically created by `define-minor-mode'."
   (easy-menu-define
     opam-switch--mode-menu
-    opam-switch--mode-keymap
+    opam-switch-mode-map
     "opam mode menu"
+    ;; FIXME: Use `:filter'?
     (cons "opam-switch"
           (opam-switch--menu-items))))
 
 ;;;###autoload
 (define-minor-mode opam-switch-mode
   "Toggle opam-switch mode.
-The mode can be enabled only if opam is found and 'opam var root' succeeds."
-  :init-value nil
+The mode can be enabled only if opam is found and \"opam var root\" succeeds."
+  ;; FIXME: Should we include the current switch in the lighter?
   :lighter " OPSW"
-  :keymap opam-switch--mode-keymap
-  :group 'opam-switch
-  (when opam-switch-mode
+  (if (not opam-switch-mode)
+      (opam-switch--reset-env)
     (condition-case sig
         (progn
           (opam-switch--root)
           (opam-switch--setup-opam-switch-mode))
       (t (setq opam-switch-mode nil)
-         (message "Opam-Switch mode disabled %s" (pp-to-string sig))))))
+         (message "Opam-Switch mode disabled: %s" (pp-to-string sig))))))
 
 (provide 'opam-switch-mode)