Vincent van Ravesteijn - TNW wrote:
Subject: Re: patch for case-insensitive reference sorting
Attaching v3 of the patch after taking into various suggestions:
* The caseInsensitiveCB should be set to disabled (at the same place
where sortCB is disabled in the constructor). This because of e.g.
read-only documents for which no updateContents is called (bug 5785,
which I could fix somewhat more).
* I think we should set the checkbox to the default of case insensitive
sorting. Thus it has to be set to checked in the constructor (unless you
change it to the case sensitive sorting checkbox).
* Why don't we need the case_insensitive_ private member? I ask this
because we do have the sort_ private member. There might be a reason why
the following is needed in updateContents():
"sortCB->setChecked(sort_);". Otherwise we could remove the sort_
member.
And some cosmetic thingies:
* We have spaces between const and & in:
+inline bool caseInsensitiveLessThan(QString const& s1, QString const&
s2)
* There should be a tab at the beginning of the next line (no spaces):
+ void caseInsensitiveToggled(bool);
Vincent
Attaching v4 of the patch after taking into account Vincent's points.
Changelog:
1. Case-&insensitive renamed to Cas&e-sensitive. Logic reworked.
2. Case-insensitive behavior by default (nobody objected to Vincent's
points and I also thought it made sense)
3. Cosmetic changes (tabs) made
4. CaseSensitiveCB disabled in constructor
5. Tooltip on sortCB updated to mention case-insensitive default
6. Tooltip on caseSensitiveCB
Thanks,
Manoj
Index: GuiRef.cpp
===================================================================
--- GuiRef.cpp (revision 28591)
+++ GuiRef.cpp (working copy)
@@ -57,6 +57,8 @@
// Enabling is set in updateRefs. Disable for now in case no
// call to updateContents follows (e.g. read-only documents).
sortCB->setEnabled(false);
+ caseSensitiveCB->setEnabled(false);
+ caseSensitiveCB->setChecked(false);
refsLW->setEnabled(false);
gotoPB->setEnabled(false);
@@ -80,6 +82,8 @@
this, SLOT(refSelected(QListWidgetItem *)));
connect(sortCB, SIGNAL(clicked()),
this, SLOT(sortToggled()));
+ connect(caseSensitiveCB, SIGNAL(clicked()),
+ this, SLOT(caseSensitiveToggled()));
connect(gotoPB, SIGNAL(clicked()),
this, SLOT(gotoClicked()));
connect(updatePB, SIGNAL(clicked()),
@@ -93,6 +97,7 @@
bc().setCancel(closePB);
bc().addReadOnly(refsLW);
bc().addReadOnly(sortCB);
+ bc().addReadOnly(caseSensitiveCB);
bc().addReadOnly(nameED);
bc().addReadOnly(referenceED);
bc().addReadOnly(typeCO);
@@ -171,10 +176,17 @@
void GuiRef::sortToggled()
{
+ caseSensitiveCB->setEnabled(sortCB->isChecked());
redoRefs();
}
+void GuiRef::caseSensitiveToggled()
+{
+ redoRefs();
+}
+
+
void GuiRef::updateClicked()
{
updateRefs();
@@ -306,7 +318,12 @@
at_ref_ = !at_ref_;
}
+inline bool caseInsensitiveLessThan(QString const & s1, QString const & s2)
+{
+ return s1.toLower() < s2.toLower();
+}
+
void GuiRef::redoRefs()
{
// Prevent these widgets from emitting any signals whilst
@@ -321,14 +338,21 @@
// the first item inserted
QString const oldSelection(referenceED->text());
- for (vector<docstring>::const_iterator iter = refs_.begin();
- iter != refs_.end(); ++iter) {
- refsLW->addItem(toqstr(*iter));
+ QStringList refsStrings;
+ vector<docstring>::const_iterator iter;
+ for (iter = refs_.begin(); iter != refs_.end(); ++iter)
+ refsStrings.append(toqstr(*iter));
+
+ if (sortCB->isEnabled() && sortCB->isChecked()) {
+ if(caseSensitiveCB->isEnabled() && caseSensitiveCB->isChecked())
+ qSort(refsStrings.begin(), refsStrings.end());
+ else
+ qSort(refsStrings.begin(), refsStrings.end(),
+ caseInsensitiveLessThan /*defined above*/);
}
+
+ refsLW->addItems(refsStrings);
- if (sortCB->isEnabled() && sortCB->isChecked())
- refsLW->sortItems();
-
referenceED->setText(oldSelection);
// restore the last selection or, for new insets, highlight
@@ -368,6 +392,7 @@
buf->getLabelList(refs_);
}
sortCB->setEnabled(!refs_.empty());
+ caseSensitiveCB->setEnabled(sortCB->isEnabled() && sortCB->isChecked());
refsLW->setEnabled(!refs_.empty());
// refsLW should only be the focus proxy when it is enabled
setFocusProxy(refs_.empty() ? 0 : refsLW);
Index: GuiRef.h
===================================================================
--- GuiRef.h (revision 28591)
+++ GuiRef.h (working copy)
@@ -38,6 +38,7 @@
void selectionChanged();
void refSelected(QListWidgetItem *);
void sortToggled();
+ void caseSensitiveToggled();
void updateClicked();
void reset_dialog();
void dialog_rejected();
Index: ui/RefUi.ui
===================================================================
--- ui/RefUi.ui (revision 28591)
+++ ui/RefUi.ui (working copy)
@@ -5,7 +5,7 @@
<rect>
<x>0</x>
<y>0</y>
- <width>347</width>
+ <width>386</width>
<height>423</height>
</rect>
</property>
@@ -16,113 +16,112 @@
<bool>true</bool>
</property>
<layout class="QGridLayout" >
- <property name="margin" >
- <number>9</number>
- </property>
- <property name="spacing" >
- <number>6</number>
- </property>
- <item row="1" column="0" colspan="3" >
- <widget class="QListWidget" name="refsLW" />
+ <item row="0" column="0" colspan="2" >
+ <widget class="QLabel" name="refsL" >
+ <property name="text" >
+ <string>La&bels in:</string>
+ </property>
+ <property name="buddy" >
+ <cstring>bufferCO</cstring>
+ </property>
+ </widget>
</item>
<item row="0" column="2" >
<widget class="QComboBox" name="bufferCO" >
<property name="sizePolicy" >
- <sizepolicy>
- <hsizetype>7</hsizetype>
- <vsizetype>0</vsizetype>
+ <sizepolicy vsizetype="Fixed" hsizetype="Expanding" >
<horstretch>0</horstretch>
<verstretch>0</verstretch>
</sizepolicy>
</property>
</widget>
</item>
- <item row="0" column="0" colspan="2" >
- <widget class="QLabel" name="refsL" >
- <property name="text" >
- <string>La&bels in:</string>
- </property>
- <property name="buddy" >
- <cstring>bufferCO</cstring>
- </property>
- </widget>
+ <item row="1" column="0" colspan="3" >
+ <widget class="QListWidget" name="refsLW" />
</item>
- <item row="6" column="0" colspan="3" >
+ <item row="2" column="0" colspan="3" >
<layout class="QHBoxLayout" >
- <property name="margin" >
- <number>0</number>
- </property>
- <property name="spacing" >
- <number>6</number>
- </property>
<item>
- <spacer>
- <property name="orientation" >
- <enum>Qt::Horizontal</enum>
- </property>
- <property name="sizeType" >
- <enum>QSizePolicy::Expanding</enum>
- </property>
- <property name="sizeHint" >
- <size>
- <width>20</width>
- <height>20</height>
- </size>
- </property>
- </spacer>
+ <layout class="QHBoxLayout" >
+ <item>
+ <widget class="QCheckBox" name="sortCB" >
+ <property name="toolTip" >
+ <string>Sort labels in alphabetical order (case-insensitively unless the Case-sensitive option is checked)</string>
+ </property>
+ <property name="text" >
+ <string>&Sort</string>
+ </property>
+ </widget>
+ </item>
+ <item>
+ <widget class="QCheckBox" name="caseSensitiveCB" >
+ <property name="enabled" >
+ <bool>false</bool>
+ </property>
+ <property name="toolTip" >
+ <string>Sort labels case-sensitively in alphabetical order</string>
+ </property>
+ <property name="text" >
+ <string>Cas&e-sensitive</string>
+ </property>
+ </widget>
+ </item>
+ </layout>
</item>
<item>
- <widget class="QPushButton" name="okPB" >
- <property name="text" >
- <string>&OK</string>
- </property>
- <property name="default" >
- <bool>true</bool>
- </property>
- </widget>
+ <layout class="QHBoxLayout" >
+ <item>
+ <widget class="QPushButton" name="updatePB" >
+ <property name="sizePolicy" >
+ <sizepolicy vsizetype="Fixed" hsizetype="MinimumExpanding" >
+ <horstretch>0</horstretch>
+ <verstretch>0</verstretch>
+ </sizepolicy>
+ </property>
+ <property name="toolTip" >
+ <string>Update the label list</string>
+ </property>
+ <property name="text" >
+ <string>&Update</string>
+ </property>
+ </widget>
+ </item>
+ <item>
+ <widget class="QPushButton" name="gotoPB" >
+ <property name="sizePolicy" >
+ <sizepolicy vsizetype="Fixed" hsizetype="MinimumExpanding" >
+ <horstretch>0</horstretch>
+ <verstretch>0</verstretch>
+ </sizepolicy>
+ </property>
+ <property name="toolTip" >
+ <string>Jump to the label</string>
+ </property>
+ <property name="text" >
+ <string>&Go to Label</string>
+ </property>
+ </widget>
+ </item>
+ </layout>
</item>
- <item>
- <widget class="QPushButton" name="applyPB" >
- <property name="text" >
- <string>&Apply</string>
- </property>
- <property name="default" >
- <bool>true</bool>
- </property>
- </widget>
- </item>
- <item>
- <widget class="QPushButton" name="closePB" >
- <property name="text" >
- <string>&Close</string>
- </property>
- </widget>
- </item>
</layout>
</item>
- <item row="5" column="1" colspan="2" >
- <widget class="QLineEdit" name="nameED" >
- <property name="enabled" >
- <bool>false</bool>
- </property>
- </widget>
- </item>
- <item row="5" column="0" >
- <widget class="QLabel" name="nameL" >
- <property name="enabled" >
- <bool>false</bool>
- </property>
+ <item row="3" column="0" >
+ <widget class="QLabel" name="referenceL" >
<property name="text" >
- <string>&Name:</string>
+ <string>&Label:</string>
</property>
<property name="alignment" >
<set>Qt::AlignLeading|Qt::AlignLeft|Qt::AlignVCenter</set>
</property>
<property name="buddy" >
- <cstring>nameED</cstring>
+ <cstring>referenceED</cstring>
</property>
</widget>
</item>
+ <item row="3" column="1" colspan="2" >
+ <widget class="QLineEdit" name="referenceED" />
+ </item>
<item row="4" column="0" >
<widget class="QLabel" name="typeLA" >
<property name="text" >
@@ -136,25 +135,10 @@
</property>
</widget>
</item>
- <item row="3" column="0" >
- <widget class="QLabel" name="referenceL" >
- <property name="text" >
- <string>&Label:</string>
- </property>
- <property name="alignment" >
- <set>Qt::AlignLeading|Qt::AlignLeft|Qt::AlignVCenter</set>
- </property>
- <property name="buddy" >
- <cstring>referenceED</cstring>
- </property>
- </widget>
- </item>
<item row="4" column="1" colspan="2" >
<widget class="QComboBox" name="typeCO" >
<property name="sizePolicy" >
- <sizepolicy>
- <hsizetype>1</hsizetype>
- <vsizetype>0</vsizetype>
+ <sizepolicy vsizetype="Fixed" hsizetype="Minimum" >
<horstretch>0</horstretch>
<verstretch>0</verstretch>
</sizepolicy>
@@ -194,60 +178,86 @@
</item>
</widget>
</item>
- <item row="3" column="1" colspan="2" >
- <widget class="QLineEdit" name="referenceED" />
- </item>
- <item row="2" column="0" >
- <widget class="QCheckBox" name="sortCB" >
- <property name="toolTip" >
- <string>Sort labels in alphabetical order</string>
+ <item row="5" column="0" >
+ <widget class="QLabel" name="nameL" >
+ <property name="enabled" >
+ <bool>false</bool>
</property>
<property name="text" >
- <string>&Sort</string>
+ <string>&Name:</string>
</property>
+ <property name="alignment" >
+ <set>Qt::AlignLeading|Qt::AlignLeft|Qt::AlignVCenter</set>
+ </property>
+ <property name="buddy" >
+ <cstring>nameED</cstring>
+ </property>
</widget>
</item>
- <item row="2" column="1" colspan="2" >
+ <item row="5" column="1" colspan="2" >
+ <widget class="QLineEdit" name="nameED" >
+ <property name="enabled" >
+ <bool>false</bool>
+ </property>
+ </widget>
+ </item>
+ <item row="6" column="0" colspan="3" >
<layout class="QHBoxLayout" >
- <property name="margin" >
- <number>0</number>
- </property>
<property name="spacing" >
<number>6</number>
</property>
+ <property name="leftMargin" >
+ <number>0</number>
+ </property>
+ <property name="topMargin" >
+ <number>0</number>
+ </property>
+ <property name="rightMargin" >
+ <number>0</number>
+ </property>
+ <property name="bottomMargin" >
+ <number>0</number>
+ </property>
<item>
- <widget class="QPushButton" name="updatePB" >
- <property name="sizePolicy" >
- <sizepolicy>
- <hsizetype>3</hsizetype>
- <vsizetype>0</vsizetype>
- <horstretch>0</horstretch>
- <verstretch>0</verstretch>
- </sizepolicy>
+ <spacer>
+ <property name="orientation" >
+ <enum>Qt::Horizontal</enum>
</property>
- <property name="toolTip" >
- <string>Update the label list</string>
+ <property name="sizeType" >
+ <enum>QSizePolicy::Expanding</enum>
</property>
+ <property name="sizeHint" >
+ <size>
+ <width>20</width>
+ <height>20</height>
+ </size>
+ </property>
+ </spacer>
+ </item>
+ <item>
+ <widget class="QPushButton" name="okPB" >
<property name="text" >
- <string>&Update</string>
+ <string>&OK</string>
</property>
+ <property name="default" >
+ <bool>true</bool>
+ </property>
</widget>
</item>
<item>
- <widget class="QPushButton" name="gotoPB" >
- <property name="sizePolicy" >
- <sizepolicy>
- <hsizetype>3</hsizetype>
- <vsizetype>0</vsizetype>
- <horstretch>0</horstretch>
- <verstretch>0</verstretch>
- </sizepolicy>
+ <widget class="QPushButton" name="applyPB" >
+ <property name="text" >
+ <string>&Apply</string>
</property>
- <property name="toolTip" >
- <string>Jump to the label</string>
+ <property name="default" >
+ <bool>true</bool>
</property>
+ </widget>
+ </item>
+ <item>
+ <widget class="QPushButton" name="closePB" >
<property name="text" >
- <string>&Go to Label</string>
+ <string>&Close</string>
</property>
</widget>
</item>
@@ -255,10 +265,6 @@
</item>
</layout>
</widget>
- <pixmapfunction></pixmapfunction>
- <includes>
- <include location="local" >qt_i18n.h</include>
- </includes>
<tabstops>
<tabstop>bufferCO</tabstop>
<tabstop>refsLW</tabstop>
@@ -272,6 +278,9 @@
<tabstop>applyPB</tabstop>
<tabstop>closePB</tabstop>
</tabstops>
+ <includes>
+ <include location="local" >qt_i18n.h</include>
+ </includes>
<resources/>
<connections/>
</ui>
