[PATCH] Edit bookmark name.

Dirk Hohndel dirk at hohndel.org
Thu Apr 3 11:09:36 PDT 2014

On Thu, 2014-04-03 at 19:12 +0200, Yosef Hamza wrote:
> last version of the batch.
> It works just as simple bookmarks with all it's functions.
> the second attachment to check saving/loading and a preview.

Thanks, Yosef,

a few comments

a) the patch looks indented correctly, but all the tabs are replaced by
8 spaces. I wonder if this is an editor setting or somthing.

b) when I try the patch and click on a bookmark to change its name, I
first get asked if I want to hide all bookmarks, and after I decline
that, then I'm ask to enter a new bookmark name. The reason for that is
that you are not creating a new QAction() object and end up reusing the
one that was used for the HideEvents action. I am quite surprised that
you didn't run into that in your testing

c) finally, your commit message was a little hard to understand. What do
you think of my proposed commit message:

Edit bookmark name

Add the option to edit the name of a bookmark to be more meaningful for
the user they prefer.
It works just as simple bookmarks and can be removed and hidden.
It won't accept names longer than 22 characters because longer names will
display as garbage text.
Also changed the code from displaying flag depending on event name to
dependind on event type.

Can you address these three issues and resend, please?



More information about the subsurface mailing list