Re: Setting location coordinates while editing a dive
On Fri, Oct 31, 2014 at 07:08:19PM -0400, John Van Ostrand wrote: > Okay, you've convinced me. This is too big for me right now. I'll fiddle > with smaller bits until I get a much better understanding of the code. This wasn't my goal at all. I would like you to work with me (and others) to continue down this path. It's not an easy "over the weekend" kind of project. But that doesn't mean that it isn't worth doing. Let's start with one specific case and a proof of concept implementation. E.g., let's add one step undo to edits. Don't mess with any of the displayed_dive vs. current_dive semantics but just implement a way to track "which dive was the last one changed" and "replace that dive with a copy of it that we made before the last edit operation". That should give you a good idea of all the areas of the code that this needs to touch. Then add the data structures that you envisioned for the undo / redo ring buffer. Once that is in place, you and I should figure out how we can work together to convert from our displayed_dive / current_dive semantic to the semantic that your approach implies where the connection between what's displayed and what is in the dive_table is much tighter. Does that make sense? /D ___ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
Re: Setting location coordinates while editing a dive
On Fri, Oct 31, 2014 at 5:43 PM, Dirk Hohndel wrote:
> On Fri, Oct 31, 2014 at 04:36:29PM -0400, John Van Ostrand wrote:
> >
> > In other words when manually adding a dive an empty dive would be added
> to
> > the dive list with fields added to the dive in as they are entered in the
> > form.
>
> Hmm. I find that somewhat awkward. But I'm not categorically rejecting it.
>
> > > > This would have a few benefits
> > > >
> > > > 1. We eliminate the "This dive is being edited" indicator and all the
> > > code
> > > > referencing "isEditing()".
> > >
> > > So this is easy to write in text, but once you deal with the widgets it
> > > gets a little tricky.
> > >
> > > Example. You edit something (let's say a tag), your cursor is still in
> > > that field, and then click on a different dive in the dive list. What
> > > happens? You have a drop down open (date, cylinder type) and you
> change to
> > > a different dive...
> > >
> > > What I'm trying to say is that the definition of "I'm editing
> something"
> > > is very intuitive, until you try to implement the finer details.
> >
> > I figured we could use a change signal of some kind so we can react to
> > changes. It looks like some widgets are already enabled. Others may need
> > some modification.
>
> That's part of the 'how'. But the question in many instances is "when do
> you issue that signal". E.g., when changing a text field, do you issue
> this with every key press? If not, then when DO you do it? We have learned
> the hard way that it's not always simple to figure out when focus has
> moved...
>
> > > 2. We don't have anything funny to do with placemarks on the map.
> > > > 3. Oops, like accidental double clicks can be backed out.
> > > > 4. Eliminate non-intuitive location/placemark functionality.
> > > > 5. Any for_each_dive will automatically include the dive being
> edited so
> > > UI
> > > > updates like placemarks don't need special cases.
> > >
> > > That last one doesn't make sense to me. Can you elaborate.
> > >
> >
> > The repopulateLabels function in globe.cpp uses the dive list to create
> > flags. It would be cleaner if the data being editing was being updated
> into
> > the dive list as each field was changed. That way functions like
> > repopulateLabels wouldn't have to retrieve data from the form to place a
> > flag.
>
> Ah, OK. That's what I suggested with adding the displayed_dive there.
> But please DO NOT break the logic that the displayed_dive is distinct and
> different from the corresponding dive on the divelist...
>
> > > > There would be a few challenges:
> > > >
> > > > 1. Single dive edits would be easy but multi-dive edits would more
> > > > complicated. They should be associated as one undo.
> > >
> > > OK
> > >
> > > > 2. Trips would be another undo case.
> > >
> > > What does that mean? You mean trip changes to the divelist?
> > > Uhhh, I want to see the data structure in which you do that :-)
> >
> > We may want to undo a trip grouping or undo edits to a trip. We need to
> set
> > up different types of undos. A TRIP_GROUP undo might be one type and
> > TRIP_EDIT undo might be another. The undo stack would have a struct that
> > contains a type and a pointer to a blob of data, and undo action would
> send
> > that blob to the function that handles that specific undo type. The blob
> > would be used by that function to perform the undo. This provides a
> > framework for allowing a wide range of different undos.
>
> Wow. Oops. Err. Yeah. That sounds easy...
>
> > A example might be this (imagine expanding struct undo_dive_edit to hold
> a
> > list of dives.)
> >
> > struct undo_item {
> > enum undo_type type;
> > void *data;
> > };
> >
> > struct undo_dive_edit {
> >int divenr;
> >struct dive *dive;
> > };
> >
> > struct undo_item undo_stack[16];
> > int undo_stack_head = 0;
> > int undo_stack_tail = 0;
> >
> > // called when date field is updated
> > void MainTab::updateDate(const QDateTime &)
> > {
> > undo_dive_create(); // create and undo for this change
> >
> > }
> >
> >
> > void undo_dive_edit_create()
> > {
> > struct undo_item undo = undo_get_current(); // Get most recent
> undo
> > struct undo_dive_edit *ude = undo->undo_data;
> >
> > // Only create an undo if this change affects a different set of
> dives
> > than the current undo.
> > if (undo.type != UNDO_DIVE_EDIT || ude->dive == selected_dive) {
> > // Create undo item.
> > struct undo_dive_edit *new_undo = malloc(sizeof(struct
> > undo_dive_edit));
> > new_undo->divenr = selected_dive;
> > new_undo->dive =
> > undo_push(UNDO_DIVE_EDIT, (void *) new_undo);
> > }
> > }
> >
> >
> > // Dive Edit Undo
> > void undo_dive_edit(void *undo_data)
> > {
> > struct undo_dive_edit *undo = undo->undo_data;
> >
> >
> > }
> >
> >
> > void undo_pop()
> > {
> >
> > if (undo_stack_head == undo_stack_tail)
> > return; // nothing to undo
> >
> > st
Re: Setting location coordinates while editing a dive
On Fri, Oct 31, 2014 at 04:36:29PM -0400, John Van Ostrand wrote:
>
> In other words when manually adding a dive an empty dive would be added to
> the dive list with fields added to the dive in as they are entered in the
> form.
Hmm. I find that somewhat awkward. But I'm not categorically rejecting it.
> > > This would have a few benefits
> > >
> > > 1. We eliminate the "This dive is being edited" indicator and all the
> > code
> > > referencing "isEditing()".
> >
> > So this is easy to write in text, but once you deal with the widgets it
> > gets a little tricky.
> >
> > Example. You edit something (let's say a tag), your cursor is still in
> > that field, and then click on a different dive in the dive list. What
> > happens? You have a drop down open (date, cylinder type) and you change to
> > a different dive...
> >
> > What I'm trying to say is that the definition of "I'm editing something"
> > is very intuitive, until you try to implement the finer details.
>
> I figured we could use a change signal of some kind so we can react to
> changes. It looks like some widgets are already enabled. Others may need
> some modification.
That's part of the 'how'. But the question in many instances is "when do
you issue that signal". E.g., when changing a text field, do you issue
this with every key press? If not, then when DO you do it? We have learned
the hard way that it's not always simple to figure out when focus has
moved...
> > 2. We don't have anything funny to do with placemarks on the map.
> > > 3. Oops, like accidental double clicks can be backed out.
> > > 4. Eliminate non-intuitive location/placemark functionality.
> > > 5. Any for_each_dive will automatically include the dive being edited so
> > UI
> > > updates like placemarks don't need special cases.
> >
> > That last one doesn't make sense to me. Can you elaborate.
> >
>
> The repopulateLabels function in globe.cpp uses the dive list to create
> flags. It would be cleaner if the data being editing was being updated into
> the dive list as each field was changed. That way functions like
> repopulateLabels wouldn't have to retrieve data from the form to place a
> flag.
Ah, OK. That's what I suggested with adding the displayed_dive there.
But please DO NOT break the logic that the displayed_dive is distinct and
different from the corresponding dive on the divelist...
> > > There would be a few challenges:
> > >
> > > 1. Single dive edits would be easy but multi-dive edits would more
> > > complicated. They should be associated as one undo.
> >
> > OK
> >
> > > 2. Trips would be another undo case.
> >
> > What does that mean? You mean trip changes to the divelist?
> > Uhhh, I want to see the data structure in which you do that :-)
>
> We may want to undo a trip grouping or undo edits to a trip. We need to set
> up different types of undos. A TRIP_GROUP undo might be one type and
> TRIP_EDIT undo might be another. The undo stack would have a struct that
> contains a type and a pointer to a blob of data, and undo action would send
> that blob to the function that handles that specific undo type. The blob
> would be used by that function to perform the undo. This provides a
> framework for allowing a wide range of different undos.
Wow. Oops. Err. Yeah. That sounds easy...
> A example might be this (imagine expanding struct undo_dive_edit to hold a
> list of dives.)
>
> struct undo_item {
> enum undo_type type;
> void *data;
> };
>
> struct undo_dive_edit {
>int divenr;
>struct dive *dive;
> };
>
> struct undo_item undo_stack[16];
> int undo_stack_head = 0;
> int undo_stack_tail = 0;
>
> // called when date field is updated
> void MainTab::updateDate(const QDateTime &)
> {
> undo_dive_create(); // create and undo for this change
>
> }
>
>
> void undo_dive_edit_create()
> {
> struct undo_item undo = undo_get_current(); // Get most recent undo
> struct undo_dive_edit *ude = undo->undo_data;
>
> // Only create an undo if this change affects a different set of dives
> than the current undo.
> if (undo.type != UNDO_DIVE_EDIT || ude->dive == selected_dive) {
> // Create undo item.
> struct undo_dive_edit *new_undo = malloc(sizeof(struct
> undo_dive_edit));
> new_undo->divenr = selected_dive;
> new_undo->dive =
> undo_push(UNDO_DIVE_EDIT, (void *) new_undo);
> }
> }
>
>
> // Dive Edit Undo
> void undo_dive_edit(void *undo_data)
> {
> struct undo_dive_edit *undo = undo->undo_data;
>
>
> }
>
>
> void undo_pop()
> {
>
> if (undo_stack_head == undo_stack_tail)
> return; // nothing to undo
>
> struct undo_item undo = undo_stack[undo_stack_head--];
>
> switch (undo.type) {
> case UNDO_DIVE_EDIT:
> undo_dive_edit(undo->data);
> break;
> case UNDO_DIVE_DELETE:
> ...
> }
> }
That looks like an interesting start. And hellishly fragile.
I'd be very careful to make sure we
Re: Setting location coordinates while editing a dive
On Fri, Oct 31, 2014 at 12:47 PM, Dirk Hohndel wrote: > On Fri, Oct 31, 2014 at 09:40:29AM -0700, Dirk Hohndel wrote: > > > I've been able to make changes so the ui.coordinates field is updated > but > > > placing flags on the globe is more challenging. Since the edited > location > > > name and coordinates only exist in the ui.location and ui.coordinates > > > variables the globe::repopulateLabels() function can't place a flag on > the > > > globe, repopulateLabels() uses the dive list for coordinates and > labels. > > > Adding code so it also checks isEditing() and places a flag based on > the > > > edited fields would add that functionality. > > > > It should use the data from displayed_dive in addition to the dive list. > > Then all you need to do is make sure things are tracked correctly in > > displayed_dive (and they should, if not, that's yet another bug). > > Oh, and in case this wasn't obvious... my sentence implied the suggestion > that you should implement that. So I won't tackle this and focus on the > other three dozen bugs that I'm aware of Jedi management tricks. ;) -- John Van Ostrand At large on sabbatical ___ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
Re: Setting location coordinates while editing a dive
On Fri, Oct 31, 2014 at 12:34 PM, Dirk Hohndel wrote:
> On Fri, Oct 31, 2014 at 11:39:27AM -0400, John Van Ostrand wrote:
> >
> > Mentioning Undo got me thinking.
> >
> > I've been mucking about trying to have globe.cpp both update the editted
> > dive *and* show the currently edit's placemark planted on the globe and
> the
> > code is uglier than I like. The idea of "undo" has me thinking of ways to
> > streamline that code and remove some oddities in the UI.
>
> He.
>
> > How about this.
> >
> > When editing dives we update the dive on each field change, storing the
> old
> > dive in an undo stack. We can accumulate similar consecutive edits to
> > reduce the number of undos.
>
> OK. That seems simple. Except when editing multiple dives. Then it gets...
> interesting. But I'll go with the idea.
>
> > This could include gas changes, events and even changes to the profile.
>
> Yep, I can see that.
>
> > Manually added dives can be done like this too.
>
> You lost me.
>
In other words when manually adding a dive an empty dive would be added to
the dive list with fields added to the dive in as they are entered in the
form.
>
> > This would have a few benefits
> >
> > 1. We eliminate the "This dive is being edited" indicator and all the
> code
> > referencing "isEditing()".
>
> So this is easy to write in text, but once you deal with the widgets it
> gets a little tricky.
>
> Example. You edit something (let's say a tag), your cursor is still in
> that field, and then click on a different dive in the dive list. What
> happens? You have a drop down open (date, cylinder type) and you change to
> a different dive...
>
> What I'm trying to say is that the definition of "I'm editing something"
> is very intuitive, until you try to implement the finer details.
>
I figured we could use a change signal of some kind so we can react to
changes. It looks like some widgets are already enabled. Others may need
some modification.
> 2. We don't have anything funny to do with placemarks on the map.
> > 3. Oops, like accidental double clicks can be backed out.
> > 4. Eliminate non-intuitive location/placemark functionality.
> > 5. Any for_each_dive will automatically include the dive being edited so
> UI
> > updates like placemarks don't need special cases.
>
> That last one doesn't make sense to me. Can you elaborate.
>
The repopulateLabels function in globe.cpp uses the dive list to create
flags. It would be cleaner if the data being editing was being updated into
the dive list as each field was changed. That way functions like
repopulateLabels wouldn't have to retrieve data from the form to place a
flag.
>
> > There would be a few challenges:
> >
> > 1. Single dive edits would be easy but multi-dive edits would more
> > complicated. They should be associated as one undo.
>
> OK
>
> > 2. Trips would be another undo case.
>
> What does that mean? You mean trip changes to the divelist?
> Uhhh, I want to see the data structure in which you do that :-)
>
We may want to undo a trip grouping or undo edits to a trip. We need to set
up different types of undos. A TRIP_GROUP undo might be one type and
TRIP_EDIT undo might be another. The undo stack would have a struct that
contains a type and a pointer to a blob of data, and undo action would send
that blob to the function that handles that specific undo type. The blob
would be used by that function to perform the undo. This provides a
framework for allowing a wide range of different undos.
A example might be this (imagine expanding struct undo_dive_edit to hold a
list of dives.)
struct undo_item {
enum undo_type type;
void *data;
};
struct undo_dive_edit {
int divenr;
struct dive *dive;
};
struct undo_item undo_stack[16];
int undo_stack_head = 0;
int undo_stack_tail = 0;
// called when date field is updated
void MainTab::updateDate(const QDateTime &)
{
undo_dive_create(); // create and undo for this change
}
void undo_dive_edit_create()
{
struct undo_item undo = undo_get_current(); // Get most recent undo
struct undo_dive_edit *ude = undo->undo_data;
// Only create an undo if this change affects a different set of dives
than the current undo.
if (undo.type != UNDO_DIVE_EDIT || ude->dive == selected_dive) {
// Create undo item.
struct undo_dive_edit *new_undo = malloc(sizeof(struct
undo_dive_edit));
new_undo->divenr = selected_dive;
new_undo->dive =
undo_push(UNDO_DIVE_EDIT, (void *) new_undo);
}
}
// Dive Edit Undo
void undo_dive_edit(void *undo_data)
{
struct undo_dive_edit *undo = undo->undo_data;
}
void undo_pop()
{
if (undo_stack_head == undo_stack_tail)
return; // nothing to undo
struct undo_item undo = undo_stack[undo_stack_head--];
switch (undo.type) {
case UNDO_DIVE_EDIT:
undo_dive_edit(undo->data);
break;
case UNDO_DIVE_DELETE:
...
}
}
> > 3. We'd still have the issue of ac
Re: Setting location coordinates while editing a dive
On Fri, Oct 31, 2014 at 09:40:29AM -0700, Dirk Hohndel wrote: > > I've been able to make changes so the ui.coordinates field is updated but > > placing flags on the globe is more challenging. Since the edited location > > name and coordinates only exist in the ui.location and ui.coordinates > > variables the globe::repopulateLabels() function can't place a flag on the > > globe, repopulateLabels() uses the dive list for coordinates and labels. > > Adding code so it also checks isEditing() and places a flag based on the > > edited fields would add that functionality. > > It should use the data from displayed_dive in addition to the dive list. > Then all you need to do is make sure things are tracked correctly in > displayed_dive (and they should, if not, that's yet another bug). Oh, and in case this wasn't obvious... my sentence implied the suggestion that you should implement that. So I won't tackle this and focus on the other three dozen bugs that I'm aware of. /D ___ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
Re: Setting location coordinates while editing a dive
On Fri, Oct 31, 2014 at 12:04:03PM -0400, John Van Ostrand wrote: > On Fri, Oct 31, 2014 at 10:45 AM, Dirk Hohndel wrote: > > > On Thu, Oct 30, 2014 at 04:22:03PM -0400, John Van Ostrand wrote: > > > I was getting very frustrated adding a dive location to an existing > > dive. I > > > was expecting to be able to add a location then select the coordinates > > from > > > the globe and have the coordinates updated and the location name and > > > coordinates associated. Neither happens. The flag is planted without text > > > and the flag disappears when I save. > > > > That's not how it's supposed to work :-( > > It's not supposed to work the way I expected or the way it does? flag should disappear when you save. > > > So I'm looking for ideas on proper behaviour. There are several > > > circumstances I can think of: > > > > > > 1. User is adding a dive.. Associate the location with the edit location > > > name of the dive. > > > 2. User is editing an existing dive. Associate the location with the edit > > > location name of the dive. > > > > I don't know what you mean by "edit location name of the dive" in both of > > these cases. > > I've looked at the code so I can write a little more concisely about it. > > 1 and 2. When adding or editing a dive dive double clicking on the globe > should add coordinates to the Coordinates field and place a marker on the > globe so the user can see that he selected appropriate coordinates *before > saving*. Neither happens, right now the action has no effect on the dive. Bug. Needs to be fixed. Sadly, this used to work :-( > I've been able to make changes so the ui.coordinates field is updated but > placing flags on the globe is more challenging. Since the edited location > name and coordinates only exist in the ui.location and ui.coordinates > variables the globe::repopulateLabels() function can't place a flag on the > globe, repopulateLabels() uses the dive list for coordinates and labels. > Adding code so it also checks isEditing() and places a flag based on the > edited fields would add that functionality. It should use the data from displayed_dive in addition to the dive list. Then all you need to do is make sure things are tracked correctly in displayed_dive (and they should, if not, that's yet another bug). /D ___ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
Re: Setting location coordinates while editing a dive
On Fri, Oct 31, 2014 at 11:39:27AM -0400, John Van Ostrand wrote: > > Mentioning Undo got me thinking. > > I've been mucking about trying to have globe.cpp both update the editted > dive *and* show the currently edit's placemark planted on the globe and the > code is uglier than I like. The idea of "undo" has me thinking of ways to > streamline that code and remove some oddities in the UI. He. > How about this. > > When editing dives we update the dive on each field change, storing the old > dive in an undo stack. We can accumulate similar consecutive edits to > reduce the number of undos. OK. That seems simple. Except when editing multiple dives. Then it gets... interesting. But I'll go with the idea. > This could include gas changes, events and even changes to the profile. Yep, I can see that. > Manually added dives can be done like this too. You lost me. > This would have a few benefits > > 1. We eliminate the "This dive is being edited" indicator and all the code > referencing "isEditing()". So this is easy to write in text, but once you deal with the widgets it gets a little tricky. Example. You edit something (let's say a tag), your cursor is still in that field, and then click on a different dive in the dive list. What happens? You have a drop down open (date, cylinder type) and you change to a different dive... What I'm trying to say is that the definition of "I'm editing something" is very intuitive, until you try to implement the finer details. > 2. We don't have anything funny to do with placemarks on the map. > 3. Oops, like accidental double clicks can be backed out. > 4. Eliminate non-intuitive location/placemark functionality. > 5. Any for_each_dive will automatically include the dive being edited so UI > updates like placemarks don't need special cases. That last one doesn't make sense to me. Can you elaborate. > There would be a few challenges: > > 1. Single dive edits would be easy but multi-dive edits would more > complicated. They should be associated as one undo. OK > 2. Trips would be another undo case. What does that mean? You mean trip changes to the divelist? Uhhh, I want to see the data structure in which you do that :-) > 3. We'd still have the issue of accidental edits, like an accidental > double-click on the globe. > 4. Other cases: Deletes, renumber, photo add/delete, imports, copy and > paste, undos (i.e. redo) Several of those would be crazy complicated to keep undo information about. Renumbers for example. We need to draw the line somewhere. > 5. Future changes will create more cases. > 6. Edited dives may appear incomplete in the dive list. What do you mean here? > To implement: > > 1. Create a structure for an undo that supports the known cases. Each > struct is an atomic undo, an array of them is the undo stack. Maybe a > complementary redo stack can be done too. OK. See above. I'd like to see the data structures that you have in mind here. > 2. Determine what actions should accumulate into an atomic undo. (e.g. > per-field undo or entire dive undo.) I'd go with a dive undo. But once again - what does that mean in the context of multi dive edits? > 2. Create an undo function for each case. > 3. Alter edits to directly change dive_list. Please explain more. > I've never done something like this before so I'm not sure if there are > some really sneaky details lurking out of sight. I promise, there are a MOUNTAIN of snarky details... But I like the basic idea. Let's start drilling down on some details and see where we get. All this is post 4.3, though. /D ___ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
Re: Setting location coordinates while editing a dive
On Fri, Oct 31, 2014 at 10:45 AM, Dirk Hohndel wrote: > On Thu, Oct 30, 2014 at 04:22:03PM -0400, John Van Ostrand wrote: > > I was getting very frustrated adding a dive location to an existing > dive. I > > was expecting to be able to add a location then select the coordinates > from > > the globe and have the coordinates updated and the location name and > > coordinates associated. Neither happens. The flag is planted without text > > and the flag disappears when I save. > > That's not how it's supposed to work :-( > It's not supposed to work the way I expected or the way it does? > > > I thought that commit 9ead871d6456f8f19f6f0fe2413513ef4449253d was the > > culprit except reverting it seemingly made no difference. > > (qt-ui/globe.cpp:135 is the effective area of that commit.) > > > > So I added a dive to see what functionality that commit was supposed to > > correct. By double clicking, the coordinates are associated with the > > location of the highlighted dive in the dive list. I can see why that's > bad. > > > > Then restoring the commit gave me the same add-dive bug, odd but I'll > > forget about that right now. > > You lost me. > It looked as if that patch may have fixed a bug while adding a dive but created one while editing a dive. I tested but the edit-dive problem was elsewhere and I couldn't see a difference in how add-dive worked so I'm not sure what bug it's supposed to have fixed. I don't think that's important right now though. > > > So I'm looking for ideas on proper behaviour. There are several > > circumstances I can think of: > > > > 1. User is adding a dive.. Associate the location with the edit location > > name of the dive. > > 2. User is editing an existing dive. Associate the location with the edit > > location name of the dive. > > I don't know what you mean by "edit location name of the dive" in both of > these cases. > I've looked at the code so I can write a little more concisely about it. 1 and 2. When adding or editing a dive dive double clicking on the globe should add coordinates to the Coordinates field and place a marker on the globe so the user can see that he selected appropriate coordinates *before saving*. Neither happens, right now the action has no effect on the dive. I've been able to make changes so the ui.coordinates field is updated but placing flags on the globe is more challenging. Since the edited location name and coordinates only exist in the ui.location and ui.coordinates variables the globe::repopulateLabels() function can't place a flag on the globe, repopulateLabels() uses the dive list for coordinates and labels. Adding code so it also checks isEditing() and places a flag based on the edited fields would add that functionality. I had assumed a list associating location names and coordinates was kept which is why I wrote "associate the location [i.e. coordinates] with the location name." > > 3. User is not editing a dive. Associate the location with the recorded > > location name of the dive. > Circumstance 3 works as expected. > Yep. > > > The problem arises that if the user changes the location while editing, > or > > cancels the edit that needs to change the location-coordinate mapping. > > Sorry, your language is just too confusing for me. There are three things > that we need to treat separately: > - location name > - coordinates > - flag placement on the globe > > Can you rephrase your three cases and your issue using these terms? Then > we can determine which scenario isn't working right and hopefully fix > it... > > > So, do we track that in the edit session and update it on a name change > or > > revert it on cancel? > > > > Or do we inform the user, on a double click, that they need to save > first? > > So this is the one part I did understand. And I responded to THAT in my > previous email. > > /D > -- John Van Ostrand At large on sabbatical ___ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
Re: Setting location coordinates while editing a dive
On Fri, Oct 31, 2014 at 10:41 AM, Dirk Hohndel wrote: > On Fri, Oct 31, 2014 at 02:51:39PM +0100, Davide DB wrote: > > On Thu, Oct 30, 2014 at 9:22 PM, John Van Ostrand > > wrote: > > > > > So, do we track that in the edit session and update it on a name > change or > > > revert it on cancel? > > > > > > Or do we inform the user, on a double click, that they need to save > first? > > > > Good question. > > Actually I always found the editing/saving process very inconsistent. > > Why I get "dive is being edited" alert just on the "dive notes" tab but I > > can change tanks, I can add events on the profile without any alert? > > Just to be precise, you cannot modify equipment without triggering the > "edit mode". But you can add a gas change. > > Yes, our handling of edits is extremely inconsistent. It has bugged me for > a long time. > > I absolutely want a way to say "never mind" when editing things. I usually > need that in the context of edits to the info page, so that's where it got > implemented. But then there are several other ways to edit things where we > don't have that. > > People have suggested an undo/redo feature but that would be an even more > complex layer to add. > > Today you can make changes in these places (I hope I didn't forget any) > > 1) Dive notes tab (date, time, location, divemaster, buddy, tags, notes) > (and once you entered edit mode, on a manually added dive, you can > change the profile in the profile, but you can only start this process > if you actually change some other field first - that's completely > broken) > Equipment tab (tanks and weights). > > The moment you edit things in Dive notes or Equipment tab, this triggers > "edit mode" and the "save / discard" functionality. > > 2) Right click on the profile (changing/adding/removing events, gas > changes, order of dive computers, delete dive computer > > All of these take immediate effect > > 3) Double click on the globe window > > Also takes immediate effect > > 4) Dive list (mostly changes to trip status, but also delete dives or shift > their times) > > All of these take immediate effect > > > I think it would be fairly simple to switch to edit mode when making > changes in the second and third case above. Not sure this can be > reasonably done in the last one. Or we could do what other software does > and not have this edit mode. So basically the moment you tab away from a > field that edit is permanent. Only if you use ESC is an edit reverted. > > I'll admit that I'd hate that, so I'd be more inclined to got with adding > edit mode to 2) and 3) above. > > Comments? Preferences Mentioning Undo got me thinking. I've been mucking about trying to have globe.cpp both update the editted dive *and* show the currently edit's placemark planted on the globe and the code is uglier than I like. The idea of "undo" has me thinking of ways to streamline that code and remove some oddities in the UI. How about this. When editing dives we update the dive on each field change, storing the old dive in an undo stack. We can accumulate similar consecutive edits to reduce the number of undos. This could include gas changes, events and even changes to the profile. Manually added dives can be done like this too. This would have a few benefits 1. We eliminate the "This dive is being edited" indicator and all the code referencing "isEditing()". 2. We don't have anything funny to do with placemarks on the map. 3. Oops, like accidental double clicks can be backed out. 4. Eliminate non-intuitive location/placemark functionality. 5. Any for_each_dive will automatically include the dive being edited so UI updates like placemarks don't need special cases. There would be a few challenges: 1. Single dive edits would be easy but multi-dive edits would more complicated. They should be associated as one undo. 2. Trips would be another undo case. 3. We'd still have the issue of accidental edits, like an accidental double-click on the globe. 4. Other cases: Deletes, renumber, photo add/delete, imports, copy and paste, undos (i.e. redo) 5. Future changes will create more cases. 6. Edited dives may appear incomplete in the dive list. To implement: 1. Create a structure for an undo that supports the known cases. Each struct is an atomic undo, an array of them is the undo stack. Maybe a complementary redo stack can be done too. 2. Determine what actions should accumulate into an atomic undo. (e.g. per-field undo or entire dive undo.) 2. Create an undo function for each case. 3. Alter edits to directly change dive_list. I've never done something like this before so I'm not sure if there are some really sneaky details lurking out of sight. -- John Van Ostrand At large on sabbatical ___ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
Re: Proxy bug [was Re: Setting location coordinates while editing a dive]
Done https://www.screenr.com/RaDN http://trac.subsurface-divelog.org/ticket/753 On Fri, Oct 31, 2014 at 4:04 PM, Davide DB wrote: > On Fri, Oct 31, 2014 at 3:56 PM, Dirk Hohndel wrote: > >> >> Please do. I guess I can think of a reason for this... it's possible that >> we only get the password when the Object is created and don't refresh it >> from the preferences when we try again. I'll look into that. >> >> > http://trac.subsurface-divelog.org/ticket/752 > > > I'm trying to speed up editing operation through the mythical copy & paste > operation but I'm having hard times... > Now it seems that also editing gas in one tank used in multiple dives > doesn't work anymore :( > > I'll try to reproduce it capturing a video. > > > > -- > Davide > https://vimeo.com/bocio/videos > -- Davide https://vimeo.com/bocio/videos ___ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
Re: Proxy bug [was Re: Setting location coordinates while editing a dive]
On Fri, Oct 31, 2014 at 3:56 PM, Dirk Hohndel wrote: > > Please do. I guess I can think of a reason for this... it's possible that > we only get the password when the Object is created and don't refresh it > from the preferences when we try again. I'll look into that. > > http://trac.subsurface-divelog.org/ticket/752 I'm trying to speed up editing operation through the mythical copy & paste operation but I'm having hard times... Now it seems that also editing gas in one tank used in multiple dives doesn't work anymore :( I'll try to reproduce it capturing a video. -- Davide https://vimeo.com/bocio/videos ___ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
Proxy bug [was Re: Setting location coordinates while editing a dive]
On Fri, Oct 31, 2014 at 03:50:04PM +0100, Davide DB wrote: > > I'm just editing my last dives and I found a bug on Proxy: > > - Proxy password has expired > - You try to download GPS point and you get "authentication error" > - You update the password and save new setting but the new password is not > used by the GPS download feature but it was correctly saved. > - Restart the application, everything works. > > I'll file a bug on trac Please do. I guess I can think of a reason for this... it's possible that we only get the password when the Object is created and don't refresh it from the preferences when we try again. I'll look into that. /D ___ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
Re: Setting location coordinates while editing a dive
On Fri, Oct 31, 2014 at 3:41 PM, Dirk Hohndel wrote: > > I'll admit that I'd hate that, so I'd be more inclined to got with adding > edit mode to 2) and 3) above. > > Comments? Preferences? > > /D > +1 PS I'm just editing my last dives and I found a bug on Proxy: - Proxy password has expired - You try to download GPS point and you get "authentication error" - You update the password and save new setting but the new password is not used by the GPS download feature but it was correctly saved. - Restart the application, everything works. I'll file a bug on trac -- Davide https://vimeo.com/bocio/videos ___ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
Re: Setting location coordinates while editing a dive
On Thu, Oct 30, 2014 at 04:22:03PM -0400, John Van Ostrand wrote: > I was getting very frustrated adding a dive location to an existing dive. I > was expecting to be able to add a location then select the coordinates from > the globe and have the coordinates updated and the location name and > coordinates associated. Neither happens. The flag is planted without text > and the flag disappears when I save. That's not how it's supposed to work :-( > I thought that commit 9ead871d6456f8f19f6f0fe2413513ef4449253d was the > culprit except reverting it seemingly made no difference. > (qt-ui/globe.cpp:135 is the effective area of that commit.) > > So I added a dive to see what functionality that commit was supposed to > correct. By double clicking, the coordinates are associated with the > location of the highlighted dive in the dive list. I can see why that's bad. > > Then restoring the commit gave me the same add-dive bug, odd but I'll > forget about that right now. You lost me. > So I'm looking for ideas on proper behaviour. There are several > circumstances I can think of: > > 1. User is adding a dive.. Associate the location with the edit location > name of the dive. > 2. User is editing an existing dive. Associate the location with the edit > location name of the dive. I don't know what you mean by "edit location name of the dive" in both of these cases. > 3. User is not editing a dive. Associate the location with the recorded > location name of the dive. Yep. > The problem arises that if the user changes the location while editing, or > cancels the edit that needs to change the location-coordinate mapping. Sorry, your language is just too confusing for me. There are three things that we need to treat separately: - location name - coordinates - flag placement on the globe Can you rephrase your three cases and your issue using these terms? Then we can determine which scenario isn't working right and hopefully fix it... > So, do we track that in the edit session and update it on a name change or > revert it on cancel? > > Or do we inform the user, on a double click, that they need to save first? So this is the one part I did understand. And I responded to THAT in my previous email. /D ___ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
Re: Setting location coordinates while editing a dive
On Fri, Oct 31, 2014 at 02:51:39PM +0100, Davide DB wrote: > On Thu, Oct 30, 2014 at 9:22 PM, John Van Ostrand > wrote: > > > So, do we track that in the edit session and update it on a name change or > > revert it on cancel? > > > > Or do we inform the user, on a double click, that they need to save first? > > Good question. > Actually I always found the editing/saving process very inconsistent. > Why I get "dive is being edited" alert just on the "dive notes" tab but I > can change tanks, I can add events on the profile without any alert? Just to be precise, you cannot modify equipment without triggering the "edit mode". But you can add a gas change. Yes, our handling of edits is extremely inconsistent. It has bugged me for a long time. I absolutely want a way to say "never mind" when editing things. I usually need that in the context of edits to the info page, so that's where it got implemented. But then there are several other ways to edit things where we don't have that. People have suggested an undo/redo feature but that would be an even more complex layer to add. Today you can make changes in these places (I hope I didn't forget any) 1) Dive notes tab (date, time, location, divemaster, buddy, tags, notes) (and once you entered edit mode, on a manually added dive, you can change the profile in the profile, but you can only start this process if you actually change some other field first - that's completely broken) Equipment tab (tanks and weights). The moment you edit things in Dive notes or Equipment tab, this triggers "edit mode" and the "save / discard" functionality. 2) Right click on the profile (changing/adding/removing events, gas changes, order of dive computers, delete dive computer All of these take immediate effect 3) Double click on the globe window Also takes immediate effect 4) Dive list (mostly changes to trip status, but also delete dives or shift their times) All of these take immediate effect I think it would be fairly simple to switch to edit mode when making changes in the second and third case above. Not sure this can be reasonably done in the last one. Or we could do what other software does and not have this edit mode. So basically the moment you tab away from a field that edit is permanent. Only if you use ESC is an edit reverted. I'll admit that I'd hate that, so I'd be more inclined to got with adding edit mode to 2) and 3) above. Comments? Preferences? /D ___ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
Re: Setting location coordinates while editing a dive
On Thu, Oct 30, 2014 at 9:22 PM, John Van Ostrand wrote: > So, do we track that in the edit session and update it on a name change or > revert it on cancel? > > Or do we inform the user, on a double click, that they need to save first? > > Good question. Actually I always found the editing/saving process very inconsistent. Why I get "dive is being edited" alert just on the "dive notes" tab but I can change tanks, I can add events on the profile without any alert? Regarding your question, we should inform the user I guess. -- Davide https://vimeo.com/bocio/videos ___ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
Setting location coordinates while editing a dive
I was getting very frustrated adding a dive location to an existing dive. I was expecting to be able to add a location then select the coordinates from the globe and have the coordinates updated and the location name and coordinates associated. Neither happens. The flag is planted without text and the flag disappears when I save. I thought that commit 9ead871d6456f8f19f6f0fe2413513ef4449253d was the culprit except reverting it seemingly made no difference. (qt-ui/globe.cpp:135 is the effective area of that commit.) So I added a dive to see what functionality that commit was supposed to correct. By double clicking, the coordinates are associated with the location of the highlighted dive in the dive list. I can see why that's bad. Then restoring the commit gave me the same add-dive bug, odd but I'll forget about that right now. So I'm looking for ideas on proper behaviour. There are several circumstances I can think of: 1. User is adding a dive.. Associate the location with the edit location name of the dive. 2. User is editing an existing dive. Associate the location with the edit location name of the dive. 3. User is not editing a dive. Associate the location with the recorded location name of the dive. The problem arises that if the user changes the location while editing, or cancels the edit that needs to change the location-coordinate mapping. So, do we track that in the edit session and update it on a name change or revert it on cancel? Or do we inform the user, on a double click, that they need to save first? -- John Van Ostrand At large on sabbatical ___ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
