Re: empty-directory predicate, native implementation

[Michael Albinus]

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?

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.

- 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.

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 ++;

> +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.

>  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.

>  This function accepts four optional arguments:
>  If FULL is non-nil, return absolute file names.  Otherwise return names
> - that are relative to the specified directory.
> +that are relative to the specified directory.
>  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.
> - NOSORT is useful if you plan to sort the result yourself.
> +NOSORT is useful if you plan to sort the result yourself.

Same here.

> +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. */)

See above.

Best regards, Michael.