Review request: plasma-pass

Albert Astals Cid aacid at
Sun Dec 16 19:14:37 GMT 2018

El dilluns, 10 de desembre de 2018, a les 14:49:28 CET, Daniel Vrátil va escriure:
> Hi folks,
> back in May I wrote a Plasma applet that serves as a GUI frontend for the pass 
> password manager [0]. I blogged about it [1], but then somewhat forgot to make 
> a release etc. Recently I started getting some emails from packagers where to 
> get a tarballs so I think it's time to get some translations in and start 
> doing official releases. Thus I'd like to ask for a review for inclusion in 
> extragear.
> The way pass works is it has a directory in which each password is stored in a 
> PGP-encrypted file, the name of the file is the name of the password. You can 
> also create subfolders to organize the passwords.
> The code of the applet is a C++ model that watches said directory and exposes 
> its content as a tree. There's also a filter proxy model which uses partial 
> string matching code from KDevelop (so you can filter for "jd at f" and it 
> matches "john.doe at").
> The applet itself sits in systray, when activated it shows the top-level list 
> of folders and password. Code-wise it contains a stack of list views, when 
> entering a subfolder it pushes a new listview with content of that folder to 
> the stack. Selecting a password decrypts it using gpg and puts it into 
> clipboard for 45 seconds. After that it clears it from the X clipboard as well 
> as Klipper.
> There's a bit of mess regarding focus handling in the QML, but the goal was to 
> make the applet fully controllable via keyboard, which wasn't easy with my  
> QML skills :) 

You have 

plasma-pass:master$ wcgrep X-KDE-PluginInfo-Name

Which one is it? I guess the second? Then you also need to update your to match that one if you change it, see applets/colorpicker in kdeplasma-addons for example

You're using old style connects, use the power of the "new one".

PlasmaPass::PasswordsModel::Node has dtor but not copy-ctor, copy-assignment. Which means that if someone ever does
 PasswordsModel::Node a;
 PasswordsModel::Node b = a;
things will break since the default implementations will just copy the children vector and bad things will happen, i suggest you delete the copy and assignment functions.

filterChanged is already a name of an existing function on QSortFilterProxyModel (i know sucks), so maybe you could rename that signal to something else? I guess this is not very important though but some people may get confused when reading the code.

In the QML side you do a few == != that would be 0.00000000001% faster doing === and !==, it's considered better JS code since "weird" promotions are not done, but your call.

I have not actually tested whether the thing works or not, but it's small enough that i guess it does :D

Just had time for this quick code review, hope you find it useful :)


> Looking forward to your feedback,
> Dan
> [0]
> [1]

More information about the kde-core-devel mailing list