Ensure we copy strings before creating buttons

This should fix #74 and #68, as well as abo-abo/swiper#1380.

make-text-button calls add-text-properties, which mutates the string
we passed in. If we also use this string with find-file, we end up
calling find-file with propertized strings.

Perhaps recentf should be more robust against propertized strings, but
fixing it in helpful is reasonable and clean.

2 files changed, 69 insertions, 45 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 3a65b36..140da48 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,25 @@ Ensure docstring references to Info nodes are converted to buttons.
 Helpful now shows all aliases for callables and variables, and
 highlights which aliases are obsolete.
 
+## recentf bug
+
+Helpful had an issue where it would call find-file with propertized
+strings. This broke various recentf features.
+
+This has been fixed, and you can check if you're running a fixed
+version by seeing whether you have a `helpful--button` function
+defined. If you do, your version is new enough.
+
+You will aso need to edit your `~/.emacs.d/recentf` and
+`recentf-save.el` to remove any lines that start with a `#`:
+
+``` emacs-lisp
+#("/usr/share/emacs/25.3.50/lisp/frame.el.gz" 0 41 (button (t) category helpful-navigate-button-button path #0 position 2815))
+```
+
+Otherwise, you will get `Invalid read syntax: "#"` when starting
+Emacs.
+
 # v0.4
 
 You can now enable edebug directly from helpful buffers!
diff --git a/helpful.el b/helpful.el
index 09e0371..a56852b 100644
--- a/helpful.el
+++ b/helpful.el
@@ -110,6 +110,16 @@ with double-quotes."
     (cl-prettyprint value)
     (s-trim (buffer-string))))
 
+(defun helpful--button (text type &rest properties)
+  ;; `make-text-button' mutates our string to add properties. Copy
+  ;; TEXT to prevent mutating our arguments, and to support 'pure'
+  ;; strings, which are read-only.
+  (setq text (substring-no-properties text))
+  (apply #'make-text-button
+         text nil
+         :type type
+         properties))
+
 (defun helpful--canonical-symbol (sym callable-p)
   "If SYM is an alias, return the underlying symbol.
 Return SYM otherwise."
@@ -149,12 +159,9 @@ Return SYM otherwise."
   (let ((obsolete-info (if callable-p
                            (get sym 'byte-obsolete-info)
                          (get sym 'byte-obsolete-variable)))
-        (sym-button (make-text-button
-                     ;; symbol-name can return a pure string, e.g. for
-                     ;; 'report-errors, so take a copy so we can add
-                     ;; properties to it.
-                     (substring (symbol-name sym)) nil
-                     :type 'helpful-describe-exactly-button
+        (sym-button (helpful--button
+                     (symbol-name sym)
+                     'helpful-describe-exactly-button
                      'symbol sym
                      'callable-p callable-p)))
     (cond
@@ -329,9 +336,9 @@ or disable if already enabled."
 
 (defun helpful--navigate-button (path &optional pos)
   "Return a button that opens PATH and puts point at POS."
-  (make-text-button
-   (abbreviate-file-name path) nil
-   :type 'helpful-navigate-button
+  (helpful--button
+   (abbreviate-file-name path)
+   'helpful-navigate-button
    'path path
    'position pos))
 
@@ -493,9 +500,9 @@ blank line afterwards."
        ;; Only create a link if this is a symbol that is bound as a
        ;; variable or callable.
        (if (or (boundp sym) (fboundp sym))
-           (make-text-button
-            sym-name nil
-            :type 'helpful-describe-button
+           (helpful--button
+            sym-name
+            'helpful-describe-button
             'symbol sym)
          (propertize sym-name
                      'face 'font-lock-constant-face))))
@@ -518,9 +525,9 @@ blank line afterwards."
        (concat
         "Info node"
         space
-        (make-text-button
-         info-node nil
-         :type 'helpful-info-button
+        (helpful--button
+         info-node
+         'helpful-info-button
          'info-node info-node))))
    docstring
    t t))
@@ -898,9 +905,9 @@ state of the current symbol."
       (when (helpful--in-manual-p helpful--sym)
         (insert
          "\n\n"
-         (make-text-button
-          "View in manual" nil
-          :type 'helpful-manual-button
+         (helpful--button
+          "View in manual"
+          'helpful-manual-button
           'symbol helpful--sym))))
 
     (when (not helpful--callable-p)
@@ -913,24 +920,24 @@ state of the current symbol."
          "\n\n")
         (when (memq (helpful--sym-value helpful--sym buf) '(nil t))
           (insert
-           (make-text-button
-            "Toggle" nil
-            :type 'helpful-toggle-button
+           (helpful--button
+            "Toggle"
+            'helpful-toggle-button
             'symbol helpful--sym
             'buffer buf)
            " "))
         (insert
-         (make-text-button
-          "Set" nil
-          :type 'helpful-set-button
+         (helpful--button
+          "Set"
+          'helpful-set-button
           'symbol helpful--sym
           'buffer buf))
         (when (custom-variable-p helpful--sym)
           (insert
            " "
-           (make-text-button
-            "Customize" nil
-            :type 'helpful-customize-button
+           (helpful--button
+            "Customize"
+            'helpful-customize-button
             'symbol helpful--sym)))))
 
     ;; Show keybindings.
@@ -956,9 +963,9 @@ state of the current symbol."
       (t
        "Could not find source file."))
      "\n\n"
-     (make-text-button
-      "Find all references" nil
-      :type 'helpful-all-references-button
+     (helpful--button
+      "Find all references"
+      'helpful-all-references-button
       'symbol helpful--sym
       'callable-p helpful--callable-p))
 
@@ -985,23 +992,21 @@ state of the current symbol."
         (insert (helpful--heading "\n\nDebugging\n")))
       (when can-edebug
         (insert
-         (make-text-button
+         (helpful--button
           (if (helpful--edebug-p helpful--sym)
               "Disable edebug"
             "Enable edebug")
-          nil
-          :type 'helpful-edebug-button
+          'helpful-edebug-button
           'symbol helpful--sym)))
       (when can-trace
         (when can-edebug
           (insert " "))
         (insert
-         (make-text-button
+         (helpful--button
           (if (trace-is-traced helpful--sym)
               "Disable tracing"
             "Enable tracing")
-          nil
-          :type 'helpful-trace-button
+          'helpful-trace-button
           'symbol helpful--sym)))
 
       (when (and
@@ -1011,18 +1016,18 @@ state of the current symbol."
 
       (when can-disassemble
         (insert
-         (make-text-button
-          "Disassemble" nil
-          :type 'helpful-disassemble-button
+         (helpful--button
+          "Disassemble"
+          'helpful-disassemble-button
           'symbol helpful--sym)))
 
       (when can-forget
         (when can-disassemble
           (insert " "))
         (insert
-         (make-text-button
-          "Forget" nil
-          :type 'helpful-forget-button
+         (helpful--button
+          "Forget"
+          'helpful-forget-button
           'symbol helpful--sym
           'callable-p helpful--callable-p))))
 
@@ -1051,9 +1056,9 @@ state of the current symbol."
          "C code is not yet loaded."
          'face 'font-lock-comment-face)
         "\n\n"
-        (make-text-button
-         "Set C source directory" nil
-         :type 'helpful-c-source-directory)))
+        (helpful--button
+         "Set C source directory"
+         'helpful-c-source-directory)))
       (t
        (helpful--syntax-highlight
         (format ";; Source file is unknown\n")))))