ag.el icon indicating copy to clipboard operation
ag.el copied to clipboard

support pt (the platinum searcher)

Open thomasf opened this issue 10 years ago • 23 comments

ag/pt are quite close to eachother featurewise. It might be a good idea to have support for both in the same codebase. Currently pt does not have all of the options that ag does so I would guess that a test to see how well it works is to just omit a few command line arguments and take it from there.. I can probably tinker a bit with that stuff..

Does it seem like a good idea?

thomasf avatar May 25 '14 17:05 thomasf

Woah, pt boasts faster searching than ag!

Yep, I'd certainly welcome patches that add support for pt to ag.el.

Wilfred avatar May 25 '14 19:05 Wilfred

As an inital attempt i thnik I'm gonna go with adding an ag-pt-* infix versionx of all defcustoms that refer to specifically to ag and one for choosing which searcher to use. Then I can make decisions if things should be done conditionally or use multiple versions or refactor out new functions as I go along.

thomasf avatar May 26 '14 07:05 thomasf

If there are any differences which are significant to ag.el, then why not just clone ag.el to pt.el? Adding a bunch of conditional code seems a bit messier IMO.

purcell avatar May 26 '14 08:05 purcell

And a wgrep-pt.el, of course. ;-)

purcell avatar May 26 '14 08:05 purcell

I know. The current ag- namespace makes things a bit confusing, to really do this proper renamespacing into alloy- and would make things less confusing.. An alternative would be to contribute the missing arguments to the pt main project instead so that nothing actually differs from an ag.el perspective. However, I have only ever written an golang code at the size of hello world so I will begin at this end to see if its feasible.

thomasf avatar May 26 '14 09:05 thomasf

the differences at a basic level are not that big when taking a quick look though... screenshot - 2014-05-26 - 11 07 05

The first command is what ag.el currently generates for a simple search and then I removed arguments until pt accepted it. The outputs seems very similar.

(Also, I belive that the -M option isnt even a part of ag atm, it might come from my own patched version,.)

thomasf avatar May 26 '14 09:05 thomasf

An alternative way to handle both searches could be to detect if ag-executable is set to pt and then filter arguments that pt does not support. That solution is depending on that people don't rename their ag or pt executables which I guess seems a bit strange to do anyway.. This ties ag.el a bit more to specific versions of pt.

thomasf avatar May 26 '14 09:05 thomasf

ok,. there already is a pull request for adding --literal https://github.com/monochromegane/the_platinum_searcher/pull/27 So maybe it's better for me to focus on adding (or lobbying if I'm lazy) --column as well to pt. It seems I should do other things now..

thomasf avatar May 26 '14 09:05 thomasf

Yes, I think that with --column added, pt would have everything ag.el needs, which would be the best outcome overall IMO.

purcell avatar May 26 '14 09:05 purcell

Implementing the column part of the matching in elisp might also be viable solution if it isn't too slow. I'll start a discussion at the other end of this as well. Also my personal reason for switching to pt is that it isn't written in C and seems easier to extend so I want to work on it.. Seems like a good code base size and scope (performance) to get acquainted with golang.

thomasf avatar May 26 '14 09:05 thomasf

FWIW, a pt.el has appeared: https://github.com/bling/pt.el

Wilfred avatar Jun 08 '14 19:06 Wilfred

At first glance, that doesn't look as complete as ag.el.

purcell avatar Jun 08 '14 20:06 purcell

yeah, but it lacks most or all features of ag.el

thomasf avatar Jun 08 '14 20:06 thomasf

I'm still happy to take patches for pt.

Wilfred avatar Jun 08 '14 20:06 Wilfred

i created pt.el to solve my specific problem of having a good searcher in windows, of which both ack and ag are lackluster. i thought about forking this project at the beginning, but decided against it because:

  1. projectile already gives me all the information i need for where/what the search.
  2. i really didn't like the idea of forking and renaming ag to pt because there would be so much code duplication.

when --column gets implemented upstream, i'd be happy to obsolete my projects, but until then, pt and wgrep-pt are available on MELPA.

bling avatar Jul 03 '14 15:07 bling

Great! Thanks for letting us know.

Wilfred avatar Jul 03 '14 15:07 Wilfred

I've implemented a from my viewpoint suitable --column implementation now which I hope will be merged.. So now onto the ag.el pt part.. ..

I've just quickly looked at how to support both ag/pt in ag.el. I have not got to the point of looking at the regex building stuff but it mostly seems that the arguments building places needs to be either moved to own functions or else there will be quite a few (if...(progn ))'s that does not seem to be very nice looking..

Below is my 5 minutes of toying to get basic searches working,... If you have any suggestions for the direction of the implementation, please let me know..

(I changed the defcustom from ag-pt to ag-use-pt and I see that it's not changed all the way through)

diff --git a/ag.el b/ag.el
index eee8f16..c3efe9b 100644
--- a/ag.el
+++ b/ag.el
@@ -40,12 +40,21 @@
 (require 'ido)  ;; completion
 (require 'find-dired) ;; find-dired-filter

+
+
 (defcustom ag-executable
   "ag"
   "Name of the ag executable to use."
   :type 'string
   :group 'ag)

+(defcustom ag-use-pt nil
+  "Use pt, the platinum searcher instead of ag.
+If ag-executable is set to ag it will automatically be set to pt
+before execution."
+  :type 'boolean
+  :group 'ag)
+
 (defcustom ag-arguments
   (list "--line-number" "--smart-case" "--nogroup" "--column" "--")
   "Default arguments passed to ag.
@@ -58,6 +67,15 @@ print line numbers when the input is a stream."
   :type '(repeat (string))
   :group 'ag)

+(defcustom ag-pt-arguments
+  (list "--smart-case" "--nogroup" "--column" "--")
+  "Default arguments passed to pt.
+
+Ag.el requires --nogroup and --column, so we recommend you add any
+additional arguments to the start of this list."
+  :type '(repeat (string))
+  :group 'ag)
+
 (defcustom ag-highlight-search nil
   "Non-nil means we highlight the current search term in results.

@@ -159,6 +177,12 @@ different window, according to `ag-reuse-window'."
    (regexp (format "*ag search regexp:%s dir:%s*" search-string directory))
    (:else (format "*ag search text:%s dir:%s*" search-string directory))))

+(defun ag/executable ()
+  "Returns either ag or pt."
+    (if (and ag-pt (string= ag-executable "ag"))
+        "pt"
+      ag-executable))
+
 (defun ag/format-ignore (ignores)
   "Prepend '--ignore' to every item in IGNORES."
   (apply #'append
@@ -169,24 +193,48 @@ different window, according to `ag-reuse-window'."
   "Run ag searching for the STRING given in DIRECTORY.
 If REGEXP is non-nil, treat STRING as a regular expression."
   (let ((default-directory (file-name-as-directory directory))
-        (arguments ag-arguments)
+        (arguments (if ag-pt
+                       ag-pt-arguments
+                     ag-arguments))
         (shell-command-switch "-c"))
-    (unless regexp
-      (setq arguments (cons "--literal" arguments)))
-    (if ag-highlight-search
-        (setq arguments (append '("--color" "--color-match" "30;43") arguments))
-      (setq arguments (append '("--nocolor") arguments)))
-    (when (char-or-string-p file-regex)
-      (setq arguments (append `("--file-search-regex" ,file-regex) arguments)))
-    (when file-type
-      (setq arguments (cons (format "--%s" file-type) arguments)))
-    (when ag-ignore-list
-      (setq arguments (append (ag/format-ignore ag-ignore-list) arguments)))
+
+    (if ag-pt
+        (progn
+          ;; (unless regexp
+          ;;   (setq arguments (cons "--literal" arguments)))
+
+          (if ag-highlight-search
+              (setq arguments (append '("--color") arguments))
+            (setq arguments (append '("--nocolor") arguments)))
+
+          ;;;; TODO these are not the same or not at all in pt:
+          ;; (when (char-or-string-p file-regex)
+          ;;   (setq arguments (append `("--file-search-regex" ,file-regex) arguments)))
+          ;; (when file-type
+          ;;   (setq arguments (cons (format "--%s" file-type) arguments)))
+          ;; (when ag-ignore-list
+          ;;   (setq arguments (append (ag/format-ignore ag-ignore-list) arguments)))
+          )
+
+      (unless regexp
+        (setq arguments (cons "--literal" arguments)))
+      (if ag-highlight-search
+          (setq arguments (append '("--color" "--color-match" "30;43") arguments))
+        (setq arguments (append '("--nocolor") arguments)))
+      (when (char-or-string-p file-regex)
+        (setq arguments (append `("--file-search-regex" ,file-regex) arguments)))
+      (when file-type
+        (setq arguments (cons (format "--%s" file-type) arguments)))
+      (when ag-ignore-list
+        (setq arguments (append (ag/format-ignore ag-ignore-list) arguments)))
+
+      )
     (unless (file-exists-p default-directory)
       (error "No such directory %s" default-directory))
     (let ((command-string
            (mapconcat #'shell-quote-argument
-                      (append (list ag-executable) arguments (list string "."))
+                      (append (list (ag/executable))
+                              arguments (list string "."))
                       " ")))
       ;; If we're called with a prefix, let the user modify the command before
       ;; running it. Typically this means they want to pass additional arguments.
@@ -464,7 +512,7 @@ See also `find-dired'."
          (buffer-name (if ag-reuse-buffers
                           "*ag dired*"
                         (format "*ag dired pattern:%s dir:%s*" regexp dir)))
-         (cmd (concat ag-executable " --nocolor -g '" regexp "' "
+         (cmd (concat (ag/executable) " --nocolor -g '" regexp "' "
                       (shell-quote-argument dir)
                       " | grep -v '^$' | sed s/\\'/\\\\\\\\\\'/ | xargs -I '{}' ls "
                       dired-listing-switches " '{}' &")))
@@ -563,7 +611,7 @@ This function is called from `compilation-filter-hook'."

 (defun ag/get-supported-types ()
   "Query the ag executable for which file types it recognises."
-  (let* ((ag-output (shell-command-to-string (format "%s --list-file-types" ag-executable)))
+  (let* ((ag-output (shell-command-to-string (format "%s --list-file-types" (ag/executable))))
          (lines (-map #'s-trim (s-lines ag-output)))
          (types (--keep (when (s-starts-with? "--" it) (s-chop-prefix "--" it )) lines))
          (extensions (--map (s-split "  " it) (--filter (s-starts-with? "." it) lines))))

thomasf avatar Jan 28 '15 22:01 thomasf

I did think of how to properly support pt in ag.el, just now while taking a shower :)

It's probably a good idea to structure the solution from a viewpoint of having two providers . A defcustom named ag-provider (with value 'ag or 'pt) can be the configuration part.
All code that needs specific provider handling can then use it's own sub namespace functions.

It feels like a clean way to do it, this way we can also (maybe) support the unobtainium searcher the day that it arrives and is so so so awesome because it can search all your data using PRISM as well :)

thomasf avatar Jan 29 '15 01:01 thomasf

Please note, pt may be fast, but it has poor regexp support in comparison to ag. Just my 2c

jasonm23 avatar May 23 '15 12:05 jasonm23

AG however does not work well with windows when I tried it sometime in the past.. Also PT being a go program it's very easy to install from source regardless of supported platform. I'll revisit this issue when I think that PT has the features it needs. PT suffers from the same problems as AG in that it seems like the maintainer doesnt have time for it at all.

thomasf avatar May 23 '15 13:05 thomasf

How about making ag.el a generic platform for searcher programs. I would like ag.el to have support for https://github.com/BurntSushi/ripgrep . I am sure there are more similar programs.

vigneshsarma avatar Dec 01 '16 07:12 vigneshsarma

I'd be keen for this too. I like the way ag.el is structured, and my dependence on ag-project (for example) makes it hard to switch to ripgrep -- there's a ripgrep.el, but it's structured quite differently.

purcell avatar Dec 01 '16 08:12 purcell

Yep, the speedup of ripgrep would be really nice. I know MELPA has socyl but I'm really interested in generalising ag.el.

I'm working on some ideas for ag.el, I'll take a look at this.

Wilfred avatar Dec 01 '16 08:12 Wilfred