You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Kalle Olavi Niemitalo <ko...@iki.fi> on 2005/08/31 19:31:59 UTC

Re: svn commit: r15968 - trunk/contrib/client-side/psvn

xsteve@tigris.org writes:

> +;; taken from esh-util: eshell-for
> +(defmacro svn-status-for (for-var for-list &rest forms)

I don't see why esh-util bothers to define this, when the dolist
macro is already widely available:
- GNU Emacs 20.7 defines it in cl-macs.
- GNU Emacs 21.4 defines it in subr, which is preloaded.
- XEmacs 21.4 defines it in cl-macs, which is preloaded.

(dolist (x y) z...) corresponds to (eshell-for x y z...).

> +;; taken from esh-util: eshell-flatten-list
> +(defun svn-status-flatten-list (args)
> +  "Flatten any lists within ARGS, so that there are no sublists."
> +  (let ((new-list (list t)))
> +    (svn-status-for a args
> +      (if (and (listp a)
> +               (listp (cdr a)))
> +          (nconc new-list (svn-status-flatten-list a))
> +        (nconc new-list (list a))))
> +    (cdr new-list)))

Here's how this might be written with the loop macro:

(defun svn-status-flatten-list (list)
  (loop for item in list
	if (listp item) nconc (svn-status-flatten-list item)
	else collect item))

Shorter, and typically much faster on long lists as well.
This is not quite optimal on deeply nested lists though,
as it reverses the list twice on each level of recursion.
Such input is better handled by a pair of functions:
a recursive one that scans the tree and pushes each atom
on a stack held in a dynamic variable, and a harness that
sets up the stack and reverses it at the end.

>  ARGLIST is a list of arguments \(which must include the command name,
>  for example: '(\"revert\" \"file1\"\)
> +ARGLIST is flattened and any every nil value is discarded.

OK, this will simplify my patch for issue #2357.

> +  (setq arglist (delete nil (svn-status-flatten-list arglist)))

svn-status-flatten-list already deletes nils, because they are
empty lists.

Re: svn commit: r15968 - trunk/contrib/client-side/psvn

Posted by Stefan Reichör <st...@xsteve.at>.
Kalle Olavi Niemitalo <ko...@iki.fi> writes:

Thanks for your suggestions. They simplify the code.

I use them in the actual psvn.el revision.


Stefan.

> xsteve@tigris.org writes:
>
>> +;; taken from esh-util: eshell-for
>> +(defmacro svn-status-for (for-var for-list &rest forms)
>
> I don't see why esh-util bothers to define this, when the dolist
> macro is already widely available:
> - GNU Emacs 20.7 defines it in cl-macs.
> - GNU Emacs 21.4 defines it in subr, which is preloaded.
> - XEmacs 21.4 defines it in cl-macs, which is preloaded.
>
> (dolist (x y) z...) corresponds to (eshell-for x y z...).
>
>> +;; taken from esh-util: eshell-flatten-list
>> +(defun svn-status-flatten-list (args)
>> +  "Flatten any lists within ARGS, so that there are no sublists."
>> +  (let ((new-list (list t)))
>> +    (svn-status-for a args
>> +      (if (and (listp a)
>> +               (listp (cdr a)))
>> +          (nconc new-list (svn-status-flatten-list a))
>> +        (nconc new-list (list a))))
>> +    (cdr new-list)))
>
> Here's how this might be written with the loop macro:
>
> (defun svn-status-flatten-list (list)
>   (loop for item in list
> 	if (listp item) nconc (svn-status-flatten-list item)
> 	else collect item))
>
> Shorter, and typically much faster on long lists as well.
> This is not quite optimal on deeply nested lists though,
> as it reverses the list twice on each level of recursion.
> Such input is better handled by a pair of functions:
> a recursive one that scans the tree and pushes each atom
> on a stack held in a dynamic variable, and a harness that
> sets up the stack and reverses it at the end.
>
>>  ARGLIST is a list of arguments \(which must include the command name,
>>  for example: '(\"revert\" \"file1\"\)
>> +ARGLIST is flattened and any every nil value is discarded.
>
> OK, this will simplify my patch for issue #2357.
>
>> +  (setq arglist (delete nil (svn-status-flatten-list arglist)))
>
> svn-status-flatten-list already deletes nils, because they are
> empty lists.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org