Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use mdfind on macos #18

Closed
mclearc opened this issue Jun 21, 2022 · 7 comments
Closed

Use mdfind on macos #18

mclearc opened this issue Jun 21, 2022 · 7 comments

Comments

@mclearc
Copy link
Contributor

mclearc commented Jun 21, 2022

Hello and thanks for the very interesting package. I use macos and I've noticed a process-lines-handling-status: find exited with status 1 error when I use denote-link-backlinks. I suspect this is because macos uses mdfind rather than find. As far as I can tell the relevant function here is

(defun denote-retrieve--proces-grep (identifier)
  "Process lines matching IDENTIFIER and return list of files."
  (let* ((default-directory (denote-directory))
         (file (file-name-nondirectory (buffer-file-name))))
    (denote-retrieve--files-in-output
     (sort
      (process-lines
       "find"
       default-directory
       "-maxdepth" "1"
       "-type" "f"
       "!" "-name" file
       "-exec"
       grep-program
       "--color=never"
       "-m"
       "1"
       "-e"
       identifier
       "{}"
       ";"
       "-print")
      #'string-lessp))))

Would it be possible to allow for customization of this function? For example, consult-locate allows the user to specify arguments (like mdfind) using consult-locate-args. Perhaps something similar could be done here. This is all just speculation though, because I haven't yet gotten the function to work appropriately using mdfind and grep.

@protesilaos
Copy link
Owner

Would it be possible to allow for customization of this function?

Yes, sure. The reason I did not venture down that path is because of the idea I have to use xref instead:

;; TODO 2022-06-15: Maybe we can do the same in a more standard way?
;; Perhaps with `xref-matches-in-files'?
;;
;; (xref-matches-in-files IDENTIFIER (denote--directory-files :absolute))

I was thinking that xref would alleviate the problem on our end, as it is supposed to work everywhere Emacs runs. What has stalled my progress is the fact that I still don't know how to process the output of that command. It is a hash table which includes file paths---I just need the paths. Once that is sorted, we have a more portable implementation.

Ultimately, we can have the xref method as the default and provide the option of an alternative.

What do you think?

@protesilaos
Copy link
Owner

Since you are here: I plan to expand the section in the manual about extending Denote. Will include consult-notes as well and provide sample configuration.

@mclearc
Copy link
Contributor Author

mclearc commented Jun 22, 2022

Ultimately, we can have the xref method as the default and provide the option of an alternative.

That sounds great. I should have a look at xref. The annoying thing about mdfind is that it cannot exclude files the way that find can -- so I'll try and see what is going wrong with this error -- in principle find should be able to work on macos too.

I plan to expand the section in the manual about extending Denote. Will include consult-notes as well and provide sample configuration.

Cool -- it seems like consult-notes will work great with Denote. Please let me know though if there is any functionality you think should be added.

@protesilaos
Copy link
Owner

I should have a look at xref.

If you have any ideas, please let me know.

The annoying thing about mdfind is that it cannot exclude files the
way that find can -- so I'll try and see what is going wrong with
this error -- in principle find should be able to work on macos too.

Maybe the error you get is due to a different version of find or
grep? Mine is GNU find/grep, whereas the Mac may have the BSD
variants. Concretely, some flag may not exist on your end.

At any rate, this is a good reason to use xref as a baseline.

Cool -- it seems like consult-notes will work great with
Denote. Please let me know though if there is any functionality you
think should be added.

I used it over the weekend. It worked and I like the interface! Though
I was busy adding the finishing touches here and did not test it
thoroughly. I plan to add it to my dotemacs and use it full-time. Will
be able to have a more informed opinion afterwards.

protesilaos added a commit that referenced this issue Jun 22, 2022
With xref we delegate to a built-in facility instead of passing a
hard-coded list of arguments to 'process-lines'.

The old method is prone to errors, such as not working on builds of
Emacs for macOS, as reported by Colin McLear in issue 18 over at the
GitHub mirror: <#18>.

Users of Emacs 28 or higher can configure 'xref-search-program' to
change from the default 'grep' to 'ripgrep', 'ugrep', or a user-defined
alternative.

THIS CHANGE IS PROVISIONAL and subject to further edits.
@protesilaos
Copy link
Owner

I pushed a commit to the xref-instead-of-find branch. Maybe it fixes your problem.

commit 92f9cb8cb9e11142f7124b10e6a03e53053c239c
Author: Protesilaos Stavrou <info@protesilaos.com>
Date:   Wed Jun 22 13:57:23 2022 +0300

    Use xref instead of relying on find+grep

    With xref we delegate to a built-in facility instead of passing a
    hard-coded list of arguments to 'process-lines'.

    The old method is prone to errors, such as not working on builds of
    Emacs for macOS, as reported by Colin McLear in issue 18 over at the
    GitHub mirror: <https://github.com/protesilaos/denote/issues/18>.

    Users of Emacs 28 or higher can configure 'xref-search-program' to
    change from the default 'grep' to 'ripgrep', 'ugrep', or a user-defined
    alternative.

    THIS CHANGE IS PROVISIONAL and subject to further edits.

 denote-retrieve.el | 41 +++++++++++++++++++----------------------
 1 file changed, 19 insertions(+), 22 deletions(-)

The diff, if it helps you:

 denote-retrieve.el | 41 +++++++++++++++++++----------------------
 1 file changed, 19 insertions(+), 22 deletions(-)

diff --git a/denote-retrieve.el b/denote-retrieve.el
index fbd5534..75724f3 100644
--- a/denote-retrieve.el
+++ b/denote-retrieve.el
@@ -96,33 +96,30 @@ (defun denote-retrieve--files-in-output (files)
                       (when (denote--only-note-p f) f))
                     files)))

-;; TODO 2022-06-15: Maybe we can do the same in a more standard way?
-;; Perhaps with `xref-matches-in-files'?
-;;
-;; (xref-matches-in-files IDENTIFIER (denote--directory-files :absolute))
+(autoload 'xref--analyze "xref")
+
+(defun denote-retrieve--xrefs (identifier)
+  "Return xrefs of IDENTIFIER in variable `denote-directory'."
+  (xref--analyze
+   (xref-matches-in-files identifier (denote--directory-files :absolute))))
+
+(defun denote-retrieve--files-in-xrefs (xrefs)
+  "Return sorted file names sans directory from XREFS.
+Parse `denote-retrieve--xrefs'."
+  (sort
+   (mapcar (lambda (x)
+             (file-name-nondirectory (car x)))
+           xrefs)
+   #'string-lessp))
+
 (defun denote-retrieve--proces-grep (identifier)
   "Process lines matching IDENTIFIER and return list of files."
   (let* ((default-directory (denote-directory))
          (file (file-name-nondirectory (buffer-file-name))))
     (denote-retrieve--files-in-output
-     (sort
-      (process-lines
-       "find"
-       default-directory
-       "-maxdepth" "1"
-       "-type" "f"
-       "!" "-name" file
-       "-exec"
-       grep-program
-       "--color=never"
-       "-m"
-       "1"
-       "-e"
-       identifier
-       "{}"
-       ";"
-       "-print")
-      #'string-lessp))))
+     (delete file
+             (denote-retrieve--files-in-xrefs
+              (denote-retrieve--xrefs identifier))))))

 (provide 'denote-retrieve)
 ;;; denote-retrieve.el ends here

@mclearc
Copy link
Contributor Author

mclearc commented Jun 22, 2022

Yes -- testing it a bit and it seems to work well! I'll close this but will let you know if I run into any problems. Thanks!

@mclearc mclearc closed this as completed Jun 22, 2022
protesilaos added a commit that referenced this issue Jun 22, 2022
This is about commit 92f9cb8 which adds the xref method to retrieve
paths to backlinks.  See issue 18 over at the GitHub mirror:
<#18>.
@protesilaos
Copy link
Owner

Thank you! Merged it and added your name to the manual's "Acknowledgements".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants