D8825: do not show edit bookmarks action if keditbookmarks is not installed

2017-11-22 Thread Harald Sitter
This revision was automatically updated to reflect the committed changes.
Closed by commit R294:5dffdee21f20: do not show edit bookmarks action if 
keditbookmarks is not installed (authored by sitter).

REPOSITORY
  R294 KBookmarks

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D8825?vs=22443&id=22732

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

AFFECTED FILES
  src/kbookmarkmanager.cpp
  src/kbookmarkmenu.cpp
  src/kbookmarkmenu_p.h

To: sitter, #frameworks, apol
Cc: apol


D8825: do not show edit bookmarks action if keditbookmarks is not installed

2017-11-22 Thread Aleix Pol Gonzalez
apol accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R294 KBookmarks

BRANCH
  master

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

To: sitter, #frameworks, apol
Cc: apol


D8825: do not show edit bookmarks action if keditbookmarks is not installed

2017-11-16 Thread Harald Sitter
sitter updated this revision to Diff 22443.
sitter added a comment.


  rebased

REPOSITORY
  R294 KBookmarks

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D8825?vs=22379&id=22443

BRANCH
  master

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

AFFECTED FILES
  src/kbookmarkmanager.cpp
  src/kbookmarkmenu.cpp
  src/kbookmarkmenu_p.h

To: sitter, #frameworks
Cc: apol


D8825: do not show edit bookmarks action if keditbookmarks is not installed

2017-11-15 Thread Harald Sitter
sitter added inline comments.

INLINE COMMENTS

> apol wrote in kbookmarkmenu_p.h:42
> Why not in the cpp file?

It's used in 2 difference cpps.

REPOSITORY
  R294 KBookmarks

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

To: sitter, #frameworks
Cc: apol


D8825: do not show edit bookmarks action if keditbookmarks is not installed

2017-11-15 Thread Aleix Pol Gonzalez
apol added a comment.


  LGTM

INLINE COMMENTS

> kbookmarkmenu_p.h:42
>  
> +#define KEDITBOOKMARKS_BINARY "keditbookmarks"
> +

Why not in the cpp file?

REPOSITORY
  R294 KBookmarks

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

To: sitter, #frameworks
Cc: apol


D8825: do not show edit bookmarks action if keditbookmarks is not installed

2017-11-14 Thread Harald Sitter
sitter created this revision.
sitter added a reviewer: Frameworks.
Restricted Application added a project: Frameworks.

REVISION SUMMARY
  keditbookmarks lives in applications, making it very likely that it is
  not installed. this does already raise an error window explaining that
  the binary is not installed, unfortunately that is fairly poor user
  experience. instead do not show the edit action if we have no editor
  installed.
  
  this is a bit unfortunate design-wise KBookmarkOwner is meant to control
  if the edit action is shown, it does however encourage deriving from it
  to control this behavior making it more than likely that devs simply derive
  and override enableOption always returning true to enable all features
  (while technically unnecessary). to deal with this we make the executable
  lookup in the menu implementation, in addition to checking enableOption.
  effectively we now have "does the owner want us to show the edit option,
  and if so can we even edit?"

TEST PLAN
  - uninstall keditbookmarks
  - konsole has no edit entry
  - install again
  - konsole has edit entry

REPOSITORY
  R294 KBookmarks

BRANCH
  master

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

AFFECTED FILES
  src/kbookmarkmanager.cpp
  src/kbookmarkmenu.cpp
  src/kbookmarkmenu_p.h

To: sitter, #frameworks