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

counsel.el: Add directory change support to counsel-git #2750

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

counsel.el: Add directory change support to counsel-git #2750

wants to merge 1 commit into from

Conversation

brownts
Copy link
Contributor

@brownts brownts commented Dec 8, 2020

Fixes #2740.

Adds a keymap for counsel-git allowing counsel-cd to be invoked. Additionally, counsel-git has been modified to allow an optional initial directory to be provided (as a result of invoking counsel-cd).

basil-conto
basil-conto requested changes Dec 8, 2020
View reviewed changes
Copy link
Collaborator

@basil-conto basil-conto left a comment

Choose a reason for hiding this comment

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

Thanks, see some comments below.

counsel.el Outdated
Comment on lines 1255 to 1258
(defvar counsel-git-map
(let ((map (make-sparse-keymap)))
(define-key map (kbd "C-x C-d") 'counsel-cd)
map))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: please always include a docstring.

@brownts
counsel.el: Add directory change support to counsel-git
8428a6d 
* counsel-git-map: Add keymap for counsel-git
* counsel-git-cmd: Adjusted command to remove --full-name
* counsel-git: Allow optional initial directory and configure keymap
@brownts brownts force-pushed the feature/counsel-git_cd branch from b2358d0 to 8428a6d Compare December 9, 2020 04:47
@brownts
Copy link
Contributor Author

brownts commented Dec 9, 2020

Thanks @basil-conto, I have now added a docstring to counsel-git-map. Additionally, nice catch on the second issue. I only tested at the git root level, so missed the issue when you move into a sub-directory. I have now addressed that by adjusting the counsel-git-cmd to not generate the complete path from the git root (via --full-name), but only relative to the directory where the search was started.

basil-conto
basil-conto approved these changes Dec 9, 2020
View reviewed changes
Copy link
Collaborator

@basil-conto basil-conto left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks, LGTM.

I'm not sure whether I can merge this without a copyright assignment to the FSF, though, given your cumulative changes to this project. Do you have a CA on file already? If not, would you be willing to? If you're unfamiliar with this, see (info "(emacs) Copyright Assignment"). Thanks.

@basil-conto basil-conto added enhancement waiting-for-ca Waiting for author's copyright assignment labels Dec 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@basil-conto basil-conto basil-conto approved these changes

Assignees
No one assigned
Labels
enhancement waiting-for-ca Waiting for author's copyright assignment
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Allow changing directory within counsel-git
2 participants
@brownts @basil-conto