dfaure added inline comments.

INLINE COMMENTS

> hein wrote in dropjob.cpp:324
> Should this be && instead of ||? "Add actions if either of those is not 
> empty" seems weird, what's the reasoning?

It's a copy of the if() on line 311. But there it makes sense. Here not so much 
;)

What if there were already some plugin actions? Then instead of the usual
separator | appActions | pluginActions would here become
separator | pluginActions | separator | appActions.

And what if there were already some app actions? Then it's even worse, we end 
up with
separator | oldAppActions | [pluginActions if any] | separator | newAppActions.

It sounds to me like

1. any previously set appActions should be removed first
2. the new app actions should be inserted into the right place, which means 
remembering where -and- remember or checking whether we need to start with a 
separator or not.

XMLGUI would handle all this automatically pretty well ;)

REPOSITORY
  R241 KIO

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

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: mart, #plasma
Cc: dfaure, hein, plasma-devel, #frameworks, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

Reply via email to