broulik abandoned this revision.
broulik added a comment.
Superseded by https://phabricator.kde.org/D7774
REPOSITORY
R245 Solid
REVISION DETAIL
https://phabricator.kde.org/D5034
To: broulik, #plasma, dhaumann, dfaure
Cc: bruns, dhaumann, plasma-devel, #frameworks, ZrenBot, progwolff,
bruns added inline comments.
INLINE COMMENTS
> fstabstorageaccess.cpp:48
>
> +const QStringList = FstabHandling::options(device->device());
> +// GVFS by default doesn't show devices outside of /media, $HOME (and
> some other locations)
Another useless use of const QList&,
dhaumann added a comment.
@broulik Ping. Is there anything that keeps you from submitting this?
REPOSITORY
R245 Solid
REVISION DETAIL
https://phabricator.kde.org/D5034
To: broulik, #plasma, dhaumann, dfaure
Cc: bruns, dhaumann, plasma-devel, #frameworks, ZrenBot, spstarr, progwolff,
bruns added inline comments.
INLINE COMMENTS
> fstabdevice.cpp:47
>
> -m_description = m_vendor + " on " + m_product;
> +const QStringList =
> FstabHandling::options(m_device).filter("x-gvfs-");
> +
Why QStringList& instead of QStringList?
> fstabdevice.cpp:51
> +if
broulik added a comment.
If no-one objects I'll push this after the next frameworks release
REPOSITORY
R245 Solid
REVISION DETAIL
https://phabricator.kde.org/D5034
To: broulik, #plasma, dfaure, dhaumann
Cc: dhaumann, plasma-devel, #frameworks, progwolff, lesliezhai, ali-mohamed,
dhaumann accepted this revision.
dhaumann added a comment.
This revision is now accepted and ready to land.
The patch looks good to me (without testing this locally here). I think the
one QStringRef can be turned by to QStringList, since you finally need the
QString anyways it seems...
broulik updated this revision to Diff 12457.
broulik added a comment.
- Cache mountOptions or else it falls out of scope and QStringRef crashes
(now I know why I should cache it :D)
REPOSITORY
R245 Solid
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D5034?vs=12456=12457
broulik updated this revision to Diff 12456.
broulik edited the summary of this revision.
broulik added a comment.
- Use QStringRef
- Drop swap of vendor/product, this will be done in a separate patch
REPOSITORY
R245 Solid
CHANGES SINCE LAST UPDATE
broulik added inline comments.
INLINE COMMENTS
> broulik wrote in fstabdevice.cpp:40-41
> I wondered that too, that's in the original patch, though, didn't really look
> into where this will end up, though :D
Just checked, this way round it makes more sense.
The vendor will be the host name
broulik added inline comments.
INLINE COMMENTS
> dhaumann wrote in fstabdevice.cpp:40-41
> Did you intentionally switch vendor and product?
I wondered that too, that's in the original patch, though, didn't really look
into where this will end up, though :D
REPOSITORY
R245 Solid
REVISION
dhaumann added a comment.
Just some minor comments, besides that, looks sane.
INLINE COMMENTS
> fstabdevice.cpp:40-41
> +if (m_device.startsWith(QLatin1String("//"))) {
> +m_vendor = m_device.mid(2, m_device.indexOf(QLatin1String("/"), 2) -
> 2);
> +m_product =
broulik created this revision.
Restricted Application added projects: Plasma, Frameworks.
Restricted Application added subscribers: Frameworks, plasma-devel.
REVISION SUMMARY
These fstab options allows an administrator to specify names and icons
intended for the user, shown in a GUI.
For
12 matches
Mail list logo
