Re: never use `eval'

["Pascal J. Bourguignon" <pjb@informatimago.com>]

Emanuel Berg <embe8573@student.uu.se> writes:

> "Pascal J. Bourguignon" <pjb@informatimago.com>
> writes:
>
>> It's always a bad idea to use eval, because eval
>> evalutes in the nil environment, not in the local
>> lexical envrionment.
>
> I have used `eval' eight times in my current setup of
> 75 files.
>
> Tho some of this code I wrote several years ago, I'd
> be happy to correct it. Any hints - general or
> specific - are appreciated.
>
> ;; From: http://user.it.uu.se/~embe8573/conf/emacs-init/faces.el
>
> (defun set-color-face (name front &optional bold back)
>   (eval `(defvar ,(make-symbol name)))
>   (eval `(setq ,(intern name)
>                `((t (:foreground ,front
>                      :background ,back
>                      :bold       ,bold) )))))

As mentionned, eval setq can be replaced by (setf symbol-value) or even
set, if you like archaic forms.

For defvar, and in general for operations like this set-color-face that
_defines_ a symbol to bind it to some kind of object, what you should do
is to write a define macro instead.  

Also, your code is higly suspicious: why do you use make-symbol?  This
will create a _different_ symbol!

     (eq (make-symbol "x") (intern "x")) --> nil

therefore your defvar will be totally useless!

Instead, you can just give a symbol to the macro.

    (defmacro define-color-face (name front &optional bold back)
       `(progn
          (defvar ,name)
          (setq ,name '((t (:foreground ,front
                            :background ,back
                            :bold ,bold))))))

    (define-color-face cf-black "black")
    cf-black --> ((t (:foreground "black" :background nil :bold nil)))



> (defun do-repeat-complex-command ()
>   (interactive)
>   (eval (car command-history) ))

This use is acceptable.


> ;; From: http://user.it.uu.se/~embe8573/conf/emacs-init/isbn.el
>
> (defun book-page-to-bibtex ()
>   (interactive)
>   (save-excursion
>     (goto-char (point-min))
>     (let ((title (get-title))
>           (data  (mapcar 'key-value
>                          '("authors?" "publisher" "published" "isbn-10")) ))
>       (eval `(create-book nil ,title ,@data) )))) ; don't INSERT

Just use apply!  

        (apply (function create-book) nil title data)


> ;; From: http://user.it.uu.se/~embe8573/conf/emacs-init/match-data-format.el
>
> (defun match-data-format (data match format-str)
>   (save-match-data
>     (string-match match data)
>     (eval `(message format-str ,@(make-match-list 1 data) ))))

Use apply.


> ;; From: http://user.it.uu.se/~embe8573/conf/emacs-init/wrap-search.el
>
> (defun wrap-search-again (prefix)
>   "Search again for the most recent search string of `wrap-search'.
> Use \\[universal-argument] \(to set the PREFIX\) to toggle case sensitiveness."
>   (interactive "p")
>   (let ((cmd (cl-dolist (cmd command-history)
>                (if (and (eq (car cmd) 'wrap-search)
>                         (not (string= (cl-caddr cmd) "")) )
>                    (cl-return cmd) ))))
>     (if cmd
>         (if (eq prefix 4)
>             (let*((old-prefix (cadr  cmd))
>                   (search-str (cl-caddr cmd))
>                   (new-prefix (if (eq old-prefix 4) 1 4))
>                   (final-cmd  `(wrap-search ,new-prefix ,search-str)) )
>               (setq command-history (cons final-cmd command-history))
>               (eval final-cmd) )

Use apply.

>           (eval cmd) )

Ok for eval.

>       (message " No previous search.") )))


Notice:

    command-history is a variable defined in `C source code'.
    Its value is shown below.

    Documentation:
    List of recent commands that read arguments from terminal.
    Each command is represented as a form to evaluate.

So clearly, (eval (first command-history)) is vetted.


-- 
__Pascal Bourguignon__                 http://www.informatimago.com/
“The factory of the future will have only two employees, a man and a
dog. The man will be there to feed the dog. The dog will be there to
keep the man from touching the equipment.” -- Carl Bass CEO Autodesk