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.

Sign up for GitHub

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

Jump to bottom

Fix bad boundary matches #188

Merged
merged 5 commits into from
Dec 17, 2017
Merged

Fix bad boundary matches #188

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bc2b1c0
add `grep` prefix for grep tests to match other/later searchers
jacktasia Dec 14, 2017
f262999
add not test #186
jacktasia Dec 14, 2017
e6d55c3
update/fix boundaries
jacktasia Dec 15, 2017
9b48da6
Merge branch 'master' of github.com:jacktasia/dumb-jump into fix-bad-…
jacktasia Dec 15, 2017
504ac7c
add more \\j not rules
jacktasia Dec 15, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 16 additions & 10 deletions dumb-jump.el
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,25 +125,25 @@ or most optimal searcher."
:type 'string)

(defcustom dumb-jump-ag-word-boundary
"(?![\\w\\?\\*-])"
"(?![a-zA-Z0-9\\?\\*-])"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The motivation for using a-zA-Z0-9 instead of \w is that you don't want it to match for _ which is the only other character not matched by this change?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some searchers it was causing bad matches as outlined in #186 . For example, saying tester was a match when jumping on test. You can see evidence of this in the early commits for this PR with the ❌s, but here's a deep link to one example: https://travis-ci.org/jacktasia/dumb-jump/jobs/316563561
Thanks again for looking at this!

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This leads me to believe for these searchers with the options we give them \w is not recognized at all.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very weird but I see the point. Np :)

"`\\b` thinks `-` is a word boundary. When this matters use `\\j` instead and ag will use this value."
:group 'dumb-jump
:type 'string)

(defcustom dumb-jump-rg-word-boundary
"($|[^\\w\\?\\*-])"
"($|[^a-zA-Z0-9\\?\\*-])"
"`\\b` thinks `-` is a word boundary. When this matters use `\\j` instead and rg will use this value."
:group 'dumb-jump
:type 'string)

(defcustom dumb-jump-git-grep-word-boundary
"($|[^\\w\\?\\*-])"
"($|[^a-zA-Z0-9\\?\\*-])"
"`\\b` thinks `-` is a word boundary. When this matters use `\\j` instead and git grep will use this value."
:group 'dumb-jump
:type 'string)

(defcustom dumb-jump-grep-word-boundary
"($|[^\\w\\?\\*-])"
"($|[^a-zA-Z0-9\\?\\*-])"
"`\\b` thinks `-` is a word boundary. When this matters use `\\j` instead and grep will use this value."
:group 'dumb-jump
:type 'string)
Expand Down Expand Up @@ -213,20 +213,26 @@ or most optimal searcher."
:type 'boolean)

(defcustom dumb-jump-find-rules
'((:type "function" :supports ("ag" "grep" "rg" "git-grep") :language "elisp" :regex "\\\((defun|cl-defun)\\s+JJJ\\j"
'((:type "function" :supports ("ag" "grep" "rg" "git-grep") :language "elisp"
:regex "\\\((defun|cl-defun)\\s+JJJ\\j"
;; \\j usage see `dumb-jump-ag-word-boundary`
:tests ("(defun test (blah)" "(defun test\n" "(cl-defun test (blah)" "(cl-defun test\n")
:not ("(defun test-asdf (blah)" "(defun test-blah\n" "(cl-defun test-asdf (blah)" "(cl-defun test-blah\n"))

:not ("(defun test-asdf (blah)" "(defun test-blah\n" "(cl-defun test-asdf (blah)"
"(cl-defun test-blah\n" "(defun tester (blah)" "(defun test? (blah)" "(defun test- (blah)"))

(:type "variable" :supports ("ag" "grep" "rg" "git-grep") :language "elisp"
:regex "\\\(defvar\\b\\s*JJJ\\j" :tests ("(defvar test " "(defvar test\n"))
:regex "\\\(defvar\\b\\s*JJJ\\j"
:tests ("(defvar test " "(defvar test\n")
:not ("(defvar tester" "(defvar test?" "(defvar test-"))

(:type "variable" :supports ("ag" "grep" "rg" "git-grep") :language "elisp"
:regex "\\\(defcustom\\b\\s*JJJ\\j" :tests ("(defcustom test " "(defcustom test\n"))
:regex "\\\(defcustom\\b\\s*JJJ\\j"
:tests ("(defcustom test " "(defcustom test\n")
:not ("(defcustom tester" "(defcustom test?" "(defcustom test-"))

(:type "variable" :supports ("ag" "grep" "rg" "git-grep") :language "elisp"
:regex "\\\(setq\\b\\s*JJJ\\j" :tests ("(setq test 123)") :not ("setq test-blah 123)"))
:regex "\\\(setq\\b\\s*JJJ\\j" :tests ("(setq test 123)")
:not ("setq test-blah 123)" "(setq tester" "(setq test?" "(setq test-"))

(:type "variable" :supports ("ag" "grep" "rg" "git-grep") :language "elisp"
:regex "\\\(JJJ\\s+" :tests ("(let ((test 123)))") :not ("(let ((test-2 123)))"))
Expand Down
38 changes: 19 additions & 19 deletions test/dumb-jump-test.el
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,10 @@
(let* ((system-type 'darwin)
(regexes (dumb-jump-get-contextual-regexes "elisp" nil 'grep))
(expected-regexes (--map (concat " -e " (shell-quote-argument it))
'("\\((defun|cl-defun)\\s+tester($|[^\\w\\?\\*-])"
"\\(defvar\\b\\s*tester($|[^\\w\\?\\*-])"
"\\(defcustom\\b\\s*tester($|[^\\w\\?\\*-])"
"\\(setq\\b\\s*tester($|[^\\w\\?\\*-])"
'("\\((defun|cl-defun)\\s+tester($|[^a-zA-Z0-9\\?\\*-])"
"\\(defvar\\b\\s*tester($|[^a-zA-Z0-9\\?\\*-])"
"\\(defcustom\\b\\s*tester($|[^a-zA-Z0-9\\?\\*-])"
"\\(setq\\b\\s*tester($|[^a-zA-Z0-9\\?\\*-])"
"\\(tester\\s+")))
(expected (concat "LANG=C grep -REn --include \\*.el --include \\*.el.gz" (s-join "" expected-regexes) " .")))
(should (string= expected (dumb-jump-generate-grep-command "tester" "blah.el" "." regexes "elisp" nil)))))
Expand All @@ -93,37 +93,37 @@
(let* ((system-type 'darwin)
(regexes (dumb-jump-get-contextual-regexes "elisp" nil 'gnu-grep))
(expected-regexes (--map (concat " -e " (shell-quote-argument it))
'("\\((defun|cl-defun)[[:space:]]+tester($|[^\\w\\?\\*-])"
"\\(defvar\\b[[:space:]]*tester($|[^\\w\\?\\*-])"
"\\(defcustom\\b[[:space:]]*tester($|[^\\w\\?\\*-])"
"\\(setq\\b[[:space:]]*tester($|[^\\w\\?\\*-])"
'("\\((defun|cl-defun)[[:space:]]+tester($|[^a-zA-Z0-9\\?\\*-])"
"\\(defvar\\b[[:space:]]*tester($|[^a-zA-Z0-9\\?\\*-])"
"\\(defcustom\\b[[:space:]]*tester($|[^a-zA-Z0-9\\?\\*-])"
"\\(setq\\b[[:space:]]*tester($|[^a-zA-Z0-9\\?\\*-])"
"\\(tester[[:space:]]+")))
(expected (concat "LANG=C grep -rEn" (s-join "" expected-regexes) " .")))
(should (string= expected (dumb-jump-generate-gnu-grep-command "tester" "blah.el" "." regexes "elisp" nil)))))

(ert-deftest dumb-jump-generate-ag-command-no-ctx-test ()
(let* ((regexes (dumb-jump-get-contextual-regexes "elisp" nil 'ag))
(expected-regexes "\\((defun|cl-defun)\\s+tester(?![\\w\\?\\*-])|\\(defvar\\b\\s*tester(?![\\w\\?\\*-])|\\(defcustom\\b\\s*tester(?![\\w\\?\\*-])|\\(setq\\b\\s*tester(?![\\w\\?\\*-])|\\(tester\\s+|\\((defun|cl-defun)\\s*.+\\(?\\s*tester(?![\\w\\?\\*-])\\s*\\)?")
(expected-regexes "\\((defun|cl-defun)\\s+tester(?![a-zA-Z0-9\\?\\*-])|\\(defvar\\b\\s*tester(?![a-zA-Z0-9\\?\\*-])|\\(defcustom\\b\\s*tester(?![a-zA-Z0-9\\?\\*-])|\\(setq\\b\\s*tester(?![a-zA-Z0-9\\?\\*-])|\\(tester\\s+|\\((defun|cl-defun)\\s*.+\\(?\\s*tester(?![a-zA-Z0-9\\?\\*-])\\s*\\)?")
(expected (concat "ag --nocolor --nogroup --elisp " (shell-quote-argument expected-regexes) " .")))
(should (string= expected (dumb-jump-generate-ag-command "tester" "blah.el" "." regexes "elisp" nil)))))

(ert-deftest dumb-jump-generate-rg-command-no-ctx-test ()
(let* ((regexes (dumb-jump-get-contextual-regexes "elisp" nil 'rg))
(expected-regexes "\\((defun|cl-defun)\\s+tester($|[^\\w\\?\\*-])|\\(defvar\\b\\s*tester($|[^\\w\\?\\*-])|\\(defcustom\\b\\s*tester($|[^\\w\\?\\*-])|\\(setq\\b\\s*tester($|[^\\w\\?\\*-])|\\(tester\\s+|\\((defun|cl-defun)\\s*.+\\(?\\s*tester($|[^\\w\\?\\*-])\\s*\\)?")
(expected-regexes "\\((defun|cl-defun)\\s+tester($|[^a-zA-Z0-9\\?\\*-])|\\(defvar\\b\\s*tester($|[^a-zA-Z0-9\\?\\*-])|\\(defcustom\\b\\s*tester($|[^a-zA-Z0-9\\?\\*-])|\\(setq\\b\\s*tester($|[^a-zA-Z0-9\\?\\*-])|\\(tester\\s+|\\((defun|cl-defun)\\s*.+\\(?\\s*tester($|[^a-zA-Z0-9\\?\\*-])\\s*\\)?")
(expected (concat "rg --color never --no-heading --line-number --type elisp " (shell-quote-argument expected-regexes) " .")))
(should (string= expected (dumb-jump-generate-rg-command "tester" "blah.el" "." regexes "elisp" nil)))))

(ert-deftest dumb-jump-generate-git-grep-command-no-ctx-test ()
(let* ((regexes (dumb-jump-get-contextual-regexes "elisp" nil 'git-grep))
(expected-regexes "\\((defun|cl-defun)\\s+tester($|[^\\w\\?\\*-])|\\(defvar\\b\\s*tester($|[^\\w\\?\\*-])|\\(defcustom\\b\\s*tester($|[^\\w\\?\\*-])|\\(setq\\b\\s*tester($|[^\\w\\?\\*-])|\\(tester\\s+|\\((defun|cl-defun)\\s*.+\\(?\\s*tester($|[^\\w\\?\\*-])\\s*\\)?")
(expected-regexes "\\((defun|cl-defun)\\s+tester($|[^a-zA-Z0-9\\?\\*-])|\\(defvar\\b\\s*tester($|[^a-zA-Z0-9\\?\\*-])|\\(defcustom\\b\\s*tester($|[^a-zA-Z0-9\\?\\*-])|\\(setq\\b\\s*tester($|[^a-zA-Z0-9\\?\\*-])|\\(tester\\s+|\\((defun|cl-defun)\\s*.+\\(?\\s*tester($|[^a-zA-Z0-9\\?\\*-])\\s*\\)?")
(excludes '("one" "two" "three"))
(expected (concat "git grep --color=never --line-number --untracked -E " (shell-quote-argument expected-regexes) " -- ./\\*.el ./\\*.el.gz \\:\\(exclude\\)one \\:\\(exclude\\)two \\:\\(exclude\\)three")))
(should (string= expected (dumb-jump-generate-git-grep-command "tester" "blah.el" "." regexes "elisp" excludes)))))

(ert-deftest dumb-jump-generate-git-grep-command-not-search-untracked-test ()
(let* ((dumb-jump-git-grep-search-untracked nil)
(regexes (dumb-jump-get-contextual-regexes "elisp" nil 'git-grep))
(expected-regexes "\\((defun|cl-defun)\\s+tester($|[^\\w\\?\\*-])|\\(defvar\\b\\s*tester($|[^\\w\\?\\*-])|\\(defcustom\\b\\s*tester($|[^\\w\\?\\*-])|\\(setq\\b\\s*tester($|[^\\w\\?\\*-])|\\(tester\\s+|\\((defun|cl-defun)\\s*.+\\(?\\s*tester($|[^\\w\\?\\*-])\\s*\\)?")
(expected-regexes "\\((defun|cl-defun)\\s+tester($|[^a-zA-Z0-9\\?\\*-])|\\(defvar\\b\\s*tester($|[^a-zA-Z0-9\\?\\*-])|\\(defcustom\\b\\s*tester($|[^a-zA-Z0-9\\?\\*-])|\\(setq\\b\\s*tester($|[^a-zA-Z0-9\\?\\*-])|\\(tester\\s+|\\((defun|cl-defun)\\s*.+\\(?\\s*tester($|[^a-zA-Z0-9\\?\\*-])\\s*\\)?")
(excludes '("one" "two" "three"))
(expected (concat "git grep --color=never --line-number -E " (shell-quote-argument expected-regexes) " -- ./\\*.el ./\\*.el.gz \\:\\(exclude\\)one \\:\\(exclude\\)two \\:\\(exclude\\)three")))
(should (string= expected (dumb-jump-generate-git-grep-command "tester" "blah.el" "." regexes "elisp" excludes)))))
Expand All @@ -132,7 +132,7 @@
(let* ((system-type 'darwin)
(dumb-jump-functions-only t)
(regexes (dumb-jump-get-contextual-regexes "elisp" nil 'grep))
(expected-regexes "\\((defun|cl-defun)\\s+tester($|[^\\w\\?\\*-])")
(expected-regexes "\\((defun|cl-defun)\\s+tester($|[^a-zA-Z0-9\\?\\*-])")
(expected (concat "LANG=C grep -REn -e " (shell-quote-argument expected-regexes) " ."))
(zexpected (concat "LANG=C zgrep -REn -e " (shell-quote-argument expected-regexes) " .")))
(should (string= expected (dumb-jump-generate-grep-command "tester" "blah.el" "." regexes "" nil)))
Expand All @@ -143,7 +143,7 @@
(ctx-type (dumb-jump-get-ctx-type-by-language "elisp" '(:left "(" :right nil)))
(dumb-jump-ignore-context nil) ;; overriding the default
(regexes (dumb-jump-get-contextual-regexes "elisp" ctx-type 'grep))
(expected-regexes "\\((defun|cl-defun)\\s+tester($|[^\\w\\?\\*-])")
(expected-regexes "\\((defun|cl-defun)\\s+tester($|[^a-zA-Z0-9\\?\\*-])")
(expected (concat "LANG=C grep -REn -e " (shell-quote-argument expected-regexes) " .")))
;; the point context being passed should match a "function" type so only the one command
(should (string= expected (dumb-jump-generate-grep-command "tester" "blah.el" "." regexes "" nil)))))
Expand All @@ -154,7 +154,7 @@
(ctx-type (dumb-jump-get-ctx-type-by-language "elisp" '(:left "(" :right nil)))
(dumb-jump-ignore-context nil) ;; overriding the default
(regexes (dumb-jump-get-contextual-regexes "elisp" ctx-type 'grep))
(expected-regexes "\\((defun|cl-defun)\\s+tester($|[^\\w\\?\\*-])")
(expected-regexes "\\((defun|cl-defun)\\s+tester($|[^a-zA-Z0-9\\?\\*-])")
(expected (concat "grep -REn -e " (shell-quote-argument expected-regexes) " .")))
(should (string= expected (dumb-jump-generate-grep-command "tester" "blah.el" "." regexes "" nil))))))

Expand All @@ -164,10 +164,10 @@
(dumb-jump-ignore-context t)
(regexes (dumb-jump-get-contextual-regexes "elisp" ctx-type nil))
(expected-regexes (--map (concat " -e " (shell-quote-argument it))
'("\\((defun|cl-defun)\\s+tester($|[^\\w\\?\\*-])"
"\\(defvar\\b\\s*tester($|[^\\w\\?\\*-])"
"\\(defcustom\\b\\s*tester($|[^\\w\\?\\*-])"
"\\(setq\\b\\s*tester($|[^\\w\\?\\*-])"
'("\\((defun|cl-defun)\\s+tester($|[^a-zA-Z0-9\\?\\*-])"
"\\(defvar\\b\\s*tester($|[^a-zA-Z0-9\\?\\*-])"
"\\(defcustom\\b\\s*tester($|[^a-zA-Z0-9\\?\\*-])"
"\\(setq\\b\\s*tester($|[^a-zA-Z0-9\\?\\*-])"
"\\(tester\\s+")))
(expected (concat "LANG=C grep -REn" (s-join "" expected-regexes) " .")))

Expand Down