org-attach-sync uses directory-empty-p (new in Emacs 28)

org-attach-sync uses directory-empty-p (new in Emacs 28)

[Kyle Meyer]

Hi Marco,

In 61e083732 (org-attach: Possibly delete empty attach directory,
2021-07-09), you added a call to directory-empty-p.  This function was
introduced in Emacs's 0806075520 (Add directory-empty-p and new argument
COUNT for directory-files-*, 2020-11-02) and hasn't yet made it into a
release.

Could you update org-attach-sync to avoid using directory-empty-p (e.g.,
by inlining it or by adding a compatibility alias)?

Thanks.

Re: org-attach-sync uses directory-empty-p (new in Emacs 28)

[Arthur Miller]

Kyle Meyer <kyle@kyleam.com> writes:

> Hi Marco,
>
> In 61e083732 (org-attach: Possibly delete empty attach directory,
> 2021-07-09), you added a call to directory-empty-p.  This function was
> introduced in Emacs's 0806075520 (Add directory-empty-p and new argument
> COUNT for directory-files-*, 2020-11-02) and hasn't yet made it into a
> release.
>
> Could you update org-attach-sync to avoid using directory-empty-p (e.g.,
> by inlining it or by adding a compatibility alias)?
>
> Thanks.

Can this help:

#+begin_src emacs-lisp

(when (version< emacs-version "28")
  (defun directory-empty-p (file-name)
    "Check if a directory contains any other files then dot-files"
    (when (file-directory-p file-name)
      (null (directory-files file-name nil
                             directory-files-no-dot-files-regexp t)))))

#+end_src

Re: org-attach-sync uses directory-empty-p (new in Emacs 28)

[Marco Wahl]

Arthur Miller <arthur.miller@live.com> writes:

> Kyle Meyer <kyle@kyleam.com> writes:
>
>> In 61e083732 (org-attach: Possibly delete empty attach directory,
>> 2021-07-09), you added a call to directory-empty-p.  This function was
>> introduced in Emacs's 0806075520 (Add directory-empty-p and new argument
>> COUNT for directory-files-*, 2020-11-02) and hasn't yet made it into a
>> release.
>>
>> Could you update org-attach-sync to avoid using directory-empty-p (e.g.,
>> by inlining it or by adding a compatibility alias)?

> Can this help:
>
> #+begin_src emacs-lisp
>
> (when (version< emacs-version "28")
>   (defun directory-empty-p (file-name)
>     "Check if a directory contains any other files then dot-files"
>     (when (file-directory-p file-name)
>       (null (directory-files file-name nil
>                              directory-files-no-dot-files-regexp t)))))
>
> #+end_src

Thanks Kyle and Arthur!

Starting from Arthur's suggestion and Kyle's hint to the compatibility
alias I put org-directory-empty-p into org-compat.el.  So there is
org-directory-empty-p now which provides the functionality of
directory-empty-p from Emacs 28 for smaller version Emacsen.

org-directory-empty-p is defined in org-compat.el.  From there
org-directory-empty-p can (and hopefully will) be easily dropped when
the minimum required Emacs version for Org switches to 28.


Thanks again and best regards,
--
Marco

Re: org-attach-sync uses directory-empty-p (new in Emacs 28)

[Maxim Nikulin]

On 11/08/2021 03:52, Marco Wahl wrote:
>> Kyle Meyer <kyle@kyleam.com> writes:
>>
>>> In 61e083732 (org-attach: Possibly delete empty attach directory,
>>> 2021-07-09), you added a call to directory-empty-p.  This function was
>>> introduced in Emacs's 0806075520 (Add directory-empty-p and new argument
>>> COUNT for directory-files-*, 2020-11-02) and hasn't yet made it into a
>>> release.
>>>
>>> Could you update org-attach-sync to avoid using directory-empty-p (e.g.,
>>> by inlining it or by adding a compatibility alias)?
> 
> Starting from Arthur's suggestion and Kyle's hint to the compatibility
> alias I put org-directory-empty-p into org-compat.el.  So there is
> org-directory-empty-p now which provides the functionality of
> directory-empty-p from Emacs 28 for smaller version Emacsen.

Unfortunately current code causes a compiler warning at least when 
Emacs-25.2 is used:

Compiling single /home/ubuntu/org-mode/lisp/org-compat.el...

In end of data:
org-compat.el:1255:1:Warning: the function ‘directory-empty-p’ is not 
known to
     be defined.

Re: org-attach-sync uses directory-empty-p (new in Emacs 28)

[Marco Wahl]

Maxim Nikulin <manikulin@gmail.com> writes:

> On 11/08/2021 03:52, Marco Wahl wrote:
>>> Kyle Meyer <kyle@kyleam.com> writes:
>>>
>>>> In 61e083732 (org-attach: Possibly delete empty attach directory,
>>>> 2021-07-09), you added a call to directory-empty-p.  This function was
>>>> introduced in Emacs's 0806075520 (Add directory-empty-p and new argument
>>>> COUNT for directory-files-*, 2020-11-02) and hasn't yet made it into a
>>>> release.
>>>>
>>>> Could you update org-attach-sync to avoid using directory-empty-p (e.g.,
>>>> by inlining it or by adding a compatibility alias)?
>> Starting from Arthur's suggestion and Kyle's hint to the
>> compatibility
>> alias I put org-directory-empty-p into org-compat.el.  So there is
>> org-directory-empty-p now which provides the functionality of
>> directory-empty-p from Emacs 28 for smaller version Emacsen.
>
> Unfortunately current code causes a compiler warning at least when
> Emacs-25.2 is used:
>
> Compiling single /home/ubuntu/org-mode/lisp/org-compat.el...
>
> In end of data:
> org-compat.el:1255:1:Warning: the function ‘directory-empty-p’ is not
> known to
>     be defined.

Thanks.

How make the compiler happy?

What about adding a declare-function for directory-empty-p?  Suggestion
for org-compat.el:

#+begin_src emacs-lisp
;;; Emacs < 28.1 compatibility
(if (version< emacs-version "28")
    (defun org-directory-empty-p (dir)
      "Return t if DIR names an existing directory containing no other files."
      (and (file-directory-p dir)
           (null (directory-files dir nil directory-files-no-dot-files-regexp t))))
  (declare-function directory-empty-p "files" (dir)) ; <-- NEW LINE TO MAKE THE COMPILER HAPPY.
  (defalias 'org-directory-empty-p #'directory-empty-p))
#+end_src

Could you please check the compile with this modification?


Best regards,
-- 
Marco

Re: org-attach-sync uses directory-empty-p (new in Emacs 28)

[Maxim Nikulin]

On 12/08/2021 04:17, Marco Wahl wrote:
> Maxim Nikulin writes:
>>
>> Compiling single /home/ubuntu/org-mode/lisp/org-compat.el...
>>
>> In end of data:
>> org-compat.el:1255:1:Warning: the function ‘directory-empty-p’ is not
>> known to
>>      be defined.
...
> 
> How make the compiler happy?

I have never fought with this kind of problem. After looking at the 
other constructs in the org-compat.el file, I have tried

(if (fboundp 'directory-empty-p)
     (defalias 'org-directory-empty-p #'directory-empty-p)
   (defun org-directory-empty-p (dir)
     ; ...

I have no idea if there are any drawbacks of such approach, I can only 
say that such form suppresses the warning.

It seems, version comparison is sensible when an existing function gets 
new arguments, not in the case of completely new function.

Re: org-attach-sync uses directory-empty-p (new in Emacs 28)

[Marco Wahl]

Maxim Nikulin <manikulin@gmail.com> writes:

>>> Compiling single /home/ubuntu/org-mode/lisp/org-compat.el...
>>>
>>> In end of data:
>>> org-compat.el:1255:1:Warning: the function ‘directory-empty-p’ is not
>>> known to
>>>      be defined.

> I have never fought with this kind of problem. After looking at the
> other constructs in the org-compat.el file, I have tried
>
> (if (fboundp 'directory-empty-p)
>     (defalias 'org-directory-empty-p #'directory-empty-p)
>   (defun org-directory-empty-p (dir)
>     ; ...
>
> I have no idea if there are any drawbacks of such approach, I can only
> say that such form suppresses the warning.

Thanks Maxim and Kyle!

The latter suggestion landed in master a few minutes ago.


Have a great day!
--
Marco

Re: org-attach-sync uses directory-empty-p (new in Emacs 28)

[Maxim Nikulin]

On 15/08/2021 16:11, Marco Wahl wrote:
> Maxim Nikulin <manikulin@gmail.com> writes:
> 
>>>> Compiling single /home/ubuntu/org-mode/lisp/org-compat.el...
>>>>
>>>> In end of data:
>>>> org-compat.el:1255:1:Warning: the function ‘directory-empty-p’ is not
>>>> known to
>>>>       be defined.
> 
>> I have never fought with this kind of problem. After looking at the
>> other constructs in the org-compat.el file, I have tried
>>
>> (if (fboundp 'directory-empty-p)
>>      (defalias 'org-directory-empty-p #'directory-empty-p)
>>    (defun org-directory-empty-p (dir)
>>      ; ...
> 
> The latter suggestion landed in master a few minutes ago.

Thank you. The warning has disappeared for current master HEAD and 
Emacs-25.2.

Maybe a brief comment in org-compat.el clarifying usage of version 
comparison vs. `fboundp' would be helpful to avoid similar issues in 
future. E.g. I am unsure concerning simple < vs. version< (whether 
attempt of version check could cause a compatibility issue). Fortunately 
latest change does not use any of them.