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

Feature request: Make counsel-locate command configurable #385

Closed
Andre0991 opened this Issue Feb 10, 2016 · 16 comments

Comments

Projects
None yet
3 participants
@Andre0991
Contributor

Andre0991 commented Feb 10, 2016

Currently locate is hardcoded, so it's not possible to change the command that counsel-locate uses.
This is inconvenient for OS X and Windows users, as the former might want to use mdfind, and the latter everything.

Reasons to use mdfind:

  • It's Spotlight's backend, so using it feels very consistent and natural;
  • locate won't work unless the user manually starts the database. mdfind works out of the box.
  • its indexing is updated as the file system changes, ie changes are immediately detected and indexed.

I propose two things:

  • Making the command configurable - in my case, I'd like to set it to mdfind
  • Auto detecting the OS and setting the appropriate command - it's quite easy to implement and it makes sense: If a Windows user installed everything, of course he will want to use it (otherwise counsel-locate is useless for him); if I'm a OS X user, mdfind is much better and works out of the box.

If you agree with my second suggestion, I recommend using the -name parameter on mdfind - it searches only on filenames (and not content), which makes the search much faster.

@abo-abo

This comment has been minimized.

Owner

abo-abo commented Feb 10, 2016

I like the suggestion. But I don't have OSX or Windows, so I'll have to trust the contributor's code in those areas.

Feel free to open a PR. I suggest to deprecate counsel-locate-options in favor of a format-style string variable counsel-locate-command.

counsel-locate-options would need to be marked with make-obsolete-variable as an obsolete variable for 0.8.0. counsel-locate-function would still test for it, but also use counsel-locate-command. On releasing 0.8.0, counsel-locate-options will finally be removed.

@Andre0991

This comment has been minimized.

Contributor

Andre0991 commented Feb 10, 2016

Cool, I'll open a PR.

@Andre0991 Andre0991 changed the title from Feature requrest: Make counsel-locate command configurable to Feature request: Make counsel-locate command configurable Feb 10, 2016

@Andre0991

This comment has been minimized.

Contributor

Andre0991 commented Feb 13, 2016

@abo-abo

How should I use counsel-locate-command and counsel-locate-options at the same time?

The issue is that counsel-locate-command (if I understand correctly what you want) already contains the full command (including parameters) - which I find a good idea. But counsel-locate-options don't play well with it - with the current code, it would add the -i parameter to mdfind, causing an error (as there's no such parameter for mdfind), for example.

Here's what I have:

(defun counsel--autodetect-locate-command ()
  "Try to set an appropriate command for `counsel-locate'
depending on the operating system."
  (cond
   ((eq system-type 'darwin)
    "mdfind -name")
   ;; TODO: find a Windows user that
   ;; can test Ivy and the Everything engine
   ;; ((eq system-type 'windows-nt)
   ;;  "es")
   (t "locate -i --regex")))

(defcustom counsel-locate-command (counsel--autodetect-locate-command)
  "Format string to use in `cousel-locate-function'.
   The default value is determined by `counsel--autodetect-locate-command'."
  :type 'stringp
  :group 'ivy)

This is the problematic part:

(defun counsel-locate-function (str)
  (if (< (length str) 3)
      (counsel-more-chars 3)
    (counsel--async-command
     ;; (format "locate %s '%s'"
     (format "%s %s '%s'"
             counsel-locate-command
             ;; (mapconcat #'identity counsel-locate-options " ")
             ""
             (counsel-unquote-regex-parens
              (ivy--regex str))))
    '("" "working...")))

Note that I commented the line that would add the parameters.

Here's counsel-locate-options (as it is now)

(defcustom counsel-locate-options (if (eq system-type 'darwin)
                                      '("-i")
                                    '("-i" "--regex"))
  "Command line options for `locate`."
  :group 'ivy
  :type  '(repeat string))

Should I just remove the darwin part? Also, having -i and --regex would be a bit misleading for the user as they are already used in counsel--autodetect-locate-command (so removing them would have no effect).

What do you suggest?

@abo-abo

This comment has been minimized.

Owner

abo-abo commented Feb 13, 2016

How should I use counsel-locate-command and counsel-locate-options at the same time?

Just remove all uses of counsel-locate-options and leave a warning that it's obsolete.
And make the code work in the same way, i.e. counsel-locate-command should be "locate -i" on darwin and "locate --regex" otherwise.

@Andre0991

This comment has been minimized.

Contributor

Andre0991 commented Feb 13, 2016

Right, I'll do that.

Should it use locate -i on darwin? I believe mdfind would be a better default for the reasons I mentioned in the first post, but it will get configurable anyway so it's not a big deal for me.

@abo-abo

This comment has been minimized.

Owner

abo-abo commented Feb 13, 2016

I think other users are used to locate -i. Is locate/mdfind installed by default, and is either objectively better?

@Andre0991

This comment has been minimized.

Contributor

Andre0991 commented Feb 13, 2016

mdfind is the backend of spotlight on Mac OS, so it's installed by default. See https://en.wikipedia.org/wiki/Spotlight_(software)

locate is installed by default too.

Those are the killer advantages of mdfind for me:

  • It is integrated with the filesystem so it gets updated automatically. You create a folder and it is already indexed. There's no need to update a database manually or set long intervals.
  • It is very familiar for OS X users, because spotlight is widely used and it is its backend
  • It works out of the box, there's no need to create the database

I found mdfind's results considerably better.
For example, I know I have a file that has git and org is its name, but I don't remember its location. I want to open it. Results:

Andres-MacBook-Air:~ andre0991$ mdfind -name git org
/Users/andre0991/Dropbox/ciencia_da_computacao/org/git.org
/Users/andre0991/Library/Caches/Metadata/Safari/History/https:%2F%2Fgithub.com%2Fsyl20bnr%2Fspacemacs%2Fissues%2F1218.webhistory
/Users/andre0991/Library/Caches/Metadata/Safari/History/https:%2F%2Fgithub.com%2Farduino-org%2FArduino%2Fissues%2F2.webhistory

Searching the same string with locate results in 59112 results, and the top ones are not what I want.

I actually stopped using find-files because I can just type the file or directory name using mdfind (with Ivy or Helm, of course) and get the right result.

I could go on and show more examples but it would be great if some OS X users say what they think about this proposal here.

@abo-abo

This comment has been minimized.

Owner

abo-abo commented Feb 13, 2016

Emacs allows defcustom to have easily select-able pre-configured values. I think it makes sense to have locate as the default one, but mdfind should be easy to customize to.

@Andre0991

This comment has been minimized.

Contributor

Andre0991 commented Feb 16, 2016

@abo-abo OK, I'll set locateas default on OS X.

I don't understand why

(counsel-unquote-regex-parens
 (ivy--regex str))

is used in the locate function. Can't it just use the string that the user entered?

Note: mdfind doesn't support regex.

@abo-abo

This comment has been minimized.

Owner

abo-abo commented Feb 16, 2016

Can't it just use the string that the user entered?

I think the issue there was that locate has a different syntax for regex groups.

@Andre0991

This comment has been minimized.

Contributor

Andre0991 commented Feb 18, 2016

Some old versions of locate don't support --regex. mdfind doesn't either. I'm not sure about es.

So for those cases (or if the user wants to change the default command to one without --regex) the code from my previous post shouldn't run. How do you suggest handling this?

@abo-abo abo-abo closed this in a50f668 Feb 19, 2016

@abo-abo

This comment has been minimized.

Owner

abo-abo commented Feb 19, 2016

Please have a look. If you wish, you can add a counsel-locate-cmd-mdfind that's similar to counsel-locate-cmd-noregex.

@Andre0991

This comment has been minimized.

Contributor

Andre0991 commented Feb 21, 2016

Thanks. I created a counsel-locate-cmd-mdfind for my config.

@abo-abo

This comment has been minimized.

Owner

abo-abo commented Feb 21, 2016

If you wish, open a PR to add it, so that it's easier for people on OSX to customize.
There's a 15 line limit for people without an Emacs copyright assignment though.

@Andre0991

This comment has been minimized.

Contributor

Andre0991 commented Feb 21, 2016

Ah, sure. I'll do it soon.

@hmelman

This comment has been minimized.

hmelman commented Feb 23, 2016

I do know how to change it, but my vote is that counsel-locate-cmd on OS X should default to counsel-locate-cmd-mdfind

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment