First patch - Delete dive(s) using the delete key

Dirk Hohndel dirk at hohndel.org
Fri Feb 6 07:20:26 PST 2015


Hi Grace,

welcome to the team!

On Fri, Feb 06, 2015 at 02:19:04PM +0300, Grace Karanja wrote:
> 
> This patch allows the user to delete dives using the delete key.

Excellent.

I have a few comments...

> diff --git a/qt-ui/divelistview.cpp b/qt-ui/divelistview.cpp
> index d4e7442..a889af8 100644
> --- a/qt-ui/divelistview.cpp
> +++ b/qt-ui/divelistview.cpp
> @@ -343,6 +345,10 @@ bool DiveListView::eventFilter(QObject *, QEvent *event)
>  	if (event->type() != QEvent::KeyPress)
>  		return false;
>  	QKeyEvent *keyEv = static_cast<QKeyEvent *>(event);
> +        if (keyEv->key() == Qt::Key_Delete) {
> +                contextMenuIndex = currentIndex();
> +                deleteDive();
> +        }

Indentation here is with spaces - we always indent with tabs (only for
continuation lines we add spaces after the initial tabs to line things up).
Have you taken a look at the file CodingStyle in the sources? It gives
you editor settings for many common editors and IDEs to make it easier.

>  	if (keyEv->key() != Qt::Key_Escape)
>  		return false;
>  	return true;
> diff --git a/qt-ui/divelistview.h b/qt-ui/divelistview.h
> index a6522fa..f375d27 100644
> --- a/qt-ui/divelistview.h
> +++ b/qt-ui/divelistview.h
> @@ -13,6 +13,7 @@
>  
>  #include <QTreeView>
>  #include <QLineEdit>
> +#include <QKeyEvent>

Since that include file isn't needed for the .h file but only for the .cpp
file, I think it would make more sense to include it there...

These two things are easy to fix for me as I apply the patch, so I won't
make you redo it, but it would be nice if you could make the change to
your editor settings so that next time there are no whitespace issues...

Thanks for that first patch! I'm looking forward to many more.

/D


More information about the subsurface mailing list