PR: Several Find in Files improvements #4056
Conversation
|
You can leave this one as it is, then rebase against |
andfoy
force-pushed
the
find_in_files_bar
branch
from
3878ca1 to
934da70
Compare
spyder/plugins/findinfiles.py
Outdated
| @@ -112,7 +124,8 @@ def include_patterns(): | |||
| r'\.ipy$|\.pyw?$|\.rst$|\.txt$', | |||
| '.', | |||
| ] | |||
| return patterns | |||
| # return patterns | |||
| return [] | |||
There was a problem hiding this comment.
Why this returns an empty list? Isn't it better to simply remove this method?
spyder/widgets/findinfiles.py
Outdated
| @@ -13,102 +13,43 @@ | |||
|
|
|||
| # Standard library imports | |||
| from __future__ import with_statement | |||
| from __future__ import unicode_literals | |||
There was a problem hiding this comment.
unicode_literals introduces a lot of problems. Please remove it.
There was a problem hiding this comment.
Also, you can simplify importing from __future__ like this
from __future__ import with_statement, print_function
spyder/widgets/findinfiles.py
Outdated
| def initialize(self, path, python_path, hg_manifest, | ||
| include, exclude, texts, text_re): | ||
| self.results = {} | ||
| self.nb = 0 |
There was a problem hiding this comment.
Please use a more descriptive name instead of nb.
spyder/widgets/findinfiles.py
Outdated
| text += ' (' + _('interrupted') + ')' | ||
| self.set_title(title+text) | ||
|
|
||
| def init(self, search_text): |
There was a problem hiding this comment.
This is a bad name, please use a better one (I mean, there's already __init__ and initialize)
|
@andfoy, I left some minor comments. After you solve them, I think this is ready to merge! But before doing any changes, please remove your last 3 commits (the ones that merge with |
|
@andfoy, the last two commits you added are a mess! I'm going to remove them, and do a proper merge with 3.x. After that you could fix my comments from yesterday. |
ccordoba12
force-pushed
the
find_in_files_bar
branch
from
4ae7238 to
8765940
Compare
ccordoba12
force-pushed
the
find_in_files_bar
branch
from
76fbcca to
bec5c0d
Compare
|
@ccordoba12 I have some CI errors |
|
You're trying to merge your changes from yesterday with my changes from today, which is again introducing the mess I mentioned before. Please revert what you did (i.e. your last merge) and redo again your changes on top of my last commit on this branch (i.e. |
|
The error persists, even if the conflicting merge was removed |
|
Ok, don't worry. I'm seeing the same error locally. I'll find a fix, push it and merge your PR. Thanks a lot for your patience :-) |
ccordoba12
changed the title
|
This long PR is (finally!) ready. Thanks @andfoy for your work! |
Fixes #4197
Fixes #4134
Fixes #2963
Fixes #2850
Fixes #2730
Fixes #1555
After checking issue #4005, I've identified that the widget in it's current incarnation doesn't allow to track the status of the search, that implies that the User doesn't know if the search is taking place or not, especially when the search is launched in a big directory. Taking in account this requirement, a progress bar and a text field that indicates the current file scanned is added. Also, some efficiency issues and codification mismatches are being tackled in the present PR.
Now the Widget should look like this while a search task is taking place
