Add tab completions for PathCombobox #2692

goanpeca · 2015-09-14T01:38:11Z

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

goanpeca · 2015-09-14T01:43:00Z

Want to give it a try @gb119?

blink1073 · 2015-09-14T01:53:23Z

  File "/Users/ssilvester/workspace/spyder-ide/spyderlib/widgets/comboboxes.py", line 200
    return
         ^
TabError: inconsistent use of tabs and spaces in indentation

goanpeca · 2015-09-14T01:56:49Z

what the f...

blink1073 · 2015-09-14T01:57:55Z

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?

goanpeca · 2015-09-14T01:59:24Z

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)

blink1073 · 2015-09-14T02:00:12Z

I removed the tab locally 😄
When I hit enter, it just closes the dialog.

blink1073 · 2015-09-14T02:00:48Z

I mean while typing, just like in the command prompt.

blink1073 · 2015-09-14T02:01:23Z

or in our completions widget in the editor.

goanpeca · 2015-09-14T02:02:38Z

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 ?

goanpeca · 2015-09-14T02:04:23Z

Yes it totally makes sense, will fiddle around a bit more ;-)

blink1073 · 2015-09-14T02:04:48Z

Dead link, but yes, that sounds right.

ccordoba12 · 2015-09-14T02:39:35Z

Cool @goanpeca! Given this new functionality, I think we should remove the green and red coloring present in that dialog. What do you think?

goanpeca · 2015-09-14T02:40:41Z

I could not agree more with you! I hate that red/green coloring! HATE!

goanpeca · 2015-09-14T05:44:55Z

@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.

blink1073 · 2015-09-14T13:07:47Z

@goanpeca, beautiful!

goanpeca · 2015-09-14T16:15:18Z

@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

gb119 · 2015-09-14T18:46:04Z

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.

goanpeca · 2015-09-20T23:38:07Z

Ok so I made a code update to get rid of colors and now display status icons.

Single tab autocompletes if only one option
Double tab shows completions if more that one option
There are three icons, valid, invalid and set. To set the path an enter is required. If the box looses focus without having pressed enter it will returned to the last selected path.

This is how it looks

Comments, @gb119 , @ccordoba12 , @blink1073 ?

goanpeca · 2015-09-20T23:54:54Z

Maybe the icon would look better to the right :-p ?

gb119 · 2015-09-21T07:03:45Z

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.....

ccordoba12 · 2015-09-28T19:00:04Z

spyderlib/plugins/inspector.py

-            self.set_default_style()
+                    self.valid.emit(False, False)
+#        else:
+#            self.set_default_style()


Please remove these comments

ccordoba12 · 2015-11-03T16:11:20Z

Ok, I tried the separator and it basically breaks all the logic

Ok, no problem. This is still an improvement ;-)

The icon I can do, but I would like some opinions

I'm strongly +1 on hiding it on focus out. It's very distracting to see it there all the time.

ccordoba12 · 2015-11-03T16:15:09Z

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?

goanpeca · 2015-11-03T17:50:24Z

@ccordoba12 ok, I will look into hiding the icon after focus out.

Can you test it on windows?

ccordoba12 · 2015-11-03T20:27:03Z

Yes, will do. I'm also going to comment our Appveyor tests for PY 2.7 so your PRs can be merged :-)

goanpeca · 2015-11-04T05:00:08Z

@ccordoba12 done!

Now the icon hides on focus out and shows on focus in. Can you test locally?

ccordoba12 · 2015-11-04T14:23:52Z

Will do later today. Thanks for the change :-)

El 04/11/15 a las 00:00, Gonzalo Peña-Castellanos escribió:

@ccordoba12 https://github.com/ccordoba12 done!

Now the icon hides on focus out and shows on focus in. Can you test
locally?

—
Reply to this email directly or view it on GitHub
#2692 (comment).

ccordoba12 · 2015-11-05T02:37:33Z

You need to rebase this one for my changes to Travis to take effect here.

… on other widgets.

ccordoba12 · 2015-11-05T17:05:10Z

Now I don't like the white space that is left in focus out:

Can't we just remove the status icon in focus out and add it in focus in?

Nodd · 2015-11-05T20:09:36Z

The text would jump left and right, that would be disturbing. One solution is to put the icon on the right side.

ccordoba12 · 2015-11-05T20:19:23Z

@Nodd's solution is nicer, and simpler to implement than mine. @goanpeca could you make it happen?

goanpeca · 2015-11-09T23:35:11Z

Icon is on the right now, and it disappears on focus out.. reappears on focus in

Any last comments before merging?

goanpeca · 2015-11-09T23:49:26Z

Its failing for one build test

https://travis-ci.org/spyder-ide/spyder/jobs/90208316

@ccordoba12, ideas?

ccordoba12 · 2015-11-16T21:28:46Z

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.

goanpeca · 2015-11-16T21:31:21Z

@ccordoba12 so merge?

ccordoba12 · 2015-11-16T21:47:27Z

spyderlib/widgets/comboboxes.py

@@ -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)
-


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

Sure, I have seen that video more than once ;-)

ccordoba12 · 2015-11-16T21:48:45Z

Very nice addition @goanpeca! Thanks for working on it :-)

Add tab completions for PathCombobox

goanpeca · 2015-11-16T21:55:59Z

👍

goanpeca added the type:Enhancement label Sep 14, 2015

goanpeca self-assigned this Sep 14, 2015

goanpeca added this to the v3.0 milestone Sep 14, 2015

goanpeca changed the title ~~Add completion for PathCombobox (Global Working dir and find files)~~ Add completion for PathCombobox Sep 14, 2015

ccordoba12 reviewed Sep 28, 2015
View reviewed changes

goanpeca added 9 commits November 4, 2015 22:05

Add completion for PathCombobox (Global Working dir and find files)

6e4351f

Remove tab!

048e751

Adapt completion behavior to resemble bash completion

70e2210

Update PathCombobox behavior and style, update pep8 and perform fixes…

acfac95

… on other widgets.

Remove comments

270b8d3

Move to helper widgets, fix typo, refactor code

ed87382

Fix cpu over use

26aeb9f

Fix combobox refresh on style change

2a8bada

Hide status icon on focus out.

bb6ff77

Move PathComboBox status icon to the right.

e69220a

ccordoba12 reviewed Nov 16, 2015
View reviewed changes

ccordoba12 changed the title ~~Add completion for PathCombobox~~ Add tab completions for PathCombobox Nov 16, 2015

ccordoba12 added a commit that referenced this pull request Nov 16, 2015

Merge pull request #2692 from goanpeca/feature/global-autocomplete

496ecfc

Add tab completions for PathCombobox

ccordoba12 merged commit 496ecfc into spyder-ide:master Nov 16, 2015

Add tab completions for PathCombobox #2692

Add tab completions for PathCombobox #2692

Conversation

goanpeca commented Sep 14, 2015

Description

Screenshot

goanpeca commented Sep 14, 2015

blink1073 commented Sep 14, 2015

goanpeca commented Sep 14, 2015

blink1073 commented Sep 14, 2015

goanpeca commented Sep 14, 2015

blink1073 commented Sep 14, 2015

blink1073 commented Sep 14, 2015

blink1073 commented Sep 14, 2015

goanpeca commented Sep 14, 2015

goanpeca commented Sep 14, 2015

blink1073 commented Sep 14, 2015

ccordoba12 commented Sep 14, 2015

goanpeca commented Sep 14, 2015

goanpeca commented Sep 14, 2015

blink1073 commented Sep 14, 2015

goanpeca commented Sep 14, 2015

gb119 commented Sep 14, 2015

goanpeca commented Sep 20, 2015

goanpeca commented Sep 20, 2015

gb119 commented Sep 21, 2015

ccordoba12 Sep 28, 2015

Choose a reason for hiding this comment

goanpeca Sep 28, 2015

Choose a reason for hiding this comment

ccordoba12 commented Nov 3, 2015

ccordoba12 commented Nov 3, 2015

goanpeca commented Nov 3, 2015

ccordoba12 commented Nov 3, 2015

goanpeca commented Nov 4, 2015

ccordoba12 commented Nov 4, 2015

ccordoba12 commented Nov 5, 2015

ccordoba12 commented Nov 5, 2015

Nodd commented Nov 5, 2015

ccordoba12 commented Nov 5, 2015

goanpeca commented Nov 9, 2015

goanpeca commented Nov 9, 2015

ccordoba12 commented Nov 16, 2015

goanpeca commented Nov 16, 2015

ccordoba12 Nov 16, 2015

Choose a reason for hiding this comment

goanpeca Nov 16, 2015

Choose a reason for hiding this comment

ccordoba12 commented Nov 16, 2015

goanpeca commented Nov 16, 2015