Reviewing merge request #454: QLabel: add setSelection, hasSelectedText, selectedText, selectionStart.
When using TextSelectableByMouse or TextSelectableByKeyboard, this
allows to programmatically control the selection. Needed in KDE's
KSqueezedTextLabel, so that mouseReleaseEvent can get the selection
and copy the un-squeezed text instead of the squeezed one.
The API is entirely copied from QLineEdit, so that it brings no surprises.
Auto-test included as well. Made me fix a few bugs in my implementation :)
Commits that would be merged:
- c06c254
- d57d3fc
QLabel: add setSelection, hasSelectedText, selectedText, selectionStart.
c06c254-d57d3fcComments
+ void setSelection(int, int);
You should put the name of the variables here.
It is not obvious it is start and length when reading the header (that could be start and end for example).
This patch looks good.
If you have some time, it would be great to add a second unit test with simulated mouse events to create a selection.
From qlineedit.h:
void setSelection(int, int);
This is where I got it from. Can you merge it as is? It didn’t seem to bother anyone in qlineedit.h ;)
I don’t really have time for a second unit test, for a feature that was already there before my patch.
Can you merge it as is? It didn’t seem to bother anyone in qlineedit.h ;)
I don’t really have time for a second unit test.
Sure, that was just small suggestions.


Add a new comment:
Login or create an account to post a comment