add rg-dwim-current-file to narrow rg-dwim even more. #55
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kidd
force-pushed
the
add-rg-dwim-current-file
branch
from
a70dcd0
to
d7978cd
Compare
dajva
requested changes
Feb 4, 2019
There was a problem hiding this comment.
Thanks, some generalt comments below. I added some details in you commit that I want to be fixed before merging.
- Running the tests locally. Should be really simple (in theory). Just install cask and then run
makefrom top dir. - One issue with this solution that I didn't think of is that the
:fileskey is still a pattern, meaning that it's not just the current file but all files with the same name in any subdir of the current dir. I don't think this is a big issue, would be worse if this was done in project dir. But, I think this would be good to at least aknowledge in the documentation. - The actual problem with the test is likely the use of
=, which according to documentation wants NUMBER-OR-MARKER as argument. At least that the error I see when running the tests locally. Try withequalor eveneqmay work.
rg.el
Outdated
| @@ -707,6 +707,14 @@ under the current directory." | |||
| :files current | |||
| :dir current) | |||
|
|
|||
| ;;;###autoload (autoload 'rg-dwim-current-file "rg.el" "" t) | |||
| (rg-define-search rg-dwim-current-file | |||
| "Search for thing at point in the current file." | |||
There was a problem hiding this comment.
I would like this to reflect that the current file is still treated as a pattern. Maybe "Search for thing at point with the current file name as pattern."?
rg.el
Outdated
| @@ -715,9 +723,10 @@ point in files matching current file under project root | |||
| directory. With \\[universal-argument] prefix (CURDIR), search is | |||
There was a problem hiding this comment.
This needs documentation of the double universal argument behavior.
rg.el
Outdated
| (rg-dwim-current-dir) | ||
| (rg-dwim-project-dir))) | ||
| (cond | ||
| ((= 4 (and (consp curdir) (car curdir))) (rg-dwim-current-dir)) |
kidd
force-pushed
the
add-rg-dwim-current-file
branch
2 times, most recently
from
172b15f
to
094023d
Compare
Following the trend of c-u meaning "more focused", this commit implements c-u c-u rg-dwim as even narrower search, searching just in the current file.
kidd
force-pushed
the
add-rg-dwim-current-file
branch
from
094023d
to
6470d52
Compare
|
Hello, and thanks for the review. Totally right on all points. The whole thing wasn't "ready" for merge yet. I think I addressed all of them. If you have further ideas on making the wording clearer, feel free to merge and edit the docstrings right away. |
dajva
approved these changes
Feb 5, 2019
|
LGTM. Merged. Thanks. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Following the trend of c-u meaning "more focused", this commit
implements c-u c-u rg-dwim as even narrower search, searching just in
the current file.
Still have to figure out how to run the tests correctly in my box.