Add support for opening counsel-git-grep in other window #3044
Conversation
New regex:"\\`\\(.*\\):\\([[:digit:]]+\\):\\(.*\\)\\'" Old regex: "\\`\\(.*?\\):\\([0-9]+\\):\\(.*\\)\\'"
counsel.el
Outdated
| (defun counsel--git-grep-file-and-line-number (x) | ||
| "Returns a cons cell of file-name and line number" | ||
| (when (string-match (rx grouped-regex-matches) x) | ||
| (cons (match-string-no-properties 1 x) (match-string-no-properties 2 x)))) |
There was a problem hiding this comment.
This function is meant to return a file name and a line number: file names are usually strings, and line numbers are usually numbers, so I still think it would be cleaner to parse the second match group as a number here, instead of passing around a string to be parsed as a number later. Then counsel--git-grep-go-to-location won't need to know that its argument is actually a string that has to first be parsed.
counsel.el
Outdated
| (cons (match-string-no-properties 1 x) (match-string-no-properties 2 x)))) | ||
|
|
||
| (defun counsel--git-grep-go-to-location (line-number) | ||
| "Go to line-number within current buffer" |
There was a problem hiding this comment.
The first line of a docstring is a short but complete sentence (ending in a full stop), and argument names are written in all-caps. If you're interested in the full details of Elisp docstring conventions, feel free to look through (elisp) Documentation Tips.
So in this case, the docstring could be something like:
"Go to LINE-NUMBER in current buffer for `counsel-git-grep'."Similarly counsel--git-grep-file-and-line-number above could be something like:
"Extract file name and line number from `counsel-git-grep' line X.
Return a pair (FILE . LINE) on success; nil otherwise."
Adds functionality described in related issue