broulik added inline comments.
INLINE COMMENTS
> main.qml:173
> + // For some hardware, headphones are a port of the build-in
> audio sink
> + var text =
> defaultSink.ports[defaultSink.activePortIndex].description;
> + var icon;
Please cache `defaultSink.ports[defaultSink.activePortIndex]` in a variable.
Also, can this be out of bounds or otherwise throw an error?
> main.qml:175
> + var icon;
> + if (defaultSink.ports[defaultSink.activePortIndex].name ==
> "analog-output-headphones") {
> + icon = Icon.formFactorIcon("headphone");
I would prefer if hardcoded values like these are put into a helper function or
`formFactorIcon` augmented to take those into account also
> main.qml:181
> +
> + if (!icon || icon == "") {
> + icon = Icon.formFactorIcon(defaultSink.formFactor);
`if (!icon) {` is sufficient
> main.qml:187
> if (!icon) {
> + text = defaultSink.description;
> // Show "muted" icon for Dummy output
The `text =` assignments seem to serve no purpose, ie. the end result would be
no different than having it down where it was
> pulseaudio.cpp:264
> connect(sink, &Sink::stateChanged, this,
> &SinkModel::updatePreferredSink);
> + connect(sink, &Sink::activePortIndexChanged, this, [this]() {
> + Q_EMIT defaultSinkChanged();
You can connect a signal to a signal:
connect(sink, &Sink::activePortIndexChanged, this, &Sink::defaultSinkChanged);
REPOSITORY
R115 Plasma Audio Volume Applet
REVISION DETAIL
https://phabricator.kde.org/D16082
To: thsurrel, #plasma, #vdg, drosca
Cc: broulik, nicolasfella, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai,
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
