Monday, May 26, 2014

Don't think of it as a bug...

...think of it as material for a blog post!

OK so there is an annoying bug in the edit display. I am putting a pale highlight on the current line (the line with the cursor) -- see the image on this post. This fixes an annoyance I had with version 1, that on a large screen it was easy to lose track of the cursor, especially after paging up or down. I'd often have to rattle the left and right arrow keys so I could spot the cursor moving.

Here's the basic code, cut down:

Note: The following is not the correct way to set a current-line highlight. Do not emulate this code. See this post for the problem with it and a later post for the correct approach.

    def _cursor_moved(self, force=False):
        tc = self.Editor.textCursor()
        self.ColNumber.setText( str( tc.positionInBlock() ) )
        tb = tc.block()
        if tb == self.last_text_block and not force :
            return # still on same line, nothing more to do
        # ...here fill in line-number, scan filename, and folio widgets...
        # Clear any highlight on the previous current line
        bc = QTextCursor(self.last_text_block)
        bc.setBlockFormat(self.normal_line_fmt)
        # remember this new current line
        self.last_text_block = tb
        # and set its highlight
        tc.setBlockFormat(self.current_line_fmt)

That's pretty simple: if the cursor is on the same line as last time, set the column number widget and bug out quick. (The force argument is so this same method can be used in certain rare cases by other routines to repaint the widgets even if the cursor hasn't moved.)

If the cursor is on a different line than the last time it moved, which we will know because the QTextBlock returned by the current cursor is not the same as before, then clear out the highlight on the previous line/block, and set a highlight on the new line/block. It never sets a highlight without also clearing one.

And it mostly works. Move the cursor with the arrow keys or by mouse-clicking, and the pale highlight moves around with it. Great!

Except in one case: if I select multiple lines—by dragging or by shift-clicking or by shift-down-arrowing—then all selected lines get the current-line highlight and it does not clear if I click somewhere else. It only clears if I run the cursor back over those lines. And here's the kicker: as I drag or shift-down-arrow to extend the selection, the line-number widget is updating properly. So the code above is being executed, recognizing that it is on a different line (or it wouldn't update the line-number widget), which means it must be clearing the highlight. But it doesn't. WUT?

Time to put in the print statements.

        dbg1 = tb.blockNumber()
        dbg2 = self.last_text_block.blockNumber()
        print('clearing block {0} setting block {1}'.format(dbg2,dbg1))

And it reports exactly what it should: Put the cursor on line 0, key shift-down-arrow to make a selection, it reports "cleaning block 0 setting block 1, clearing block 1 setting block 2" and so on, but the highlights on line 0 and 1 are not clearing. They are partly hidden by the brighter selection highlight, but there. Here's a picture.

If I click on line 4, the highlight on line 2, the last current line, is cleared, as is the selection highlight, but the highlights on lines 0 and 1 remain.

So there's no logic error, but some issue with setting a block format when the block is under another format, the selection highlight. I note that setting the current_line_fmt works, even when selection is in progress. You can see it sticking out from under the selection on line 2 above. But setting the normal_line_fmt doesn't take. What's the difference? It is initialized so:

        self.normal_line_fmt = QTextBlockFormat()

It's an empty, default format object. Whereas the current line format is set up:

        self.current_line_fmt.setBackground(colors.get_current_line_brush())

It is a format object with an explicit background brush. (The colors module returns basically QBrush(QColor('#FAFAE0')); eventually the user will be able to choose this color as a Preference.) Anyway the current line format has an explicit background brush and the normal format does not. Should I give it one, and if so, what color? Back to the Qt Assistant...

Answer: No. I changed the initialization of the normal format:

        self.normal_line_fmt = QTextBlockFormat()
        self.normal_line_fmt.setBackground(QBrush(Qt.white))

I also tried Qt.color0, the transparent color. The bug continues. So it is not the lack of an explicit background brush that prevents the clearing. Back to the Assistant...

Ah. Re-reading the doc for QTextCursor.setBlockFormat() I note it reads "Sets the block format of the current block (or all blocks that are contained in the selection) to format." Oops.

The problem, I betcha, is in the line

tc.setBlockFormat(self.current_line_fmt)

Because tc is the actual cursor of the current document, the cursor that represents the current selection. And when it is used to set the block format of the "current" block, it sets a block format for "all blocks in the selection" which includes the prior block whose format was just cleared!

And that's it. If I set the current line format using a new cursor with no selection, it all works. So the final lines of code now read:

        # clear any highlight on the previous current line
        temp_cursor = QTextCursor(self.last_text_block)
        temp_cursor.setBlockFormat(self.normal_line_fmt)
        # remember this new current line
        self.last_text_block = tb
        # and set its highlight
        temp_cursor = QTextCursor(tb)
        temp_cursor.setBlockFormat(self.current_line_fmt)

Make a new text cursor based on the current block, one which has (by default) no selection. Use that to set the format for the current line. It affects only the actual current line, not any other lines that might be in the current user selection. Bug fixed! Do a commit and break for lunch!

No comments: