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

Proposal: add swiper option to go to the start of the match, instead of its end #942

Closed
edkolev opened this Issue Apr 4, 2017 · 4 comments

Comments

Projects
None yet
3 participants
@edkolev
Contributor

edkolev commented Apr 4, 2017

Hi, and thanks for this package.

Would you accept a PR which adds an option to swiper which will make swiper to go the beginning of the match, not its end?

Something like this:

diff --git a/swiper.el b/swiper.el
index 7aa2875..754b9bf 100644
--- a/swiper.el
+++ b/swiper.el
@@ -683,36 +683,38 @@ WND, when specified is the window."
 (defun swiper--action (x)
   "Goto line X."
   (let ((ln (1- (read (or (get-text-property 0 'swiper-line-number x)
                           (and (string-match ":\\([0-9]+\\):.*\\'" x)
                                (match-string-no-properties 1 x))))))
         (re (ivy--regex ivy-text)))
     (if (null x)
         (user-error "No candidates")
       (with-ivy-window
         (unless (equal (current-buffer)
                        (ivy-state-buffer ivy-last))
           (switch-to-buffer (ivy-state-buffer ivy-last)))
         (goto-char swiper--point-min)
         (funcall (if swiper-use-visual-line
                      #'line-move
                    #'forward-line)
                  ln)
         (re-search-forward re (line-end-position) t)
+        (when (goto-char swiper--point-min) ;; and some other var is true, maybe `swiper-action-land-on-match-start`
+          (goto-char (match-beginning 0)))
         (swiper--ensure-visible)
         (cond (swiper-action-recenter
                (recenter))
               (swiper--current-window-start
                (set-window-start (selected-window) swiper--current-window-start)))
         (when (/= (point) swiper--opoint)
           (unless (and transient-mark-mode mark-active)
             (when (eq ivy-exit 'done)
               (push-mark swiper--opoint t)
               (message "Mark saved where search started"))))
         (add-to-history
          'regexp-search-ring
          re
          regexp-search-ring-max)
         (when (and (bound-and-true-p evil-mode)
                    (eq evil-search-module 'evil-search))
           (add-to-history 'evil-ex-search-history re)
           (setq evil-ex-search-pattern (list re t t))
@abo-abo

This comment has been minimized.

Owner

abo-abo commented Apr 4, 2017

What's the use case? Going to the end of the match is a very reasonable default; isearch does this as well.

@edkolev

This comment has been minimized.

Contributor

edkolev commented Apr 4, 2017

After using vim, and now evil, I'm used to this behaviour - vim's and evil's default search (both evil's own search and evil's isearch wrapper) go to the start.

I common use case I have is:

  1. search for foo with swiper (I have / mapped to swiper)
  2. replace the match with evil's cgn - but this surprises me because it replaces the next match, instead of the one that the point is on.

Another use case I have (again vim habit):

  1. wanting to change text from point to foo, I press c/foo RET - this changes text from point till foo and foo - I'm not used to this behaviour.

It's all personal preference based on muscle memory I guess.

@abo-abo

This comment has been minimized.

Owner

abo-abo commented Apr 4, 2017

Look at this code already in swiper--action:

(when (and (bound-and-true-p evil-mode)
                   (eq evil-search-module 'evil-search))
          (add-to-history 'evil-ex-search-history re)
          (setq evil-ex-search-pattern (list re t t))
          (when evil-ex-search-persistent-highlight
            (evil-ex-search-activate-highlight evil-ex-search-pattern)))

Maybe there's a way to make cgn to work with that.

Otherwise, please PR with a new user option, off by default.

@dieggsy

This comment has been minimized.

Contributor

dieggsy commented Apr 4, 2017

I think adding the user option as proposed here (thus moving to beginning of match) is probably the way to go, if cgn merely depends on the position of cursor relative to match. This feature was next on my idea list for swiper, but I haven't yet filled out my copyright assignment, so I'm glad others have interest in this!

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