-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Add tab completions for PathCombobox #2692
Add tab completions for PathCombobox #2692
Conversation
Want to give it a try @gb119? |
File "/Users/ssilvester/workspace/spyder-ide/spyderlib/widgets/comboboxes.py", line 200
return
^
TabError: inconsistent use of tabs and spaces in indentation |
what the f... |
I'd expect to be able to hit Tab to make a completion instead of taking me to the next widget, what do you think? |
hmmm did you use it? You mean hitting tab once you have selected from the list should be the same as enter (enter is the current behaviour) |
I removed the tab locally 😄 |
I mean while typing, just like in the command prompt. |
or in our completions widget in the editor. |
Ahhh hmm ok then that means that you want the inline mode, instead of the popup mode, http://doc.qt.io/qt-4.8/qcompleter.html#CompletionMode-enum ? |
Yes it totally makes sense, will fiddle around a bit more ;-) |
Dead link, but yes, that sounds right. |
Cool @goanpeca! Given this new functionality, I think we should remove the green and red coloring present in that dialog. What do you think? |
I could not agree more with you! I hate that red/green coloring! HATE! |
@blink1073 can you give it a try now? (use tab and double tab... it should feel like bash completion) @ccordoba12 I have not changed the colors yet. |
@goanpeca, beautiful! |
@ccordoba12 can you try this one out? I think we can leave the colors for now... and make those changes in a new PR, lets keep this one short |
Looks nice. On the colours issue - it's not great UI design to use red/green fonts to signify things as the most common form of colour blindness is red/gree (!). A small tick/cross emblem at the start of the path might have worked better - probably still has a value even with autocomplete as the user can typoe any sort of random string in there. |
Ok so I made a code update to get rid of colors and now display status icons.
This is how it looks Comments, @gb119 , @ccordoba12 , @blink1073 ? |
Maybe the icon would look better to the right :-p ? |
Very nice :-) I guess the icon location is a matter of taste, but I was going left on the analogy with the way valid https sites are shown in most web browsers (at least the ones I use...) which modify the appearance of the left side of the adress entry field. But you guys are the coders not me..... |
self.set_default_style() | ||
self.valid.emit(False, False) | ||
# else: | ||
# self.set_default_style() |
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 these 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.
Ok
Ok, no problem. This is still an improvement ;-)
I'm strongly +1 on hiding it on focus out. It's very distracting to see it there all the time. |
The AppVeyor failure is related to conda-build being unable to do a git checkout with non-ascii chars on its log. I think that means we should stop testing on Python 2.7 until that bug is fixed. @goanpeca, what do you think? |
@ccordoba12 ok, I will look into hiding the icon after focus out. Can you test it on windows? |
Yes, will do. I'm also going to comment our Appveyor tests for PY 2.7 so your PRs can be merged :-) |
@ccordoba12 done! Now the icon hides on focus out and shows on focus in. Can you test locally? |
Will do later today. Thanks for the change :-) El 04/11/15 a las 00:00, Gonzalo Peña-Castellanos escribió:
|
You need to rebase this one for my changes to Travis to take effect here. |
… on other widgets.
The text would jump left and right, that would be disturbing. One solution is to put the icon on the right side. |
Its failing for one build test https://travis-ci.org/spyder-ide/spyder/jobs/90208316 @ccordoba12, ideas? |
It was a random failure. I think you can restart the build when that happens, as all other @spyder-ide/core-developers :-) If not, please let me know how to change that. |
@ccordoba12 so merge? |
@@ -195,7 +280,7 @@ class UrlComboBox(PathComboBox): | |||
def __init__(self, parent, adjust_to_contents=False): | |||
PathComboBox.__init__(self, parent, adjust_to_contents) | |||
self.editTextChanged.disconnect(self.validate) | |||
|
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 blanks only if they're around the code you're touching. This is not a problem in this case because we almost never touch comboboxes
, so it's not a blocker for merging :-)
But I've had major headaches trying to merge things between 2.3 and master when that rule is not followed :-)
My idea is to (slowly) move to remove blanks and adhere to pep8, by following Raymond Hettinger advice: https://www.youtube.com/watch?v=wf-BqAjZb8M
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.
Sure, I have seen that video more than once ;-)
Very nice addition @goanpeca! Thanks for working on it :-) |
Add tab completions for PathCombobox
👍 |
Fixes #2677
Description
The PathComboBox now has auto completion which improves the UX. This affects the Global Working Directory Toolbar and the Find in Files pane.
Screenshot