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

2020-03-25 Thread Nathaniel Graham
ngraham added a comment.


  UI refinement is here: D28280 

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

2020-03-24 Thread 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&id=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

2020-03-23 Thread 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&id=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

2020-03-23 Thread 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&id=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

2020-03-23 Thread 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


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

2020-03-23 Thread Nathaniel Graham
ngraham added a comment.


  Ok, will do. I just have a few comments about the backend bits, just minor 
stuff. Overall this is really good! I plan to polish the UI after this lands, 
but left comments on the front-end component anyway so you can sharpen your QML 
skills (which are quite good already). :) Feel free to consider them more 
educational than actionable. :)

INLINE COMMENTS

> filteredfoldermodel.cpp:36
>  namespace {
> +QString normalizeTrailingSlashes(QString&& input) {
> +if (!input.endsWith('/'))

this handy little function feels like it wants to be in KCoreAddons or something

> filteredfoldermodel.cpp:47
> +if (!str.endsWith('/'))
> +str += QLatin1Char('/');
>  }

Could probably just call `normalizeTrailingSlashes()` here

> filteredfoldermodel.cpp:83
> +if (displayName.startsWith(homePath))
> +displayName.replace(0, homePath.length(), QStringLiteral("~/"));
>  

use braces for single-line if statements here, as done immediately below

> filteredfoldermodel.cpp:208
> +auto excluded = addTrailingSlashes(m_settings->excludedFolders());
> +auto included = addTrailingSlashes(m_settings->folders());
> +if (entry.enableIndex) {

There's a lot of use of `auto`  in this file for simple `QStringList` objects 
where I think it would be better to just declare the type

> main.qml:100
>  
>  RowLayout {
> +Layout.alignment: Qt.AlignRight

Since this layout now only has one item in it, there's no longer a need for a 
layout at all

> main.qml:112
> +
> +Component {
> +id: directoryConfigDelegate

Since this is only used once, it could simply be declared inline

  delegate: Kirigami.SwipeListItem {
  id: listItem
  onClicked: {
  [blabla]
  }

> main.qml:122
> +
> +RowLayout {
> +Kirigami.Icon {

set spacing: units.smallSpacing on the layout instead of adding an empty item 
between objects for spacing

> main.qml:133
> +ColumnLayout {
> +RowLayout {
> +Kirigami.Icon {

ditto

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

2020-03-20 Thread 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

2020-03-20 Thread 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

2020-03-20 Thread Nathaniel Graham
ngraham added a comment.


  Yes, it's true that our UIs are frequently inconsistent; that's why the 
#consistency  goal exists. A 
big part of this goal is to improve the usability of common components like the 
Kirigami SwipeListItem so that people are more comfortable using it as-is and 
not overriding bits of or re-implementing something themselves.
  
  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?

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

2020-03-19 Thread 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

2020-03-19 Thread 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&id=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

2020-03-19 Thread 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

2020-03-19 Thread Nathaniel Graham
ngraham added a comment.


  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?
  
  > 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.
  
  > 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.

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

2020-03-19 Thread 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


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

2020-03-19 Thread Nathaniel Graham
ngraham requested changes to this revision.
ngraham added a reviewer: mart.
ngraham added a comment.
This revision now requires changes to proceed.


  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.
  - 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.
  - 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.

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

2020-03-13 Thread Nathaniel Graham
ngraham added a comment.


  Looks interesting, will take a look soon.

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

2020-03-13 Thread 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&id=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

2020-03-13 Thread 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

2020-03-13 Thread 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

2020-03-13 Thread 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