Adding integration with more online dive logs GSoC 2015

[Dirk Hohndel]

On Tue, Mar 10, 2015 at 11:08:14PM +0200, Yosef Hamza wrote:
> Check this and let me know what you think.
> 
> This is for the second one, I will leave the first one for now and check
> the new changes made there by Tomaz.

The code flow has changed quite a bit for that one, so yes, read what's
there before continuing.

Did you resend the one that deals with negative time? The code was right,
just the commit message didn't seem so great...


> diff --git a/qt-ui/profile/profilewidget2.cpp b/qt-ui/profile/profilewidget2.cpp
> index cfcd25d..17a2eb9 100644
> --- a/qt-ui/profile/profilewidget2.cpp
> +++ b/qt-ui/profile/profilewidget2.cpp
> @@ -1383,6 +1383,20 @@ void ProfileWidget2::changeGas()
>  	struct gasmix gasmix;
>  	int seconds = timeAxis->valueAt(scenePos);
>  
> +	if (seconds == 0) {
> +		struct event **events = &current_dc->events;
> +		struct event *temp;
> +		while (*events) {
> +			temp = (*events)->next;
> +			if (event_is_gaschange(*events) && (*events)->time.seconds == 0) {
> +				remove_event(*events);
> +				mark_divelist_changed(true);
> +				replot();
> +			}
> +			*events = temp;
> +		}
> +	}
> +

Let me nit-pick a little :-)

a) the events are ordered by time, so the moment you have one where
   time.seconds > 0 you can stop
b) even easier, we have a helper function you could use:
	get_next_event(ev, "gaschange");
   so you call this with current_dc->events
   if you get an event back AND its time stamp is 0, remove that event
   try again, again with current_dc->events as pointer
   repeat until you don't find a gas change
c) I wouldn't call replot() in the middle; while it's unlikely that there
   would be two gas changes at t=0, still one replot() would be sufficient,
   right? So I'd have a little flag (bool eventRemoved) and then call
   mark_divelist_changed() and replot() if eventRemoved is true.

Makes sense?

I know that this might feel frustrating. You keep sending patches and I
keep criticizing the code and not taking them. This really is intended to
help you and to make sure you understand the code and the code we have in
the end makes sense. Sure, I could just fix this myself (that would be a
lot less work for me) - but it's unlikely that you'd learn something if I
did that.

/D