device-automounter moved to kdereview

[Kevin Ottens]

On Sunday 9 August 2009 21:34:45 Trever Fischer wrote:
> I've moved device-automounter out of playground and into kdereview. After
> the review, I hope for it to end up in kdebase/runtime/solid/.

Shouldn't be a problem I guess if we can agree on a few changes. :-)

> Then after that, I hope to combine it with the solid-actions-kcm to let
> it handle other automatic execution of actions when devices get attached.

Well, at least for the configuration parts that looks like the way to go. All 
this stuff should be presented at one place (from the user pov). You'll 
probably have to discuss with Ben Cooksley for that who is in charge of the 
action kcm.

BTW, most of the people working on the hardware related topics are on kde-
hardware-devel so you probably want to register there.

> device-automounter is a small kded plugin and kcm page that adds removable
> media automounting to KDE. Its more than blindly automatic, since it has a
> little bit more logic (further describe in the SETTINGS file) to make it
> smarter than your average automounter, while still behaving without
> configuration as a naive user would expect.

I took a look at it and it's mostly doing it blindly albeit a few heuristics 
as you mentionned. In any case that's the kind of stuff which can't work in a 
multi-user environment, and it can also cause troubles when someone is 
deleting/creating/formatting partitions. So here is my (short) list of 
proposed changes.

1) bool shouldAutomount = deviceAutomount || (enabled && typeCondition && 
((automountKnown && known) || lastSeenMounted));
really has to become:
bool shouldAutomount = enabled && (... rest of the condition);

If I set automounting to disabled, I really want it disabled period. There 
should not be some special exception to that.

2) AutomountEnabled should have false by default. So that means that (with the 
previous change) automounting is completely shutdown by default. We can't 
really afford having something doing automounting enabled by default for the 
reasons I pointed out above (multi-user envs, formatting, etc.)

3) I agree with the other people who commented in this thread that using the 
UDI in the GUI doesn't work, it's really confusing. Moreover some of the logic 
there needs to be refined as for some reason when I plugged my usb flash drive 
(containing only one volume) I got 8 devices reported in the "Device 
Overrides" list to always automount...
BTW a small nitpick on the KCM, the first column appears really tiny by 
default here which makes the Name column unreadable.

> Everything passed krazy2all, with the exception of line 40 in
> kded/DeviceAutomounter.cpp because I can't remember how to get krazy2 to
> ignore the foreach checker. My rationale for that is that using
> Solid::VolumeAccess::setup() requires a non-const Device, so two copy
> constructors is cheaper than converting from a Device to a QString udi and
> back. If I'm wrong, feel free to correct me/point and laugh.

This you could solve by ditching the foreach construct in favor of a for loop 
using C++ style iterators.

Regards.
-- 
Kévin 'ervin' Ottens, http://ervin.ipsquad.net
"Ni le maître sans disciple, Ni le disciple sans maître,
Ne font reculer l'ignorance."
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20090815/65c76d51/attachment.sig>