Home
Reading
Searching
Subscribe
Sponsors
Statistics
Posting
Contact
Spam
Lists
Links
About
Hosting
Filtering
Features Download
Marketing
Archives
FAQ
Blog
 
Gmane
From: Roger Crew <crew-UfE5FQABeHN2Qaki92YDXw <at> public.gmane.org>
Subject: [PATCH] remove runtime dependence on cl.el
Newsgroups: gmane.comp.version-control.git.magit
Date: Wednesday 28th April 2010 00:07:34 UTC (over 7 years ago)
This patch removes the runtime dependences on the cl package.
Luckily, there weren't too many of these.  

From the GNU emacs lisp manual (Appendix D.1 coding conventions)

   * Please don't require the `cl' package of Common Lisp extensions
     at run time.  Use of this package is optional, and it is not part
     of the standard Emacs namespace.  If your package loads `cl' at
     run time, that could cause name clashes for users who don't use
     that package.

     However, there is no problem with using the `cl' package at
     compile time, with `(eval-when-compile (require 'cl))'.  That's
     sufficient for using the macros in the `cl' package, because the
     compiler expands them before generating the byte-code.

There's also the small matter that many of the function implementations 
in cl, striving for the full generality of Common Lisp (much of which
is completely useless in Emacs), turn out to be horrible.

E.g., for a fun time, dig down through

      (find-if pred list :from-end t), 

and look at what it ACTUALLY does when you finish macroexpanding
everything.  It tests *every* element of the list against the
predicate, not just the rightmost ones stopping when it finds the
first match.  Once it determines the rightmost match, it then retains
NOT the element itself, but its *ordinal* position N, which then gets
used in (elt list N), meaning ANOTHER listwalk, just to get the
element back in order to return it.  Nor is the byte-compiler anywhere
near smart enough to optimize this away (I'm not sure *any* compiler
would be...)

I'll grant cl has some useful macros in it, but it comes bundled with a
lot of crap and you need to be really careful about what you use.  For
many things, you're better off rolling your own functionality using
the standard routines available (e.g., while, mapcar, and reverse are
all written directly in C).

And you most definitely do NOT want to be foisting the crap on
everybody else, hence the need to keep it out of the runtime.

Meanwhile, here's The Patch:

	Modified magit.el
diff --git a/magit.el b/magit.el
index dcacba5..4d89754 100644
--- a/magit.el
+++ b/magit.el
@@ -55,8 +55,8 @@
 ;; - Amending commits other than HEAD.
 ;; - 'Subsetting', only looking at a subset of all files.
 
-(require 'cl)
-(require 'parse-time)
+(eval-when-compile (require 'cl))
+(eval-when-compile (require 'parse-time))
 (require 'log-edit)
 (require 'easymenu)
 (require 'diff-mode)
@@ -385,7 +385,7 @@ Many Magit faces inherit from this one by default."
 		 (let ((sub (magit-remove-conflicts
 			     (mapcar (lambda (entry)
 				       (let ((dir (directory-file-name
-						   (subseq entry 0 (- (length key))))))
+						   (substring entry 0 (- (length key))))))
 					 (cons (concat (file-name-nondirectory dir) "/" key)
 					       entry)))
 				     value))))
@@ -646,7 +646,7 @@ BODY must leave point at the end of the created
section.
 
 If TYPE is nil, the section won't be highlighted."
   (declare (indent 2))
-  (let ((s (gensym)))
+  (let ((s (make-symbol "*section*")))
     `(let* ((,s (magit-new-section ,title ,type))
 	    (magit-top-section ,s))
        (setf (magit-section-beginning ,s) (point))
@@ -691,12 +691,12 @@ If TYPE is nil, the section won't be highlighted."
   "Find in subsection of section TOP the section at the path PATH."
   (if (null path)
       top
-    (let ((sec (find-if (lambda (s) (equal (car path)
-					   (magit-section-title s)))
-			(magit-section-children top))))
-      (if sec
-	  (magit-find-section (cdr path) sec)
-	nil))))
+    (let ((secs (magit-section-children top)))
+      (while (and secs (not (equal (car path) 
+				   (magit-section-title (car secs)))))
+	(setq secs (cdr secs)))
+      (and (car secs) 
+	   (magit-find-section (cdr path) (car secs))))))
 
 (defun magit-section-path (section)
   "Return the path of SECTION."
@@ -906,7 +906,10 @@ Default value for TOP is `magit-top-section'"
 (defun magit-section-any-hidden (section)
   "Return true if SECTION or any of its children is hidden."
   (or (magit-section-hidden section)
-      (some #'magit-section-any-hidden (magit-section-children section))))
+      (let ((kids (magit-section-children section)))
+	(while (and kids (not (magit-section-any-hidden (car kids))))
+	  (setq kids (cdr kids)))
+	kids)))
 
 (defun magit-section-collapse (section)
   "Show SECTION and hide all its children."
@@ -1130,8 +1133,8 @@ where SECTION-TYPE describe section where BODY will
be run."
   (declare (indent 1))
   (let ((section (car head))
 	(info (cadr head))
-	(type (gensym))
-	(context (gensym))
+	(type (make-symbol "*type*"))
+	(context (make-symbol "*context*"))
 	(opname (caddr head)))
     `(let* ((,section (magit-current-section))
 	    (,info (magit-section-info ,section))
@@ -1300,8 +1303,10 @@ FUNC should leave point at the end of the modified
region"
       (goto-char (process-mark proc))
       ;; Find last ^M in string.  If one was found, ignore everything
       ;; before it and delete the current line.
-      (let ((ret-pos (position ?\r string :from-end t)))
-	(cond (ret-pos
+      (let ((ret-pos (length string)))
+	(while (and (>= (setq ret-pos (1- ret-pos)) 0)
+		    (/= ?\r (aref string ret-pos))))
+	(cond ((>= ret-pos 0)
 	       (goto-char (line-beginning-position))
 	       (delete-region (point) (line-end-position))
 	       (insert (substring string (+ ret-pos 1))))
@@ -2187,10 +2192,14 @@ insert a line to tell how to insert more of them"
             (sha1 (match-string 2))
             (msg (match-string 4))
             (refs (when (match-string 3)
-                    (remove-if (lambda (s)
-                                 (or (string= s "tag:")
-                                     (string= s "HEAD"))) ; as of 1.6.6
-                               (split-string (match-string 3) "[(), ]"
t)))))
+		    (delq nil
+			  (mapcar 
+			   (lambda (s)
+			     (and (not
+				   (or (string= s "tag:")
+				       (string= s "HEAD"))) ; as of 1.6.6
+				  s))
+			   (split-string (match-string 3) "[(), ]" t))))))
         (delete-region (point-at-bol) (point-at-eol))
         (insert (funcall magit-present-log-line-function chart sha1 refs
msg))
         (goto-char (point-at-bol))
@@ -2841,10 +2850,11 @@ in both working tree and staging area.
     (or info
 	(error "No rewrite in progress"))
     (let* ((pending (cdr (assq 'pending info)))
-	   (first-unused (find-if (lambda (p)
-				    (not (plist-get (cdr p) 'used)))
-				  pending
-				  :from-end t))
+	   (first-unused 
+	    (let ((rpend (reverse pending)))
+	      (while (and rpend (plist-get (cdr (car rpend)) 'used))
+		(setq rpend (cdr rpend)))
+	      (car rpend)))
 	   (commit (car first-unused)))
       (cond ((not first-unused)
 	     (magit-rewrite-stop t))
@@ -3582,8 +3592,12 @@ With a non numeric prefix ARG, show all entries"
 	(let* ((excluded (magit-file-lines ".git/info/wazzup-exclude"))
 	       (all-branches (magit-list-interesting-refs))
 	       (branches (if all all-branches
-			   (remove-if (lambda (b) (member (cdr b) excluded))
-				      all-branches)))
+			   (delq nil (mapcar
+				      (lambda (b)
+					(and (not 
+					      (member (cdr b) excluded))
+					     b))
+				      all-branches))))
 	       (reported (make-hash-table :test #'equal)))
 	  (dolist (branch branches)
 	    (let* ((name (car branch))
 
CD: 3ms