> On Aug. 10, 2013, 7:26 p.m., Mark Kretschmann wrote: > > I'm not so sure about the warning that is shown when an unwritable UMS > > collection is loaded. First of all it seems to me this would belong on a > > higher level, as other collections could be read-only as well. Second, I'm > > not sure a warning is called for. It's not a critical situation. And last, > > doesn't it suffice to show an error when the actual writing failed? > > Frank Meerkoetter wrote: > There won't be an error message when the actual writing fails, as the > user is not offered certain operations (move/copy to/organize) for read only > collections. :-) > > I am not sure what would be a good solution. I have to admit that I added > the warning as an afterthought after having fixed the isWriteable() for UMS. > The reason was that i found it highly intransparent why certain operations > are not longer offered. > > The question is how to make it transparent to the user why move/copy/org. > are disabled for a certain UMS collection? > > Perhaps the solution is to leave isWriteable() as it was (always return > true for UMS) and to generate error messages when operations fail? > >
I think that leaving isWritable() that returns true despite the fact that collection is read-only is hacky at best, and dead wrong at worst. If we ever find ourselves needing to do something like that, we have a sure sign of a faulty design. That being said, I completely agree with Mark about it belonging on the higher level. It may be nice to get a visual hint that a collection is read-only, but it should do so for every collection type, not only for UMS. FWIW, I would instantly grasp the concept if the relevant actions were added but disabled (as currently not being added at all). Maybe add a tooltip on hover saying that the collection is read-only, and I'm sure everything would be clear enough. Just something to consider, I'm not necessarily convinced it's the best way. - Konrad ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111991/#review37483 ----------------------------------------------------------- On Aug. 10, 2013, 4:44 p.m., Frank Meerkoetter wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/111991/ > ----------------------------------------------------------- > > (Updated Aug. 10, 2013, 4:44 p.m.) > > > Review request for Amarok. > > > Description > ------- > > If an UMS-collection is non-writeable (think freshly created ext4 on an > USB-stick) a copy/move to > collection will silently fail. It can only be seen in the debug logs on the > console. > > The reason is that the UMS-collection is not actually checking if its > writeable. Its just assuming > to be always writeable. > > My change consists of two parts: > 1. I check if the UMS-collection is actually writeable (mount point and if > set the "music folder"). > 2. To give the user a hint what is going on I added a warning message, > informing the user that > this collection is read-only (otherwise he might be confused why certain > operations (move/copy/organize) > are missing. > > > Diffs > ----- > > src/core-impl/collections/umscollection/UmsCollection.h 749ff81 > src/core-impl/collections/umscollection/UmsCollection.cpp 028966e > src/core-impl/collections/umscollection/UmsCollectionLocation.cpp 1cd1ba8 > > Diff: http://git.reviewboard.kde.org/r/111991/diff/ > > > Testing > ------- > > I have tested with an USB-Stick containing an ext4 filesystem. First i had > the base-dir owned by root.root (non-writeable), next i made it writeable and > verified that its correctly detected as such. Next i created a "Music" folder > and configured it for this USB-Stick. I checked that it was detected as > writeable. Next i made the "Music" folder non-writeable and checked that > amarok also detected that. > > > Thanks, > > Frank Meerkoetter > >
_______________________________________________ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
