[PATCH] Edit Bookmarks name

[Dirk Hohndel]

I'll respond to the last feedback that I had given you - but this is to
the patch you sent 15 minutes ago:

On Tue, 2014-03-25 at 09:49 -0700, Dirk Hohndel wrote:
> On Tue, 2014-03-25 at 17:55 +0200, Yosef Hamza wrote:
> 
> big parts of your patch have "4 space" indentation.

Still true

> can you explain in the commit message why you switched the pixmap to be
> a flag?

Still missing

> you always add the option to "Edit name" to the context menu if there is
> any event - but this only makes sense for bookmark events, right?

fixed

> the comment "so we know what to remove" makes no sense in your case (it
> made sense for the code which you copied).

fixed

> changing the event->name is an interesting approach. Subsurface has not
> been super consistent about the way in which we handle events. The
> libdivecomputer design specifies the event->type, yet the subsurface
> code often uses the event name to track things which likely is a mistake
> in what was implemented in the very early code and was never fixed.
> 
> So in a way I'm OK with overwriting the name, but there's a price to be
> paid:
> - you need to make sure that this doesn't break anything else in the
> code
> - you need to make sure that you can save and load such events and
> things behave correctly
> - you need to add a test dive that has such a modified event in it

I don't see any indication that you have done that. Should be in the
commit message

> And of course you can ONLY do the name change if the event is of type
> SAMPLE_EVENT_BOOKMARK

Fixing the third comment above fixed this as well

Please address the remaining issues.

/D