-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Suggest ag -Q
when relevant
#561
Conversation
This detects when `ag` suggests the `-Q` option, and adds it.
b090596
to
d2e0a19
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice additions. I've gathered some comments.
Thanks!
|
||
matching_command = Command(script='ag \\(', stderr=stderr) | ||
|
||
@pytest.mark.parametrize('command', [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, add a new line here. [E302 expected 2 blank lines, found 1]
@@ -0,0 +1,11 @@ | |||
from thefuck.utils import for_app | |||
from thefuck.utils import replace_argument |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove this unused import. [F401 imported but unused]
@pytest.mark.parametrize('command, new_command', [ | ||
(matching_command, 'ag -Q \\(')]) | ||
def test_get_new_command(command, new_command): | ||
assert get_new_command(command) == new_command |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd recommend refactoring the test as the following:
import pytest
from thefuck.rules.ag_literal import get_new_command, match
from tests.utils import Command
@pytest.fixture
def stderr():
return ('ERR: Bad regex! pcre_compile() failed at position 1: missing )\n'
'If you meant to search for a literal string, run ag with -Q\n')
@pytest.mark.parametrize('script', ['ag \('])
def test_match(script, stderr):
assert match(Command(script=script, stderr=stderr))
@pytest.mark.parametrize('script', ['ag foo'])
def test_not_match(script):
assert not match(Command(script=script))
@pytest.mark.parametrize('script, new_cmd', [
('ag \(', 'ag -Q \(')])
def test_get_new_command(script, new_cmd, stderr):
assert get_new_command((Command(script=script, stderr=stderr))) == new_cmd
|
||
@for_app('ag') | ||
def match(command): | ||
return 'run ag with -Q' in command.stderr |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't endswith()
a slightly better option than in
?
Thanks for the review, @scorphus! I've made some changes addressing your comments. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You nailed it, @josephfrazier! 👍
Thanks!
Thanks! |
Suggest `ag -Q` when relevant
This detects when
ag
suggests the-Q
option, and adds it.