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

Further Support "rg" Backend #394

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

zozowell
Copy link

@zozowell zozowell commented Oct 6, 2022

What are changes in this PR

Silver Searcher is a bit of out of active maintenance recently, it's regretful, and I'm recently trouble a lot by that "ag" doesn't recognize the negated rules in gitignore, so it caused some weird behaviors in a repo like this one. It's a regret, I've been using helm-ag for a long while, thanks for the great efforts of the contributors, this tool benefits me a lot, especially for its robust command line option support and async helm source support (helm-do-ag). I noticed for a while that, helm-ag does attempt to support other searcher, including "pt", "ack" and "rg". However, I don't know why, the support just stops at replacing default base command with helm-ag-base-command custom variable, it doesn't really support the option syntax difference between searchers.

As "rg" now is the most popular searcher, it outweights others in performance, and it is under active development. To me, the good news is it supports gitignore well. After some painful tries on other emacs packages, like "rg", pure "helm grep" with rg backend etc..., it's hard to find a good balance between "helm", "rg" and "project search". So, I finally tried to harden helm-ag to support "rg" better. Hope this effort can make slight sense.

The changes I made in this PR includes:

  • when you set helm-ag-base-command with a rg command, like "rg --no-heading", helm-ag now:
    • supports ".rgignore"
    • supports recognize helm-ag-ignore-patterns, and convert them into rg compatible format.
  • some maintenance changes:
    • replaced deprecated helm-attr and helm-attrset with 'helm-get-attrandhelm-set-attr`.
    • merged and normalized helm-ag--construct-command and helm-ag--construct-do-ag-command, they do same thing basically in different formats, very confusing. Now, consolidated under helm-ag--construct-command.
  • add some new test cases.

Tests

➜ make test
Testing...
eask test ert ./test/*.el
Running Eask in the development environment
Press Ctrl+C to cancel.

Executing script inside Emacs...

✓ Checking Emacs version 28.1... done!
✓ Checking system darwin... done!
✓ Try constructing the package-descriptor (helm-ag.el)... succeeded!
Unmatched website URL ’https://github.com/syohex/emacs-helm-ag’; add (website-url "https://github.com/syohex/emacs-helm-ag") to Eask-file
Keywords header is optional, but it’s often recommended
✓ Loading Eask file in /Users/zozo/CodeCenter/helm-ag/Eask... done!

Loading package information... done ✓
Loading /Users/zozo/CodeCenter/helm-ag/test/test-util.el (source)...
Running 23 tests (2022-10-06 22:03:02+0800, selector ‘t’)
   passed   1/23  construct-command (0.000362 sec)
   passed   2/23  construct-command-for-rg (0.000237 sec)
   passed   3/23  construct-command-in-do-ag (0.001074 sec)
   passed   4/23  construct-command-this-file (0.000169 sec)
   passed   5/23  construct-command-with-extra-option-in-do-ag (0.000194 sec)
   passed   6/23  construct-command-with-ignore-options-for-rg (0.000168 sec)
   passed   7/23  construct-command-with-ignore-patterns (0.000164 sec)
   passed   8/23  construct-command-with-options (0.000256 sec)
   passed   9/23  construct-command-with-options-for-rg (0.000179 sec)
   passed  10/23  construct-command-with-options-in-input (0.000531 sec)
   passed  11/23  convert-to-emacs-lisp-regexp (0.000267 sec)
   passed  12/23  emacs-lisp-regexp-to-pcre (0.000125 sec)
   passed  13/23  join-pattern (0.000221 sec)
   passed  14/23  judge-ignore-case (0.000072 sec)
   passed  15/23  parse-query (0.000242 sec)
   passed  16/23  search-this-file-p (0.000135 sec)
   passed  17/23  set-command-features (0.000143 sec)
   passed  18/23  split-string (0.000186 sec)
   passed  19/23  transform-for-files (0.000073 sec)
   passed  20/23  transform-for-this-file (0.000051 sec)
   passed  21/23  validate-regexp-with-invalid-regexp (0.000041 sec)
   passed  22/23  validate-regexp-with-valid-regexp (0.000040 sec)
   passed  23/23  visited-buffers (0.013920 sec)

Ran 23 tests, 23 results as expected, 0 unexpected (2022-10-06 22:03:02+0800, 0.021485 sec)

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

Successfully merging this pull request may close these issues.

1 participant