Re: empty-directory predicate, native implementation

[Arthur Miller]

Michael Albinus <michael.albinus@gmx.de> writes:

> Arthur Miller <arthur.miller@live.com> writes:
>
> Hi Arthur,
>
>> Attached is patch which return nil early.
>
> Thanks for this. I haven't seen your name on the FSF file; I guess your
> legal papers are on the way?
They are not, but they can be if someone send me link. I've been looking
at this page:

https://www.gnu.org/prep/maintain/html_node/Copyright-Papers.html#Copyright-Papers

but I am not really sure where do I send request.

> In the meantime, we could prepare the other changes. These are
>
> - Adapt the description of directory-files and
>   directory-files-and-attributes in doc/lispref/files.texi.
>
> - Add an entry to etc/NEWS.
>
> - Adapt the file name handlers. These are ange-ftp-directory-files,
>   ange-ftp-directory-files-and-attributes, tramp-handle-directory-files,
>   tramp-handle-directory-files-and-attributes,
>   tramp-adb-handle-directory-files-and-attributes,
>   tramp-crypt-handle-directory-files,
>   tramp-rclone-handle-directory-files,
>   tramp-sh-handle-directory-files-and-attributes,
>   tramp-smb-handle-directory-files.

Aha, so it is how it works; didn't have time to look into tramp yet.
Ok, I'll do; I'll sent in patch(es) when I am done. Probably not today
though but during the weekend of beginning of the next week.

So many changes just for one signature :-).
> - Adapt resp extend ERT tests. These are tramp-test16-directory-files,
>   tramp-test19-directory-files-and-attributes,
>   tramp-archive-test16-directory-files and
>   tramp-archive-test19-directory-files-and-attributes.
>
> - Optionally (but much appreciated), write new ERT tests for
>   directory-files and directory-files-and-attributes.

Will have to look at ERT, I suppose it is not too complicated,
opportunity to learn it.

> And here are some minor comments about your patch:
>
>> +      if (FIXNATP(return_count))
>> +        {
>> +          if (ind == last)
>> +            break;
>> +          ind ++;
>> +        }
>> +
>
> Wouldn't it be more efficient to test
>
>       if (!last && ind == last)
>       break;
>       ind ++;
It will :-)

I mainly try to be clear "what I intent" for some future reader; since
previous code uses lisp.h macros, I used same. I thought that one extra
'if' is not that much of a penalty, I'll gladly shortcircut with !last.

>> +DEFUN ("directory-files", Fdirectory_files, Sdirectory_files, 1, 5, 0,
>>         doc: /* Return a list of names of files in DIRECTORY.
>>  There are three optional arguments:
>>  If FULL is non-nil, return absolute file names.  Otherwise return names
>> @@ -296,9 +312,14 @@
>>  If MATCH is non-nil, mention only file names that match the regexp MATCH.
>>  If NOSORT is non-nil, the list is not sorted--its order is unpredictable.
>>   Otherwise, the list returned is sorted with `string-lessp'.
>> - NOSORT is useful if you plan to sort the result yourself.  */)
>> + NOSORT is useful if you plan to sort the result yourself.
>> +If COUNT is  non-nil, the function will return max of COUNT and length
>> + files, where length is number of files in the directory. Order
>> + in which files are returned is not guaranteed and is file system and
>> + OS dependent. COUNT has to be an integral number in interval
>> + [1,COUNT]. If 0 files are requested the function will return nil. */)
>
> I know what you mean, but I had to read it twice in order to
> understand. Why not simply
>
> If COUNT is a natural number, the function will return up to COUNT files.
I was thinking myself that the last argument took as much doc-space as
comment for the rest of entire function :-). I tried to be as
no-doubt-clear as well as not-so-mathematical, but I definitely think
your version is much nicer. We go with that.

>>  DEFUN ("directory-files-and-attributes", Fdirectory_files_and_attributes,
>> -       Sdirectory_files_and_attributes, 1, 5, 0,
>> -       doc: /* Return a list of names of files and their attributes in 
>> DIRECTORY.
>> +       Sdirectory_files_and_attributes, 1, 6, 0, doc
>> +       : /* Return a list of names of files and their attributes in 
>> DIRECTORY.
>>  Value is a list of the form:
>>
>> -  ((FILE1 . FILE1-ATTRS) (FILE2 . FILE2-ATTRS) ...)
>> +((FILE1 . FILE1-ATTRS) (FILE2 . FILE2-ATTRS) ...)
>
> Why do you remove the indentation? It is intentional, I believe.
I don't. It is my Emacs. I have by defualt "linux" as identation
style; and changed to "gnu" and even setq default, but Emacs arsed
about identing "linux" style anyway; so I "manually" idented everything
I sent in that changed, and I have just missed those since I didn't
edited those at all.