[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: empty-directory predicate, native implementation
From: |
Michael Albinus |
Subject: |
Re: empty-directory predicate, native implementation |
Date: |
Sun, 18 Oct 2020 13:52:26 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) |
Arthur Miller <arthur.miller@live.com> writes:
Hi Arthur,
> Please check that handlers stuff. I am not sure how to test it, and am
> not sure I get those correct; especially tramp-crypt.
See comments below.
> I am using a regular expression from Dired+ by Drew in two places. I
> have mention it the comment in ert tests, but don't know how to mention
> it in the example in manual. Maybe remove example, or maybe it can stay
> without creds?
You don't need that regexp from Drew. We have the constant
directory-files-no-dot-files-regexp for that purpose.
> I also discovered that I wasn't covered with FIXNUM all the way; thought
> it was unsigned int so -1 would be converted into ffffffff, which
> would just yeild all results. Seems like fixnum is not what I thought so I
> have added test for <=0 which return nil.
What's wrong using FIXNATP and XFIXNAT everywhere?
> Ert test are mostly foxused on new argument. I haven't found something
> related to dired in test/src so I have created dired-tests.el. If it
> shoudl be in some other place I can rename it.
test/src/dired-tests.el is perfect. Maybe you add a comment, that just
the tests for COUNT are added.
> diff --git a/doc/lispref/files.texi b/doc/lispref/files.texi
> index 3b8b4fb3a9..2f15176e79 100644
> --- a/doc/lispref/files.texi
> +++ b/doc/lispref/files.texi
>
> +If @var{count} is non-@code{nil}, the function will return first
> +@var{count} number of files, or all files, whichever occurs
> +first. @var{count} has to be a natural number (> 0). You can use this
> +function to short circuit evaluation in case you are just interested to
> +find if a directory is empty or not (request one file and tell it to
> +ignore dot-files).
In Emacs documentation, we use two spaces after an end-of-sentence
period. Furthermore, pls say "file name" instead of "file"; this is what
we return.
> +@example
> +@group
> + (null (directory-files directory-name nil
> + "^\\([^.]\\|\\.\\([^.]\\|\\..\\)\\).*" t 1))
Please format properly. It shall be
(null
(directory-files directory-name
nil directory-files-no-dot-files-regexp t 1))
> diff --git a/etc/NEWS b/etc/NEWS
> index 1838b6b38a..25c54d3dfe 100644
> --- a/etc/NEWS
> +++ b/etc/NEWS
> +** 'diirectory-files' function can now take an additional count parameter
Pls fix the spelling error. Finish the header line with a period. Use
capital letters COUNT.
> +This option makes directory-files return COUNT first files in
It is not an option, but rather an optional parameter. Quote
directory-files. Use two spaces. Use "file names". Fix "vill".
> The option is useful for checking if
> +directory is empty since it will check at most 3 files when COUNT = 1.
As it stands it is not understandable, without mentioning the respective
regexp. Better, you keep this sentence out.
> diff --git a/lisp/net/ange-ftp.el b/lisp/net/ange-ftp.el
> index 0cb8d7cb83..335a07914c 100644
> --- a/lisp/net/ange-ftp.el
> +++ b/lisp/net/ange-ftp.el
> +(defun ange-ftp-directory-files (directory &optional full match nosort
> + count)
Could be in the same line.
> @@ -3444,18 +3444,19 @@ ange-ftp-directory-files
> (setq files
> (cons (if full (concat directory f) f) files))))
> (nreverse files)))
> - (apply 'ange-ftp-real-directory-files directory full match v19-args)))
> + (apply 'ange-ftp-real-directory-files directory full match nosort
> count)))
That is the fallback. But I don't see an implementation of
COUNT. (Should be in the "while tail" loop).
> (defun ange-ftp-directory-files-and-attributes
> - (directory &optional full match nosort id-format)
> + (directory &optional full match nosort attrs id-format count)
What it attrs good for?
> - (ange-ftp-directory-files directory full match nosort))
> + (ange-ftp-directory-files directory full match nosort attrs
> + id_format count))
Remove attrs. Use id-format.
> (ange-ftp-real-directory-files-and-attributes
> - directory full match nosort id-format)))
> + directory full match nosort attrs id-format count)))
Remove attrs.
> --- a/lisp/net/tramp-adb.el
> +++ b/lisp/net/tramp-adb.el
> @@ -312,7 +312,7 @@ tramp-adb-handle-directory-files-and-attributes
> (copy-tree
> (with-tramp-file-property
> v localname (format "directory-files-and-attributes-%s-%s-%s-%s"
> - full match id-format nosort)
> + full match id-format nosort count)
If you want to add count to the property name, you must adapt the format
string.
I miss the implementation of count.
> diff --git a/lisp/net/tramp-crypt.el b/lisp/net/tramp-crypt.el
> index 3e96daa7b1..bda3735db4 100644
> --- a/lisp/net/tramp-crypt.el
> +++ b/lisp/net/tramp-crypt.el
> +;; This function does not seem to pass match and nosort into
> +;; directory-files at all; is that intentional or bug?
ATM: yes. Leave it as it is, we could change later on. Add FIXME in the comment.
> diff --git a/lisp/net/tramp-rclone.el b/lisp/net/tramp-rclone.el
> index 3701bfc22c..787eead807 100644
> --- a/lisp/net/tramp-rclone.el
> +++ b/lisp/net/tramp-rclone.el
> +;; This function did not pass nosort arguemnt into directory-files
> +;; not sure if intentional or bug
You can remove the comment, because you did it right.
> diff --git a/lisp/net/tramp-sh.el b/lisp/net/tramp-sh.el
> index 15eab0a4de..7a969979ac 100644
> +;; what about perl & sta -> need to fix list to count?
That's correct. Mark the comment as FIXME.
> diff --git a/lisp/net/tramp-smb.el b/lisp/net/tramp-smb.el
> index 1b6af2a2e3..62135f514d 100644
> --- a/lisp/net/tramp-smb.el
> +++ b/lisp/net/tramp-smb.el
> + (let ((result nil)
> + (numres 0))
numres is not used. The rest I need to test later; I'm currently just
dry reading.
> diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
> index 6d44ad23ad..a99af70196 100644
> --- a/lisp/net/tramp.el
> +++ b/lisp/net/tramp.el
> +(defun tramp-handle-directory-files (directory &optional full match
> + nosort count)
This fits into one line (80 chars). I'm missiong an implementation for count.
> diff --git a/src/dired.c b/src/dired.c
> index 1584b6acf0..1fc8dd27fa 100644
> --- a/src/dired.c
> +++ b/src/dired.c
> @@ -17,7 +17,6 @@
> + if (FIXNUMP(return_count))
> + {
> + last = XFIXNUM (return_count);
> + if (last <= 0)
> + return Qnil;
> + }
This is superfluous. Remove it.
> +If COUNT is non-nil, the function will return max of COUNT and length
> + files, where length is number of files in the directory. COUNT has to
> + be a natural number > 0. */)
So you haven't changed the docstring?
> 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) ...)
...
Still the indentation doesn't fit with the original.
> diff --git a/test/src/dired-tests.el b/test/src/dired-tests.el
> new file mode 100644
> index 0000000000..3b739e59cc
> --- /dev/null
> +++ b/test/src/dired-tests.el
> +;; Copyright (C) 2015-2020 Free Software Foundation, Inc.
This shall be just 2020.
> + ;; nodots expression from dired+ by Drew A.
> + (nodots "^\\([^.]\\|\\.\\([^.]\\|\\..\\)\\).*"))
Not needed. Use directory-files-no-dot-files-regexp.
> + (message name)
That's OK for debugging purposes. Finally, the tests shall be silent.
> + (make-directory name)
Tests shall not kepp garbage. Please delete the directory at the end of
the test, preferred as unwindorm of unwind-protect.
> +(ert-deftest directory-files-and-attributes-tests ()
Same comments apply.
> Best regards
Best regards, Michael.
- RE: empty-directory predicate, native implementation, (continued)
- RE: empty-directory predicate, native implementation, Drew Adams, 2020/10/17
- Re: empty-directory predicate, native implementation, Arthur Miller, 2020/10/17
- RE: empty-directory predicate, native implementation, Drew Adams, 2020/10/17
- Re: empty-directory predicate, native implementation, Arthur Miller, 2020/10/17
- Re: empty-directory predicate, native implementation, Arthur Miller, 2020/10/17
- RE: empty-directory predicate, native implementation, Drew Adams, 2020/10/17
- Re: empty-directory predicate, native implementation, Arthur Miller, 2020/10/17
- Re: empty-directory predicate, native implementation, Michael Albinus, 2020/10/18
- Re: empty-directory predicate, native implementation, Eli Zaretskii, 2020/10/17
- RE: empty-directory predicate, native implementation, Drew Adams, 2020/10/18
- Re: empty-directory predicate, native implementation,
Michael Albinus <=
- RE: empty-directory predicate, native implementation, Drew Adams, 2020/10/18
- Re: empty-directory predicate, native implementation, Michael Albinus, 2020/10/18
- Re: empty-directory predicate, native implementation, Stefan Monnier, 2020/10/18
- RE: empty-directory predicate, native implementation, Drew Adams, 2020/10/18
- Re: empty-directory predicate, native implementation, Arthur Miller, 2020/10/18
- Re: empty-directory predicate, native implementation, Arthur Miller, 2020/10/18
- Re: empty-directory predicate, native implementation, Michael Albinus, 2020/10/19
- Re: empty-directory predicate, native implementation, Arthur Miller, 2020/10/19
- Re: empty-directory predicate, native implementation, Michael Albinus, 2020/10/19
- Re: empty-directory predicate, native implementation, Stefan Monnier, 2020/10/15