D9568: Add a way to reset changes to system-wide color schemes

Kurt Hindenburg noreply at phabricator.kde.org
Tue Jan 23 02:08:28 UTC 2018


hindenburg added a comment.


  Thanks for working on this

INLINE COMMENTS

> ColorSchemeManager.cpp:224
> +
> +    if (fileInfo.exists()) {
> +        return dirInfo.isWritable();

I'm not sure I understand this -  how can the colorscheme file not exist?  and why return true when it does not exist?

can't you just return dirInfo.isWritable(); ?

> ColorSchemeManager.cpp:240
> +    // i.e. resetting the colorscheme
> +    if (paths.count() > 1) {
> +        return true;

actually you could use return (paths.count() > 1)

> EditProfileDialog.cpp:942
> +
> +        bool on = ColorSchemeManager::instance()->canResetColorScheme(name);
> +        _ui->resetColorSchemeButton->setEnabled(on);

can you name this better than on?  isResetable?

I'm not sure the "Reset" is the best way to describe what we're doing.  Nothing comes to mind though.

REPOSITORY
  R319 Konsole

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

To: ahmadsamir, hindenburg, #konsole
Cc: #konsole, ngraham, hindenburg
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/konsole-devel/attachments/20180123/5a8b1c93/attachment-0001.html>


More information about the konsole-devel mailing list