Richard Heck wrote:
>
> Still seeking comment on this patch, or permission to commit.
>
> =====
>
> The attached patch addresses these bugs
> http://bugzilla.lyx.org/show_bug.cgi?id=3741
> http://bugzilla.lyx.org/show_bug.cgi?id=3756
> in a way that is compatible with earlier discussion with Abdel. Here's
> what it does:
>
> * In the "Available" list, double clicking or hitting Enter adds the
> citation. Hitting Ctrl-Enter adds the citation and closes the
> dialog. Previously, enter did what Ctrl-Enter now does if there
> was no other citation already selected.
> * In the "Selected" list, Delete and Backspace delete the selection.
> Ctrl-Delete and Ctrl-Backspace clear the whole list.
>
> The only way I could find to implement this was to install an event
> filter on these widgets.
>
> I've also made two other changes. One was to use the rejected() signal
> instead of checking for Qt::Key_Escape in a KeyPressEvent handler. The
> other is in two places, of which this is an example:
>
> void QCitationDialog::on_addPB_clicked()
> {
> + QModelIndexList const selIdx =
> + availableLV->selectionModel()->selectedIndexes();
> + if (selIdx.empty())
> + return;
> + QModelIndex const & idxToAdd = selIdx.first();
> + if (!idxToAdd.isValid())
> + return;
> QModelIndex idx = selectedLV->currentIndex();
> - form_->addKey(availableLV->currentIndex());
> + form_->addKey(idxToAdd);
> if (idx.isValid())
> selectedLV->setCurrentIndex(idx);
> selectedLV->selectionModel()->reset();
>
> The point of this is to get the currently highlighted index, if such
> there is, as availableLV->currentIndex() will return the index where the
> cursor is, even if that is not highlighted. As the dialog now works,
> this doesn't make any difference---it's always highlighted---but it
> seemed worth being careful.
>
> I tried to do something like this so I didn't have the same code in two
> places:
> QModelIndex & QCitationDialog::getSelectedIndex(QListView *) {
> QModelIndexList const selIdx =
> availableLV->selectionModel()->selectedIndexes();
> if (selIdx.empty())
> return QModelIndex(); //this is an invalid one.
> return selIdx.first();
> }
> But that failed: warning: returns reference to temporary. Any ideas how
> to avoid this and still return a reference? Or could I return a smart
returning "const QModelIndex &" should be save. But you could also
return may value, copying QModelIndex isn't that expensive. The Qt
code return often by value, QModelIndex was designed for this.
> pointer of some kind?
>
> Looking to commit, possibly with improvements.
>
> Richard
>
>
> ------------------------------------------------------------------------
>
> Index: QCitationDialog.cpp
> ===================================================================
> --- QCitationDialog.cpp (revision 18580)
> +++ QCitationDialog.cpp (working copy)
> @@ -65,6 +65,9 @@
> connect(selectedLV->selectionModel(),
> SIGNAL(currentChanged(const QModelIndex &, const QModelIndex
> &)),
> this, SLOT(selectedChanged(const QModelIndex &, const
> QModelIndex &)));
> + connect(this, SIGNAL(rejected()), this, SLOT(cleanUp()));
> + availableLV->installEventFilter(this);
> + selectedLV->installEventFilter(this);
> }
>
>
> @@ -73,18 +76,63 @@
> }
>
>
> -void QCitationDialog::keyPressEvent(QKeyEvent * event)
> +bool QCitationDialog::eventFilter(QObject * obj, QEvent * event)
> {
> - if (event->key() == Qt::Key_Escape) {
> - form_->clearSelection();
> - form_->clearParams();
> - event->accept();
> - close();
> - } else
> - event->ignore();
> + if (obj == availableLV) {
> + if (event->type() != QEvent::KeyPress)
> + return QObject::eventFilter(obj, event);
> + QKeyEvent * keyEvent = static_cast<QKeyEvent *>(event);
> + int const keyPressed = keyEvent->key();
> + Qt::KeyboardModifiers const keyModifiers =
> keyEvent->modifiers();
> + //Enter key without modifier will add current item
> + //...with control modifier will add it and close the dialog
> + if ((keyPressed == Qt::Key_Enter || keyPressed ==
> Qt::Key_Return) &&
> + (!keyModifiers ||
> + (keyModifiers == Qt::ControlModifier) ||
> + (keyModifiers == Qt::KeypadModifier) ||
> + (keyModifiers == (Qt::ControlModifier |
> Qt::KeypadModifier))
> + )
> + ) {
> + if (addPB->isEnabled())
> + on_addPB_clicked();
> + if (keyModifiers & Qt::ControlModifier)
> + on_okPB_clicked();
> + event->accept();
> + return true;
> + }
> + } else if (obj == selectedLV) {
> + //Delete or backspace key will delete current item
> + //...with control modifier will clear the list
> + if (event->type() != QEvent::KeyPress)
> + return QObject::eventFilter(obj, event);
> + QKeyEvent * keyEvent = static_cast<QKeyEvent *>(event);
> + int const keyPressed = keyEvent->key();
> + Qt::KeyboardModifiers const keyModifiers =
> keyEvent->modifiers();
> + if (keyPressed == Qt::Key_Delete || keyPressed ==
> Qt::Key_Backspace) {
> + if (keyModifiers == Qt::NoModifier &&
> deletePB->isEnabled())
> + on_deletePB_clicked();
> + else if (keyModifiers == Qt::ControlModifier) {
> + form_->clearSelection();
> + update();
> + } else
> + //ignore it otherwise
> + return QObject::eventFilter(obj, event);
> + event->accept();
> + return true;
> + }
> + }
> + return QObject::eventFilter(obj, event);
> }
>
>
> +void QCitationDialog::cleanUp()
> +{
> + form_->clearSelection();
> + form_->clearParams();
> + close();
> +}
> +
> +
> void QCitationDialog::closeEvent(QCloseEvent * e)
> {
> form_->clearSelection();
> @@ -300,7 +348,6 @@
> {
> if (!idx.isValid())
> return;
> -
> availableLV->selectionModel()->reset();
> update();
> }
> @@ -317,21 +364,18 @@
> {
> if (!idx.isValid())
> return;
> -
> selectedLV->selectionModel()->reset();
> update();
> }
>
>
> -void QCitationDialog::on_availableLV_activated(const QModelIndex & idx)
> +void QCitationDialog::on_availableLV_doubleClicked(const QModelIndex & idx)
> {
> if (isSelected(idx))
> return;
>
> selectedLV->selectionModel()->reset();
> on_addPB_clicked();
> - if (selectedLV->model()->rowCount() == 1)
> - on_okPB_clicked();
> }
>
>
> @@ -342,8 +386,15 @@
>
> void QCitationDialog::on_addPB_clicked()
> {
> + QModelIndexList const selIdx =
> + availableLV->selectionModel()->selectedIndexes();
> + if (selIdx.empty())
> + return;
> + QModelIndex const & idxToAdd = selIdx.first();
> + if (!idxToAdd.isValid())
> + return;
> QModelIndex idx = selectedLV->currentIndex();
> - form_->addKey(availableLV->currentIndex());
> + form_->addKey(idxToAdd);
> if (idx.isValid())
> selectedLV->setCurrentIndex(idx);
> selectedLV->selectionModel()->reset();
> @@ -353,7 +404,13 @@
>
> void QCitationDialog::on_deletePB_clicked()
> {
> - QModelIndex idx = selectedLV->currentIndex();
> + QModelIndexList selIdx =
> + selectedLV->selectionModel()->selectedIndexes();
> + if (selIdx.empty())
> + return;
> + QModelIndex & idx = selIdx.first();
> + if (!idx.isValid())
> + return;
> int nrows = selectedLV->model()->rowCount();
>
> form_->deleteKey(idx);
>
--
Peter Kümmel
