D21575: Add Cursor

[Stefan Gerlach]

sgerlach added inline comments.

INLINE COMMENTS

> CartesianCoordinateSystem.cpp:711
> +
> +	if (limit) {
> +		// set to max/min if passed over

use temp vars here too?

> CartesianPlot.cpp:2641
> +			emit q->cursor1EnableChanged(cursor1Enable);
> +		}else{
> +			cursor0Enable = true;

use spaces: } else {

> CartesianPlot.cpp:2661
> +
> +		if(logicalPos.x() < xMin)
> +			logicalPos.setX(xMin);

space after "if": if (...)

> CartesianPlot.cpp:2690
> +
> +	if (cursorNumber == 0)
> +		cursor0Enable = true;

maybe better:
cursorNumber == 0 ? cursor0Enable = true : cursor1Enable = true;

> CartesianPlot.cpp:2804
> +	QPointF p1(logicalPos.x(), 0);
> +	if (cursorNumber == 0)
> +		cursor0Pos = p1;

maybe better:
cursorNumber == 0 ? cursor0Pos = p1 : cursor1Pos = p1;

> CartesianPlot.cpp:2972
> +			double cursorPenWidth2 = cursorPen.width()/2;
> +			cursorPenWidth2 = cursorPenWidth2 < 10 ? 10 : cursorPenWidth2;
> +			if ((cursor0Enable && abs(point.x()-cSystem->mapLogicalToScene(QPointF(cursor0Pos.x(),yMin)).x()) < cursorPenWidth2) ||

same as
if (cursorPenWidth2 < 10)
 cursorPenWidth2 = 10;

> XYCurve.cpp:2314
>  			double value = column[row];
>  			if (abs(value - x) <= abs(prevValue - x)) { // "<=" prevents also that row - 1 become < 0
>  					prevValue = value;

do you really want abs() defined as
int abs(int)
or fabs() defined as
double fabs(double)
or even qAbs() defined as
T qAbs(const T&)

please check all places where abs() is used

> WorksheetView.cpp:1816
>  		m_cartesianPlotMouseMode = CartesianPlot::ZoomYSelectionMode;
> +	else if (action == cartesianPlotCursorModeAction) {
> +        m_cartesianPlotMouseMode = CartesianPlot::Cursor;

no brackets for one-liners

> CartesianPlotDock.cpp:103
> +	ui.cbCursorLineStyle->clear();
> +	ui.cbCursorLineStyle->addItem("NoPen", 0);
> +	ui.cbCursorLineStyle->addItem("SolidLine", 1);

This is Qt::PenStyle, right?
Can we use a global const QStringList containing the names? Also translation would be good :-)

> XYCurveDock.cpp:513
> +		QString path = m_curve->xColumnPath().split("/").last();
> +		if (m_curve->xColumn()){
> +			path += QString("\t ")+m_curve->xColumn()->plotDesignationString();

space before {

REPOSITORY
  R262 LabPlot

REVISION DETAIL
  https://phabricator.kde.org/D21575

To: Murmele, asemke
Cc: sgerlach, kde-edu, narvaez, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20190719/2ce007f3/attachment-0001.html>