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

counsel-git-grep-other-window #3043

Open
flooose opened this issue Apr 16, 2024 · 3 comments
Open

counsel-git-grep-other-window #3043

flooose opened this issue Apr 16, 2024 · 3 comments
Labels
enhancement waiting-for-ca Waiting for author's copyright assignment

Comments

@flooose
Copy link

flooose commented Apr 16, 2024

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.

@basil-conto
Copy link
Collaborator

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 assign@gnu.org; they will be able to guide you through the process if you have any specific questions:

Please email the following information to assign@gnu.org, 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 <basil@contovou.net>

@basil-conto basil-conto added enhancement waiting-for-ca Waiting for author's copyright assignment labels Apr 16, 2024
@flooose
Copy link
Author

flooose commented Apr 17, 2024

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
Copy link
Author

flooose commented Apr 25, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement waiting-for-ca Waiting for author's copyright assignment
Projects
None yet
Development

No branches or pull requests

2 participants