D20169: Add profile support interface for TerminalInterface

Pino Toscano noreply at phabricator.kde.org
Sun Apr 21 20:38:29 BST 2019


pino added a comment.


  General notes on the API (not a konsole developer though):
  
  - an abstract virtual destructor is needed
  - in kdelibs/kf5 lingo, the "get" as prefix of API getters is generally not used
  - `setCurrentProfile` instead of `changeCurrentProfile`? the latter makes me think it does changes to the current profile, rather than setting another profile as current
  - wrt `changeCurrentProfile`: what will it do if the specified profile name does not exist? should the API return true/false to indicate the switch actually succeeded?
  - wrt `getProfilePath`: what if an application does not store a profile as single filename, for example as directory, or even stored e.g. in a DB? maybe it would be better to have this as property of a profile
  - is a `setProfileProperty` worth having?
  - is an API to list the available properties worth having?
  
  Also: this file is still not installed, so it cannot be used. Maybe just add the new interface to the existing `kde_terminal_interface.h`?

INLINE COMMENTS

> terminalprofileinterface.h:1
> +// interface.h
> +// Copyright (C)  2002  Dominique Devriese <devriese at kde.org>

either put the right filename, or just remove (IMHO better)

> terminalprofileinterface.h:2
> +// interface.h
> +// Copyright (C)  2002  Dominique Devriese <devriese at kde.org>
> +

this is certainly yours now

> terminalprofileinterface.h:19
> +
> +#ifndef KDELIBS_TERMINALPROFILEINTERFACE_H
> +#define KDELIBS_TERMINALPROFILEINTERFACE_H

I'd go with `KPARTS_etc` (instead of `KDELIBS_etc`)

REPOSITORY
  R306 KParts

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

To: mschiller, hindenburg, #konsole, #frameworks, cfeck, hein
Cc: pino, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20190421/570a175c/attachment-0001.html>


More information about the Kde-frameworks-devel mailing list