D18038: Fix semantics for ghns_exclude

[Dan Leinir Turthra Jensen]

This revision was automatically updated to reflect the committed changes.
Closed by commit R304:301fe73569ba: Fix semantics for ghns_exclude (authored by 
leinir).

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18038?vs=48859=49339

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

AFFECTED FILES
  autotests/knewstuffentrytest.cpp
  src/core/engine.cpp
  src/core/engine.h
  src/core/tagsfilterchecker.cpp
  src/core/tagsfilterchecker.h
  tests/testdata/entry.xml

To: leinir, ronaldv, #kde_store, ngraham
Cc: kde-frameworks-devel, michaelh, ZrenBot, ngraham, bruns, akiraohgaki, 
alexanderschmidt, siyuandong, ronaldv, mikesomov, starbuck

D18038: Fix semantics for ghns_exclude

[Dan Leinir Turthra Jensen]

leinir edited the summary of this revision.

REPOSITORY
  R304 KNewStuff

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

To: leinir, ronaldv, #kde_store, ngraham
Cc: kde-frameworks-devel, michaelh, ZrenBot, ngraham, bruns, akiraohgaki, 
alexanderschmidt, siyuandong, ronaldv, mikesomov, starbuck

D18038: Fix semantics for ghns_exclude

[Dan Leinir Turthra Jensen]

leinir added a comment.


  In D18038#391684 , @ngraham wrote:
  
  > Thanks, in addition to the testing tool working, this patch seems to 
actually fix the issue in production (e.g. "Tree on Island" is no longer 
visible in the wallpaper downloader), and as far as I can tell the code is 
sane. Thanks for the additional documentation and commenting too.
  >
  > Should this be marked as actually fixing 402888? If so, it should be `BUG: 
402888`
  
  
  Yay! :D i'm quite keen on documentation being solid, i know what it's like to 
arrive at something which is... less than well documented ;) Great stuff, nice 
to know it works for people not me :)
  
  I think it should probably fix said bug (which also will show you what i mean 
when i say it doesn't work for me ;) ).

REPOSITORY
  R304 KNewStuff

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

To: leinir, ronaldv, #kde_store, ngraham
Cc: kde-frameworks-devel, michaelh, ZrenBot, ngraham, bruns, akiraohgaki, 
alexanderschmidt, siyuandong, ronaldv, mikesomov, starbuck

D18038: Fix semantics for ghns_exclude

[Nathaniel Graham]

ngraham accepted this revision.
ngraham added a comment.
This revision is now accepted and ready to land.


  Thanks, in addition to the testing tool working, this patch seems to actually 
fix the issue in production (e.g. "Tree on Island" is no longer visible in the 
wallpaper downloader), and as far as I can tell the code is sane. Thanks for 
the additional documentation and commenting too.
  
  Should this be marked as actually fixing 402888? If so, it should be `BUG: 
402888`

REPOSITORY
  R304 KNewStuff

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

To: leinir, ronaldv, #kde_store, ngraham
Cc: kde-frameworks-devel, michaelh, ZrenBot, ngraham, bruns, akiraohgaki, 
alexanderschmidt, siyuandong, ronaldv, mikesomov, starbuck

D18038: Fix semantics for ghns_exclude

[Dan Leinir Turthra Jensen]

leinir added a comment.


  In D18038#390748 , @ngraham wrote:
  
  > What is the test tool? Can you help a total n00b like me learn how to test 
KNewStuff patches like these?
  
  
  The test tool is khotnewstuff_test in tests/. It's not installed, but once 
compiled it's in (build)/bin/khotnewstuff_test and can be run from there once 
the libraries are installed. For the most useful output, be sure to turn on all 
of the KNewStuff debug output, and then pass it a knsrc file. For example, what 
i use is
  
QT_LOGGING_RULES="org.kde.knewstuff.*=true" ./bin/khotnewstuff_test 
/etc/xdg/peruse.knsrc
  
  You then get a nice little UI which lets you run a couple of tests (engine 
test initialises the engine and lists the first page of content in the 
requested categories using the default sort order, and if you turn on the 
option underneath that, it will also attempt to download them). If you don't 
give it a knsrc file, it'll just use the static data found in tests/testdata.
  
  As you can tell, it's a super simple tool, and could of course be improved :) 
Haven't really had the need to do so, since the more advanced features seem 
better tested on live installs, and this one is more for testing out the very 
most basic functionality (which is just sort of expected to work in proper 
applications).
  
  > Also, the correct formatting is `CCBUG: 402888`. See 
https://community.kde.org/Infrastructure/Phabricator#Formatting_your_patch
  
  Ah yes, quite, i've only been using those hooks for about a decade ;) You'd 
imagine i would be able to remember them at some point. Not that the bug hooks 
work for me, mind, but still :P

REPOSITORY
  R304 KNewStuff

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

To: leinir, ronaldv, #kde_store, ngraham
Cc: kde-frameworks-devel, michaelh, ZrenBot, ngraham, bruns, akiraohgaki, 
alexanderschmidt, siyuandong, ronaldv, mikesomov, starbuck

D18038: Fix semantics for ghns_exclude

[Nathaniel Graham]

ngraham added a comment.


  What is the test tool? Can you help a total n00b like me learn how to test 
KNewStuff patches like these?
  
  Also, the correct formatting is `CCBUG: 402888`. See 
https://community.kde.org/Infrastructure/Phabricator#Formatting_your_patch

REPOSITORY
  R304 KNewStuff

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

To: leinir, ronaldv, #kde_store, ngraham
Cc: kde-frameworks-devel, michaelh, ZrenBot, ngraham, bruns, akiraohgaki, 
alexanderschmidt, siyuandong, ronaldv, mikesomov, starbuck

D18038: Fix semantics for ghns_exclude

[Dan Leinir Turthra Jensen]

leinir created this revision.
leinir added reviewers: ronaldv, KDE Store.
leinir added projects: KNewStuff, KDE Store.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
leinir requested review of this revision.

REVISION SUMMARY
  After much discussion on the topic, it came to light that the specific 
grammar used for the default tag in KNSCore was incorrect, as it was an action 
verb (rather than an adjective the way the metadata tags are supposed to be). 
While this does mean that previous versions of Frameworks will not have the 
filtering functionality by default, it is otherwise non-invasive and causes no 
side effects apart from the server-suggested filtering working.
  
  CCBUG: https://bugs.kde.org/show_bug.cgi?id=402888

TEST PLAN
  Run the test tool without this patch: Items which are supposed to be excluded 
are not excluded
  Run the test tool with this patch: Items which are supposed to be excluded 
from listings are excluded.

REPOSITORY
  R304 KNewStuff

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

AFFECTED FILES
  autotests/knewstuffentrytest.cpp
  src/core/engine.cpp
  src/core/engine.h
  src/core/tagsfilterchecker.cpp
  src/core/tagsfilterchecker.h
  tests/testdata/entry.xml

To: leinir, ronaldv, #kde_store
Cc: kde-frameworks-devel, michaelh, ZrenBot, ngraham, bruns, akiraohgaki, 
alexanderschmidt, siyuandong, ronaldv, mikesomov, starbuck