Qtconsole menu #887

Merged
merged 52 commits into from Oct 20, 2011

3 participants

@fperez
IPython member

This PR is meant to continue the work started by @Carreau in #813, which was sadly interrupted by a robbery. Since we don't know when @Carreau will be able to resume regular work, I've rebased his branch on top of master (at his last commit it had a small conflict), and I'm putting it up here on a common branch that several of us could in principle work on.

Unfortunately, when this happened the branch was actually in a broken state, because the qtconsole doesn't start, failing with:

/home/fperez/usr/lib/python2.7/site-packages/IPython/frontend/qt/console/qtconsoleapp.py in init_window_shortcut()
   1134                 statusTip="Toggle menubar betwin visible and not",
   1135                 triggered=self.toggle_menu_bar)
-> 1136             self.window_menu.addAction(self.toggle_menu_bar_act)
   1137 
   1138     def toggle_menu_bar(self):
AttributeError: 'IPythonQtConsoleApp' object has no attribute 'window_menu'

I'm not sure what happened, since a few days ago I'd played with the branch and it was broken. I haven't had a chance to bisect it yet.

Basically, the point of putting this PR here is to see whether we can find the resources to finish up @Carreau's work to include it in 0.12. It's a large PR that had very nice functionality, and I would love to have his contribution in 0.12. But it's a lot of complex code that we can't just dump in there unless we're reasonably confident it's in good shape.

@epatters had given @Carreau already a ton of feedback, so you're the best positioned to gauge whether it's just missing a tiny bit of (doable) work or major effort that is not realistic in a short timeframe. I certainly won't take any action without your input.

In summary, before this can be merged, it needs:

  • fixing the current brokenness, since the qt console won't even start.
  • finishing up the remaining comments that were in-flight in the discussion for #813.
  • any other checks/fixes that @epatters might deem necessary.

We'll see if it can be done before 0.12 or not...

Carreau added some commits Sep 17, 2011
@Carreau Carreau create menu bar on OSX with some action
	console_widget:
		modifie/create some QAction to be able to put it in menu bar
			-made print_action and export_action public
			-transform increase/decrease/rest front_size
			 to QAction
			-create qaction for undo/redo

	MainWindow
		Create the menubar with the following structure
		And link each menu with its action in console Widget
		File
		|__Print
		|__Save as XML
		|__Select ALL
		Edit
		|__Undo
		|__Redo
		Font
		|__Increase font size
		|__Decresae font size
		|__Restet Font size
		Window
		|__Minimize
		|__Maximize
		|__Full Screen
		Magic
		|__Reset
		|__History
		|__Export History
		|__Who
		|__Whos
		|__Who_ls
		Help
		|_Open online Help

		The 'Magic' Menu, execute %magic after 'exit_keyboard' and pasting
		'%magic'.  The 'Export History' menu prompt the user for a file name
		and a range before pasting `%save filename range`.

	This this also fix a bug in toggle full screen where
	Full Screen -> (try to) Minimizing -> Out Of Full Screen
	is broken (grey background for all screen), at least on OSX
3bceb23
@Carreau Carreau Add icon to qtconsole app
	It's clearly not definitive, as most of the file are not in the right
	location and icon should be graphically improved, so il will certainly be rebased later.
	But as it goes with UI improvemetn in the menubar I add it to this branch.

	Note: About the svg Icon, I've done it from scratch in inkscape, inspired by
	iTerm2 icon, which is under GPL
	the .icns file is OSX Specific.
c181f23
@Carreau Carreau Fix last Commit Resources (Icones)
		Did export the wrong area with inkscape, making the Icons to Have the
		wrong aspect ratio once loaded in qt. Use this excuse to refine
		shadow/lightnig and general color
cfd327e
@Carreau Carreau Improve 'save history' menu (%save)
	Real file chooser menu, get the current session history range by default.
	If the file already exist, the user will be prompted twice if he/she really
	want to be override the existing file ( by the gui, and the magic afterward)
	One might also to add the (-r) %save option into the gui.
cf3ca98
@Carreau Carreau disable some QAction by default, remove OSX only, wrap in try/except 2230aba
@Carreau Carreau move some action in main windows as asked by @epatters.
	not all haves benn moves as i'm not sure yet how to handle some of them
5932a85
@Carreau Carreau move %history and %save action into mainwindow 9439e88
@Carreau Carreau Just indent a commentary with the right numbur of tabs and re-wrap it 3bdac69
@Carreau Carreau tab management new/existing kernel.
	working 2 kinds of tab
	with Ctrl+Shift+T -> tab attached on same kernel
	with Ctrl+T -> tab attached on new kernel

	closing event management is way far from the one of the mainWindow
	and "first tab" is still handeled differently as the other ones as some action
	are still attaches to it directly
eb0e7f3
@Carreau Carreau Send all action to active front instead of first tab 3bcd8ff
@Carreau Carreau trying to move closing logic on a tab basis 870cdf7
@Carreau Carreau Handle all the case of tab closing and clean the code 814dd41
@Carreau Carreau fx tab name and already existing kernel before application launch
	tabs are now names "kernel i"

	starting the application with --existing ... use to attached all new tabs
	to the same kernel. New tab attached to new kernel now start a local kernel
	on random port and connect to it.

	rename some variable to be more explicit.
4f5f50a
@Carreau Carreau Fix: crash when cancel closing slave tab e0cf8ec
@Carreau Carreau New tab on focus when created ffae8ff
@Carreau Carreau Add Shortcut Next/Previous tab cf2bb28
@Carreau Carreau Create Keyboard Accelerator for MenuBar b0f2f4d
@Carreau Carreau Break Hard dependency to Pyside ? 370944b
@Carreau Carreau don't quit if asked once not to shutdown a kernel 95617a5
@Carreau Carreau remove duplicate function "pasteMagic", and change code not to use it 6d414bb
@Carreau Carreau decamelCaseify : tabWidget -> tab_widget 58ef988
@Carreau Carreau decamelcasify : closeTab close_tab e73f585
@Carreau Carreau decamelcasify: 3 more changes f7c8a99
@Carreau Carreau decamelcase : activeFrontend active_frontend d8febfe
@Carreau Carreau decamelcasify: a few more dee94ca
@Carreau Carreau a few more deCamelCasification 0c9f556
@Carreau Carreau remove all binary trace from Icons, use directly SVG 51ebc86
@Carreau Carreau one commit again of decamelcasify. 57e7f84
@Carreau Carreau use svg icon also for dialog box.
	Note that svg doesn't render the shadow casting in QT, Maybe it's an
	inkscape specific feature
e80884f
@Carreau Carreau move open online help to be on all plateform 564d1ca
@Carreau Carreau unwrap adding action from try/except, add %guiref and '?' in help menu c48eab0
@Carreau Carreau add some blank lines cad795a
@Carreau Carreau Create all_magic_menu. use webbrowser to open url 58e0679
@Carreau Carreau reorganise menu
	add cut,copy paste, toggle menu bar (but not on os x)
	remove history oneliner from history console
0fc39ed
@Carreau Carreau remove old comment no longer usefull 9c42726
@Carreau Carreau move tab management to "Kernel" menu ef6e68c
@Carreau Carreau add copy_raw, interrupt kernel and restart kernel into menu 3d888bb
@Carreau
IPython member

Hi,
thank you for continuing my work, I really apreciate
I've got an old debian lenny up and running but on 512k, and not a laptop so I can't bring it to work where to update fast.

Just for the broken state, just try

self.window_menu.addAction(self.toggle_menu_bar_act)

to

self.**window**.window_menu.addAction(self.toggle_menu_bar_act)
``̀`

it's the inconvenient of  having qtconsole and mainWindow in one file... I'm mixing up when moving by searching in a file...

And by the way, the police found finger print ( a woman at first look according to one of them ) , which they told me  was pretty rare as thieves oven wear gloves.
I've also learn that they have 2 kind of powder for fingerprint: black for clear object and white for dark object !

see you soon hopefully.
@minrk
IPython member

Has been updated to use the connection files that were introduced in master since this work started. Seems to work fine now.

@fperez
IPython member

OK, summary status of where we are with this. The good news: it's looking pretty good, from the user side, the functionality is great. A few things of note:

  • the menu hide action isn't working, whether triggered from the Window menu or the kbd. Don't know why.
  • No keybindings to navigate between tabs. Ctrl-PgUp/PgDown should do that.

Other than these two, I think from the user side this is good to go (I'm going to make some tiny wording fixes in a second). @Carreau, if you have some suggestions or can find a bit of time/resources to pitch in, that would be great.

@epatters, what's your take on the internals regarding the Qt code? I won't merge this until you give it the final all-clear, so your input would be very welcome.

@fperez
IPython member

One more thing: the File menu should have a Quit action at the bottom, most people will expect that...

@fperez
IPython member

OK, I've made some small fixes to the text of some menu entries.

If we can fix the three points above (File/Quit, menu hide, tab keyboard navigation), this is good to merge as far as I'm concerned.

But @epatters has the final say on the internals of the Qt code, since he knows it 100x better than I and ultimately maintains it.

@minrk
IPython member

cmd/ctrl-w should also probably close tabs. I don't think it does.

@fperez
IPython member

agreed con c-w for tab close, plus c-q for quit.

@Carreau Carreau commented on an outdated diff Oct 18, 2011
IPython/frontend/qt/console/qtconsoleapp.py
+ self.prev_tab_act = QtGui.QAction("Pre&vious Tab",
+ self.window,
+ shortcut="Ctrl+PgDown",
+ statusTip="Cahange to next tab",
+ triggered=self.window.prev_tab)
+
+ self.next_tab_act = QtGui.QAction("Ne&xt Tab",
+ self.window,
+ shortcut="Ctrl+PgUp",
+ statusTip="Cahange to next tab",
+ triggered=self.window.next_tab)
@Carreau
IPython member
Carreau added a note Oct 18, 2011

This should handle next/prev tab
(typo btw 'Cahange')

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Carreau
IPython member
@fperez
IPython member
@fperez
IPython member

One more thing: in the qtconsole, Ctrl-P by default does history recall, so we should leave that alone. In this branch it triggers the print dialog, which is a much rarer need than efficient keyboard workflow.

@minrk
IPython member

Right - it's ctrl-shift-P on Linux, cmd-P on OSX, to avoid conflict.

@minrk
IPython member

I've fixed the ctrl-p shortcut, and added ctrl-w locally, but I think a bit more reorganization of menu items is in oder. Do we want the menu organization to match standard apps?

There are pretty well established conventions for the roles and ordering of menu entries.

For instance:

  • 'new tab' is typically under File
  • font size / fullscreen under a View menu
  • ordering of undo/redo cut/copy/paste in Edit
@fperez
IPython member
@minrk
IPython member

I've fixed the ctrl-p shortcut, and added ctrl-w locally

Where did you push?

I haven't pushed yet, I'm still working locally.

I've also done a little bit more reorganization, that I'll push soon.

A few more things I would change unless there are objections:

  • Move all the shortcuts into the MainWindow object, rather than having most of them there, and some of them in the IPythonApp
  • change tabs and fullscreen shortcuts are hidden on non-OSX, but I think we might as well leave them in the menu, since they are not universal, so people might find them useful, if only for learning/remembering the shortcuts.
  • I am also adjusting a few shortcuts on OSX, to match platform conventions:
    • fullscreen: cmd-shift-f
    • switch tabs is cmd-shift-left/right

Can I ask why the non-OSX fullscreen shortcut is ctrl-meta-space? I see F11 on gnome-terminal.

@fperez
IPython member
@minrk
IPython member

I'm testing and just about ready to push.

@Carreau asked if hiding the menu bar works on platforms that support it, and I tested it, and the answer is no. At first, the logic was just backwards (a miscreant 'not', easily found and fixed), but Qt has an interesting behavior: When the MenuBar is set as invisible, all shortcuts attached to it are no longer accessible. So hiding the menu bar means making it impossible to activate any of the shortcuts (including showing the menu bar again).

I can fix this easily enough, by attaching all menu bar actions to the application itself as well, to preserve their availability.

@minrk minrk Work on QtConsole Menu
* add new_frontend_factory, slave_frontend_factory callable arguments to MainWindow
  constructor, decoupling MainWindow and the IPythonApp a bit
* create *all* menu items in MainWindow.init_menubar, rather than some in the IPythonApp
* split init_menu_bar into methods for each menu, to ease organization
* attach all menu bar actions to the MainWindow as well, so that they are still
  accessible when Menu Bar is invisible
* add View menu for Fullscreen and Fonts
* organized menu items to match standard patterns
* Fewer platform-dependent menu items
5e65a46
@fperez
IPython member
@minrk
IPython member

Okay, I pushed with those changes, and (in a separate commit) gave MainWindow its own file as @epatters suggested.

@fperez
IPython member

Great! A few comments:

  • I'd change the menu shortcut to Ctrl-Shift-M instead of Meta. It's the only one using meta, and Ctrl-Shift-M is already used for menubar toggle in apps like konsole.

  • The tab nav keybindings are backwards from the behavior on chrome and firefox: pgup should go left, pgdown should go right.

  • There's a strange focus bug: open several tabs, then close one with C-w. The focus doesn't return to the window so it's impossible to type. I have to switch focus to a different window and back to the console to recover focus. No amount of clicking on the qtconsole helps. A visual cue of the problem is that the blinking cursor disappears.

  • I was wondering if we should change the close action on the whole window to ask about closing all tabs, and shutting them all down if affirmative. Right now if you have 10 tabs open, there's no way to close them all without answering 'yes' 10 times.

Awesome job, Min. This is almost ready to go, I'm excited. It makes the qt console a really appealing environment.

@minrk
IPython member

Sure, sorry - I misread your Ctrl-S-M before for some reason. I'll fix pgup/down as well.

Closing everything with Quit should get a single-confirm, and then close everything or do nothing. I'll look into that, as well as the weird focus bug.

@fperez
IPython member
@minrk minrk cleanup some closing logic
* Single prompt on quit event
* Disabled leaving the kernel running on window close, as it doesn't make as much sense
  now that there are multiple kernel windows
* Tabs respect confirm-exit setting
* Removed some obsolete kwargs from MainWindow.__init__, that are no longer used

Closing logic is still a bit wonky, depending on the ordering of tabs closed (masters, slaves, etc.).

also fix some spelling errors, and remove errant semicolons
8b6601c
@minrk
IPython member

Should be single dialog on app quit / window close, now, and the confirm-exit setting is respected.

Definitely not 100% sure the closing logic is right, as now with possibly many tabs closing out of order, there are a large number of possible cases, and the little mess that I had made with the close dialog when there could be only one window is now pretty tangled.

@fperez
IPython member
@minrk
IPython member

Ctrl/Cmd-Q for quit? And listing it at the bottom of the file menu?
I think people will expect that now that it's an app.

I used QKeySequence.Quit, which I figure we should use unless there is conflict. I assume that's Ctrl-Q on Linux.

Of course, it is supposed to already be at the bottom of File, but I inverted the sys.platform != darwin check so I could test on my mac, but never put it back. That should be fixed now.

C-S-p for print not working on linux.
focus bug remains, I don't know if you'd taken a stab at that...

I haven't checked on this, as I've been mostly testing on my Mac. I'll try on Linux after dinner. I haven't seen the focus bug in OSX, and printing works fine.

@minrk
IPython member

The problem with print (and save, select-all, etc.) is that the widget events are still bound to key-combos via the context-menu, and ambiguous events don't resolve (helpful printed error messages, though). I'll investigate.

Re: focus - it looks like the tab itself is highlighted, rather than the IPython area, after closing a tab without confirmation (either slave, or any with --no-confirm-exit). For me, hitting TAB moves focus back to the text area, but I'll look into why it's different.

@minrk minrk prevent widget shortcut conflicts with menu proxies
set some menu bar contexts to Widget, preventing conflict.

Essentially, these events will never be called by shortcut, as the events
for which they are a proxy are already bound to the shortcut, and get it instead.

This is a bit ugly, because the menu shortcuts aren't really active - they depend
on the widget shortcuts being the same.
915fe1f
@minrk
IPython member

Fixed the conflict with a slightly dirty trick of setting the menu bar shortcut context (for those that conflict) to widget-only. This means that they won't ever get invoked by shortcut (but they will display their shortcut keys), and the widget shortcuts actually do all the work.

@minrk
IPython member

disallowing focus in the tab-bar itself seems to have fixed the focus issue.

minrk added some commits Oct 19, 2011
@minrk minrk stop channels in background threads
stop_channels() *must* be called, to prevent fd count rising, but
stop_channels waits up to 0.2s up to 4 times, and it shouldn't get in the
way of the UI.  This just calls stop_channels in a thread, to protect
UI responsiveness.
aec0034
@minrk minrk Fix another small bug in stripping kernel args
Problem case: flags and aliases with the same name (e.g. existing)

would attempt removal twice, raising ValueError
cfc7e4a
@Carreau
IPython member

So much things in one night...
I've got a new (time-shared-with-my-girlfriend) computer, so I might be able to soon help. just need to reinstall everything

Can I ask why the non-OSX fullscreen shortcut is ctrl-meta-space? I see F11 on gnome-terminal.

There is no 'full screen' shortcut (in my knowledge ) on Mac, so I put one I use in a terminal, I may have missed a Qt.Shortcut.Fullscreen. Fxx Keys are not dirrecty accessible on newest mac, bind to volumes control for example

I was wondering if we should change the close action on the whole window to ask about closing all tabs, and shutting them all down if affirmative. Right now if you have 10 tabs open, there's no way to close them all without answering 'yes' 10 times.

I was thinking of a 3 way option:
-cancel
-close all
-ask each time

a more complexe option might be to add a "yes for all" or "no for all" button

Definitely not 100% sure the closing logic is right, as now with possibly many tabs closing out of order, there are a large number of possible cases, and the little mess that I had made with the close dialog when there could be only one window is now pretty tangled.

I had some issues with it too :-)

BTW, i just though of a 'bug' : tab names are based on number of open tabs.
I might mess up names if you closes tab without closing kernels. We might prefer counting the number of opened kernel...

@minrk
IPython member

Adding as a side note, that my somewhat stale install of Qt 4.7.1 did not support svg icons (who knows why). I updated Qt to current (4.7.4) and the icon works now.

@minrk
IPython member

Can I ask why the non-OSX fullscreen shortcut is ctrl-meta-space? I see F11 on gnome-terminal.

There is no 'full screen' shortcut (in my knowledge ) on Mac, so I put one I use in a terminal, I may have missed a
Qt.Shortcut.Fullscreen. Fxx Keys are not directly accessible on newest mac, bind to volumes control for example

OSX may not have a perfectly uniform standard, but cmd-ctrl-F is very common (Preview, Chrome, Safari, GitX), and approximately 100% of the exceptions to this are apps that use cmd-opt-F or cmd-shift-F, so I think there's a well established reference.

BTW, i just though of a 'bug' : tab names are based on number of open tabs.
It might mess up names if you closes tab without closing kernels. We might prefer counting the number of opened kernel...

right you are - open two tabs, close the first, open a third, and you have two tabs labeled 'kernel 1'. easy fix is to just use a monotonic counter.

@minrk minrk prevent reusing kernel IDs
previous method of using tab count allowed collisions when closing and reopening tabs.
2059d0a
@fperez
IPython member

OK, as far as I'm concerned, this is good to go. Anyone object? Otherwise I say we merge it. There's a huge amount of work in here, and the resulting functionality is very, very nice. I don't want it to go stale after you guys having put so much high quality work in.

Thanks a lot to @carreau, @minrk and @epatters for this awesome contribution!!

@Carreau
IPython member

I'm ok for it... will work on improving again in a new branch.
Thank to all of you for continuing without me !

@minrk minrk add 4-space soft-tabs to qtconsole
pressing tab inserts four spaces, so there should never be any '\t' characters
inserted by typing.

closes gh-900
closes gh-513
33cfae8
@minrk
IPython member

I've been using it myself, and it seems reliable. I would bet on there being some problems if you close the right combination of tabs in a particular order, but I keep trying a variety of combos, and it seems well behaved.

@fperez
IPython member

OK, I'm pulling it in, then... We can always keep fine-tuning afterwards, but there's no question this is high-quality work we want in. Thanks again everyone!

@fperez fperez merged commit 78f4eff into master Oct 20, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment