Doing It "Pythonically" (with added thought)

I'm starting work on the Find module. One of the features of that is that each of the input fields—the Find text and each of three independent Replace texts—has a memory pop-up: a button that pops up a menu of the ten most recent strings used in that field. It's a very nice help when you are alternating between two or three complex regex searches. (And stolen from Guiguts and BBEdit.)

The details of this widget were of course encapsulated in a class; for V.2 it is class RecallMenuButton. In V.1 this was a QComboBox because that is easy to present. I maintained the recent strings in a QStringList, and any time the list was updated the widget could reload itself in one call, self.insertItems(list). However, I wanted it to look like a square button, not a list, so I had it set its own max width to 29px. Under Windows that didn't work with the default style, so I had to set it to a non-native style "CleanLooks", and that no longer exists.

Anyway for V.2 I am using a "command button" which is a button that has an associated menu. I maintain a python list of QActions, one for each string. When the menu for the button emits the aboutToShow signal, I clear the menu and add the list of actions to it.

So I was starting to code the remember() method of this class, which takes a string and, if it is in the list now, deletes it; then adds it to the front of the list. But some considerations:

• The string might very likely be in the list already as the first item, because it will be frequent to Find or Replace the same string over and over.
• The list might be empty, at least the first time.
• The string might not be in the list at all.
• The list should never exceed MAX_STRINGS in length.

So the V.1 code is about a dozen lines, with a Fortran-like loop over the list to find and remove the string if it exists. But I'd like to do it more "pythonically" this time. I came up with this:

    def remember(self, string):
        new_stack = [act for act in self.string_stack if string != act.text()]
        new_stack[0:0] = QAction(string)
        self.string_stack = new_stack[0:self.MAX_STRINGS]

After composing which I said,

Now, this does not try to short-cut the presumptively common case of the string already being on top of the stack. If the added string is at the front of the stack it will be removed and added back in the same position. Is this a waste of time? Yes, but the test for that special case would look like:

    def remember(self, string):
        if len(self.string_stack):
            if string == self.string_stack[0].text() :
                return

...and that much extra code would surely waste as much time as it saved, or nearly. Or would it? Hmmm.

Edit: no, it would not be worth it and here's why. The Find UI is only going to "remember" a find or replace string if that string is manually edited by the user. When the Find or Replace line-edit field is filled by the program (from the remembered-strings popup menu or from a user-loadable macro button) a flag is cleared. Said flag is only set when the user edits in the field (the textChanged signal). When the field is actually used (Find or Replace action done), the flag is tested and remember() is called only if the flag is set. Thus remember() is only ever called for strings that the user has entered or altered. This greatly reduces both the number of calls to remember() and the chances that a remembered string already exists at any position in the stack. So there is no point in guarding against the input string being at stack[0].

Now you see it, now you don't

First I added about 4 more tests to the mainwindow script shown in a video in the prior post. One of these revealed an interesting bug. The test was, to use File>Open and pick a file that was already open. The result should be to cause the existing edit view of that file to pop to the front (not to open a second copy of the file).

That worked as it should, but then I combined it with another test. The other test was to choose, not somebook.txt but somebook.txt.meta. This should check that somebook.txt exists, and if it does, open it (treating the book and its .meta file as identical). That test also worked as it should. But when I combined the two, opening somebook.txt.meta when somebook.txt was already open, I got two open copies of somebook.txt. Huh?

It was a matter of when the different checks were made, of course, and a small rearrangement of code fixed it. But it's fun taking an attack stance toward my own code and breaking it.

Then I moved on to the notes panel, and the first test of a disappearing Edit menu and it works a treat. I had to modify the mainwindow so that it offered a module method for retrieving the Menu Bar. Then the notes widget could do this:

        ed_menu = QMenu(C.ED_MENU_EDIT,self)
        ed_menu.addAction(C.ED_MENU_UNDO,self.undo,QKeySequence.Undo)
        ed_menu.addAction(C.ED_MENU_REDO,self.redo,QKeySequence.Redo)
        ...cut, copy, paste...
        self.edit_menu = mainwindow.get_menu_bar().addMenu(ed_menu)
        self.edit_menu.setVisible(False)

Adding a menu to the menu bar returns a "menu action", and a QAction has a property visible which, if it is set to False, means the action, or in this case the menu, is not shown. Further down in the notes panel code is:

    def focusInEvent(self, event):
        self.edit_menu.setVisible(True)
    def focusOutEvent(self, event):
        self.edit_menu.setVisible(False)

And it works just loverly. Click in the Notes panel, the Edit menu appears. Click in the edit view, it disappears. When I finish building notes view I will go back into Edit view and give it an Edit menu of its own, with quite a few more actions.

Dude, where's my tab?

Well, rolling along here at ppqt central, we have the basics of a main window, now it is time to add the File menu and the actions it commands. First up is File>New. Easy-peasy one would think, since the _new() method is already in place and being used as part of initialization.

        self.file_menu = self.menu_bar.addMenu(_TR('Menu bar', '&File'))
        # Populate the File menu with actions.
        #  New -> _new()
        work = self.file_menu.addAction( _TR('File menu','&New') )
        work.setShortcut(C.CTL_N)
        work.setToolTip( _TR('File:New tooltip','Create a new, empty document') )
        work.triggered.connect(self._new)

Add a QMenu('File') (appropriately translated) to the application QMenuBar. To it, add an action 'New' and connect its signal to the proper slot. And this should work. So test it. Up comes the app and oooh! It has a File menu with only one item, New. And selecting that executes the code of the _new() method. I have verified this and also that each time File>New is selected, the app creates an empty Book with the names Untitled-0, Untitled-1, Untitled-2 and so on, and adds them to the edit tabset on the left side. And the tabset.addTab() method returns a tab index of 0, 1, 2 as expected.

Except ... the tabs don't appear. The added tabs are not shown; the only visible tab is the first one, for "Untitled-0". Wut?

The code for adding a New document is exactly the same as for adding an existing opened file, and that works. The unit test driver fakes up settings showing two previously opened files and the app opens them and they appear in separate tabs (as shown two posts back).

OK run the unit test and it opens two existing books. Now do New. Hmm. The debug printout reports "added Untitled-1 at index 1" even though there are two open books, so the name should be "Untitled-2" and the index, 2. Add the printout to the _open() method as well.

added realbook.txt at index 0
added small_book.txt at index 1
added Untitled-1 at index 1

No, that's not right. Should be Untitled-2 at index 2. It's like the _new() was happening in a completely different Mainwindow object, operating on a different book sequence number and different tabset. Could that be?

Yes it could be! The unit test driver created three MainWindow objects in sequence. The first is used to check that various settings are properly saved, like this:

settings.clear()
mw = mainwindow.MainWindow(settings)
...do things like mw.move(new position)...
mw.close() # force writing settings
...read values out of settings and assert things about them...
mw = None # destroy main window

And then another round of that: load the QSettings with fake info; create a new MainWindow object in mw; close it; look at settings values; assign mw=None.

Then finally do that one more time: load the QSettings object this time with two valid "previously-open" files; create mw=mainwindow.MainWindow(settings); call app._exec() and interact with the window.

If the first two rounds are commented out, so only the third MainWindow is created, it works: File>New makes a new file that appears in the edit tab bar.

Conclusion: somehow the QApplication is holding onto one or both of the first-created MainWindow objects. And it remembers how that (or those) objects created File menus. And somehow the File>New action is being directed to the local data owned by one of those zombie main windows, even though both of them had executed their .close() methods and all references to both had been overwritten in the Python code.

So: split the unit-test into two or three separate programs... boring.

Edit: On later consideration, I think the link is in the File menu QActions. Each QAction has a reference to the "slot" to handle its the triggered() signal. Each slot is specified using the main window's "self" pointer, e.g.

    act.triggered.connect(self._new)

That "self" value is a reference to the current instance of the MainWindow class, a QWidget derivative. Each time I created a MainWindow instance, it initialized by creating a QMenuBar and putting File>New and File>Open actions in it. Somehow the first of these "stuck" in the app's memory, and kept that object alive, not garbage-collected, even though I'd deleted every reference I had to it. Any use of the Mac OS menu bar's File>New, regardless of the fact that MainWindow #3 was running and active, still invoked the code in the zombie MainWindow #1 (or #2?).