D29326: Bookmarks Runner: Fix extraction of firefox profile

[Stefan Brüns]

bruns requested changes to this revision.
bruns added a comment.
This revision now requires changes to proceed.


  The summary needs some text which works without any images
  
  The code needs some comment which tells how the selection logic works and why 
it is as it is.

INLINE COMMENTS

> firefox.cpp:184
>  } else {
> -// There are multiple profiles, find the default one
> +const QStringList installConfig = 
> firefoxProfile.groupList().filter(QRegularExpression("Install.*"));
> +if (installConfig.size() == 1) {

Missing start of string anchor (^)

> firefox.cpp:185
> +const QStringList installConfig = 
> firefoxProfile.groupList().filter(QRegularExpression("Install.*"));
> +if (installConfig.size() == 1) {
> +profilePath = 
> firefoxProfile.group(installConfig.first()).readEntry("Default");

Can this ever be > 1?

REPOSITORY
  R120 Plasma Workspace

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

To: alex, #plasma, ngraham, bruns, broulik
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28619: BookmarksRunner: Remove caching of profile database path

[Stefan Brüns]

bruns requested changes to this revision.
bruns added a comment.
This revision now requires changes to proceed.


  Please just remove the `writeEntry`.
  
  You can then propose "But what about renaming the dbfile entry, maybe to 
alternate_dbfile?" as another review.

REPOSITORY
  R120 Plasma Workspace

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

To: alex, #plasma, bruns, broulik
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28619: BookmarksRunner: Remove caching of profile database path

[Stefan Brüns]

bruns added a comment.


  There are two possible ways of changing the caching, and each one will break 
behavior for one of two different groups of users:
  
  1. People who rely on the krunner following the default profile.
  2. People who rely on krunner using the dbfile config entry.
  
  I would argue for just removing the `grp.writeEntry("dbfile", m_dbFile)` 
line. This keeps the current behavior for the second group, and still solves 
the problem for the first group when there is no config yet. Only users of the 
second group with an existing config have to do a one-time change to their 
config, e.g. resetting/removing the "dbfile" config value.

REPOSITORY
  R120 Plasma Workspace

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

To: alex, #plasma, bruns, broulik
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28473: BookmarksRunner: Early return and formatting

[Stefan Brüns]

bruns added a comment.


  LGTM - @broulik ?

REPOSITORY
  R120 Plasma Workspace

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

To: alex, bruns, broulik, #plasma
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28473: BookmarksRunner: Early return and formatting

[Stefan Brüns]

bruns added inline comments.

INLINE COMMENTS

> firefox.cpp:166
> +if (!QSqlDatabase::isDriverAvailable(QStringLiteral("QSQLITE"))) {
> +qWarning() << "SQLITE driver isn't available";
> +return;

Thanks, though I was wrong about "no categories yet".

  #include "bookmarks_debug"
  ...
  qCWarning(RUNNER_BOOKMARKS) << ...

REPOSITORY
  R120 Plasma Workspace

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

To: alex, bruns, broulik, #plasma
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28473: BookmarksRunner: Early return and formatting

[Stefan Brüns]

bruns added inline comments.

INLINE COMMENTS

> firefox.cpp:198
> -if (profilePath.isEmpty()) {
> -//qDebug() << "No default firefox profile found";
> -return;

Can you leave this in, and even promote it to an active qWarning* - if we have 
a db and profiles file, but can't determine the default profile, something is 
definitely wrong.

- better would be a qCWarning, but we have no categories here yet -> TODO

> firefox.cpp:214
>  } else {
> -//qDebug() << "SQLITE driver isn't available";
>  }

This can IMHO also be promoted to an active warning.

REPOSITORY
  R120 Plasma Workspace

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

To: alex, bruns, broulik, #plasma
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28439: BoorkmarksRunner: Set QIcon instead of Favicon pointer in BookmarkMatch

[Stefan Brüns]

bruns added a comment.


  @davidedmundson - is `QIcon(QImage(...)) `save?
  
  the AppstreamRunner suffers from the same problem, it uses 
`match->setIcon(/*wrapped*/QIcon::fromTheme())`.

REPOSITORY
  R120 Plasma Workspace

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

To: alex, apol, broulik, davidedmundson
Cc: bruns, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28439: BoorkmarksRunner: Set QIcon instead of Favicon pointer in BookmarkMatch

[Stefan Brüns]

bruns added a comment.


  In D28439#638188 , @davidedmundson 
wrote:
  
  > QIcon isn't thread safe.
  
  
  I think this is related to https://phabricator.kde.org/D5889?id=14601 ?
  
  I think we are save here, as the QIcon is created from an image stored on 
disk (absolute path to e.g. PNG), not from a theme.

REPOSITORY
  R120 Plasma Workspace

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

To: alex, apol, broulik, davidedmundson
Cc: bruns, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28439: BoorkmarksRunner: Set QIcon instead of Favicon pointer in BookmarkMatch

[Stefan Brüns]

bruns added a comment.


  In D28439#638216 , @alex wrote:
  
  > In D28439#638188 , 
@davidedmundson wrote:
  >
  > > QIcon isn't thread safe.
  >
  >
  > Sorry, I wasn't aware about that and haven't found anything when I had a 
quick look at the docs before submitting this patch.
  
  
  https://api.kde.org/frameworks/krunner/html/classPlasma_1_1AbstractRunner.html

REPOSITORY
  R120 Plasma Workspace

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

To: alex, apol, broulik, davidedmundson
Cc: bruns, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28196: BUGFIX BookmarksRunner: Fix wrong default profile extraction, remove caching

[Stefan Brüns]

bruns requested changes to this revision.
bruns added a comment.
This revision now requires changes to proceed.


  This should be 3 submission:
  
  1. early return/indentation changes (preparation)
  2. always read default profile from FF config, do not write "dbfile" to 
runner config
  3. fix lookup of default  profile name

REPOSITORY
  R120 Plasma Workspace

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

To: alex, bruns, z3ntu, apol, broulik, davidedmundson, #plasma
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28196: BUGFIX BookmarksRunner: Fix wrong default profile extraction, remove caching

[Stefan Brüns]

bruns added a comment.


  Also keep code lines at < 80 characters

REPOSITORY
  R120 Plasma Workspace

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

To: alex, bruns, z3ntu, apol, broulik, davidedmundson, #plasma
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28196: BUGFIX BookmarksRunner: Fix wrong default profile extraction, remove caching

[Stefan Brüns]

bruns added a comment.


  Regarding the summary:
  
  1. Wrap the lines at <80 characters
  2. Describe the screenshot textually

REPOSITORY
  R120 Plasma Workspace

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

To: alex, bruns, z3ntu, apol, broulik, davidedmundson, #plasma
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28196: BUGFIX BookmarksRunner: Fix wrong default profile extraction, remove caching

[Stefan Brüns]

bruns added a comment.


  Can you split out the early-return part for the QSQLite driver and submit it 
as a preparation patch?
  
  It adds quite some noise to the diff due to the whitespace changes (though I 
fully agree it is the better coding pattern).

REPOSITORY
  R120 Plasma Workspace

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

To: alex, bruns, z3ntu, apol, broulik, davidedmundson, #plasma
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28280: [kcms/baloo] Refine UI and prepare for more indexing options in the future

[Stefan Brüns]

bruns added a comment.


  In D28280#634471 , @ngraham wrote:
  
  > Such a complicated setup is probably not going to be typical, I expect. But 
for that we could have an Advanced Settings sheet that allows people to 
customize things with great detail. However we probably don't have to worry too 
much about that for the purpose of this patch since those advanced features 
haven't been made available to the UI yet.
  
  
  
  
  In D28280#634471 , @ngraham wrote:
  
  > Such a complicated setup is probably not going to be typical, I expect. But 
for that we could have an Advanced Settings sheet that allows people to 
customize things with great detail. However we probably don't have to worry too 
much about that for the purpose of this patch since those advanced features 
haven't been made available to the UI yet.
  
  
  We already have 5 of the 7 options, only "Metadata" is new. Though, currently 
only as a global setting.

REPOSITORY
  R119 Plasma Desktop

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

To: ngraham, #vdg, #baloo, #plasma
Cc: bruns, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28280: [kcms/baloo] Refine UI and prepare for more indexing options in the future

[Stefan Brüns]

bruns added a comment.


  In D28280#634405 , @ngraham wrote:
  
  > In D28280#634388 , @bruns wrote:
  >
  > > Try do imagine how the combobox will look like when you add 'Index 
hidden' to the config options.
  >
  >
  > That would be fine. Comboboxes handle many options pretty well.
  
  
  I am more concerned how a user will handle it:
  
  [Disabled ; Filenames+Tags+Comments ; Metadata(Authors,EXIF,etc) ; FullText] 
x [NotHidden ; All] -> 8 options
  
  > In D28280#634390 , @bruns wrote:
  > 
  >> For the "Add folder setting...":
  >>  How about a config dialog **after** the File Dialog?
  >>
  >> This may seem over the top for now, but the next step is to make the 
excludeFilters and mimetype filters configurale from the KCM, and then also per 
folder.
  > 
  > 
  > Oh, that's interesting! Sounds cool. We can probably add that 
post-file-selection dialog for the indexing settings that require it, but since 
not all will, I'm not sure it should be added for all of them.

REPOSITORY
  R119 Plasma Desktop

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

To: ngraham, #vdg, #baloo, #plasma
Cc: bruns, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28280: [kcms/baloo] Refine UI and prepare for more indexing options in the future

[Stefan Brüns]

bruns added a comment.


  For the "Add folder setting...":
  How about a config dialog **after** the File Dialog?
  
  This may seem over the top for now, but the next step is to make the 
excludeFilters and mimetype filters configurabe from the KCM, and then also per 
folder.

REPOSITORY
  R119 Plasma Desktop

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

To: ngraham, #vdg, #baloo, #plasma
Cc: bruns, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28280: [kcms/baloo] Refine UI and prepare for more indexing options in the future

[Stefan Brüns]

bruns added a comment.


  Try do imagine how the combobox will look like when you add 'Index hidden' to 
the config options.

REPOSITORY
  R119 Plasma Desktop

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

To: ngraham, #vdg, #baloo, #plasma
Cc: bruns, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28025: [Baloo KCM] Complete overhaul of the include/exclude folder list

[Stefan Brüns]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:7498b41a1979: [Baloo KCM] Complete overhaul of the 
include/exclude folder list (authored by bruns).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28025?vs=78333=78404

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

AFFECTED FILES
  kcms/baloo/filteredfoldermodel.cpp
  kcms/baloo/filteredfoldermodel.h
  kcms/baloo/package/contents/ui/main.qml

To: bruns, #baloo, #vdg, ngraham, mart
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28025: [Baloo KCM] Complete overhaul of the include/exclude folder list

[Stefan Brüns]

bruns updated this revision to Diff 78333.
bruns marked 2 inline comments as done.
bruns added a comment.


  update2

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28025?vs=78332=78333

BRANCH
  baloo_config_rework

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

AFFECTED FILES
  kcms/baloo/filteredfoldermodel.cpp
  kcms/baloo/filteredfoldermodel.h
  kcms/baloo/package/contents/ui/main.qml

To: bruns, #baloo, #vdg, ngraham, mart
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28025: [Baloo KCM] Complete overhaul of the include/exclude folder list

[Stefan Brüns]

bruns updated this revision to Diff 78332.
bruns marked 2 inline comments as done.
bruns added a comment.


  update

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28025?vs=78064=78332

BRANCH
  baloo_config_rework

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

AFFECTED FILES
  kcms/baloo/filteredfoldermodel.cpp

To: bruns, #baloo, #vdg, ngraham, mart
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28025: [Baloo KCM] Complete overhaul of the include/exclude folder list

[Stefan Brüns]

bruns marked 2 inline comments as done.
bruns added inline comments.

INLINE COMMENTS

> ngraham wrote in filteredfoldermodel.cpp:36
> this handy little function feels like it wants to be in KCoreAddons or 
> something

Now its likely inlined by the compiler ...

> ngraham wrote in main.qml:112
> Since this is only used once, it could simply be declared inline
> 
>   delegate: Kirigami.SwipeListItem {
>   id: listItem
>   onClicked: {
>   [blabla]
>   }

IMHO it is to large to be declared inline. The toplevel layout is ~90 lines and 
can be viewed on a single screen now.

REPOSITORY
  R119 Plasma Desktop

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

To: bruns, #baloo, #vdg, ngraham, mart
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28208: Move sni icon handling logic from data engine to applet

[Stefan Brüns]

bruns added inline comments.

INLINE COMMENTS

> kmaterka wrote in StatusNotifierItem.qml:90
> Yes, you are correct, my mistake. So `StatusNotifierItemSource` has no margin 
> and `KIconLoader` has 5% of icon size - another inconsistency.
> What about 'always round when multiplying by a non-integer value'? I don't 
> know exactly why, but AFAIK not-rounded number can cause inconsistent 
> behavior in QML.
> @ngraham can you give a word of explanation?

The `0.05 * iconSize` in KIconLoader has an implicit `math.floor` by the int 
conversion.

REPOSITORY
  R120 Plasma Workspace

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

To: davidre, kmaterka, broulik, mart, #plasma, #vdg
Cc: bruns, ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28025: [Baloo KCM] Complete overhaul of the include/exclude folder list

[Stefan Brüns]

bruns added a comment.


  In D28025#631358 , @ngraham wrote:
  
  > If you're okay with me modifying the UI in a follow-up patch, I'll accept 
the UI in its current form. Is that acceptable?
  
  
  Thats the intention of all this - get it in a usable state, and improve it 
gradually.

REPOSITORY
  R119 Plasma Desktop

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

To: bruns, #baloo, #vdg, ngraham, mart
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28025: [Baloo KCM] Complete overhaul of the include/exclude folder list

[Stefan Brüns]

bruns added a comment.


  TODO: T12840 

REPOSITORY
  R119 Plasma Desktop

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

To: bruns, #baloo, #vdg, ngraham, mart
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28025: [Baloo KCM] Complete overhaul of the include/exclude folder list

[Stefan Brüns]

bruns added a comment.


  F8185503: Screenshot_20200320_022339.png 


REPOSITORY
  R119 Plasma Desktop

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

To: bruns, #baloo, #vdg, ngraham, mart
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28025: [Baloo KCM] Complete overhaul of the include/exclude folder list

[Stefan Brüns]

bruns updated this revision to Diff 78064.
bruns added a comment.


  Use Kirigami.Action for Trash

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28025?vs=77574=78064

BRANCH
  baloo_config_rework

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

AFFECTED FILES
  kcms/baloo/filteredfoldermodel.cpp
  kcms/baloo/filteredfoldermodel.h
  kcms/baloo/package/contents/ui/main.qml

To: bruns, #baloo, #vdg, ngraham, mart
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28025: [Baloo KCM] Complete overhaul of the include/exclude folder list

[Stefan Brüns]

bruns added a comment.


  In D28025#630882 , @ngraham wrote:
  
  > In D28025#630849 , @bruns wrote:
  >
  > > Having only inclusion/exclusion is a temporary state. I plan to add more 
settings for each path, so having an "add" button for each possible state will 
no longer be feasible.
  > >
  > > Also, "removing" a path from the "included"  list is not the same as 
excluding it - the state depends on the state of its next ancestor. This exact 
type of mixup has led to the messy state the current KCM is in.
  >
  >
  > Then maybe we should rethink the UI, because that's what it currently 
suggests. What are the other states you're planning to add?
  
  
  Honestly, no, thats not what it suggests. It clearly tells if a folder should 
be searched (magnifying glass) or not (minus sign). Currently a checkbox would 
do, but thats a) not scalable b) non-obvious (does checked equal included or 
excluded)?
  
  I have already mentioned meta-data vs full-text. Also file-name only is often 
wanted.
  
  >> The Items are inspired by the Desktop Effects KCM. I have searched through 
the HIG for considerable time, unfortunately it lacks any specific information 
what to do, or any usable examples. If you can provide any examples where you 
think it is done the "right way (TM)", please go for it.
  > 
  > Yeah, we need to add more examples and better guidance to the HIG for sure. 
However you must be looking at an old version of the Desktop Effects KCM 
because the git master version shows what I'm talking about:  F8185313: 
Screenshot_20200319_161856.png 
  > 
  > You can also look at the Desktop effects KCM, the Activities KCM, or 
Discover's Settings page.
  
  Lets see:
  
  - Desktop Effects KCM:  It has 3 state checkboxes where this clearly violates 
HIG. The "Video" button is a toggle button, but to get its meaning you have to 
hover it for the tooltip. It has a "Get new Desktop Effects" button on the top 
button, which not only installs new effects, but must also be used to 
**uninstall** effects.
  - Activities: "Create New..." on the bottom **right** (HIG violation).
  
  So much for your poster childs ...
  
  Yes, it does use custom buttons. The second one can be trivially replaced by 
a Kirigami.Action (or I could use "flat" style, and it would look and behave 
exactly like a Kirigami.Action, so no "custom button appearance" nor 
"behavior"). The "Enable/Disable indexing" button is custom. It can be 
trivially changed to a Kirigami.Action by moving the text to the tooltip, but 
IMHO thats much worse from a usability view.
  
  >> The search/excluded icons are just the first implemented state column. 
There will be more columns. Having the full state in textual form for each 
entry will look awkward, and having it in textual form only will make it much 
harder to get the current state for a given path.
  >> 
  >> More fine granular settings have been requested several times. Having a 
content indexer run on ~/Downloads poses a security risk. Running full-text 
indexing on ~/Documents/MyCppProjects/ is definitely subject to a users 
preference, while most users would expect ~/Pictures/, ~/Videos/ and ~/Music/ 
to be scanned for metadata.
  >> 
  >> The current model and visual representation are complete nonsense, from a 
programmers as well as a users view. This definitely gets the model in a usable 
and extensible state, and shows the real state to the user (instead of showing 
some invented entries, and leaving out the other real half), and also makes it 
configurable. The important part here is the model. The view/delegate can be 
extended even by some person who is not familiar with baloo internals.
  > 
  > In general I'm not a fan of patches that change both the backend and UI and 
say, "well, we can make a better UI later." Let's do it now, or we might forget 
to do it later, or do the backend bits in a way that make it impossible to do 
the UI in a user-friendly way. It wounds like what we really need here is a 
true multi-column table, like the one in the git master version of the System 
Tray applet.
  
  Thats not what I said. It has a sound model with this change. The model is 
extensible. The model represents the configuration correctly. Based on this 
model, everybody can polish the UI.
  
  If you can provide a better model, a better UI, please do it. The new model 
and UI addresses all the complaints I am aware of.

REPOSITORY
  R119 Plasma Desktop

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

To: bruns, #baloo, #vdg, ngraham, mart
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28025: [Baloo KCM] Complete overhaul of the include/exclude folder list

[Stefan Brüns]

bruns added a comment.


  In D28025#630729 , @ngraham wrote:
  
  > I have some UI suggestions:
  >
  > - Have a button to add an exclusion path as well as a button to add an 
inclusion path, rather than a single Add Setting button, which is a rather 
jargony, programmer-centric way of presenting the feature.
  
  
  Having only inclusion/exclusion is a temporary state. I plan to add more 
settings for each path, so having an "add" button for each possible state will 
no longer be feasible.
  
  Also, "removing" a path from the "included"  list is not the same as 
excluding it - the state depends on the state of its next ancestor. This exact 
type of mixup has led to the messy state the current KCM is in.
  
  > - For consistency, use the typical way of assigning actions to Kirigami 
SwipeListItems, rather than implementing custom button appearance and behavior. 
If you did this because the Kirigami SwipeListItem has no provision to display 
an inline action with text as well as an icon, let's change the component to 
support that.
  
  The Items are inspired by the Desktop Effects KCM. I have searched through 
the HIG for considerable time, unfortunately it lacks any specific information 
what to do, or any usable examples. If you can provide any examples where you 
think it is done the "right way (TM)", please go for it.
  
  > - Instead of having the list item expand when clicked to reveal whether 
it's included or excluded, display that information in textual form in the same 
line, and no need to repeat the same path. This would optionally allow you to 
remove the magnifying glass and minus sign icons.
  
  The search/excluded icons are just the first implemented state column. There 
will be more columns. Having the full state in textual form for each entry will 
look awkward, and having it in textual form only will make it much harder to 
get the current state for a given path.
  
  More fine granular settings have been requested several times. Having a 
content indexer run on ~/Downloads poses a security risk. Running full-text 
indexing on ~/Documents/MyCppProjects/ is definitely subject to a users 
preference, while most users would expect ~/Pictures/, ~/Videos/ and ~/Music/ 
to be scanned for metadata.
  
  The current model and visual representation are complete nonsense, from a 
programmers as well as a users view. This definitely gets the model in a usable 
and extensible state, and shows the real state to the user (instead of showing 
some invented entries, and leaving out the other real half), and also makes it 
configurable. The important part here is the model. The view/delegate can be 
extended even by some person who is not familiar with baloo internals.

REPOSITORY
  R119 Plasma Desktop

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

To: bruns, #baloo, #vdg, ngraham, mart
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28024: [Baloo KCM] Expose "Index hidden" config setting

[Stefan Brüns]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:491ce0f1d78f: [Baloo KCM] Expose Index hidden 
config setting (authored by bruns).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28024?vs=77558=77662

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

AFFECTED FILES
  kcms/baloo/package/contents/ui/main.qml

To: bruns, #baloo, #vdg, ngraham
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28024: [Baloo KCM] Expose "Index hidden" config setting

[Stefan Brüns]

bruns added inline comments.

INLINE COMMENTS

> ngraham wrote in main.qml:59
> Instead adding these new layouts, you could just set the following on each 
> checkbox that needs to be indented:
> 
>   Layout.leftMargin: !LayoutMirroring.enabled ? Kirigami.Units.largeSpacing : > 0
>   Layout.rightMargin: LayoutMirroring.enabled ? Kirigami.Units.largeSpacing : > 0
> 
> IMO that's a bit cleaner.

The effect would be different, as currently there already is some margin.

Also, this has to be repeated for each item, and I plan to add (at least) one 
more option.

REPOSITORY
  R119 Plasma Desktop

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

To: bruns, #baloo, #vdg, ngraham
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28025: [Baloo KCM] Complete overhaul of the include/exclude folder list

[Stefan Brüns]

bruns updated this revision to Diff 77574.
bruns added a comment.


  tabs

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28025?vs=77560=77574

BRANCH
  baloo_config_rework

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

AFFECTED FILES
  kcms/baloo/filteredfoldermodel.cpp
  kcms/baloo/filteredfoldermodel.h
  kcms/baloo/package/contents/ui/main.qml

To: bruns, #baloo, #vdg, ngraham
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28025: [Baloo KCM] Complete overhaul of the include/exclude folder list

[Stefan Brüns]

bruns added inline comments.

INLINE COMMENTS

> main.qml:169
> +id: removeFolder
> + visible: model.deletable
> +icon.name: "user-trash"

Spurious tab

REPOSITORY
  R119 Plasma Desktop

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

To: bruns, #baloo, #vdg, ngraham
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28025: [Baloo KCM] Complete overhaul of the include/exclude folder list

[Stefan Brüns]

bruns added a comment.


  F8174362: Screenshot_20200313_145738.png 

  
  F8174361: Screenshot_20200313_145811.png 


REPOSITORY
  R119 Plasma Desktop

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

To: bruns, #baloo, #vdg, ngraham
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28025: [Baloo KCM] Complete overhaul of the include/exclude folder list

[Stefan Brüns]

bruns created this revision.
bruns added reviewers: Baloo, VDG, ngraham.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
bruns requested review of this revision.

REVISION SUMMARY
  The current "Excluded folders" list in the KCM is quite awkward:
  
  1. It tries to mimic baloos automatic exclusion of external drives, but fails 
doing so and adds almost any external drive even when not mounted below an 
indexable path.
  
  2. Deleting an autogenerated entry actually **adds** it to the included 
folder list, and then hides it.
  
  3. There is no way to show the included folder list, or add any entries to it.
  
  Remove the custom "excluded mounts" heuristic from the KCM and retrieve
  the additional (not explicitly configured) excluded ones from baloo.
  
  Replace the "excluded list" with a common list for included and excluded
  folders, and flag its state. This also makes it easy to add additional
  properties later.
  
  Create a new UI delegate for each config list item, allowing to enable
  and disable indexing for each entry. Make the "delete" actually always
  delete a config entry, and make the control inline. Move the "Add" button
  to the *right* bottom of the list (in accordance with UI guidelines) and
  add some text to it.
  
  Depends on D28024 

REPOSITORY
  R119 Plasma Desktop

BRANCH
  baloo_config_rework

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

AFFECTED FILES
  kcms/baloo/filteredfoldermodel.cpp
  kcms/baloo/filteredfoldermodel.h
  kcms/baloo/package/contents/ui/main.qml

To: bruns, #baloo, #vdg, ngraham
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28024: [Baloo KCM] Expose "Index hidden" config setting

[Stefan Brüns]

bruns created this revision.
bruns added reviewers: Baloo, VDG, ngraham.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
bruns requested review of this revision.

REVISION SUMMARY
  Also inset both "Index file content" and "Index hidden" so it is more
  obvious both are subordinate to the global "Enable indexing".

REPOSITORY
  R119 Plasma Desktop

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

AFFECTED FILES
  kcms/baloo/package/contents/ui/main.qml

To: bruns, #baloo, #vdg, ngraham
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27917: [Baloo] Fix firstRun and config change logic

[Stefan Brüns]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:550cf9cafb19: [Baloo] Fix firstRun and config change 
logic (authored by bruns).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27917?vs=77183=77456

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

AFFECTED FILES
  kcms/baloo/kcm.cpp
  kcms/baloo/kcm.h

To: bruns, #baloo, ngraham, #plasma
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27917: [Baloo] Fix firstRun and config change logic

[Stefan Brüns]

bruns created this revision.
bruns added reviewers: Baloo, ngraham.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
bruns requested review of this revision.

REVISION SUMMARY
  FirstRun should only be set if baloo was *never ever* run before, and
  this case is already handled by baloo internally.
  
  Do no try to do a baloo config refresh if it has just been quit.
  
  Also avoid triggering a config update immediately after starting baloo,
  it will do another walk of the directory tree.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  master

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

AFFECTED FILES
  kcms/baloo/kcm.cpp
  kcms/baloo/kcm.h

To: bruns, #baloo, ngraham
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27636: Remove redundant match for mountpoints

[Stefan Brüns]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:0acbfba3b0e4: Remove redundant match for mountpoints 
(authored by bruns).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27636?vs=76333=76432

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

AFFECTED FILES
  kcms/baloo/filteredfoldermodel.cpp

To: bruns, #baloo, ngraham, davidedmundson
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27636: Remove redundant match for mountpoints

[Stefan Brüns]

bruns created this revision.
bruns added reviewers: Baloo, ngraham, davidedmundson.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
bruns requested review of this revision.

REVISION SUMMARY
  m_excludeList is populated from m_settings->excludeFolders(), so one of
  the checks is redundant.
  
  Remove the first and not the second one, as the first does not use
  normalized pathes and may miss some matches.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  baloo_config_rework

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

AFFECTED FILES
  kcms/baloo/filteredfoldermodel.cpp

To: bruns, #baloo, ngraham, davidedmundson
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27613: Remove bogus mount points check when starting baloo on save

[Stefan Brüns]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:9dc634bdb1ec: Remove bogus mount points check when 
starting baloo on save (authored by bruns).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27613?vs=76254=76332

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

AFFECTED FILES
  kcms/baloo/kcm.cpp
  kcms/baloo/kcm.h

To: bruns, #baloo, ngraham
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27612: Remove unused functions

[Stefan Brüns]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:cd52299862ea: Remove unused functions (authored by bruns).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27612?vs=76252=76331

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

AFFECTED FILES
  kcms/baloo/filteredfoldermodel.cpp
  kcms/baloo/filteredfoldermodel.h

To: bruns, #baloo, ngraham, davidedmundson
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27611: Invert logic of shouldShowMountPoint -> ignoredMountPoint

[Stefan Brüns]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:31e823b6a67a: Invert logic of shouldShowMountPoint - 
ignoredMountPoint (authored by bruns).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27611?vs=76329=76330

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

AFFECTED FILES
  kcms/baloo/filteredfoldermodel.cpp
  kcms/baloo/filteredfoldermodel.h

To: bruns, #baloo, ngraham
Cc: usta, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27611: Invert logic of shouldShowMountPoint -> ignoredMountPoint

[Stefan Brüns]

bruns updated this revision to Diff 76329.
bruns retitled this revision from "Invert logic of shouldShowMountPoint -> 
ignoreMountPoint" to "Invert logic of shouldShowMountPoint -> 
ignoredMountPoint".
bruns edited the summary of this revision.
bruns added a comment.


  Update commit message (I hate Phabricator)

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27611?vs=76253=76329

BRANCH
  master

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

AFFECTED FILES
  kcms/baloo/filteredfoldermodel.cpp
  kcms/baloo/filteredfoldermodel.h

To: bruns, #baloo, ngraham
Cc: usta, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27613: Remove bogus mount points check when starting baloo on save

[Stefan Brüns]

bruns created this revision.
bruns added reviewers: Baloo, ngraham.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
bruns requested review of this revision.

REVISION SUMMARY
  The check stops baloo if non of the included pathes is *currently*
  mounted, i.e. if a device is mounted later baloo is not running and
  can not pick up the device.
  
  Depends on D27612 

REPOSITORY
  R119 Plasma Desktop

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

AFFECTED FILES
  kcms/baloo/kcm.cpp
  kcms/baloo/kcm.h

To: bruns, #baloo, ngraham
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27611: Invert logic of shouldShowMountPoint -> ignoreMountPoint

[Stefan Brüns]

bruns updated this revision to Diff 76253.
bruns added a comment.


  ignoreMountPoint -> ignoredMountPoint
  whitespace

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27611?vs=76251=76253

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

AFFECTED FILES
  kcms/baloo/filteredfoldermodel.cpp
  kcms/baloo/filteredfoldermodel.h

To: bruns, #baloo, ngraham
Cc: usta, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27611: Invert logic of shouldShowMountPoint -> ignoreMountPoint

[Stefan Brüns]

bruns added a comment.


  In D27611#616556 , @usta wrote:
  
  > 2 Question : 
  >  1- isnt that method should be const'ified ?
  
  
  Its not a member function ...

REPOSITORY
  R119 Plasma Desktop

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

To: bruns, #baloo, ngraham
Cc: usta, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27612: Remove unused functions

[Stefan Brüns]

bruns updated this revision to Diff 76252.
bruns added a comment.


  rebase

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27612?vs=76250=76252

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

AFFECTED FILES
  kcms/baloo/filteredfoldermodel.cpp
  kcms/baloo/filteredfoldermodel.h

To: bruns, #baloo, ngraham
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27611: Invert logic of shouldShowMountPoint -> ignoreMountPoint

[Stefan Brüns]

bruns updated this revision to Diff 76251.
bruns added a comment.


  move out of class

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27611?vs=76249=76251

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

AFFECTED FILES
  kcms/baloo/filteredfoldermodel.cpp
  kcms/baloo/filteredfoldermodel.h

To: bruns, #baloo, ngraham
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27612: Remove unused functions

[Stefan Brüns]

bruns created this revision.
bruns added reviewers: Baloo, ngraham.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
bruns requested review of this revision.

REVISION SUMMARY
  showMessage() is only declared, but never defined.
  
  includeFolders() and fetchMountPoint(url) are not used.
  
  Depends on D27611 

REPOSITORY
  R119 Plasma Desktop

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

AFFECTED FILES
  kcms/baloo/filteredfoldermodel.cpp
  kcms/baloo/filteredfoldermodel.h

To: bruns, #baloo, ngraham
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27611: Invert logic of shouldShowMountPoint -> ignoreMountPoint

[Stefan Brüns]

bruns created this revision.
bruns added reviewers: Baloo, ngraham.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
bruns requested review of this revision.

REVISION SUMMARY
  Instead of negating the logic values in several places, just invert
  the logic.

REPOSITORY
  R119 Plasma Desktop

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

AFFECTED FILES
  kcms/baloo/filteredfoldermodel.cpp
  kcms/baloo/filteredfoldermodel.h

To: bruns, #baloo, ngraham
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27579: Remove attempt to start long gone baloo_file_cleaner

[Stefan Brüns]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:616a99165eda: Remove attempt to start long gone 
baloo_file_cleaner (authored by bruns).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27579?vs=76181=76199

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

AFFECTED FILES
  kcms/baloo/kcm.cpp

To: bruns, #baloo, ngraham
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27579: Remove attempt to start long gone baloo_file_cleaner

[Stefan Brüns]

bruns created this revision.
bruns added reviewers: Baloo, ngraham.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
bruns requested review of this revision.

REVISION SUMMARY
  The standalone index cleaner has been replaced almost 5 years by
  functionality implemented in baloo_file.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  baloo_config_rework

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

AFFECTED FILES
  kcms/baloo/kcm.cpp

To: bruns, #baloo, ngraham
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25991: [KFilePlacesModel] Fix supported scheme check for devices

[Stefan Brüns]

bruns added inline comments.

INLINE COMMENTS

> kfileplacesmodel.cpp:758
> +KFilePlacesItem *item = nullptr;
> +if (deviceAvailable) {
> +item = new KFilePlacesItem(bookmarkManager, 
> bookmark.address(), udi);

you have dropped the `allowedHere` check. Probably just `if (!allowedHere) 
continue`.

And for reasons unknown to me, `tags:` is not filtered per app - move the 
allowedHere check to the very beginning of the `while(...)` loop.

> kfileplacesmodel.cpp:759
> +if (deviceAvailable) {
> +item = new KFilePlacesItem(bookmarkManager, 
> bookmark.address(), udi);
> +if (!item->hasSupportedScheme(supportedSchemes)) {

Does KFilePlacesItem have a move constructor? You can do the heap allocation 
then only if necessary.

REPOSITORY
  R120 Plasma Workspace

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

To: broulik, #frameworks, dfaure, ngraham
Cc: bruns, ngraham, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, 
fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25280: [Baloo KCM] Purge index when disabling indexing

[Stefan Brüns]

bruns added a comment.


  Agree with David/Thomas, first we need a "Suspend" button or similar.

REPOSITORY
  R119 Plasma Desktop

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

To: ngraham, tcanabrava, #plasma, #vdg, #baloo
Cc: bruns, cfeck, davidedmundson, plasma-devel, LeGast00n, The-Feren-OS-Dev, 
jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D23112: Add a event Spy for GtkFileChooser recent files

[Stefan Brüns]

bruns added inline comments.

INLINE COMMENTS

> ivan wrote in GtkEventSpy.cpp:126
> No need to compare chars with strings:
> 
>   if (!exec.isEmpty() && exec[0] == '\'' && exec.back() == '\'')

missing `QChar('\'')`, otherwise it breaks with QT_NO_CAST_FROM_ASCII

-> `if (exec.startsWith(QChar('\'') && exec.endsWith(QChar('\'')) {...`

> GtkEventSpy.cpp:144
> +// remove space and any caracter after
> +const int spaceIdex = exec.indexOf(" ");
> +if (spaceIdex != -1) {

`spaceI_n_dex`

REPOSITORY
  R161 KActivity Manager Service

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

To: meven, #frameworks, ivan
Cc: bruns, ngraham, broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, 
jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D22333: Move Solid::Device::listFromQuery calls to a separate thread

[Stefan Brüns]

bruns added a comment.


  In D22333#496544 , @davidedmundson 
wrote:
  
  > Kinda, you're summary missing a key part.
  >  (the commit description is a bit poor)
  >
  > This patch does the following:
  >
  > - It does the search in the other thread. That creates and iterates every 
possible device. This is expensive as there are lots of potential devices. We 
fetch pretty much everything udisks knows.
  > - From the given list we recreate the select few devices that are relevant. 
This is relatively cheap. It does still block as you point out, but now for a 
much much smaller set of objects compared to before.
  >
  >   (on my laptop that's 56 devices created when searching, resulting in 8 
useful devices).
  
  
  Just for clarification:
  
  On my system, `solid-hardware5 list` returns 77 devices, but with the 
Predicate Filter I am down to 10, and this is filtered by the hotplug engine 
down to 7.
  
  Half of the devices (32) are serial ports. These are returned by the udev 
backend, but when combined with the "Camera | PortableMediaPlayer" predicate, 
the udev backend returns an empty list and nothing is instantiated.
  
  The enumeration in the udev backend, especially 'PortableMediaPlayer' has 
become much cheaper since https://phabricator.kde.org/D21379.
  
  The Udisks backend indeed returns to much, and unfortunately it creates a 
backend device for each block device, which implies a number of DBus 
roundtrips. But this would come down to a single DBus call when D19677 
 lands.

REPOSITORY
  R120 Plasma Workspace

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

To: apol, #plasma, davidedmundson, bruns
Cc: anthonyfieroni, bruns, plasma-devel, LeGast00n, jraleigh, fbampaloukas, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D22333: Move Solid::Device::listFromQuery calls to a separate thread

[Stefan Brüns]

bruns added a comment.


  In D22333#494852 , @anthonyfieroni 
wrote:
  
  > 
https://phabricator.kde.org/source/solid/browse/master/src/solid/devices/frontend/devicemanager.cpp$301
  >  @bruns, It's backend per thread
  
  
  Yes, you are correct, so there are no threading issues, sorry.
  
  On the other hand, this IMHO makes this approach even less profitable:
  
  1. every backend is initialized once from the two helpers (assuming both 
dataengines are used).
  2. after the threads finish, all but the relevant UDIs are thrown away (and 
care must be taken not to use anything else - the devices would be dangling 
pointers in the now gone thread storage)
  3. the first completing thread triggers the initialization of the backends 
again - in the main thread.
  
  As the initialization happens implicitly by the Device instantiation, for 
e.g. UDisks there are two synchronous DBus calls now, one for introspection 
from the manager backend and one for the device itself from the device backend. 
Both are done in the main thread, and we will block even longer.
  
  So any gain here is AFAICS from the fact the constructor no longer blocks on 
a DBus call.

REPOSITORY
  R120 Plasma Workspace

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

To: apol, #plasma, davidedmundson, bruns
Cc: anthonyfieroni, bruns, plasma-devel, LeGast00n, jraleigh, fbampaloukas, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D22333: Move Solid::Device::listFromQuery calls to a separate thread

[Stefan Brüns]

bruns added a comment.


  In D22333#494810 , @apol wrote:
  
  > In D22333#494774 , @bruns wrote:
  >
  > > Also, the code is calling non-threadsafe code from multiple threads now 
(e.g. once from each the two dataengines helper threads). Each one will call 
the udisks2 `Manager::deviceCache()` method.
  >
  >
  > There's a Backend per-thread and deviceCache is not static so each thread 
will call its own.
  
  
  Sorry, but you are wrong, look for
  `Q_GLOBAL_STATIC(Solid::DeviceManagerStorage, globalDeviceStorage)`

REPOSITORY
  R120 Plasma Workspace

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

To: apol, #plasma, davidedmundson, bruns
Cc: bruns, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D22333: Move Solid::Device::listFromQuery calls to a separate thread

[Stefan Brüns]

bruns added a comment.


  In D22333#494415 , @apol wrote:
  
  > In D22333#494389 , @bruns wrote:
  >
  > > Again, where is it blocking? Which backend?
  >
  >
  > udisks2 mainly, but every backend can block by its virtue.
  
  
  The code can still block, as every invocation of 'Solid::Device(udi)` does a 
blocking DBus call to udisks2.
  
  Also, the code is calling non-threadsafe code from multiple threads now (e.g. 
once from each the two dataengines helper threads). Each one will call the 
udisks2 `Manager::deviceCache()` method.

REPOSITORY
  R120 Plasma Workspace

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

To: apol, #plasma, davidedmundson, bruns
Cc: bruns, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D22333: Move Solid::Device::listFromQuery calls to a separate thread

[Stefan Brüns]

bruns added a comment.


  In D22333#494384 , @apol wrote:
  
  > In D22333#494253 , @bruns wrote:
  >
  > > In D22333#494152 , @apol wrote:
  > >
  > > > In D22333#494146 , @bruns 
wrote:
  > > >
  > > > > Why not just a singleshot timer from the constructor? Avoids any 
initial blocking ...
  > > >
  > > >
  > > > Initial, but doesn't fix the problem. We could potentially delay this 
few seconds instead, but we'd still be getting an odd ~500ms freeze randomly
  > > >  Or we can just decide to move blocking or heavy algorithms to separate 
threads and enjoy our multicore computers and a fluid experience.
  > >
  > >
  > > Where does the blocking happen? How do you guarantee none of the later 
call block? Large parts of the code are executed in the main thread anyway, 
only the initial list creation happens in a worker thread.
  >
  >
  > listFromQuery is the big blocking call we are moving into a separate thread.
  
  
  Again, where is it blocking? Which backend?
  
  >> Also, if this is a generic problem, why only fix it in the dataengines, 
not in Solid itself?
  > 
  > This would mean refactoring how Solid works and even coming up with 
entirely new concepts. I don't discard doing it at some point but I don't think 
plasmashell startup blockage is reason enough to redo a framework that has been 
working for about 5 years as is.
  
  listFromQuery can be replaced by an asynchronous "enumerate(predicate)" call 
which uses the existing DeviceAdded signal. This would also remove the inherent 
race between the listFromQuery and DeviceAdded/DeviceRemoved.

REPOSITORY
  R120 Plasma Workspace

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

To: apol, #plasma, davidedmundson, bruns
Cc: bruns, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D22333: Move Solid::Device::listFromQuery calls to a separate thread

[Stefan Brüns]

bruns added a comment.


  In D22333#494152 , @apol wrote:
  
  > In D22333#494146 , @bruns wrote:
  >
  > > Why not just a singleshot timer from the constructor? Avoids any initial 
blocking ...
  >
  >
  > Initial, but doesn't fix the problem. We could potentially delay this few 
seconds instead, but we'd still be getting an odd ~500ms freeze randomly
  >  Or we can just decide to move blocking or heavy algorithms to separate 
threads and enjoy our multicore computers and a fluid experience.
  
  
  Where does the blocking happen? How do you guarantee none of the later call 
block? Large parts of the code are executed in the main thread anyway, only the 
initial list creation happens in a worker thread.
  
  Also, if this is a generic problem, why only fix it in the dataengines, not 
in Solid itself?

REPOSITORY
  R120 Plasma Workspace

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

To: apol, #plasma, davidedmundson, bruns
Cc: bruns, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D22333: Move Solid::Device::listFromQuery calls to a separate thread

[Stefan Brüns]

bruns requested changes to this revision.
bruns added a comment.
This revision now requires changes to proceed.


  Why not just a singleshot timer from the constructor? Avoids any initial 
blocking ...

REPOSITORY
  R120 Plasma Workspace

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

To: apol, #plasma, davidedmundson, bruns
Cc: bruns, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D22296: [ksmserver] Remove obsolete KDELibs4Support dependency

[Stefan Brüns]

This revision was automatically updated to reflect the committed changes.
Closed by commit R120:7fa46fa74987: [ksmserver] Remove obsolete KDELibs4Support 
dependency (authored by bruns).

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22296?vs=61475=61476

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

AFFECTED FILES
  ksmserver/CMakeLists.txt
  ksmserver/logout.cpp
  ksmserver/server.h

To: bruns, #plasma, broulik, davidedmundson
Cc: apol, sitter, anthonyfieroni, plasma-devel, LeGast00n, jraleigh, 
fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, mart

D22296: [ksmserver] Remove obsolete KDELibs4Support dependency

[Stefan Brüns]

bruns updated this revision to Diff 61475.
bruns marked an inline comment as done.
bruns retitled this revision from "Port KSMServer to Solid::Power, drop 
KDELibs4Support requirement" to "[ksmserver] Remove obsolete KDELibs4Support 
dependency".
bruns edited the summary of this revision.
bruns removed subscribers: anthonyfieroni, sitter, apol.
bruns added a comment.


  remove inhibition

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22296?vs=61249=61475

BRANCH
  ksmserver_no_kdelibs4support

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

AFFECTED FILES
  ksmserver/CMakeLists.txt
  ksmserver/logout.cpp
  ksmserver/server.h

To: bruns, #plasma, broulik, davidedmundson
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart, anthonyfieroni, sitter

D22296: Port KSMServer to Solid::Power, drop KDELibs4Support requirement

[Stefan Brüns]

bruns marked an inline comment as done.
bruns added inline comments.

INLINE COMMENTS

> anthonyfieroni wrote in logout.cpp:399-400
> Make job autodelete if not.

This is no KJob

REPOSITORY
  R120 Plasma Workspace

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

To: bruns, #plasma, broulik, davidedmundson
Cc: anthonyfieroni, sitter, apol, plasma-devel, LeGast00n, jraleigh, 
fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, mart

D22296: Port KSMServer to Solid::Power, drop KDELibs4Support requirement

[Stefan Brüns]

bruns added a comment.


  In D22296#492147 , @sitter wrote:
  
  >
  
  
  
  
  > So, in order to port to the API, it'd first need finishing up really.
  
  The question is - do we really want this call here, or not. As is, it is just 
an expensive noop.
  
  **Iff** we want the inhibition, I see 3 possibilities:
  
  1. Teach powerdevil to behave correctly:
- do the inhibition immediately
- keep running until all proxied inhibitions are released (maybe this is 
inversed - do not kill powerdevil from ksmserver).
- somehow interact with powerdevil from ksmserver
  2. Take an inhibitor lock via DBus (i.e. call 
`org.freedesktop.login1.Inhibit`) directly
  3. Take an inhibitor lock using `Solid::Power::inhibit`
  
  (3.) is nothing more than a thin wrapper around (2.) see 
https://github.com/KDE/solid/blob/master/src/solid/power/backends/freedesktop/fdinhibition.cpp
  Although Solid::Power would give some abstraction, the reality is:
  
  - only blocking inhibitions are supported
  - only implementation is `org.freedesktop.login1`
  - Solid::Power is very incomplete, e.g. the `statesJob` is just a stub.
  
  My preference is (0.) or (2.).

REPOSITORY
  R120 Plasma Workspace

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

To: bruns, #plasma, broulik, davidedmundson
Cc: sitter, apol, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, mart

D22312: [DeviceNotifications] Keep last message for each device

[Stefan Brüns]

This revision was automatically updated to reflect the committed changes.
Closed by commit R120:bf811e448c76: [DeviceNotifications] Keep last message for 
each device (authored by bruns).

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22312?vs=61292=61294

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

AFFECTED FILES
  dataengines/devicenotifications/ksolidnotify.cpp

To: bruns, #plasma, broulik, ngraham
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22312: [DeviceNotifications] Keep last message for each device

[Stefan Brüns]

bruns created this revision.
bruns added reviewers: Plasma, broulik, ngraham.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
bruns requested review of this revision.

REVISION SUMMARY
  Ejecting e.g. a USB stick removes it from the bus (bus power is removed),
  but the last notification should be kept, otherwise a message like
  "The device can be savely removed" immediately disappears.
  
  Unfortunately it is almost impossible to tell when a device has been
  physically removed (only indirectly, when a new device is plugged into
  the same port), so allow some small leakage - in case the same device is
  later replugged, no leakage happens.

TEST PLAN
  Card reader:
  1.insert medium, mount, unmount
  
-> message "Device can be savely removed" is shown in device notifier
  
  for 5 seconds
  
  2. eject medium, reinsert -> device appears without message and with default 
label

REPOSITORY
  R120 Plasma Workspace

BRANCH
  cleanup_devicenotifier

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

AFFECTED FILES
  dataengines/devicenotifications/ksolidnotify.cpp

To: bruns, #plasma, broulik, ngraham
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22296: Port KSMServer to Solid::Power, drop KDELibs4Support requirement

[Stefan Brüns]

bruns added a comment.


  After having looked into the callchain again, I am not sure what the best 
action here is:
  
  The old code did a DBus call of org.freedesktop.PowerManagement.Inhibit, 
which is implemented by powerdevil. Powerdevil only "schedules" the inhibition, 
but delays it for 5 seconds:
  
https://github.com/KDE/powerdevil/blob/master/daemon/powerdevilpolicyagent.cpp#L521
  
  ... but closing the session triggers a shutdown of powerdevil, i.e. the 
inhibition was not enforced at all (we only did a pointless synchronous DBus 
call to powerdevil).
  
  What about just removing the code?

REPOSITORY
  R120 Plasma Workspace

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

To: bruns, #plasma, broulik, davidedmundson
Cc: apol, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, mart

D22296: Port KSMServer to Solid::Power, drop KDELibs4Support requirement

[Stefan Brüns]

bruns created this revision.
bruns added reviewers: Plasma, broulik, davidedmundson.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
bruns requested review of this revision.

REVISION SUMMARY
  As KDELibs4Support pulled in a number of Frameworks, we have to
  explicitly add these now.

REPOSITORY
  R120 Plasma Workspace

BRANCH
  ksmserver_no_kdelibs4support

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

AFFECTED FILES
  ksmserver/CMakeLists.txt
  ksmserver/logout.cpp
  ksmserver/server.h

To: bruns, #plasma, broulik, davidedmundson
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D21414: Allow 'Exclude folders' section in Baloo KCM to fill window

[Stefan Brüns]

bruns added a comment.


  The code change looks sane for me, although I haven't tested it.
  
  The summary should mention:
  
  - what is currently wrong (space consumed by spacer)
  - what was done to fix it (spacer removed, so the folder list can stretch)
  
  Can you add a before/after screenshot here (the one from the BR is fine for 
before)?
  
  The config layout was imported (more or less) as is 4 years ago, maybe 
inherited from nepomuk. Hard to tell if the spacer ever had a purpose.

REPOSITORY
  R119 Plasma Desktop

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

To: kishoreg, #baloo, #vdg, ngraham
Cc: bruns, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D19796: [Device Notifier] Restore Solid notification messages

[Stefan Brüns]

bruns requested changes to this revision.
bruns added a comment.
This revision now requires changes to proceed.


  This is just much to complicated, adding layer over layer does not help.
  
  Also, the second problem is unrelated to the change you mention - the 
"hasMessage" property could only be true for one DeviceItem, before and after.

INLINE COMMENTS

> DeviceItem.qml:50
>  
> -readonly property bool hasMessage: statusSource.lastUdi == udi && 
> statusSource.data[statusSource.last] ? true : false
> -readonly property var message: hasMessage ? 
> statusSource.data[statusSource.last] || ({}) : ({})
> +readonly property bool hasMessage: statusSource.trigger && 
> statusSource.lastMessages[udi]
> +readonly property var message: hasMessage ? 
> statusSource.lastMessages[udi] : ({})

trigger is essentially always true

> DeviceItem.qml:125
>  // otherwise the last message will show again when this device 
> reappears
> -statusSource.clearMessage()
> +statusSource.clearMessage(udi)
>  

the item should not mess with the internals of the statusSource

> FullRepresentation.qml:108
>  if (!plasmoid.expanded) {
> -statusSource.clearMessage()
> +statusSource.clearMessages()
>  }

This is IMHO wrong, if the plasmoid shows an error message and I happen to 
close it by clicking elsewhere, I can no longer see the error message, although 
it still applies

> FullRepresentation.qml:181
>  state: sdSource.data[udi] ? sdSource.data[udi].State : 0
> -isRoot: sdSource.data[udi]["File Path"] === "/"
> +isRoot: sdSource.data[udi] ? sdSource.data[udi]["File Path"] === 
> "/" : false
>  

This is an unrelated change, and fixes an error introduced in 
https://phabricator.kde.org/R120:d1a5507bd57aa74c18392354dcd43b65e15ee491

> devicenotifier.qml:218
> +// Source is formatted as follows: " notification"
> +var udi = sources[i].split(' ')[0]
> +if (data[sources[i]].error.length > 0) {

would be much simpler to use `udi = data[sources[i]].udi`

REPOSITORY
  R120 Plasma Workspace

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

To: thsurrel, #plasma, broulik, bruns
Cc: anthonyfieroni, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D20383: [KRunner] Avoid writing history if addToHistory is idempotent

[Stefan Brüns]

This revision was automatically updated to reflect the committed changes.
Closed by commit R120:9686c05c7226: [KRunner] Avoid writing history if 
addToHistory is idempotent (authored by bruns).

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20383?vs=55793=55815

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

AFFECTED FILES
  krunner/view.cpp

To: bruns, #plasma, broulik, apol
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D20382: [KRunner] Move trivial check in addToHistory to the front

[Stefan Brüns]

This revision was automatically updated to reflect the committed changes.
Closed by commit R120:4a5f62fee952: [KRunner] Move trivial check in 
addToHistory to the front (authored by bruns).

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20382?vs=55747=55814

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

AFFECTED FILES
  krunner/view.cpp

To: bruns, #plasma, broulik, apol
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D20383: [KRunner] Avoid writing history if addToHistory is idempotent

[Stefan Brüns]

bruns updated this revision to Diff 55793.
bruns added a comment.


  use const

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20383?vs=55748=55793

BRANCH
  arcpatch-D20383

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

AFFECTED FILES
  krunner/view.cpp

To: bruns, #plasma, broulik, apol
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D20383: [KRunner] Avoid writing history if addToHistory is idempotent

[Stefan Brüns]

bruns created this revision.
bruns added reviewers: Plasma, broulik, apol.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
bruns requested review of this revision.

REVISION SUMMARY
  It is not uncommon to run the same command repeatedly. In this case,
  the history is actually unchanged - the item is removed from the first
  position, and prepended again.

TEST PLAN
  run the same command twice, config file is not rewritten
  run a new command, config is updated

REPOSITORY
  R120 Plasma Workspace

BRANCH
  krunner_history

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

AFFECTED FILES
  krunner/view.cpp

To: bruns, #plasma, broulik, apol
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D20382: [KRunner] Move trivial check in addToHistory to the front

[Stefan Brüns]

bruns created this revision.
bruns added reviewers: Plasma, broulik, apol.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
bruns requested review of this revision.

REVISION SUMMARY
  No functional change.

TEST PLAN
  compiles, runs

REPOSITORY
  R120 Plasma Workspace

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

AFFECTED FILES
  krunner/view.cpp

To: bruns, #plasma, broulik, apol
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D19823: Port to the new install directory for knsrc files

[Stefan Brüns]

bruns added a comment.


  Now plasma-workspace depends on KNewStuff 5.57

REPOSITORY
  R120 Plasma Workspace

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

To: apol, ngraham
Cc: bruns, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D19963: Avoid serializing base64 encoded favicon data twice

[Stefan Brüns]

This revision was automatically updated to reflect the committed changes.
Closed by commit R856:c3611dc98170: Avoid serializing base64 encoded favicon 
data twice (authored by bruns).

REPOSITORY
  R856 Plasma Browser Integration

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19963?vs=54527=55509

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

AFFECTED FILES
  extension/extension.js

To: bruns, #plasma, broulik
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D19963: Avoid serializing base64 encoded favicon data twice

[Stefan Brüns]

bruns added a comment.


  Ping!

REPOSITORY
  R856 Plasma Browser Integration

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

To: bruns, #plasma, broulik
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D19963: Avoid serializing base64 encoded favicon data twice

[Stefan Brüns]

bruns added a comment.


  Ping!

REPOSITORY
  R856 Plasma Browser Integration

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

To: bruns, #plasma, broulik
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D19963: Avoid serializing base64 encoded favicon data twice

[Stefan Brüns]

bruns created this revision.
bruns added reviewers: Plasma, broulik.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
bruns requested review of this revision.

REVISION SUMMARY
  The URL is ignored in case the image data is available. Avoid pushing
  it twice via DBus.

TEST PLAN
  1. start dbus-monitor
  2. type some string in krunner
  
  before: dict entries contain both favIconData and favIconUrl, with
  
identical data
  
  after:  only favIconData is part of the dict
  
  Favicons are still shown in the krunner results

REPOSITORY
  R856 Plasma Browser Integration

BRANCH
  favicondata

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

AFFECTED FILES
  extension/extension.js

To: bruns, #plasma, broulik
Cc: plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18890: Add button to reset index database and repair Baloo crashing

[Stefan Brüns]

bruns added a comment.


  In D18890#410137 , @davidedmundson 
wrote:
  
  > If we know the DB is corrupted, and it's just a cache, why do we need a 
user facing button?
  
  
  There are many possible ways of corruption:
  
  - non-decodable values
  - entries with dangling parent IDs
  - IDs of no longer existing documents
  - 
  
  These have different reasons, not all reasons are known and understood, not 
all corruptions are critical.
  
  There are issues with races in the file system, where items are 
deleted/created/modified while these are added to the DB. Unfortunately, Qt 
offers no way to handle it, the only save method is to use the 
openat/fstatat/... functions. This is possible to fix, but until then, rinse 
and repeat.
  
  The non-decodable values are hard to handle, this would at least require 
scrubbing the whole DB or building it from scratch. But until it is known what 
causes this corruption, we end up in doing it again and again.
  
  Some of these are quite safe to just ignore, but some parts a lacking sanity 
checks. On the other hand, D12336  is 
waiting for review for 10 months ...

REPOSITORY
  R119 Plasma Desktop

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

To: guoyunhe, #plasma, #baloo
Cc: mart, davidedmundson, bruns, ngraham, plasma-devel, jraleigh, GB_2, 
ragreen, Pitel, domson, ashaposhnikov, astippich, spoorun, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, abrahams, sebas, apol

D18890: Add button to reset index database and repair Baloo crashing

[Stefan Brüns]

bruns added inline comments.

INLINE COMMENTS

> kcm.cpp:197
> +const QString rm = QStandardPaths::findExecutable(QStringLiteral("rm"));
> +const QString balooctl = 
> QStandardPaths::findExecutable(QStringLiteral("balooctl"));
> +const QString baloofile = 
> QStandardPaths::findExecutable(QStringLiteral("baloo_file"));

IMHO this whole block should be a single, blocking command in balooctl, i.e. 
move the whole logic to balooctl.

> kcm.cpp:210
> +
> +// "balooctl start" isn't reliable, so here we run "baloo_file" directly
> +QProcess::startDetached(baloofile, QStringList());

Care to elaborate?

REPOSITORY
  R119 Plasma Desktop

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

To: guoyunhe, #plasma, #baloo
Cc: bruns, ngraham, plasma-devel, jraleigh, GB_2, ragreen, Pitel, 
ashaposhnikov, astippich, spoorun, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, abrahams, sebas, apol, mart

D18890: Add button to reset index database and repair Baloo crashing

[Stefan Brüns]

bruns added a comment.


  In D18890#409359 , @ngraham wrote:
  
  > Thanks! This is pretty good as-is, and I can confirm that it works just 
fine. However once the user presses the button, there's no further feedback, 
which could encourage them to repeatedly press it again--not a good idea. Maybe 
while the initial index is being generated, we could display a progress spinner 
(and a label too) and disable the button. What do you think?
  
  
  No, please no progress dialog, a full rebuild can take hours ...

REPOSITORY
  R119 Plasma Desktop

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

To: guoyunhe, #plasma, #baloo
Cc: bruns, ngraham, plasma-devel, jraleigh, GB_2, ragreen, Pitel, 
ashaposhnikov, astippich, spoorun, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, abrahams, sebas, apol, mart

[kio-extras] [Bug 329687] previewing an HTML file from a local file system causes network retrievals while generating the thumbnail

[Stefan Brüns]

https://bugs.kde.org/show_bug.cgi?id=329687

Stefan Brüns  changed:

   What|Removed |Added

 CC||stefan.bruens@rwth-aachen.d
   ||e
 Status|REPORTED|RESOLVED
 Resolution|--- |FIXED

--- Comment #2 from Stefan Brüns  ---
The HTML thumbnailer has been removed completely:
https://phabricator.kde.org/D15095

-- 
You are receiving this mail because:
You are the assignee for the bug.

D16410: [Bookmarks Runner] Avoid leaking FetchSqlite instances

[Stefan Brüns]

This revision was automatically updated to reflect the committed changes.
Closed by commit R120:9e90fdd2a593: [Bookmarks Runner] Avoid leaking 
FetchSqlite instances (authored by bruns).

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16410?vs=44170=45490

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

AFFECTED FILES
  runners/bookmarks/browsers/firefox.cpp

To: bruns, #frameworks, mart
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16409: [Bookmarks Runner] Open database connection in the query thread

[Stefan Brüns]

This revision was automatically updated to reflect the committed changes.
Closed by commit R120:145caeac1050: [Bookmarks Runner] Open database connection 
in the query thread (authored by bruns).

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16409?vs=44169=45489

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

AFFECTED FILES
  runners/bookmarks/fetchsqlite.cpp
  runners/bookmarks/fetchsqlite.h

To: bruns, #frameworks, mart
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16405: Add debug category for bookmarks runner

[Stefan Brüns]

This revision was automatically updated to reflect the committed changes.
Closed by commit R120:62804f6becb9: Add debug category for bookmarks runner 
(authored by bruns).

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16405?vs=44164=45352

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

AFFECTED FILES
  plasma-workspace.categories
  runners/bookmarks/CMakeLists.txt

To: bruns, #frameworks, davidedmundson
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16404: [Bookmarks Runner] Cleanup tests CMakeList

[Stefan Brüns]

This revision was automatically updated to reflect the committed changes.
Closed by commit R120:5ba0a08b8312: [Bookmarks Runner] Cleanup tests CMakeList 
(authored by bruns).

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16404?vs=44161=45351

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

AFFECTED FILES
  runners/bookmarks/CMakeLists.txt
  runners/bookmarks/tests/CMakeLists.txt

To: bruns, #frameworks, mart
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16530: Drop X11 core fonts code from startkde/startplasma

[Stefan Brüns]

This revision was automatically updated to reflect the committed changes.
Closed by commit R120:5374513266ee: Drop X11 core fonts code from 
startkde/startplasma (authored by bruns).

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16530?vs=44499=45152

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

AFFECTED FILES
  startkde/startkde.cmake
  startkde/startplasma.cmake

To: bruns, #plasma, fvogt, davidedmundson
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16404: [Bookmarks Runner] Cleanup tests CMakeList

[Stefan Brüns]

bruns added a comment.


  Anyone?

REPOSITORY
  R120 Plasma Workspace

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

To: bruns, #frameworks
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16530: Drop X11 core fonts code from startkde/startplasma

[Stefan Brüns]

bruns added a comment.


  In D16530#355078 , @fvogt wrote:
  
  > Looks good to me, but I don't know enough about old X11 stuff like this to 
accept it.
  
  
  @fvogt - maybe you can poke a coworker?
  
  Someone else?

REPOSITORY
  R120 Plasma Workspace

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

To: bruns, #plasma, fvogt, davidedmundson
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16530: Drop X11 core fonts code from startkde/startplasma

[Stefan Brüns]

bruns added a comment.


  Friendly reminder ...

REPOSITORY
  R120 Plasma Workspace

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

To: bruns, #plasma, fvogt, davidedmundson
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16404: [Bookmarks Runner] Cleanup tests CMakeList

[Stefan Brüns]

bruns added a comment.


  Ping!

REPOSITORY
  R120 Plasma Workspace

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

To: bruns, #frameworks
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16530: Drop X11 core fonts code from startkde/startplasma

[Stefan Brüns]

bruns created this revision.
bruns added reviewers: Plasma, fvogt.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
bruns requested review of this revision.

REVISION SUMMARY
  Qt and GTK based applications have switched to using fontconfig for fonts
  exclusively, so this code is mostly unused.
  
  As for the remaining legacy X11 applications:
  
  - mkfontdir only supports bitmap (e.g. PCF, BDF) fonts. For e.g. truetype 
fonts, a mkfontscale call would be required. Lack of complaints hints this is 
not used at all.
  - the system dirs (sys_odir, sys_fdir) use KDEDIR(S), which is deprecated. 
The dirs do not exist, thus the fontpath is unchanged.
  - system dirs are typically allready included in the XServer font path
  
  TLDR: (scalable) fonts in user dirs are not picked up due to missing
  mkfontscale calls, and system dirs are already included.
  
  This removes the dependency on xset and mkfontdir. The latter is important,
  as e.g. on openSUSE the availability of mkfontdir triggers costly, extra
  setup steps for X Core fonts on package installation.

REPOSITORY
  R120 Plasma Workspace

BRANCH
  drop_x_core_font_support

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

AFFECTED FILES
  startkde/startkde.cmake
  startkde/startplasma.cmake

To: bruns, #plasma, fvogt
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D12498: Fully remove `Application Name` from Details panel

[Stefan Brüns]

This revision was automatically updated to reflect the committed changes.
Closed by commit R121:eb9c4c080427: Fully remove `Application Name` from 
Details panel (authored by sharvey, committed by bruns).

REPOSITORY
  R121 Policykit (Polkit) KDE Agent

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12498?vs=44184=44206

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

AFFECTED FILES
  AuthDialog.cpp
  AuthDialog.h
  authdetails.ui

To: bruns, ngraham, davidedmundson, sharvey
Cc: davidedmundson, bruns, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D16417: Improve replacement text when action description is not provided

[Stefan Brüns]

This revision was automatically updated to reflect the committed changes.
Closed by commit R121:9cf71f424950: Improve replacement text when action 
description is not provided (authored by bruns).

REPOSITORY
  R121 Policykit (Polkit) KDE Agent

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16417?vs=44183=44205

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

AFFECTED FILES
  AuthDialog.cpp

To: bruns, #plasma, ngraham, davidedmundson
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16417: Improve replacement text when action description is not provided

[Stefan Brüns]

bruns added inline comments.

INLINE COMMENTS

> ngraham wrote in AuthDialog.cpp:359
> I would prefer "not available" over "not provided", but I won't choose this 
> as my hill to die on. :)
> 
> Do we actually need to repeat the word 'Description' here?

Btw, "not provided" is your own proposal ;-)

REPOSITORY
  R121 Policykit (Polkit) KDE Agent

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

To: bruns, #plasma, ngraham, davidedmundson
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15885: Clean UI file for `details` portion of UI

[Stefan Brüns]

bruns abandoned this revision.
bruns added a comment.


  Folded back into D12498 

REPOSITORY
  R121 Policykit (Polkit) KDE Agent

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

To: bruns, ngraham, davidedmundson, sharvey
Cc: bruns, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart