Re: [PATCH] git.el: Add a git-grep command

[Xavier Maillard <xma@gnu.org>]

   Xavier Maillard <xma@gnu.org> writes:

   > Hi,
   >
   >    Here is a modification with inclusion of git-grep only when the grep
   >    library is available. 
   >
   >    +(require 'grep () t)
   >
   >    +(when (featurep 'grep)
   >    +  (defvar git-grep-history nil)
   >    +
   >    +  (defun git-grep (regexp &optional files dir)
   >
   > Why not just do something like this ?
   >
   > (when (require 'grep () t)
   >       (defvar ...)
   >       (defun git-grep ...))

   Because I wanted require to stay on top of the file, but I didn't want
   to put the rest of the git-grep stuff there.

Good point. Though, you can still "embed" the require form
directly into the defun. This is thing I have already seen in the
past. Dunno if it is a convention or a coding style but something
like:

(defun git-grep ()
 "Docstring"
 (interactive)
 (when (require 'grep nil t)
       here the rest
  ))

is doable too. Maybe the if-else form would be better though with
an else clause to (error "No grep package foud.").

Well just kidding, your patch is okay ;)

	Xavier
-- 
http://www.gnu.org
http://www.april.org
http://www.lolica.org