swiper icon indicating copy to clipboard operation
swiper copied to clipboard

counsel-git-grep-other-window

Open flooose opened this issue 10 months ago • 3 comments

Good morning!

I've frequently found myself wanting to counsel-git-grep, but having the results open in another window. I was able to achieve this by splitting up counsel-git-grep-action in the following way:

(defun counsel-git-grep-action (x)
  "Go to occurrence X in current Git repository."
  (when (string-match "\\`\\(.*?\\):\\([0-9]+\\):\\(.*\\)\\'" x)
    (let ((file-name (match-string-no-properties 1 x))
          (line-number (match-string-no-properties 2 x)))
      (find-file (expand-file-name
                  file-name
                  (ivy-state-directory ivy-last)))
      (counsel-git-grep-go-to-location line-number))))

(defun counsel-git-grep-action-other-window (x)
  "Go to occurrence X in current Git repository, other window."
  (when (string-match "\\`\\(.*?\\):\\([0-9]+\\):\\(.*\\)\\'" x)
    (let ((file-name (match-string-no-properties 1 x))
          (line-number (match-string-no-properties 2 x)))
      (find-file-other-window (expand-file-name
                  file-name
                  (ivy-state-directory ivy-last)))
      (counsel-git-grep-go-to-location line-number))))

(defun counsel-git-grep-go-to-location (line-number)
  (goto-char (point-min))
  (forward-line (1- (string-to-number line-number)))
  (when (re-search-forward (ivy--regex ivy-text t) (line-end-position) t)
    (when swiper-goto-start-of-match
      (goto-char (match-beginning 0))))
  (swiper--ensure-visible)
  (run-hooks 'counsel-grep-post-action-hook)
  (unless (eq ivy-exit 'done)
    (swiper--cleanup)
    (swiper--add-overlays (ivy--regex ivy-text))))

and then setting an ivy-action to the command counsel-git-grep

(ivy-set-actions
 'counsel-git-grep
 '(("w" counsel-git-grep-action-other-window "other window")))

I'm wondering, if

  1. this functionality is already possible, and I simply overlooked it?
  2. there is any interest in adding this to counsel and this approach is generally the form that it should take? (I realize there is some opportunity to DRY these functions a little more.

I've seen CONTRIBUTING.org and the section about Copyright Assignment. I'll be happy to fill out the form, make any suggested changes to the code above and prepare a PR, if there is interest in adding this functionality.

flooose avatar Apr 16 '24 12:04 flooose

Hello, thanks for looking into this!

(ivy-set-actions
 'counsel-git-grep
 '(("w" counsel-git-grep-action-other-window "other window")))

Note that w is one of the three default actions provided by Ivy OOTB (for all completion functions):

  • o: default action (equivalent to pressing RET)
  • i: insert candidate at point
  • w: copy candidate to kill ring

So we should probably avoid overriding w.

A common (but frustratingly not consistent) key for "other window" actions in Ivy/Swiper/Counsel is j: you can look at the results of

(counsel-git-grep "\"j\"")

in swiper.git.

I'm wondering, if

1. this functionality is already possible, and I simply overlooked it?

No and yes:

  • No: counsel-git-grep and adjacent commands like counsel-ag, counsel-rg, etc. AFAICT do not define an "other window" action.

  • Yes: Emacs 28 introduced a strictly more general mechanism for frequent operations like "other window" and "other frame"; here are the relevant etc/NEWS announcements:

    *** The key prefix 'C-x 5 5' displays next command buffer in a new frame. It's bound to the command 'other-frame-prefix' that requests the buffer of the next command to be displayed in a new frame.

    *** The key prefix 'C-x 4 4' displays next command buffer in a new window. It's bound to the command 'other-window-prefix' that requests the buffer of the next command to be displayed in a new window.

So, if you are on a sufficiently recent Emacs version, you could e.g. M-x counsel-git-grep RET foobar C-x 4 4 RET.

2. there is any interest in adding this to counsel

Yes, I would be more than happy to merge something like this (Counsel has to maintain existing features for backward compatibility even if vanilla Emacs provides a more general mechanism).

and this approach is generally the form that it should take?

Yes, the idea is to provide a core internal subroutine that does the general work, and call it from "this window", "other window", etc. wrappers which can be specified via ivy-set-actions.

I'll be happy to fill out the form

Excellent! Here's the form skeleton to fill in and email to [email protected]; they will be able to guide you through the process if you have any specific questions:

Please email the following information to [email protected], and we will send you
the assignment form for your past and future changes.

Please use your full legal name (in ASCII characters) as the subject line of
the message.
----------------------------------------------------------------------
REQUEST: SEND FORM FOR PAST AND FUTURE CHANGES

[What is the name of the program or package you're contributing to?]
Emacs

[Did you copy any files or text written by someone else in these changes?
 Even if that material is free software, we need to know about it.]


[Do you have an employer who might have a basis to claim to own your changes?
 Do you attend a school which might make such a claim?]


[For the copyright registration, what country are you a citizen of?]


[What year were you born?]


[Please write your email address here.]


[Please write your postal address here.]


[Which files have you changed so far, and which new files have you written
 so far?]


[Additional people we should notify about the progress of the assignment.]
Basil L. Contovounesios <[email protected]>

basil-conto avatar Apr 16 '24 16:04 basil-conto

Good to know about the more general options available in Emacs. Thank you.

I'll get process started for the copyright assignment and get a PR with the code changes ready in the mean time.

flooose avatar Apr 17 '24 21:04 flooose

No movement on the Copyright Assignment, but PR is up if you want to start giving feedback.

flooose avatar Apr 25 '24 22:04 flooose