D17595: Upstream Dolphin's file rename dialog

[Emmanuel Pescosta]

emmanuelp added a comment.


  In D17595#473825 , @meven wrote:
  
  > I guess we need now to reach to schwarz...@gmail.com romaninflianskas 
mfuchs chinmoyr mitchell emateli emmanuelp freininghaus to have lgplv2+ 
approval.
  
  
  Added myself to the relicense-check script, including the  lgplv2+ approval.

REPOSITORY
  R241 KIO

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

To: meven, #frameworks, #dolphin, broulik, ngraham
Cc: emmanuelp, ltoscano, bruns, meven, dhaumann, pino, kde-frameworks-devel, 
LeGast00n, michaelh, ngraham

D5138: Fill UDS_CREATION_TIME with the value of st_birthtime on FreeBSD

[Emmanuel Pescosta]

emmanuelp added a comment.


  In https://phabricator.kde.org/D5138#97024, @adridg wrote:
  
  > So, Linux, sort out your *stat(2) interfaces first and we'll get back to 
you.
  
  
  xstat patches have been merged into 4.11

REPOSITORY
  R241 KIO

BRANCH
  master

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

To: tcberner, arrowdodger, rakuco, dfaure, adridg
Cc: emmanuelp, #frameworks

Re: Review Request 129394: [filenamesearch] Fix huge ram usage in kded module

[Emmanuel Pescosta]

> On Nov. 21, 2016, 9:34 a.m., David Faure wrote:
> > filenamesearch/kded/filenamesearchmodule.cpp, line 84
> > <https://git.reviewboard.kde.org/r/129394/diff/1/?file=485431#file485431line84>
> >
> > Well, if dirUrl looks like 
> > "filenamesearch:?search=file=file:///path/to/file" then dirUrl.path() 
> > is empty, and this code is incorrect (it should use the query item "url", 
> > not the path). What am I missing?
> 
> Anthony Fieroni wrote:
> This is a big misunderstanding mainly by me. Emitted url should contains 
> query with new path ?
> for (const QString  : files) {
> const QUrl url(file);
> if (!url.isLocalFile()) {
> continue;
> }
> const QString urlPath = url.path();
> for (const QUrl  : m_searchUrls) {
> QUrlQuery urlQuery(dirUrl);
> QString str = urlQuery.queryItemValue(QStringLiteral("url"));
> if (urlPath.startsWith(QUrl(str).path())) {
> QUrl temp;
> temp.setScheme(QStringLiteral("filenamesearch"));
> urlQuery.removeQueryItem(QStringLiteral("url");
> urlQuery.addQueryItem(QStringLiteral('url"), url);
> temp.setQuery(urlQuery);
> fileList << temp;
> }
> }
> }
> 
> David Faure wrote:
> Maybe, but I'm still in the dark about something. How can KDirLister cope 
> with listing such URLs? It wants a directory URL and files inside that 
> directory. Such a filenamesearch URL doesn't look like it's a file inside a 
> directory, in terms of paths. Ideally I would look into the code to 
> understand what is being done but I'm short on time.
> 
> Does kio_filenamesearch really return items from listDir(), which have an 
> empty path too, just like the listed directory? I would assume this breaks 
> many things in KDirLister.
> 
> Please clarify with the dolphin people (or whoever wrote the 
> filenamesearch KIO) about the URL structure, then it will be straightforward 
> to do the URL conversions in this code.
> 
> Anthony Fieroni wrote:
> I'm invited Emmanuel, who knows? 
> https://github.com/KDE/dolphin/blob/1710304e9ba926d2aec4226d00974b826f9bcbc0/src/kitemviews/kfileitemmodel.cpp#L123
>  url("filenamesearch:?search=file=file:///path/to/file") in slot 
> https://github.com/KDE/dolphin/blob/1710304e9ba926d2aec4226d00974b826f9bcbc0/src/kitemviews/kfileitemmodel.cpp#L77
>  comes results

Peter wrote the filenamesearch slave.

Filenamesearch URLs are only used to initiate a search. The URL contains the 
search start-folder, the pattern used for matching and an optional 'check 
contents' query item which can be yes or no. The filenamesearch ioslave uses 
all this query items to perform the search. It recursively opens each folder 
via KCoreDirLister (starting with the start-folder of the filenamesearch URL) 
and iterates trough the item list of the directory, every matching item is then 
listed via `listEntry` by using the UDSEntry of the matching item (see 1). So 
kio_filenamesearch can only return items with an empty path if the underlying 
ioslave (local, smb, ftp, ...) returns items with an empty path.

[1] 
https://github.com/KDE/kio-extras/blob/master/filenamesearch/kio_filenamesearch.cpp#L103


- Emmanuel


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129394/#review101007
---


On Nov. 14, 2016, 12:44 p.m., Anthony Fieroni wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129394/
> ---
> 
> Review request for KDE Frameworks, Anthony Fieroni, David Faure, and Emmanuel 
> Pescosta.
> 
> 
> Repository: kio-extras
> 
> 
> Description
> ---
> 
> Bug is introduced in https://git.reviewboard.kde.org/r/129297/
> When is fixed new kio-extras realease is needed for 16.08 branch.
> 
> 
> Diffs
> -
> 
>   filenamesearch/kded/filenamesearchmodule.cpp 3f9f582 
> 
> Diff: https://git.reviewboard.kde.org/r/129394/diff/
> 
> 
> Testing
> ---
> 
> No big ram usage but still not works as expected.
> 1. Perform search in Dolphin
> 2. Delete one result item
> 3. View must be update, but it's not
> 
> 
> Thanks,
> 
> Anthony Fieroni
> 
>

Re: Review Request 129083: [KIconLoader] Take into account devicePixelRatio for overlays

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129083/#review100354
---




src/kiconloader.cpp (line 514)
<https://git.reviewboard.kde.org/r/129083/#comment67358>

int -> qreal



src/kiconloader.cpp (line 516)
<https://git.reviewboard.kde.org/r/129083/#comment67357>

    QPointF


- Emmanuel Pescosta


On Oct. 6, 2016, 10 p.m., Kai Uwe Broulik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129083/
> ---
> 
> (Updated Oct. 6, 2016, 10 p.m.)
> 
> 
> Review request for Dolphin, KDE Frameworks and David Edmundson.
> 
> 
> Bugs: 369640
> https://bugs.kde.org/show_bug.cgi?id=369640
> 
> 
> Repository: kiconthemes
> 
> 
> Description
> ---
> 
> Keeps them from being painted outside view and scales them with the image 
> they're supposed to be painted on.
> 
> 
> Diffs
> -
> 
>   src/kiconloader.cpp 5315ea4 
> 
> Diff: https://git.reviewboard.kde.org/r/129083/diff/
> 
> 
> Testing
> ---
> 
> Ran dolphin with device pixel ratio 2, got my symolic link icon again.
> Ran it with device pixel ratio 1, looked as it did before.
> 
> (couldn't figure out why the VCS icons don't show up though)
> 
> 
> Thanks,
> 
> Kai Uwe Broulik
> 
>

Re: Review Request 128466: Rename Checksums tab to Integrity

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128466/#review97482
---


Ship it!




Good idea!

- Emmanuel Pescosta


On July 16, 2016, 2:35 p.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128466/
> ---
> 
> (Updated July 16, 2016, 2:35 p.m.)
> 
> 
> Review request for KDE Frameworks, KDE Usability and Dominik Haumann.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> Dominik suggested to rename the `Checksums` tab to `Integrity`, so that we 
> can "free" the Checksums string and use it as the title of the groupbox below 
> (in place of the current `Share` string, which can be confusing).
> 
> Preview in the attached screenshot.
> 
> 
> Diffs
> -
> 
>   src/widgets/checksumswidget.ui 03c64db 
>   src/widgets/kpropertiesdialog.cpp 808765c 
> 
> Diff: https://git.reviewboard.kde.org/r/128466/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> Before
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/07/16/6771ed06-c803-4d18-abe3-91e4f97c8c76__checksums-tab.png
> After
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/07/16/b2cd12c8-6bbf-4123-9e8e-59cb0c29cbdb__Spectacle.TJ7614.png
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 128088: Load KFileItemAction plugins with json metadata

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128088/#review96284
---


Ship it!




Ship It!

- Emmanuel Pescosta


On June 5, 2016, 8:14 p.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128088/
> ---
> 
> (Updated June 5, 2016, 8:14 p.m.)
> 
> 
> Review request for Dolphin and KDE Frameworks.
> 
> 
> Repository: dolphin
> 
> 
> Description
> ---
> 
> Dolphin currently doesn't support JSON-only KFileItemAction plugins (without 
> a .desktop file).
> This patch adds support for them both in the context menu and in the settings 
> dialog (Services pane).
> 
> Since a plugin can have both a .desktop file and JSON metadata (as is the the 
> case for `kactivitymanagerd_fileitem_linking_plugin`), we need to be sure to 
> not add twice the plugin in the context menu. A QSet is used to prevent this 
> issue.
> 
> Open question: do we want to install these json-only plugins in a specific 
> subdirectory of `$QT_PLUGIN_PATH`? Like `$QT_PLUGIN_PATH/kf5/kio` or 
> something else? This way we could tell Dolphin to only look for plugins 
> within that subfolder (instead of checking every plugin in `$QT_PLUGIN_PATH`).
> 
> 
> Diffs
> -
> 
>   src/dolphincontextmenu.cpp dfec76144dafd9210aa8b6f45458ded98e27ab53 
>   src/settings/services/servicessettingspage.cpp 
> 4cfa2327c9fa77a6124b4d90040a9784cc8180c5 
> 
> Diff: https://git.reviewboard.kde.org/r/128088/diff/
> 
> 
> Testing
> ---
> 
> * Build Ark master after commit c369264d6c
> * Make sure that the Compress and Extract service menus are shown
> * Make sure that the menus above can be enabled/disabled in the Services pane 
> in the settings
> * Make sure that `kactivitymanagerd_fileitem_linking_plugin` is not shown 
> twice in the context menu
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127779: use system colors for monochrome icons

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127779/#review94957
---




src/kiconloader.cpp (lines 843 - 845)
<https://git.reviewboard.kde.org/r/127779/#comment64465>

Missing in the SVG path.

IMHO it would be nicer if you put the SVG stuff into a new static function 
and return a byte array which can then be feed into the QImageReader. The 
"QImageReader from byte array" should then take the same remaining code path as 
the "QImageReader from path" to avoid breakages.


- Emmanuel Pescosta


On April 28, 2016, 1:53 p.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127779/
> ---
> 
> (Updated April 28, 2016, 1:53 p.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: kiconthemes
> 
> 
> Description
> ---
> 
> Breeze uses stylesheet information to color its icons with some "named" 
> colors that change depending from the current system color scheme, this is 
> already used since some time in the icons used by the Plasma shell.
> But QWidget applications have the same problem, when the user changes the 
> color scheme from breeze to breeze dark (or any color scheme), most of the 
> icons will be black on black.
> This patch clones (a bit simplified and taking only the most "important" 
> colors) the logic used by Plasma::Svg to color icons with the stylesheet.
> 
> even tough it's doing more things at icon generation, an application that 
> uses a lot of icons like Dolphin doesn't seem to have noticeable startup time 
> difference, even when the image cache is not present yet, so i hope is not an 
> unacceptable performance tradeoff (successive loads are unchanged as are from 
> the image cache).
> 
> still some questions and things that can be optimized, like
> 
> * an optional key in the theme metadata file to explicitly enable this, to 
> not have it running in themes that don't care?
> 
> * can i use karchive in this framework?, so it would work with svgz icons as 
> well
> 
> * right now to refresh icons at runtime it depends from a patch in the colors 
> kcm to emit iconchanges as well, alternatively kiconloader could watch for 
> kcolorscheme changes as well, but i don't think is strictly necessary
> 
> 
> Diffs
> -
> 
>   src/kiconloader.cpp 75ab482 
> 
> Diff: https://git.reviewboard.kde.org/r/127779/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> dadel1.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/04/28/0fc42425-947c-479e-9759-09c7a703a456__dadel1.png
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127587: [Kate view] Correct override cursor

[Emmanuel Pescosta]

> On April 8, 2016, 9:29 a.m., Kåre Särs wrote:
> > What is it that needs fixing?
> > 
> > I don't have a problem in either case listed.

> 3. Context menu is closed

I can reproduce this problem


- Emmanuel


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127587/#review94421
---


On April 8, 2016, 6:36 a.m., Anthony Fieroni wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127587/
> ---
> 
> (Updated April 8, 2016, 6:36 a.m.)
> 
> 
> Review request for Kate, KDE Frameworks, Dominik Haumann, and Kåre Särs.
> 
> 
> Bugs: 361075
> https://bugs.kde.org/show_bug.cgi?id=361075
> 
> 
> Repository: ktexteditor
> 
> 
> Description
> ---
> 
> Currsor must be override after these conditions:
> 1. Drag move ends
> 2. Minimize / restore / maximize
> 3. Context menu is closed
> 
> 
> Diffs
> -
> 
>   src/view/kateviewinternal.cpp ca5acfc 
> 
> Diff: https://git.reviewboard.kde.org/r/127587/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Anthony Fieroni
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127410: Fix crash in rename dialog (KF 5.20)

[Emmanuel Pescosta]

> On March 17, 2016, 1:29 p.m., Aleix Pol Gonzalez wrote:
> > Change looks good.
> > 
> > Would it be possible to get a test?

> Would it be possible to get a test?

autotest (is it allowed to show a dialog in KF autotests?) or more like 
kpropertiesdialogtest?


- Emmanuel


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127410/#review93651
---


On March 17, 2016, 10:37 a.m., Emmanuel Pescosta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127410/
> ---
> 
> (Updated March 17, 2016, 10:37 a.m.)
> 
> 
> Review request for KDE Frameworks, Andreas Sturmlechner and David Faure.
> 
> 
> Bugs: 360488
> https://bugs.kde.org/show_bug.cgi?id=360488
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> Fix crash in rename dialog
> 
> ResizePanels must only be called in case of overwrite, all other cases 
> violate the preconditions.
> 
> @Andreas: Can you please try the patch?
> 
> 
> Diffs
> -
> 
>   src/widgets/renamedialog.cpp 2d456c7 
> 
> Diff: https://git.reviewboard.kde.org/r/127410/diff/
> 
> 
> Testing
> ---
> 
> Yes, I tried it by overwriting a file with the same file and with a different 
> file -> no crash :)
> 
> 
> Thanks,
> 
> Emmanuel Pescosta
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127410: Fix crash in rename dialog (KF 5.20)

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127410/
---

(Updated March 19, 2016, 11:23 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks, Andreas Sturmlechner and David Faure.


Changes
---

Submitted with commit bf37e9aa27a2321f4a6ce7188a0901fabd47c962 by Emmanuel 
Pescosta to branch master.


Bugs: 360488
https://bugs.kde.org/show_bug.cgi?id=360488


Repository: kio


Description
---

Fix crash in rename dialog

ResizePanels must only be called in case of overwrite, all other cases violate 
the preconditions.

@Andreas: Can you please try the patch?


Diffs
-

  src/widgets/renamedialog.cpp 2d456c7 

Diff: https://git.reviewboard.kde.org/r/127410/diff/


Testing
---

Yes, I tried it by overwriting a file with the same file and with a different 
file -> no crash :)


Thanks,

Emmanuel Pescosta

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127410: Fix crash in rename dialog (KF 5.20)

[Emmanuel Pescosta]

> On March 17, 2016, 1:29 p.m., Aleix Pol Gonzalez wrote:
> > Change looks good.
> > 
> > Would it be possible to get a test?
> 
> Emmanuel Pescosta wrote:
> > Would it be possible to get a test?
> 
> autotest (is it allowed to show a dialog in KF autotests?) or more like 
> kpropertiesdialogtest?
> 
> David Faure wrote:
> Yes ideally this would be an autotest. We have autotests showing up 
> dialogs already, no problem there. What's a bit tricky is the introspection 
> to find the buttons and "click" them. Or just calling reject() would have 
> done the job for this particular crash (after some event processing).

> We have autotests showing up dialogs already

Ok great :)

Is it ok for everyone when I push this fix now, so that downstream can apply it 
to their KF 5.20 release, and provide the test next week? I need to dive into 
the introspection stuff first, so this will need some time ;)


- Emmanuel


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127410/#review93651
---


On March 17, 2016, 10:37 a.m., Emmanuel Pescosta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127410/
> ---
> 
> (Updated March 17, 2016, 10:37 a.m.)
> 
> 
> Review request for KDE Frameworks, Andreas Sturmlechner and David Faure.
> 
> 
> Bugs: 360488
> https://bugs.kde.org/show_bug.cgi?id=360488
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> Fix crash in rename dialog
> 
> ResizePanels must only be called in case of overwrite, all other cases 
> violate the preconditions.
> 
> @Andreas: Can you please try the patch?
> 
> 
> Diffs
> -
> 
>   src/widgets/renamedialog.cpp 2d456c7 
> 
> Diff: https://git.reviewboard.kde.org/r/127410/diff/
> 
> 
> Testing
> ---
> 
> Yes, I tried it by overwriting a file with the same file and with a different 
> file -> no crash :)
> 
> 
> Thanks,
> 
> Emmanuel Pescosta
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Review Request 127410: Fix crash in rename dialog (KF 5.20)

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127410/
---

Review request for KDE Frameworks, Andreas Sturmlechner and David Faure.


Bugs: 360488
https://bugs.kde.org/show_bug.cgi?id=360488


Repository: kio


Description
---

Fix crash in rename dialog

ResizePanels must only be called in case of overwrite, all other cases violate 
the preconditions.

@Andreas: Can you please try the patch?


Diffs
-

  src/widgets/renamedialog.cpp 2d456c7 

Diff: https://git.reviewboard.kde.org/r/127410/diff/


Testing
---

Yes, I tried it by overwriting a file with the same file and with a different 
file -> no crash :)


Thanks,

Emmanuel Pescosta

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127004: WIP: OpenFileManagerWindowJob

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127004/#review92157
---




src/widgets/openfilemanagerwindowjob.h (lines 79 - 102)
<https://git.reviewboard.kde.org/r/127004/#comment62869>

Maybe add them as properties?



src/widgets/openfilemanagerwindowjob.h (lines 109 - 116)
<https://git.reviewboard.kde.org/r/127004/#comment62868>

I would turn them into namespace functions, e.g. highlightInFileManager.

(And I would only provide the url list version instead of both, because 
with C++11 it is as easy to use as the single url version, but this is only my 
personal opinion so feel free to ignore it ;)



src/widgets/openfilemanagerwindowjob.cpp (lines 95 - 97)
<https://git.reviewboard.kde.org/r/127004/#comment62871>

I'm afraid that this code will become unreadable once the code of the other 
platforms will be added (do they have other fallbacks? or only KRun? what if 
dbus is available on those platforms? )

This begs for the strategy pattern (instead of the unmaintainable ifdef 
crap) ;)

You could inject the right strategy into the private class instance in the 
OpenFileManagerWindowJob::ctor and call a private class method here, which uses 
the injected strategy. In addition to that every strategy should call the 
fallback strategy which they think is best.

KRun invocation can then also be implemented as a strategy, which is 
directly used when no dbus is available, or within the dbus strategy as 
fallback when the dbus call failed.


- Emmanuel Pescosta


On Feb. 7, 2016, 7:20 p.m., Kai Uwe Broulik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127004/
> ---
> 
> (Updated Feb. 7, 2016, 7:20 p.m.)
> 
> 
> Review request for KDE Frameworks, Emmanuel Pescosta and Martin Gräßlin.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This job uses the org.freedesktop.FileManager1 interface to highlight files 
> within a certain folder, useful for eg. downloaded files or a "open 
> containing folder" functionality.
> 
> 
> Diffs
> -
> 
>   src/widgets/CMakeLists.txt 87dac50 
>   src/widgets/openfilemanagerwindowjob.h PRE-CREATION 
>   src/widgets/openfilemanagerwindowjob.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/127004/diff/
> 
> 
> Testing
> ---
> 
> With Dolphin's org.freedesktop.FileManager1 service installed, I got a file 
> highlighted properly. Without it Dolphin opened the folder without 
> highlighting (KRun fallback).
> 
> Not sure what to do with the startup id stuff
> 
> 
> Thanks,
> 
> Kai Uwe Broulik
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126392: Fix some Clazy warnings in kcoreaddons

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126392/#review89700
---



src/lib/io/kdirwatch.cpp (line 1067)
<https://git.reviewboard.kde.org/r/126392/#comment61448>

This is wrong (if I'm not wrong ;)

De Morgan's law [1] says that A or B <=> not ((not A) and (not B))

Where in this case:
A, B = list.count 
(not A), (not B) = list.empty

[1] https://en.wikipedia.org/wiki/De_Morgan's_laws



src/lib/io/kdirwatch_p.h (line 109)
<https://git.reviewboard.kde.org/r/126392/#comment61449>

Same here


- Emmanuel Pescosta


On Dec. 18, 2015, 4:33 a.m., Andrey Cygankov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126392/
> ---
> 
> (Updated Dec. 18, 2015, 4:33 a.m.)
> 
> 
> Review request for KDE Frameworks and Aleix Pol Gonzalez.
> 
> 
> Repository: kcoreaddons
> 
> 
> Description
> ---
> 
> Fix some Clazy warnings:
> - unneeded heap allocation with QString
> - midRef() non-usage
> - isEmpty() non-usage
> 
> 
> Diffs
> -
> 
>   autotests/ktexttohtmltest.cpp 81c3fc0 
>   src/lib/io/kbackup.h ead2de5 
>   src/lib/io/kbackup.cpp ec8b8b2 
>   src/lib/io/kdirwatch.cpp 34b32b7 
>   src/lib/io/kdirwatch_p.h 4483961 
>   src/lib/io/kmessage.cpp 305318c 
>   src/lib/io/kprocess.cpp 1df61c0 
>   src/lib/io/kurlmimedata.cpp 4095ee1 
>   src/lib/kaboutdata.h 0dbd7a0 
>   src/lib/kaboutdata.cpp 5d9a55e 
>   src/lib/randomness/krandom.cpp 2eb80d2 
>   src/lib/text/kmacroexpander.cpp 0d05ffd 
>   src/lib/text/kstringhandler.cpp 826ddcb 
>   src/lib/text/ktexttohtml.cpp d58f5c0 
>   src/lib/util/kshell.cpp 11892ce 
> 
> Diff: https://git.reviewboard.kde.org/r/126392/diff/
> 
> 
> Testing
> ---
> 
> Compiling without errors
> 
> 
> Thanks,
> 
> Andrey Cygankov
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 125988: [KUrlCompletion] Add autocompletion for '.' input which offers all hidden files/folders

[Emmanuel Pescosta]

> On Dec. 6, 2015, 12:19 p.m., David Faure wrote:
> > OK I debugged this further, and I found why this didn't match my 
> > expectations of the QUrl behaviour:
> > 
> > QUrl::fromLocalFile("/tmp/.") leads to file:///tmp rather than 
> > file:///tmp/. I consider this a bug (when NormalizePathSegments isn't set), 
> > I'll discuss it with Thiago and possibly fix it in Qt.
> 
> David Faure wrote:
> Urgh, it's my own commit aba336c2b4ad8926dc8a000718bbb7f8a6d5a72d in 
> qurl.cpp ...
> 
> David Faure wrote:
> Suggested fix posted to https://codereview.qt-project.org/143134

Nice. Thanks for fixing it in Qt :)

So how should we proceed with this RR?
Wrap all "." test cases with a #if QT_VERSION >= QT_VERSION_CHECK(5, 6, 0)?


- Emmanuel


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125988/#review89168
---


On Nov. 28, 2015, 10:36 p.m., Emmanuel Pescosta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125988/
> ---
> 
> (Updated Nov. 28, 2015, 10:36 p.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Bugs: 354981
> https://bugs.kde.org/show_bug.cgi?id=354981
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> Currently KUrlCompletion only offers autocompletion for hidden folders when 
> you input at least one additional character after the dot. 
> With this patch all hidden folders will be offered when only a dot is present.
> 
> This behaviour is test covered.
> 
> 
> Diffs
> -
> 
>   autotests/kurlcompletiontest.cpp ca8563c 
>   src/widgets/kurlcompletion.cpp c6764e4 
> 
> Diff: https://git.reviewboard.kde.org/r/125988/diff/
> 
> 
> Testing
> ---
> 
> All test cases for KUrlCompletion pass
> 
> 
> Thanks,
> 
> Emmanuel Pescosta
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Purpose as a KDE Framework

[Emmanuel Pescosta]

On Dienstag, 1. Dezember 2015 15:01:10 CET Aleix Pol wrote:

Hi,
looks really interesting!

> I'd like to see it used in other cases than sharing as well as in other 
applications

Would you accept a plugin which can be used to compare selected files? - e.g. 
by starting kompare

This would replace the hard wired "compare" action from DolphinMainWindow with 
a nice plugin.

But anyway, looking forward to make use of it in Dolphin  :)

Cheers,
Emmanuel


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 125988: [KUrlCompletion] Add autocompletion for '.' input which offers all hidden files/folders

[Emmanuel Pescosta]

> On Nov. 26, 2015, 9:20 a.m., David Faure wrote:
> > src/widgets/kurlcompletion.cpp, line 531
> > <https://git.reviewboard.kde.org/r/125988/diff/1/?file=415715#file415715line531>
> >
> > are you sure this hunk is necessary?
> > QUrl has no special handling of "." in paths.
> > So m_fileName = m_kurl.fileName() would also just set it to ".", like 
> > your special code does.
> > 
> > (and if that's right then I don't see the purpose of the two new 
> > vars...)
> > 
> > Wasn't only the last hunk of the patch that really made a difference?

> are you sure this hunk is necessary?

According to the tests yes, I had to add those special handling to make the 
tests pass.
In the second patch I have added some additional tests to prove that the check 
"ends with /. or is equal to ." instead of just "ends with ." is needed.

> Wasn't only the last hunk of the patch that really made a difference?

Yes it made a difference, but it didn't fix all the problems, namely:

* KUrlCompletionTest::testLocalAbsolutePath: Completion with '.', should find 
all hidden folders
* KUrlCompletionTest::testLocalRelativePath: Completion with '.', should find 
all hidden folders


- Emmanuel


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125988/#review88860
---


On Nov. 9, 2015, 9:30 a.m., Emmanuel Pescosta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125988/
> ---
> 
> (Updated Nov. 9, 2015, 9:30 a.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Bugs: 354981
> https://bugs.kde.org/show_bug.cgi?id=354981
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> Currently KUrlCompletion only offers autocompletion for hidden folders when 
> you input at least one additional character after the dot. 
> With this patch all hidden folders will be offered when only a dot is present.
> 
> This behaviour is test covered.
> 
> 
> Diffs
> -
> 
>   autotests/kurlcompletiontest.cpp ca8563c 
>   src/widgets/kurlcompletion.cpp c6764e4 
> 
> Diff: https://git.reviewboard.kde.org/r/125988/diff/
> 
> 
> Testing
> ---
> 
> All test cases for KUrlCompletion pass
> 
> 
> Thanks,
> 
> Emmanuel Pescosta
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 125988: [KUrlCompletion] Add autocompletion for '.' input which offers all hidden files/folders

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125988/
---

(Updated Nov. 28, 2015, 10:36 p.m.)


Review request for KDE Frameworks and David Faure.


Changes
---

Added some additional tests to prove that the check "ends with /. or is equal 
to ." instead of just "ends with ." is needed.


Bugs: 354981
https://bugs.kde.org/show_bug.cgi?id=354981


Repository: kio


Description
---

Currently KUrlCompletion only offers autocompletion for hidden folders when you 
input at least one additional character after the dot. 
With this patch all hidden folders will be offered when only a dot is present.

This behaviour is test covered.


Diffs (updated)
-

  autotests/kurlcompletiontest.cpp ca8563c 
  src/widgets/kurlcompletion.cpp c6764e4 

Diff: https://git.reviewboard.kde.org/r/125988/diff/


Testing
---

All test cases for KUrlCompletion pass


Thanks,

Emmanuel Pescosta

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Review Request 126009: [KUrlNavigator] Fix url encoding problems in KUrlNavigator

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126009/
---

Review request for KDE Frameworks and David Faure.


Bugs: 354678
https://bugs.kde.org/show_bug.cgi?id=354678


Repository: kio


Description
---

Fix url encoding problems (e.g. when the path contains a '%') in KUrlNavigator

These changes also fix a weird behavior of KUrlNavigator when button 0 is
pressed. KUrlNavigator always used the local path '/' for button 0 and ignored
the scheme and authority. The networkPathButtonIndex* and ftpPathButtonIndex*
test rows test exactly this problem.


Diffs
-

  autotests/kurlnavigatortest.h e65e8ef 
  autotests/kurlnavigatortest.cpp 25d1ed6 
  src/filewidgets/kurlnavigator.cpp 28d750e 

Diff: https://git.reviewboard.kde.org/r/126009/diff/


Testing
---

All tests pass


Thanks,

Emmanuel Pescosta

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126009: [KUrlNavigator] Fix url encoding problems in KUrlNavigator

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126009/
---

(Updated Nov. 9, 2015, 10:15 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and David Faure.


Changes
---

Submitted with commit 9dbe36f734b5b839b2a6a934fad29d639e954498 by Emmanuel 
Pescosta to branch master.


Bugs: 354678
https://bugs.kde.org/show_bug.cgi?id=354678


Repository: kio


Description
---

Fix url encoding problems (e.g. when the path contains a '%') in KUrlNavigator

These changes also fix a weird behavior of KUrlNavigator when button 0 is
pressed. KUrlNavigator always used the local path '/' for button 0 and ignored
the scheme and authority. The networkPathButtonIndex* and ftpPathButtonIndex*
test rows test exactly this problem.


Diffs
-

  autotests/kurlnavigatortest.h e65e8ef 
  autotests/kurlnavigatortest.cpp 25d1ed6 
  src/filewidgets/kurlnavigator.cpp 28d750e 

Diff: https://git.reviewboard.kde.org/r/126009/diff/


Testing
---

All tests pass


Thanks,

Emmanuel Pescosta

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Review Request 125988: [KUrlCompletion] Add autocompletion for '.' input which offers all hidden files/folders

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125988/
---

Review request for KDE Frameworks.


Bugs: 354981
https://bugs.kde.org/show_bug.cgi?id=354981


Repository: kio


Description
---

Currently KUrlCompletion only offers autocompletion for hidden folders when you 
input at least one additional character after the dot. 
With this patch all hidden folders will be offered when only a dot is present.

This behaviour is test covered.


Diffs
-

  autotests/kurlcompletiontest.cpp ca8563c 
  src/widgets/kurlcompletion.cpp c6764e4 

Diff: https://git.reviewboard.kde.org/r/125988/diff/


Testing
---

All test cases for KUrlCompletion pass


Thanks,

Emmanuel Pescosta

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 125928: Ship the "new file templates" in the kiofilewidgets library using a .qrc

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125928/#review87903
---


+1 otherwise :)


src/filewidgets/knewfilemenu.cpp (lines 581 - 582)
<https://git.reviewboard.kde.org/r/125928/#comment60305>

You can hand over the srcFile directly to storedPut, storedPut has an 
overload which takes a QIODevice


- Emmanuel Pescosta


On Nov. 3, 2015, 9:24 a.m., David Faure wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125928/
> ---
> 
> (Updated Nov. 3, 2015, 9:24 a.m.)
> 
> 
> Review request for KDE Frameworks, Christoph Cullmann, Frank Reininghaus, Rex 
> Dieter, and Harald Sitter.
> 
> 
> Bugs: 353642
> https://bugs.kde.org/show_bug.cgi?id=353642
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This solves the conflict with kde-baseapps 15.08.x, and simplifies deployment
> on Win/Mac.
> 
> BUG: 353642
> Change-Id: Id4f9e364ceeea6b1b93910aa100ff22ce3fd76a7
> 
> 
> Diffs
> -
> 
>   src/new_file_templates/templates.qrc PRE-CREATION 
>   autotests/knewfilemenutest.cpp 968e16e9e8c5d0680c1e6e00af1403f3f5d9976f 
>   src/CMakeLists.txt 27b0f2b0e86a0087d343226e8db056109b3cd5ab 
>   src/filewidgets/CMakeLists.txt 6ec254971055d1d1ecf5cf7a6d4637b82a52c27d 
>   src/filewidgets/knewfilemenu.cpp 577206dfa9c89c4db4750f1880547bfb9b6214e7 
>   src/new_file_templates/CMakeLists.txt 
> 0684b1871b35757ebe59c71cff01104bc388bb0f 
> 
> Diff: https://git.reviewboard.kde.org/r/125928/diff/
> 
> 
> Testing
> ---
> 
> Unittest (./knewfilemenutest)
> 
> 
> Thanks,
> 
> David Faure
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 125788: [kio thumbnailer] Recognize image/webp

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125788/#review87359
---

Ship it!


Ship It!

- Emmanuel Pescosta


On Oct. 25, 2015, 12:51 p.m., Kai Uwe Broulik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125788/
> ---
> 
> (Updated Oct. 25, 2015, 12:51 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kio-extras
> 
> 
> Description
> ---
> 
> When I download a .webp file the system claims it to be image/webp, not 
> image/x-webp which we already support.
> 
> 
> Diffs
> -
> 
>   thumbnail/imagethumbnail.desktop e64341b 
> 
> Diff: https://git.reviewboard.kde.org/r/125788/diff/
> 
> 
> Testing
> ---
> 
> I now get thumbnails for .webp files
> 
> 
> Thanks,
> 
> Kai Uwe Broulik
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Review Request 125538: [kio-trash] Add file system freespace retrieval to trash kioslave

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125538/
---

Review request for KDE Frameworks and David Faure.


Repository: kio


Description
---

Gives the user info about the trash occupancy when opening the trash


Diffs
-

  src/ioslaves/trash/kio_trash.h e6160c4 
  src/ioslaves/trash/kio_trash.cpp 7ca8ad0 
  src/ioslaves/trash/trashimpl.h 9881d8e 
  src/ioslaves/trash/trashimpl.cpp 5a2832a 

Diff: https://git.reviewboard.kde.org/r/125538/diff/


Testing
---

Works fine (with and without maximum size limit)


Thanks,

Emmanuel Pescosta

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 125538: [kio-trash] Add file system freespace retrieval to trash kioslave

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125538/
---

(Updated Oct. 6, 2015, 7:40 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and David Faure.


Changes
---

Submitted with commit 6128e94f689d416abb60255bb3792f127e52783c by Emmanuel 
Pescosta to branch master.


Repository: kio


Description
---

Gives the user info about the trash occupancy when opening the trash


Diffs
-

  src/ioslaves/trash/kio_trash.h e6160c4 
  src/ioslaves/trash/kio_trash.cpp 7ca8ad0 
  src/ioslaves/trash/trashimpl.h 9881d8e 
  src/ioslaves/trash/trashimpl.cpp 5a2832a 

Diff: https://git.reviewboard.kde.org/r/125538/diff/


Testing
---

Works fine (with and without maximum size limit)


Thanks,

Emmanuel Pescosta

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 125538: [kio-trash] Add file system freespace retrieval to trash kioslave

[Emmanuel Pescosta]

> On Oct. 6, 2015, 10:27 a.m., Aleix Pol Gonzalez wrote:
> > src/ioslaves/trash/kio_trash.cpp, line 617
> > <https://git.reviewboard.kde.org/r/125538/diff/1/?file=410023#file410023line617>
> >
> > Maybe use qCDebug? So it can be disabled and so.

Trash doesn't use categorized logging atm so I decided to use qDebug.

IMHO categorized logging should be added in a separate patch instead.


- Emmanuel


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125538/#review86411
---


On Oct. 6, 2015, 9:40 a.m., Emmanuel Pescosta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125538/
> ---
> 
> (Updated Oct. 6, 2015, 9:40 a.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> Gives the user info about the trash occupancy when opening the trash
> 
> 
> Diffs
> -
> 
>   src/ioslaves/trash/kio_trash.h e6160c4 
>   src/ioslaves/trash/kio_trash.cpp 7ca8ad0 
>   src/ioslaves/trash/trashimpl.h 9881d8e 
>   src/ioslaves/trash/trashimpl.cpp 5a2832a 
> 
> Diff: https://git.reviewboard.kde.org/r/125538/diff/
> 
> 
> Testing
> ---
> 
> Works fine (with and without maximum size limit)
> 
> 
> Thanks,
> 
> Emmanuel Pescosta
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 125400: Allow trash: in addition to trash:/ as url for listDir

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125400/
---

(Updated Sept. 26, 2015, 9:54 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and David Faure.


Changes
---

Submitted with commit c89897d0aa6bbc926ed860a3a050d6aaae007a9c by Emmanuel 
Pescosta to branch master.


Bugs: 353181
https://bugs.kde.org/show_bug.cgi?id=353181


Repository: kio


Description
---

Allow trash: in addition to trash:/ as url for listDir (calls listRoot)

BUG: 353181
FIXED-IN: 5.15.0

Same behavior as Nautilus ;)


Diffs
-

  src/ioslaves/trash/kio_trash.cpp 1597833 

Diff: https://git.reviewboard.kde.org/r/125400/diff/


Testing
---


Thanks,

Emmanuel Pescosta

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Review Request 125044: Fix crash when hovering the 'Activities' context menu entry and constantly moving the mouse cursor around

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125044/
---

Review request for KDE Frameworks and Ivan Čukić.


Bugs: 352207
http://bugs.kde.org/show_bug.cgi?id=352207


Repository: kactivities


Description
---

Only spawn one action loader (thread) when the actions of the 
FileItemLinkingPlugin are not initialized already.


Diffs
-

  src/workspace/fileitemplugin/FileItemLinkingPlugin.cpp 353d941 
  src/workspace/fileitemplugin/FileItemLinkingPlugin_p.h 05a39b4 

Diff: https://git.reviewboard.kde.org/r/125044/diff/


Testing
---


Thanks,

Emmanuel Pescosta

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 125044: Fix crash when hovering the 'Activities' context menu entry and constantly moving the mouse cursor around

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125044/
---

(Updated Sept. 4, 2015, 8:08 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Ivan Čukić.


Changes
---

Submitted with commit 821abd02047f15aceb106cd390df2cea502c72bf by Emmanuel 
Pescosta to branch master.


Bugs: 351585 and 352207
http://bugs.kde.org/show_bug.cgi?id=351585
http://bugs.kde.org/show_bug.cgi?id=352207


Repository: kactivities


Description
---

Only spawn one action loader (thread) when the actions of the 
FileItemLinkingPlugin are not initialized already.


Diffs
-

  src/workspace/fileitemplugin/FileItemLinkingPlugin.cpp 353d941 
  src/workspace/fileitemplugin/FileItemLinkingPlugin_p.h 05a39b4 

Diff: https://git.reviewboard.kde.org/r/125044/diff/


Testing
---


Thanks,

Emmanuel Pescosta

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 125044: Fix crash when hovering the 'Activities' context menu entry and constantly moving the mouse cursor around

[Emmanuel Pescosta]

> On Sept. 4, 2015, 3:14 p.m., Luca Beltrame wrote:
> > Can you check whether it also fixes bug 351585? To me the issue seems 
> > related.
> 
> Luca Beltrame wrote:
> I applied the patch locally, fixes also the aforementioned bug.
> 
> Luca Beltrame wrote:
> It just occurred to me that kactivities does not use RB for patch 
> tracking. You need to use phabricator, at phabricator.kde.org.
> 
> Ivan Čukić wrote:
> +1 for Phab

> I applied the patch locally, fixes also the aforementioned bug.

Great thanks for testing!

> +1 for Phab

Next time ;)


- Emmanuel


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125044/#review84820
---


On Sept. 4, 2015, 3:01 p.m., Emmanuel Pescosta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125044/
> ---
> 
> (Updated Sept. 4, 2015, 3:01 p.m.)
> 
> 
> Review request for KDE Frameworks and Ivan Čukić.
> 
> 
> Bugs: 352207
> http://bugs.kde.org/show_bug.cgi?id=352207
> 
> 
> Repository: kactivities
> 
> 
> Description
> ---
> 
> Only spawn one action loader (thread) when the actions of the 
> FileItemLinkingPlugin are not initialized already.
> 
> 
> Diffs
> -
> 
>   src/workspace/fileitemplugin/FileItemLinkingPlugin.cpp 353d941 
>   src/workspace/fileitemplugin/FileItemLinkingPlugin_p.h 05a39b4 
> 
> Diff: https://git.reviewboard.kde.org/r/125044/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Emmanuel Pescosta
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 125044: Fix crash when hovering the 'Activities' context menu entry and constantly moving the mouse cursor around

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125044/
---

(Updated Sept. 4, 2015, 10:06 p.m.)


Review request for KDE Frameworks and Ivan Čukić.


Changes
---

Added bug 351585 from Luca Beltrame, which is also fixed with this patch


Bugs: 351585 and 352207
http://bugs.kde.org/show_bug.cgi?id=351585
http://bugs.kde.org/show_bug.cgi?id=352207


Repository: kactivities


Description
---

Only spawn one action loader (thread) when the actions of the 
FileItemLinkingPlugin are not initialized already.


Diffs
-

  src/workspace/fileitemplugin/FileItemLinkingPlugin.cpp 353d941 
  src/workspace/fileitemplugin/FileItemLinkingPlugin_p.h 05a39b4 

Diff: https://git.reviewboard.kde.org/r/125044/diff/


Testing
---


Thanks,

Emmanuel Pescosta

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 124690: Show custom folder icons again

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124690/
---

(Updated Aug. 11, 2015, 6:18 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks, Àlex Fiestas and David Faure.


Changes
---

Submitted with commit d8aafbc37daab3cd62ee77430a55b3d72ccc12e0 by Emmanuel 
Pescosta to branch master.


Bugs: 350612
https://bugs.kde.org/show_bug.cgi?id=350612


Repository: kio


Description
---

Show custom folder icons again

Set mimetype as known when we assign the mime type for inode/directory

This bug was introduced by commit 46cd152f

BUG: 350612
FIXED-IN: 5.14


Diffs
-

  autotests/kfileitemtest.h 95612ed 
  autotests/kfileitemtest.cpp 4a87469 
  src/core/kfileitem.cpp 344ac67 

Diff: https://git.reviewboard.kde.org/r/124690/diff/


Testing
---

KFileItemTest::testIconNameForCustomFolderIcons() fails without the patch but 
works with the patch.

Folder icons are displayed properly again in Dolphin.


Thanks,

Emmanuel Pescosta

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Review Request 124690: Show custom folder icons again

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124690/
---

Review request for KDE Frameworks, Àlex Fiestas and David Faure.


Bugs: 350612
https://bugs.kde.org/show_bug.cgi?id=350612


Repository: kio


Description
---

Show custom folder icons again

Set mimetype as known when we assign the mime type for inode/directory

This bug was introduced by commit 46cd152f

BUG: 350612
FIXED-IN: 5.14


Diffs
-

  autotests/kfileitemtest.h 95612ed 
  autotests/kfileitemtest.cpp 4a87469 
  src/core/kfileitem.cpp 344ac67 

Diff: https://git.reviewboard.kde.org/r/124690/diff/


Testing
---

KFileItemTest::testIconNameForCustomFolderIcons() fails without the patch but 
works with the patch.

Folder icons are displayed properly again in Dolphin.


Thanks,

Emmanuel Pescosta

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 123940: Use KPluginLoader::factory() when loading KIO::DndPopupMenuPlugin

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123940/#review80967
---

Ship it!


Looks good otherwise. :)


src/widgets/dropjob.cpp (line 297)
https://git.reviewboard.kde.org/r/123940/#comment55465

The result of create can be null (on error).


- Emmanuel Pescosta


On May 30, 2015, 12:09 a.m., Ragnar Thomsen wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/123940/
 ---
 
 (Updated May 30, 2015, 12:09 a.m.)
 
 
 Review request for Dolphin, KDE Frameworks, Emmanuel Pescosta, and Harald 
 Hvaal.
 
 
 Repository: kio
 
 
 Description
 ---
 
 This patch fixes an error when loading the DndPopupMenuPlugin of Ark 
 (frameworks branch). DndPopupMenuPlugins use the K_PLUGIN_FACTORY_WITH_JSON 
 macro, and therefore KPluginLoader::factory() should be used instead of 
 KPluginLoader::instance() which is used by KPluginMetaData::instantiate().
 
 The DndPopupMenuPlugin is used by Ark to enable the Extract here menu 
 option when dragging an archive in Dolphin.
 
 
 Diffs
 -
 
   src/widgets/dropjob.cpp 63beb0a 
 
 Diff: https://git.reviewboard.kde.org/r/123940/diff/
 
 
 Testing
 ---
 
 The Extract here option now appears when dragging an archive in Dolphin 
 (current frameworks branch), and extraction works as expected.
 
 
 Thanks,
 
 Ragnar Thomsen
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 123525: Remove bookmarks syncing from KFilePlacesModel and use user-places.xbel only.

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123525/
---

(Updated April 29, 2015, 1:38 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks.


Changes
---

Submitted with commit e919daeb5b144b146dbf459fadb116729b866d21 by Emmanuel 
Pescosta to branch master.


Bugs: 345174
https://bugs.kde.org/show_bug.cgi?id=345174


Repository: kio


Description
---

Remove bookmarks syncing from KFilePlacesModel and use user-places.xbel only, 
as discussed on the frameworks ML.


Diffs
-

  src/filewidgets/CMakeLists.txt 37c3f26 
  src/filewidgets/kfileplacesmodel.cpp 6c92012 
  src/filewidgets/kfileplacessharedbookmarks.cpp 3e7ffb9 
  src/filewidgets/kfileplacessharedbookmarks_p.h 2f9f4b8 

Diff: https://git.reviewboard.kde.org/r/123525/diff/


Testing
---


Thanks,

Emmanuel Pescosta

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 123523: KPropertiesDialog::KPropertiesDialog(urls) and KPropertiesDialog::showDialog(urls)

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123523/
---

(Updated April 29, 2015, 1:38 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks.


Changes
---

Submitted with commit e41e0c922a641cea68a1f2d360bb2bb0fa8896f6 by Emmanuel 
Pescosta to branch master.


Repository: kio


Description
---

Added KPropertiesDialog::KPropertiesDialog(urls) and 
KPropertiesDialog::showDialog(urls) as convenience methods for non-file-manager 
applications, where you have a list of QUrls rather than a KFileItemList.


Diffs
-

  src/widgets/kpropertiesdialog.h 8d98b35 
  src/widgets/kpropertiesdialog.cpp 37fa88f 

Diff: https://git.reviewboard.kde.org/r/123523/diff/


Testing
---


Thanks,

Emmanuel Pescosta

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Review Request 123523: KPropertiesDialog::KPropertiesDialog(urls) and KPropertiesDialog::showDialog(urls)

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123523/
---

Review request for KDE Frameworks.


Repository: kio


Description
---

Added KPropertiesDialog::KPropertiesDialog(urls) and 
KPropertiesDialog::showDialog(urls) as convenience methods for non-file-manager 
applications, where you have a list of QUrls rather than a KFileItemList.


Diffs
-

  src/widgets/kpropertiesdialog.h 8d98b35 
  src/widgets/kpropertiesdialog.cpp 37fa88f 

Diff: https://git.reviewboard.kde.org/r/123523/diff/


Testing
---


Thanks,

Emmanuel Pescosta

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 123525: Remove bookmarks syncing from KFilePlacesModel and use user-places.xbel only.

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123525/
---

(Updated April 27, 2015, 2:23 p.m.)


Review request for KDE Frameworks.


Bugs: 345174
https://bugs.kde.org/show_bug.cgi?id=345174


Repository: kio


Description
---

Remove bookmarks syncing from KFilePlacesModel and use user-places.xbel only, 
as discussed on the frameworks ML.


Diffs
-

  src/filewidgets/CMakeLists.txt 37c3f26 
  src/filewidgets/kfileplacesmodel.cpp 6c92012 
  src/filewidgets/kfileplacessharedbookmarks.cpp 3e7ffb9 
  src/filewidgets/kfileplacessharedbookmarks_p.h 2f9f4b8 

Diff: https://git.reviewboard.kde.org/r/123525/diff/


Testing
---


Thanks,

Emmanuel Pescosta

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Review Request 123525: Remove bookmarks syncing from KFilePlacesModel and use user-places.xbel only.

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123525/
---

Review request for KDE Frameworks.


Repository: kio


Description
---

Remove bookmarks syncing from KFilePlacesModel and use user-places.xbel only, 
as discussed on the frameworks ML.


Diffs
-

  src/filewidgets/CMakeLists.txt 37c3f26 
  src/filewidgets/kfileplacesmodel.cpp 6c92012 
  src/filewidgets/kfileplacessharedbookmarks.cpp 3e7ffb9 
  src/filewidgets/kfileplacessharedbookmarks_p.h 2f9f4b8 

Diff: https://git.reviewboard.kde.org/r/123525/diff/


Testing
---


Thanks,

Emmanuel Pescosta

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 123523: KPropertiesDialog::KPropertiesDialog(urls) and KPropertiesDialog::showDialog(urls)

[Emmanuel Pescosta]

 On April 27, 2015, 5:10 p.m., David Faure wrote:
  src/widgets/kpropertiesdialog.cpp, line 279
  https://git.reviewboard.kde.org/r/123523/diff/1/?file=363715#file363715line279
 
  Ouch! A nested event loop! Nasty. Opens the door to unexpected 
  reentrancy
  
  Not to mention blocking the GUI for a very long time if this is done 
  with many urls over a very slow protocol.
  
  Better write a multi-stat job if we don't have one, and let the 
  application use that, and open the properties dialog from a slot.
  
  Alternatively, the properties dialog could itself delay showing itself 
  until it has stat'ed the urls (using a slot, not a blocking nested event 
  loop).

Copied the code from KPropertiesDialog(url) incl. the warning from 
showDialog(url), but I agree that this
should be fixed. (also for KPropertiesDialog(url))

 Better write a multi-stat job if we don't have one

Ok I'll have a look at it :)


- Emmanuel


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123523/#review79577
---


On April 27, 2015, 1:49 p.m., Emmanuel Pescosta wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/123523/
 ---
 
 (Updated April 27, 2015, 1:49 p.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Repository: kio
 
 
 Description
 ---
 
 Added KPropertiesDialog::KPropertiesDialog(urls) and 
 KPropertiesDialog::showDialog(urls) as convenience methods for 
 non-file-manager applications, where you have a list of QUrls rather than a 
 KFileItemList.
 
 
 Diffs
 -
 
   src/widgets/kpropertiesdialog.h 8d98b35 
   src/widgets/kpropertiesdialog.cpp 37fa88f 
 
 Diff: https://git.reviewboard.kde.org/r/123523/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Emmanuel Pescosta
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Places bookmark bug - Help needed

[Emmanuel Pescosta]

Hi everyone,

I want to ask you, if you can help me to fix the Places bookmarks
disappear bug [1], which was introduced by commit
2b5c6e42fc8114c6854ce490b9cdd4356be683d9 in kdelibs [2].

The adoption for Dolphin [3] has no effect on this bug.

I have done some research and I'm a little bit confused why
KFilePlacesModel has a (not exported) KFilePlacesSharedBookmarks, which
syncs the bookmarks (user-places.xbel and kfileplaces/bookmarks.xml) on
each change. What is the purpose of it? Is it to keep backward
compatibility for bookmarks.xml users?

My assumption is, that before the bookmarks race condition has been fixed,
each places change in Dolphin implicitly triggered a bookmarks sync in the
KFilePlacesModel.
Do we need to sync the bookmarks in Dolphin as well? But why do we store
the bookmarks in two files? Why not only in bookmarks.xml or
user-places.xbel?

So the possible solutions I see are:
* Store the bookmarks in one file (user-places.xbel) and remove the syncing
* Move KFilePlacesSharedBookmarks to KBookmarks and integrate it into the
current code so that all clients of KBookmarks automatically make use of it

Thanks in advance! :)

Cheers,
Emmanuel

[1] https://bugs.kde.org/show_bug.cgi?id=345174
[2]
http://quickgit.kde.org/?p=kdelibs.gita=commith=2b5c6e42fc8114c6854ce490b9cdd4356be683d9
[3] https://git.reviewboard.kde.org/r/122640/
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 123443: Make it possible to call a put job from an IODevice

[Emmanuel Pescosta]

 On April 22, 2015, 9 a.m., Emmanuel Pescosta wrote:
  src/widgets/accessmanager.cpp, line 255
  https://git.reviewboard.kde.org/r/123443/diff/1/?file=362233#file362233line255
 
  IMO you should always verify if the data is readable, because not all 
  lib users are trustworthy (assert isn't meant for input validation of 
  untrustworthy callers - public api)
 
 Aleix Pol Gonzalez wrote:
 I can do it, but QNetworkAccessManager doesn't do that, and I'd say it's 
 better to be as similar to QNAM as possible.

Ah ok sry for the noice, didn't know that.
Just assumed that they validate it because they mention data must be opened 
for reading in their API docs.


- Emmanuel


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123443/#review79325
---


On April 22, 2015, 1:46 a.m., Aleix Pol Gonzalez wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/123443/
 ---
 
 (Updated April 22, 2015, 1:46 a.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Repository: kio
 
 
 Description
 ---
 
 Also adopts it when using KIO::AccessManager::put. Otherwise it was doing a 
 ::readAll and then passed the QByteArray around, which isn't very elegant.
 
 
 Diffs
 -
 
   autotests/accessmanagertest.cpp 5e4988f 
   autotests/jobtest.h ef0ec57 
   autotests/jobtest.cpp b11e9f9 
   src/core/storedtransferjob.h d3e1106 
   src/core/storedtransferjob.cpp 7f81d00 
   src/widgets/accessmanager.cpp 239281e 
 
 Diff: https://git.reviewboard.kde.org/r/123443/diff/
 
 
 Testing
 ---
 
 New test for the new method.
 Tests still pass.
 
 
 Thanks,
 
 Aleix Pol Gonzalez
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 123443: Make it possible to call a put job from an IODevice

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123443/#review79325
---



src/widgets/accessmanager.cpp (line 255)
https://git.reviewboard.kde.org/r/123443/#comment54156

IMO you should always verify if the data is readable, because not all lib 
users are trustworthy (assert isn't meant for input validation of untrustworthy 
callers - public api)


- Emmanuel Pescosta


On April 22, 2015, 1:46 a.m., Aleix Pol Gonzalez wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/123443/
 ---
 
 (Updated April 22, 2015, 1:46 a.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Repository: kio
 
 
 Description
 ---
 
 Also adopts it when using KIO::AccessManager::put. Otherwise it was doing a 
 ::readAll and then passed the QByteArray around, which isn't very elegant.
 
 
 Diffs
 -
 
   autotests/accessmanagertest.cpp 5e4988f 
   autotests/jobtest.h ef0ec57 
   autotests/jobtest.cpp b11e9f9 
   src/core/storedtransferjob.h d3e1106 
   src/core/storedtransferjob.cpp 7f81d00 
   src/widgets/accessmanager.cpp 239281e 
 
 Diff: https://git.reviewboard.kde.org/r/123443/diff/
 
 
 Testing
 ---
 
 New test for the new method.
 Tests still pass.
 
 
 Thanks,
 
 Aleix Pol Gonzalez
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 122616: Use KIO::FileSystemFreeSpaceJob in KPropertiesDialog

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122616/
---

(Updated March 20, 2015, 11:18 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks.


Changes
---

Submitted with commit 9793f3ae8d1739bec2a4a167f249c4f270aabdad by Emmanuel 
Pescosta to branch master.


Repository: kio


Description
---

Use KIO::FileSystemFreeSpaceJob to retrieve free space information in 
KPropertiesDialog, this change enables KPropertiesDialog to show free space 
information of remote file systems as well (e.g. smb)

When the free space job failed Unknown size will be shown (same behavior as 
in Dolphin)


Diffs
-

  src/widgets/kpropertiesdialog.cpp 0dc4881 
  src/widgets/kpropertiesdialog_p.h 5858eed 

Diff: https://git.reviewboard.kde.org/r/122616/diff/


Testing
---

Works for local and smb file systems


Thanks,

Emmanuel Pescosta

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 123081: sftp ioslave: Check for stat...@openssh.com extension in sftpProtocol::fileSystemFreeSpace

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123081/
---

(Updated March 20, 2015, 2:50 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Andreas Schneider.


Changes
---

Submitted with commit e8d5a84156c6fc94d12894fb8ef9070b1b5cce99 by Emmanuel 
Pescosta to branch master.


Repository: kio-extras


Description
---

Check if the ssh server supports the stat...@openssh.com extension before we 
make use of it.

@anschneider:
I don't know if you prefer ==0 or ! in this case, because kio_sftp.cpp uses 
all two styles.


Diffs
-

  sftp/kio_sftp.cpp 36bb28b 

Diff: https://git.reviewboard.kde.org/r/123081/diff/


Testing
---


Thanks,

Emmanuel Pescosta

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Review Request 123081: sftp ioslave: Check for stat...@openssh.com extension in sftpProtocol::fileSystemFreeSpace

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123081/
---

Review request for KDE Frameworks and Andreas Schneider.


Repository: kio-extras


Description
---

Check if the ssh server supports the stat...@openssh.com extension before we 
make use of it.

@anschneider:
I don't know if you prefer ==0 or ! in this case, because kio_sftp.cpp uses 
all two styles.


Diffs
-

  sftp/kio_sftp.cpp 36bb28b 

Diff: https://git.reviewboard.kde.org/r/123081/diff/


Testing
---


Thanks,

Emmanuel Pescosta

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 122978: Add unit test for crash of bug 343976

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122978/#review77577
---



src/platformtheme/kdeplatformsystemtrayicon.cpp
https://git.reviewboard.kde.org/r/122978/#comment53287

unchecked m_menu


- Emmanuel Pescosta


On March 16, 2015, 1:04 p.m., Martin Gräßlin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/122978/
 ---
 
 (Updated March 16, 2015, 1:04 p.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Bugs: 343976
 https://bugs.kde.org/show_bug.cgi?id=343976
 
 
 Repository: frameworkintegration
 
 
 Description
 ---
 
 Use a QPointer for QMenu of SystemTrayMenu
 
 We need better memory management as the QMenu the SystemTrayMenu creates
 might be deleted externally causing a double delete if the so-far
 QScopedPoiter cleans up.
 
 Switch to QPointer and call deleteLater if the pointer is still valid
 during destroying the SystemTrayMenu.
 
 BUG: 343976
 CHANGELOG: Fix possible crash when destroying a QSystemTrayIcon (triggered by 
 Trojita)
 
 
 Diffs
 -
 
   autotests/CMakeLists.txt e8ed6a99bb45969231ba21b8c5588e093bbbe3ee 
   autotests/ksni_unittest.cpp PRE-CREATION 
   src/platformtheme/kdeplatformsystemtrayicon.h 
 8f13a36ae7cebed7499ce8a160f3533665b909ac 
   src/platformtheme/kdeplatformsystemtrayicon.cpp 
 ce3d3de9f9cf479a5fe3af65ef7fb8ec2b9e0d29 
 
 Diff: https://git.reviewboard.kde.org/r/122978/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Martin Gräßlin
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 122614: Fix a porting bug in the http ioslave

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122614/
---

(Updated March 7, 2015, 11:33 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks.


Repository: kio


Description
---

Fix the file/directory name extraction + base dir handling in the http ioslave

name was always empty for directories because thisURL ends with a slash and 
fileName() will return an empty string, so href.text() was used as UDS_NAME 
instead.


Diffs
-

  src/ioslaves/http/http.cpp 78f50f9 

Diff: https://git.reviewboard.kde.org/r/122614/diff/


Testing
---

Works (see screenshots)


File Attachments


Webdav (Bug)
  
https://git.reviewboard.kde.org/media/uploaded/files/2015/02/18/d0ae2572-3770-4781-88e3-4d40e626fd85__webdav_bug.png
Webdav (Fixed)
  
https://git.reviewboard.kde.org/media/uploaded/files/2015/02/18/de96eb2e-45d8-4388-83b6-77199f677c75__webdav_fixed.png


Thanks,

Emmanuel Pescosta

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 122620: Implement quota-used-bytes and quota-available-bytes in http ioslave

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122620/
---

(Updated March 7, 2015, 1:18 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks.


Repository: kio


Description
---

Implement quota-used-bytes and quota-available-bytes from RFC 4331 to enable 
free space information in http ioslave.


Diffs
-

  src/ioslaves/http/http.h f0dc027 
  src/ioslaves/http/http.cpp 78f50f9 

Diff: https://git.reviewboard.kde.org/r/122620/diff/


Testing
---

Tested with Owncloud (share.kde.org)


File Attachments


webdav_freespace.png
  
https://git.reviewboard.kde.org/media/uploaded/files/2015/02/18/c90a8f70-e12b-44b3-a0b8-651f1ee7d602__webdav_freespace.png


Thanks,

Emmanuel Pescosta

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 122621: Implemented SlaveBase::GetFileSystemFreeSpace in sftp ioslave

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122621/#review77149
---


Ping

- Emmanuel Pescosta


On Feb. 18, 2015, 12:14 p.m., Emmanuel Pescosta wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/122621/
 ---
 
 (Updated Feb. 18, 2015, 12:14 p.m.)
 
 
 Review request for KDE Frameworks and Andreas Schneider.
 
 
 Repository: kio-extras
 
 
 Description
 ---
 
 Implemented SlaveBase::GetFileSystemFreeSpace to enable free space 
 information in sftp ioslave.
 
 
 Diffs
 -
 
   sftp/kio_sftp.h e088472 
   sftp/kio_sftp.cpp 057fcd4 
 
 Diff: https://git.reviewboard.kde.org/r/122621/diff/
 
 
 Testing
 ---
 
 Works
 
 
 Thanks,
 
 Emmanuel Pescosta
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 122620: Implement quota-used-bytes and quota-available-bytes in http ioslave

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122620/#review77164
---


ping


src/ioslaves/http/http.h
https://git.reviewboard.kde.org/r/122620/#comment53015

use a better comment


- Emmanuel Pescosta


On Feb. 18, 2015, 11:05 a.m., Emmanuel Pescosta wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/122620/
 ---
 
 (Updated Feb. 18, 2015, 11:05 a.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Repository: kio
 
 
 Description
 ---
 
 Implement quota-used-bytes and quota-available-bytes from RFC 4331 to enable 
 free space information in http ioslave.
 
 
 Diffs
 -
 
   src/ioslaves/http/http.h f0dc027 
   src/ioslaves/http/http.cpp 78f50f9 
 
 Diff: https://git.reviewboard.kde.org/r/122620/diff/
 
 
 Testing
 ---
 
 Tested with Owncloud (share.kde.org)
 
 
 File Attachments
 
 
 webdav_freespace.png
   
 https://git.reviewboard.kde.org/media/uploaded/files/2015/02/18/c90a8f70-e12b-44b3-a0b8-651f1ee7d602__webdav_freespace.png
 
 
 Thanks,
 
 Emmanuel Pescosta
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 122755: Add testcases for KIO::del()

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122755/#review77157
---



autotests/deletejobtest.cpp
https://git.reviewboard.kde.org/r/122755/#comment53013

This has broken the KIO build 
(http://build.kde.org/job/kio_stable_qt5/123/console)

QSignalSpy::QSignalSpy(const QObject * object, PointerToMemberFunction 
signal) is available since Qt 5.4 but KIO uses Qt 5.2

Please use the old QSignalSpy::QSignalSpy(const QObject * object, const 
char * signal) instead

This applies for all other QSignalSpy usages in this test


- Emmanuel Pescosta


On March 6, 2015, 10:57 p.m., Martin Blumenstingl wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/122755/
 ---
 
 (Updated March 6, 2015, 10:57 p.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Repository: kio
 
 
 Description
 ---
 
 This adds some automated testcases for KIO::del().
 
 My own code was buggy but I didn't notice this until I wrote those tests.
 Maybe you want to have them upstream to make sure noone breaks KIO::del.
 
 
 Diffs
 -
 
   autotests/deletejobtest.cpp PRE-CREATION 
   autotests/CMakeLists.txt f613c1a 
   autotests/deletejobtest.h PRE-CREATION 
 
 Diff: https://git.reviewboard.kde.org/r/122755/diff/
 
 
 Testing
 ---
 
 Tests are passing
 
 
 Thanks,
 
 Martin Blumenstingl
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 122614: Fix a porting bug in the http ioslave

[Emmanuel Pescosta]

 On March 7, 2015, 10:42 a.m., David Faure wrote:
  src/ioslaves/http/http.cpp, line 854
  https://git.reviewboard.kde.org/r/122614/diff/1/?file=350094#file350094line854
 
  I don't like the use of section, when we have QUrl API to do this 
  cleanly.
  
  Store thisURL.adjusted(StripTrailingSlash), and use fileName() on that?
 
 David Faure wrote:
 Heh ok race condition ;)

Fixed 14 minutes ago :)


- Emmanuel


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122614/#review77150
---


On March 7, 2015, 10:29 a.m., Emmanuel Pescosta wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/122614/
 ---
 
 (Updated March 7, 2015, 10:29 a.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Repository: kio
 
 
 Description
 ---
 
 Fix the file/directory name extraction + base dir handling in the http ioslave
 
 name was always empty for directories because thisURL ends with a slash and 
 fileName() will return an empty string, so href.text() was used as UDS_NAME 
 instead.
 
 
 Diffs
 -
 
   src/ioslaves/http/http.cpp 78f50f9 
 
 Diff: https://git.reviewboard.kde.org/r/122614/diff/
 
 
 Testing
 ---
 
 Works (see screenshots)
 
 
 File Attachments
 
 
 Webdav (Bug)
   
 https://git.reviewboard.kde.org/media/uploaded/files/2015/02/18/d0ae2572-3770-4781-88e3-4d40e626fd85__webdav_bug.png
 Webdav (Fixed)
   
 https://git.reviewboard.kde.org/media/uploaded/files/2015/02/18/de96eb2e-45d8-4388-83b6-77199f677c75__webdav_fixed.png
 
 
 Thanks,
 
 Emmanuel Pescosta
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 122621: Implemented SlaveBase::GetFileSystemFreeSpace in sftp ioslave

[Emmanuel Pescosta]

 On March 7, 2015, 11:05 a.m., David Faure wrote:
  sftp/kio_sftp.h, line 215
  https://git.reviewboard.kde.org/r/122621/diff/1/?file=350140#file350140line215
 
  not really a todo for this particular class then, it wouldn't change ;)

Just copied the comment from smb ioslave, but I agree that this comment is 
really bad - changed it ;)


- Emmanuel


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122621/#review77155
---


On Feb. 18, 2015, 12:14 p.m., Emmanuel Pescosta wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/122621/
 ---
 
 (Updated Feb. 18, 2015, 12:14 p.m.)
 
 
 Review request for KDE Frameworks and Andreas Schneider.
 
 
 Repository: kio-extras
 
 
 Description
 ---
 
 Implemented SlaveBase::GetFileSystemFreeSpace to enable free space 
 information in sftp ioslave.
 
 
 Diffs
 -
 
   sftp/kio_sftp.h e088472 
   sftp/kio_sftp.cpp 057fcd4 
 
 Diff: https://git.reviewboard.kde.org/r/122621/diff/
 
 
 Testing
 ---
 
 Works
 
 
 Thanks,
 
 Emmanuel Pescosta
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 122614: Fix a porting bug in the http ioslave

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122614/
---

(Updated March 7, 2015, 10:29 a.m.)


Review request for KDE Frameworks.


Changes
---

use QUrl::fileName instead of QString::section(-1)


Repository: kio


Description
---

Fix the file/directory name extraction + base dir handling in the http ioslave

name was always empty for directories because thisURL ends with a slash and 
fileName() will return an empty string, so href.text() was used as UDS_NAME 
instead.


Diffs (updated)
-

  src/ioslaves/http/http.cpp 78f50f9 

Diff: https://git.reviewboard.kde.org/r/122614/diff/


Testing
---

Works (see screenshots)


File Attachments


Webdav (Bug)
  
https://git.reviewboard.kde.org/media/uploaded/files/2015/02/18/d0ae2572-3770-4781-88e3-4d40e626fd85__webdav_bug.png
Webdav (Fixed)
  
https://git.reviewboard.kde.org/media/uploaded/files/2015/02/18/de96eb2e-45d8-4388-83b6-77199f677c75__webdav_fixed.png


Thanks,

Emmanuel Pescosta

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 122621: Implemented SlaveBase::GetFileSystemFreeSpace in sftp ioslave

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122621/
---

(Updated March 7, 2015, 11:42 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Andreas Schneider.


Repository: kio-extras


Description
---

Implemented SlaveBase::GetFileSystemFreeSpace to enable free space information 
in sftp ioslave.


Diffs
-

  sftp/kio_sftp.h e088472 
  sftp/kio_sftp.cpp 057fcd4 

Diff: https://git.reviewboard.kde.org/r/122621/diff/


Testing
---

Works


Thanks,

Emmanuel Pescosta

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 122616: Use KIO::FileSystemFreeSpaceJob in KPropertiesDialog

[Emmanuel Pescosta]

 On March 7, 2015, 11:01 a.m., David Faure wrote:
  Ship It!

Will push it after 5.8 has been tagged, because it introduces a new i18n 
string: Unknown size


- Emmanuel


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122616/#review77154
---


On Feb. 18, 2015, 9:17 a.m., Emmanuel Pescosta wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/122616/
 ---
 
 (Updated Feb. 18, 2015, 9:17 a.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Repository: kio
 
 
 Description
 ---
 
 Use KIO::FileSystemFreeSpaceJob to retrieve free space information in 
 KPropertiesDialog, this change enables KPropertiesDialog to show free space 
 information of remote file systems as well (e.g. smb)
 
 When the free space job failed Unknown size will be shown (same behavior as 
 in Dolphin)
 
 
 Diffs
 -
 
   src/widgets/kpropertiesdialog.cpp 0dc4881 
   src/widgets/kpropertiesdialog_p.h 5858eed 
 
 Diff: https://git.reviewboard.kde.org/r/122616/diff/
 
 
 Testing
 ---
 
 Works for local and smb file systems
 
 
 Thanks,
 
 Emmanuel Pescosta
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 122755: Add testcases for KIO::del()

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122755/#review76769
---


Thanks for the test! :)

Maybe you can add some negative test cases (like read-only files - have a look 
at pastetest.cpp) as well and check if KIO sets the correct error codes?
That would be really awesome!

(Another test case which can be added to this test is in 
https://bugs.kde.org/show_bug.cgi?id=340950)

Please note that I'm not a KIO developer, so better wait for an ack from a KIO 
dev before you implement my suggestions. ;)


autotests/deldirtest.h
https://git.reviewboard.kde.org/r/122755/#comment52841

Maybe name it DeleteJobTest instead of DelDirTest? (because deleting of 
files and folders gets tested, so DelDir isn't correct)



autotests/deldirtest.h
https://git.reviewboard.kde.org/r/122755/#comment52844

can be a const method



autotests/deldirtest.cpp
https://git.reviewboard.kde.org/r/122755/#comment52843

I would add a number of files parameter to createEmptyTestFiles instead 
of using a global definintion or constant.



autotests/deldirtest.cpp
https://git.reviewboard.kde.org/r/122755/#comment52845

KJob::NoError instead of 0?


- Emmanuel Pescosta


On Feb. 28, 2015, 4:28 p.m., Martin Blumenstingl wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/122755/
 ---
 
 (Updated Feb. 28, 2015, 4:28 p.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Repository: kio
 
 
 Description
 ---
 
 This adds some automated testcases for KIO::del().
 
 My own code was buggy but I didn't notice this until I wrote those tests.
 Maybe you want to have them upstream to make sure noone breaks KIO::del.
 
 
 Diffs
 -
 
   autotests/CMakeLists.txt f613c1a 
   autotests/deldirtest.h PRE-CREATION 
   autotests/deldirtest.cpp PRE-CREATION 
 
 Diff: https://git.reviewboard.kde.org/r/122755/diff/
 
 
 Testing
 ---
 
 Tests are passing
 
 
 Thanks,
 
 Martin Blumenstingl
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Split kde-baseapps?

[Emmanuel Pescosta]

 Let's make sure we can do this for the next release :)

It's almost ready for the next (15.08) release ;)
David Edmundson helped me a lot with moving Dolphin into a new repo, see
http://quickgit.kde.org/?p=dolphin.git

Cheers,
Emmanuel

2015-02-26 1:02 GMT+01:00 Albert Astals Cid aa...@kde.org:

 El Dimecres, 25 de febrer de 2015, a les 11:13:37, Emmanuel Pescosta va
 escriure:
   We can make a release of it.
 
  Thanks! But I think it's already too late to move Dolphin into a new
 repo,
  given that the freeze is only a few hours away.
  Bad timing from my side ...

 Let's make sure we can do this for the next release :)

 Cheers,
   Albert

 
  Cheers,
  Emmanuel
 
  2015-02-25 11:02 GMT+01:00 Vishesh Handa m...@vhanda.in:
   On Tue, Feb 24, 2015 at 10:46 PM, Albert Astals Cid aa...@kde.org
 wrote:
   El Dimarts, 24 de febrer de 2015, a les 21:02:03, Emmanuel Pescosta va
  
   escriure:
 Hope you don't want to do this before tomorrow, do you?
   
If moving Dolphin to a new repo is still possible then yes, because
 I
  
   would
  
prefer a KF5 based version of Dolphin in 15.04 :)
  
   AFAIK you depend on unreleased baloo-widgets?
  
   We can make a release of it.
  
   I hadn't bothered till now since no one needed it.
  
  
   --
   Vishesh Handa
  
   ___
   Kde-frameworks-devel mailing list
   Kde-frameworks-devel@kde.org
   https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 122621: Implemented SlaveBase::GetFileSystemFreeSpace in sftp ioslave

[Emmanuel Pescosta]

 On Feb. 25, 2015, 11:24 a.m., Vishesh Handa wrote:
  sftp/kio_sftp.cpp, line 247
  https://git.reviewboard.kde.org/r/122621/diff/1/?file=350141#file350141line247
 
  The code indentation seems wrong.

Just followed the coding style (1 tab - 2 spaces) from kio_sftp.cpp

Should I use the frameworks coding style instead?


- Emmanuel


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122621/#review76592
---


On Feb. 18, 2015, 12:14 p.m., Emmanuel Pescosta wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/122621/
 ---
 
 (Updated Feb. 18, 2015, 12:14 p.m.)
 
 
 Review request for KDE Frameworks and Andreas Schneider.
 
 
 Repository: kio-extras
 
 
 Description
 ---
 
 Implemented SlaveBase::GetFileSystemFreeSpace to enable free space 
 information in sftp ioslave.
 
 
 Diffs
 -
 
   sftp/kio_sftp.h e088472 
   sftp/kio_sftp.cpp 057fcd4 
 
 Diff: https://git.reviewboard.kde.org/r/122621/diff/
 
 
 Testing
 ---
 
 Works
 
 
 Thanks,
 
 Emmanuel Pescosta
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Split kde-baseapps?

[Emmanuel Pescosta]

 Hope you don't want to do this before tomorrow, do you?

If moving Dolphin to a new repo is still possible then yes, because I would
prefer a KF5 based version of Dolphin in 15.04 :)
But if you think we should wait until the 15.04 freeze is over and release
a kdelibs4 version instead, is also ok.

Cheers,
Emmanuel

2015-02-24 19:51 GMT+01:00 Albert Astals Cid aa...@kde.org:

 El Dimarts, 24 de febrer de 2015, a les 10:55:20, Emmanuel Pescosta va
 escriure:
  Hi everyone,
 
   We can split off Dolphin as soon as Dolphin is konq-lib free
 
  Dolphin is almost ready for splitting (pending Review Request:
  https://git.reviewboard.kde.org/r/122687/) or merging (frameworks -
 master)

 You can't merge frameworks to master until the rest of the repo is done.

  How should we proceed?

 Hope you don't want to do this before tomorrow, do you?

 Cheers,
   Albert

 
  Cheers,
  Emmanuel
 
  2014-09-29 15:45 GMT+02:00 Emmanuel Pescosta 
 emmanuelpescosta...@gmail.com
- dolphin: could be split out, I added emmanuel as a CC to know his
  
   opinion
  
   Thanks for adding me and sorry for the late response.
  
   +1 for splitting up kde-baseapps
   We can split off Dolphin as soon as Dolphin is konq-lib free (David has
   already done a huge amount of work in this field :)
  
   Cheers,
   Emmauel
  
   2014-09-25 1:01 GMT+02:00 Aleix Pol aleix...@kde.org:
   On Wed, Sep 24, 2014 at 5:39 PM, Luca Beltrame lbeltr...@kde.org
 wrote:
   Michael Palimaka wrote:
Are there any plans to split frameworks-based kde-baseapps?
  
   IMO it would make sense first to consult with the various
 applications'
   maintainers and / or review what's in there and what should happen to
   it.
  
   Well let's see:
   - dolphin: could be split out, I added emmanuel as a CC to know his
   opinion
   - kdialog: should go to kde-cli-tools?
   - kioslave/about: I guess it should go either to kioslave-extras or
   konqueror itself. I guess it's here because the applications use it,
 so
   kioslave.
   - kfind: looks like it could be split. Doesn't look maintained, we can
   decide where to put it later on. It's an executable, so it's easy to
 move
   around.
   - konqplugins: By the name sounds like it should go wherever konqueror
   goes. Or do other applications use them?
   - nsplugins: same, wherever konqueror goes?
   - konqueror: split? David will read this for sure. :D
  
   I hope this helps.
   Aleix


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Split kde-baseapps?

[Emmanuel Pescosta]

Hi everyone,

 We can split off Dolphin as soon as Dolphin is konq-lib free

Dolphin is almost ready for splitting (pending Review Request:
https://git.reviewboard.kde.org/r/122687/) or merging (frameworks - master)

How should we proceed?

Cheers,
Emmanuel

2014-09-29 15:45 GMT+02:00 Emmanuel Pescosta emmanuelpescosta...@gmail.com
:

  - dolphin: could be split out, I added emmanuel as a CC to know his
 opinion

 Thanks for adding me and sorry for the late response.

 +1 for splitting up kde-baseapps
 We can split off Dolphin as soon as Dolphin is konq-lib free (David has
 already done a huge amount of work in this field :)

 Cheers,
 Emmauel

 2014-09-25 1:01 GMT+02:00 Aleix Pol aleix...@kde.org:

 On Wed, Sep 24, 2014 at 5:39 PM, Luca Beltrame lbeltr...@kde.org wrote:

 Michael Palimaka wrote:

  Are there any plans to split frameworks-based kde-baseapps?

 IMO it would make sense first to consult with the various applications'
 maintainers and / or review what's in there and what should happen to it.


 Well let's see:
 - dolphin: could be split out, I added emmanuel as a CC to know his
 opinion
 - kdialog: should go to kde-cli-tools?
 - kioslave/about: I guess it should go either to kioslave-extras or
 konqueror itself. I guess it's here because the applications use it, so
 kioslave.
 - kfind: looks like it could be split. Doesn't look maintained, we can
 decide where to put it later on. It's an executable, so it's easy to move
 around.
 - konqplugins: By the name sounds like it should go wherever konqueror
 goes. Or do other applications use them?
 - nsplugins: same, wherever konqueror goes?
 - konqueror: split? David will read this for sure. :D

 I hope this helps.
 Aleix



___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Review Request 122614: Fix a porting bug in the http ioslave

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122614/
---

Review request for KDE Frameworks.


Repository: kio


Description
---

Fix the file/directory name extraction + base dir handling in the http ioslave

name was always empty for directories because thisURL ends with a slash and 
fileName() will return an empty string, so href.text() was used as UDS_NAME 
instead.


Diffs
-

  src/ioslaves/http/http.cpp 78f50f9 

Diff: https://git.reviewboard.kde.org/r/122614/diff/


Testing
---

Works (see screenshots)


File Attachments


Webdav (Bug)
  
https://git.reviewboard.kde.org/media/uploaded/files/2015/02/18/d0ae2572-3770-4781-88e3-4d40e626fd85__webdav_bug.png


Thanks,

Emmanuel Pescosta

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Review Request 122616: Use KIO::FileSystemFreeSpaceJob in KPropertiesDialog

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122616/
---

Review request for KDE Frameworks.


Repository: kio


Description
---

Use KIO::FileSystemFreeSpaceJob to retrieve free space information in 
KPropertiesDialog, this change enables KPropertiesDialog to show free space 
information of remote file systems as well (e.g. smb)

When the free space job failed Unknown size will be shown (same behavior as 
in Dolphin)


Diffs
-

  src/widgets/kpropertiesdialog.cpp 0dc4881 
  src/widgets/kpropertiesdialog_p.h 5858eed 

Diff: https://git.reviewboard.kde.org/r/122616/diff/


Testing
---

Works for local and smb file systems


Thanks,

Emmanuel Pescosta

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 122614: Fix a porting bug in the http ioslave

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122614/
---

(Updated Feb. 18, 2015, 9:11 a.m.)


Review request for KDE Frameworks.


Changes
---

Add screenshot of fixed version


Repository: kio


Description
---

Fix the file/directory name extraction + base dir handling in the http ioslave

name was always empty for directories because thisURL ends with a slash and 
fileName() will return an empty string, so href.text() was used as UDS_NAME 
instead.


Diffs
-

  src/ioslaves/http/http.cpp 78f50f9 

Diff: https://git.reviewboard.kde.org/r/122614/diff/


Testing
---

Works (see screenshots)


File Attachments (updated)


Webdav (Bug)
  
https://git.reviewboard.kde.org/media/uploaded/files/2015/02/18/d0ae2572-3770-4781-88e3-4d40e626fd85__webdav_bug.png
Webdav (Fixed)
  
https://git.reviewboard.kde.org/media/uploaded/files/2015/02/18/de96eb2e-45d8-4388-83b6-77199f677c75__webdav_fixed.png


Thanks,

Emmanuel Pescosta

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Review Request 122620: Implement quota-used-bytes and quota-available-bytes in http ioslave

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122620/
---

Review request for KDE Frameworks.


Repository: kio


Description
---

Implement quota-used-bytes and quota-available-bytes from RFC 4331 to enable 
free space information in http ioslave.


Diffs
-

  src/ioslaves/http/http.h f0dc027 
  src/ioslaves/http/http.cpp 78f50f9 

Diff: https://git.reviewboard.kde.org/r/122620/diff/


Testing
---

Tested with Owncloud (share.kde.org)


File Attachments


webdav_freespace.png
  
https://git.reviewboard.kde.org/media/uploaded/files/2015/02/18/c90a8f70-e12b-44b3-a0b8-651f1ee7d602__webdav_freespace.png


Thanks,

Emmanuel Pescosta

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Review Request 122621: Implemented SlaveBase::GetFileSystemFreeSpace in sftp ioslave

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122621/
---

Review request for KDE Frameworks and Andreas Schneider.


Repository: kio-extras


Description
---

Implemented SlaveBase::GetFileSystemFreeSpace to enable free space information 
in sftp ioslave.


Diffs
-

  sftp/kio_sftp.h e088472 
  sftp/kio_sftp.cpp 057fcd4 

Diff: https://git.reviewboard.kde.org/r/122621/diff/


Testing
---

Works


Thanks,

Emmanuel Pescosta

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 122616: Use KIO::FileSystemFreeSpaceJob in KPropertiesDialog

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122616/#review76244
---



src/widgets/kpropertiesdialog.cpp
https://git.reviewboard.kde.org/r/122616/#comment52568

memo: set Unknown size as text


- Emmanuel Pescosta


On Feb. 18, 2015, 9:17 a.m., Emmanuel Pescosta wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/122616/
 ---
 
 (Updated Feb. 18, 2015, 9:17 a.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Repository: kio
 
 
 Description
 ---
 
 Use KIO::FileSystemFreeSpaceJob to retrieve free space information in 
 KPropertiesDialog, this change enables KPropertiesDialog to show free space 
 information of remote file systems as well (e.g. smb)
 
 When the free space job failed Unknown size will be shown (same behavior as 
 in Dolphin)
 
 
 Diffs
 -
 
   src/widgets/kpropertiesdialog.cpp 0dc4881 
   src/widgets/kpropertiesdialog_p.h 5858eed 
 
 Diff: https://git.reviewboard.kde.org/r/122616/diff/
 
 
 Testing
 ---
 
 Works for local and smb file systems
 
 
 Thanks,
 
 Emmanuel Pescosta
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 121461: Fix for bug 341773 and minor changes

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/121461/#review71854
---



src/core/global.cpp
https://git.reviewboard.kde.org/r/121461/#comment50069

std::all_of() maybe?

std::all_of(basename.cbegin(), basename.cend(), [](QChar c) { return c == 
QLatin1Char('.'); });


- Emmanuel Pescosta


On Dec. 12, 2014, 11:56 a.m., Arjun AK wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/121461/
 ---
 
 (Updated Dec. 12, 2014, 11:56 a.m.)
 
 
 Review request for KDE Frameworks and David Faure.
 
 
 Bugs: 341773
 https://bugs.kde.org/show_bug.cgi?id=341773
 
 
 Repository: kio
 
 
 Description
 ---
 
 1.Fix bug 341773 (Replace dialog suggests incorrect filename when it contains 
 dots). Make use of `QMimeDatabase::suffixForFileName` for calculating the 
 extension.
 2. Rename `.foo` and `..foo` into `.foo 1` and `..foo 1` respectively, 
 instead of `1.foo` and `.1.foo`, since they are hidden files and we shouldn't 
 be changing that.
 
 
 Diffs
 -
 
   autotests/globaltest.cpp 06cb500 
   src/core/global.cpp f18ac10 
 
 Diff: https://git.reviewboard.kde.org/r/121461/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Arjun AK
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 121447: Return inode/directory when isDir returns true (kfileitem)

[Emmanuel Pescosta]

 On Dec. 11, 2014, 3:27 p.m., Mark Gaiser wrote:
  src/core/kfileitem.cpp, line 255
  https://git.reviewboard.kde.org/r/121447/diff/1/?file=332652#file332652line255
 
  -- add here
  
  } else {
  // Fix for IO slaves that don't set UDS_MIME_TYPE for a folder.
  if (m_fileMode  QT_STAT_MASK) == QT_STAT_DIR) {
 m_entry.insert(KIO::UDSEntry::UDS_MIME_TYPE, inode/directory);
 m_mimeType = db.mimeTypeForName(inode/directory);
 m_bMimeTypeKnown = true;
  }
  }
  
  Not tested! Just written in comment box :)
  
  I think that's about all you'd need to fix this.
  But if this is accaptable is probably up to David to decide.
  
  I'm also not 100% sure that you catch all cases when readUDSEntry().

+1

I also think that this is better way to fix it.
Avoids code duplication and the correct mime type for folders is set a early as 
possible, so other code can rely on it.


- Emmanuel


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/121447/#review71804
---


On Dec. 11, 2014, 2:22 p.m., Àlex Fiestas wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/121447/
 ---
 
 (Updated Dec. 11, 2014, 2:22 p.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Repository: kio
 
 
 Description
 ---
 
 If we know that the item is a dir, return directly the correct mimetype for 
 directories.
 
 More info of why this is needed at:
 https://git.reviewboard.kde.org/r/120909/
 
 
 Diffs
 -
 
   src/core/kfileitem.cpp 6a2cfa5 
 
 Diff: https://git.reviewboard.kde.org/r/121447/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Àlex Fiestas
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 120605: cleanup overlay icon usage

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120605/#review69638
---


Ping? No git account?

- Emmanuel Pescosta


On Oct. 16, 2014, 5:32 p.m., Stefan Brüns wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120605/
 ---
 
 (Updated Oct. 16, 2014, 5:32 p.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Bugs: 339193
 https://bugs.kde.org/show_bug.cgi?id=339193
 
 
 Repository: kio
 
 
 Description
 ---
 
 The locked overlay should be added also for directories, as there is no
 distinct unreadable directory icon.
 No overlay icon for gzip files with .gz file ending, there is a
 mimetype icon for gzip files.
 
 BUGS: 339193
 
 Signed-off-by: Stefan Brüns stefan.bru...@rwth-aachen.de
 
 
 Diffs
 -
 
   src/core/kfileitem.cpp 74dc069dc964f4fb0040a3dab916ff0d1e26602c 
 
 Diff: https://git.reviewboard.kde.org/r/120605/diff/
 
 
 Testing
 ---
 
 Patched kio, see BR for new screenshot
 
 
 Thanks,
 
 Stefan Brüns
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 120886: Show script execution prompt only if asked to

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120886/#review69406
---



src/widgets/krun.h
https://git.reviewboard.kde.org/r/120886/#comment48510

since 5.4



src/widgets/krun.h
https://git.reviewboard.kde.org/r/120886/#comment48509

Better name instead of b? showPrompt maybe?


- Emmanuel Pescosta


On Oct. 29, 2014, 12:01 p.m., Arjun AK wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120886/
 ---
 
 (Updated Oct. 29, 2014, 12:01 p.m.)
 
 
 Review request for KDE Frameworks and David Faure.
 
 
 Repository: kio
 
 
 Description
 ---
 
 KRun shouldn't show script/desktop execution prompts by default, but only if 
 asked to by the application, since KRun is also used by launchers, whose sole 
 purpose is to execute programs.
 
 
 Diffs
 -
 
   src/widgets/krun.h 545c3a9 
   src/widgets/krun.cpp af5e8dc 
 
 Diff: https://git.reviewboard.kde.org/r/120886/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Arjun AK
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 120666: Get user's permission before executing scripts or desktop files

[Emmanuel Pescosta]

 On Oct. 23, 2014, 1:14 p.m., Emmanuel Pescosta wrote:
  src/widgets/krun.cpp, line 1013
  https://git.reviewboard.kde.org/r/120666/diff/2/?file=321616#file321616line1013
 
  Any reason why using show instead of exec? Exec would avoid the rather 
  complex and error-prone code path.
  
  IMHO we should prefer a blocking dialog in this case, because it asks 
  the user for permission.
 
 Arjun AK wrote:
 IMHO we should prefer a blocking dialog in this case, because it asks 
 the user for permission.
 
 faure: Are you okay with this?
 
 Frank Reininghaus wrote:
 Note that you have to be *extremely* careful when calling exec(), which 
 runs a nested event loop. Anything can happen inside such a loop, including 
 quitting the application. See https://git.reviewboard.kde.org/r/118858/ and 
 the links in the comments there fore more information.
 
 If you replace show() by exec() in your patch, you might get a crash in 
 the line
 
 m_dialogNotYetShown = false;
 
 if the application quits inside the nested event loop.
 
 Emmanuel Pescosta wrote:
  m_dialogNotYetShown = false;
 
 This variable won't be needed anymore. Only the result of exec() is 
 needed.
 
 David Faure wrote:
 Please please keep the non-modal dialog.
 
 Modal dialogs are very annoying - and even more so when called from 
 plasma: suddenly your whole taskbar and desktop is unusable, just because 
 somewhere there's a modal dialog.

 and even more so when called from plasma: suddenly your whole taskbar and 
 desktop is unusable

This is indeed really bad.


- Emmanuel


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120666/#review68990
---


On Oct. 22, 2014, 6:10 p.m., Arjun AK wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120666/
 ---
 
 (Updated Oct. 22, 2014, 6:10 p.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Repository: kio
 
 
 Description
 ---
 
 This patch makes KIO show a dialog box asking the user what to do (either 
 open it using a text editor or execute it) when he clicks on a script or a 
 desktop file.
 
 See also: https://git.reviewboard.kde.org/r/120171/
 
 
 Diffs
 -
 
   src/widgets/CMakeLists.txt 4060cdf 
   src/widgets/executablefileopendialog.h PRE-CREATION 
   src/widgets/executablefileopendialog.cpp PRE-CREATION 
   src/widgets/krun.cpp 6ac42da 
   src/widgets/krun_p.h 69e2e98 
 
 Diff: https://git.reviewboard.kde.org/r/120666/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Arjun AK
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 120666: Get user's permission before executing scripts or desktop files

[Emmanuel Pescosta]

 On Oct. 23, 2014, 11:43 a.m., David Faure wrote:
  Thanks for the patch.
  
  Calling init() again surprised me a bit, since this method was never called 
  twice before, but OK, anything else would be much more invasive. I would 
  suggest to at least add a comment e.g. in the call to showPrompt(), to say 
  this will call init() again once the dialog is closed or something.
  
  Some small things below.

 Calling init() again surprised me a bit, since this method was never called 
 twice before

Why not splitting up this method?

Just renaming the old init method and hide it, then create a new init function 
which checks if a dialog is needed or not and load the settings. The old init 
function can then be called in the dialog finished handler, or directly in the 
new init method when no dialog is needed at all.

 anything else would be much more invasive

And I don't think this will be too invasive ;)


- Emmanuel


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120666/#review68968
---


On Oct. 22, 2014, 6:10 p.m., Arjun AK wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120666/
 ---
 
 (Updated Oct. 22, 2014, 6:10 p.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Repository: kio
 
 
 Description
 ---
 
 This patch makes KIO show a dialog box asking the user what to do (either 
 open it using a text editor or execute it) when he clicks on a script or a 
 desktop file.
 
 See also: https://git.reviewboard.kde.org/r/120171/
 
 
 Diffs
 -
 
   src/widgets/CMakeLists.txt 4060cdf 
   src/widgets/executablefileopendialog.h PRE-CREATION 
   src/widgets/executablefileopendialog.cpp PRE-CREATION 
   src/widgets/krun.cpp 6ac42da 
   src/widgets/krun_p.h 69e2e98 
 
 Diff: https://git.reviewboard.kde.org/r/120666/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Arjun AK
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 120666: Get user's permission before executing scripts or desktop files

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120666/#review68987
---



src/widgets/executablefileopendialog.h
https://git.reviewboard.kde.org/r/120666/#comment48253

m_



src/widgets/krun.cpp
https://git.reviewboard.kde.org/r/120666/#comment48252

Lambda expression maybe?

connect(dialog, ExecutableFileOpenDialog::finished, [this, dialog](int 
result) {
...
}

Avoids the cast hack and the need of a member variable.


- Emmanuel Pescosta


On Oct. 22, 2014, 6:10 p.m., Arjun AK wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120666/
 ---
 
 (Updated Oct. 22, 2014, 6:10 p.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Repository: kio
 
 
 Description
 ---
 
 This patch makes KIO show a dialog box asking the user what to do (either 
 open it using a text editor or execute it) when he clicks on a script or a 
 desktop file.
 
 See also: https://git.reviewboard.kde.org/r/120171/
 
 
 Diffs
 -
 
   src/widgets/CMakeLists.txt 4060cdf 
   src/widgets/executablefileopendialog.h PRE-CREATION 
   src/widgets/executablefileopendialog.cpp PRE-CREATION 
   src/widgets/krun.cpp 6ac42da 
   src/widgets/krun_p.h 69e2e98 
 
 Diff: https://git.reviewboard.kde.org/r/120666/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Arjun AK
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 120666: Get user's permission before executing scripts or desktop files

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120666/#review68990
---



src/widgets/krun.cpp
https://git.reviewboard.kde.org/r/120666/#comment48256

Any reason why using show instead of exec? Exec would avoid the rather 
complex and error-prone code path.

IMHO we should prefer a blocking dialog in this case, because it asks the 
user for permission.


- Emmanuel Pescosta


On Oct. 22, 2014, 6:10 p.m., Arjun AK wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120666/
 ---
 
 (Updated Oct. 22, 2014, 6:10 p.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Repository: kio
 
 
 Description
 ---
 
 This patch makes KIO show a dialog box asking the user what to do (either 
 open it using a text editor or execute it) when he clicks on a script or a 
 desktop file.
 
 See also: https://git.reviewboard.kde.org/r/120171/
 
 
 Diffs
 -
 
   src/widgets/CMakeLists.txt 4060cdf 
   src/widgets/executablefileopendialog.h PRE-CREATION 
   src/widgets/executablefileopendialog.cpp PRE-CREATION 
   src/widgets/krun.cpp 6ac42da 
   src/widgets/krun_p.h 69e2e98 
 
 Diff: https://git.reviewboard.kde.org/r/120666/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Arjun AK
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 120666: Get user's permission before executing scripts or desktop files

[Emmanuel Pescosta]

 On Oct. 23, 2014, 1:14 p.m., Emmanuel Pescosta wrote:
  src/widgets/krun.cpp, line 1013
  https://git.reviewboard.kde.org/r/120666/diff/2/?file=321616#file321616line1013
 
  Any reason why using show instead of exec? Exec would avoid the rather 
  complex and error-prone code path.
  
  IMHO we should prefer a blocking dialog in this case, because it asks 
  the user for permission.
 
 Arjun AK wrote:
 IMHO we should prefer a blocking dialog in this case, because it asks 
 the user for permission.
 
 faure: Are you okay with this?
 
 Frank Reininghaus wrote:
 Note that you have to be *extremely* careful when calling exec(), which 
 runs a nested event loop. Anything can happen inside such a loop, including 
 quitting the application. See https://git.reviewboard.kde.org/r/118858/ and 
 the links in the comments there fore more information.
 
 If you replace show() by exec() in your patch, you might get a crash in 
 the line
 
 m_dialogNotYetShown = false;
 
 if the application quits inside the nested event loop.

 m_dialogNotYetShown = false;

This variable won't be needed anymore. Only the result of exec() is needed.


- Emmanuel


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120666/#review68990
---


On Oct. 22, 2014, 6:10 p.m., Arjun AK wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120666/
 ---
 
 (Updated Oct. 22, 2014, 6:10 p.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Repository: kio
 
 
 Description
 ---
 
 This patch makes KIO show a dialog box asking the user what to do (either 
 open it using a text editor or execute it) when he clicks on a script or a 
 desktop file.
 
 See also: https://git.reviewboard.kde.org/r/120171/
 
 
 Diffs
 -
 
   src/widgets/CMakeLists.txt 4060cdf 
   src/widgets/executablefileopendialog.h PRE-CREATION 
   src/widgets/executablefileopendialog.cpp PRE-CREATION 
   src/widgets/krun.cpp 6ac42da 
   src/widgets/krun_p.h 69e2e98 
 
 Diff: https://git.reviewboard.kde.org/r/120666/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Arjun AK
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 120605: cleanup overlay icon usage

[Emmanuel Pescosta]

 On Oct. 16, 2014, 8:09 p.m., Emmanuel Pescosta wrote:
  Looks good! +1 from my side
  
  (Please use the CCBUG hook when you commit this patch, because it only 
  fixes one mentioned problem)
 
 Stefan Brüns wrote:
 Any more comments? Is this a SHIP IT?
 
 BTW: The same applies to SC 4, should I backport it to 4.14?

 Any more comments? Is this a SHIP IT?

The KIO maintainer (dfaure) will give you a ship it.


- Emmanuel


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120605/#review68567
---


On Oct. 16, 2014, 5:32 p.m., Stefan Brüns wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120605/
 ---
 
 (Updated Oct. 16, 2014, 5:32 p.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Bugs: 339193
 https://bugs.kde.org/show_bug.cgi?id=339193
 
 
 Repository: kio
 
 
 Description
 ---
 
 The locked overlay should be added also for directories, as there is no
 distinct unreadable directory icon.
 No overlay icon for gzip files with .gz file ending, there is a
 mimetype icon for gzip files.
 
 BUGS: 339193
 
 Signed-off-by: Stefan Brüns stefan.bru...@rwth-aachen.de
 
 
 Diffs
 -
 
   src/core/kfileitem.cpp 74dc069dc964f4fb0040a3dab916ff0d1e26602c 
 
 Diff: https://git.reviewboard.kde.org/r/120605/diff/
 
 
 Testing
 ---
 
 Patched kio, see BR for new screenshot
 
 
 Thanks,
 
 Stefan Brüns
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 120605: cleanup overlay icon usage

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120605/#review68567
---


Looks good! +1 from my side

(Please use the CCBUG hook when you commit this patch, because it only fixes 
one mentioned problem)

- Emmanuel Pescosta


On Oct. 16, 2014, 5:32 p.m., Stefan Brüns wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120605/
 ---
 
 (Updated Oct. 16, 2014, 5:32 p.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Bugs: 339193
 https://bugs.kde.org/show_bug.cgi?id=339193
 
 
 Repository: kio
 
 
 Description
 ---
 
 The locked overlay should be added also for directories, as there is no
 distinct unreadable directory icon.
 No overlay icon for gzip files with .gz file ending, there is a
 mimetype icon for gzip files.
 
 BUGS: 339193
 
 Signed-off-by: Stefan Brüns stefan.bru...@rwth-aachen.de
 
 
 Diffs
 -
 
   src/core/kfileitem.cpp 74dc069dc964f4fb0040a3dab916ff0d1e26602c 
 
 Diff: https://git.reviewboard.kde.org/r/120605/diff/
 
 
 Testing
 ---
 
 Patched kio, see BR for new screenshot
 
 
 Thanks,
 
 Stefan Brüns
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Split kde-baseapps?

[Emmanuel Pescosta]

 - dolphin: could be split out, I added emmanuel as a CC to know his
opinion

Thanks for adding me and sorry for the late response.

+1 for splitting up kde-baseapps
We can split off Dolphin as soon as Dolphin is konq-lib free (David has
already done a huge amount of work in this field :)

Cheers,
Emmauel

2014-09-25 1:01 GMT+02:00 Aleix Pol aleix...@kde.org:

 On Wed, Sep 24, 2014 at 5:39 PM, Luca Beltrame lbeltr...@kde.org wrote:

 Michael Palimaka wrote:

  Are there any plans to split frameworks-based kde-baseapps?

 IMO it would make sense first to consult with the various applications'
 maintainers and / or review what's in there and what should happen to it.


 Well let's see:
 - dolphin: could be split out, I added emmanuel as a CC to know his opinion
 - kdialog: should go to kde-cli-tools?
 - kioslave/about: I guess it should go either to kioslave-extras or
 konqueror itself. I guess it's here because the applications use it, so
 kioslave.
 - kfind: looks like it could be split. Doesn't look maintained, we can
 decide where to put it later on. It's an executable, so it's easy to move
 around.
 - konqplugins: By the name sounds like it should go wherever konqueror
 goes. Or do other applications use them?
 - nsplugins: same, wherever konqueror goes?
 - konqueror: split? David will read this for sure. :D

 I hope this helps.
 Aleix

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 119809: KIO: New job RestoreJob, public API KIO::restoreFromTrash().

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119809/#review64687
---



src/core/restorejob.cpp
https://git.reviewboard.kde.org/r/119809/#comment45245

constBegin() because m_urlsIterator is a const iterator


- Emmanuel Pescosta


On Aug. 17, 2014, 11:01 a.m., David Faure wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/119809/
 ---
 
 (Updated Aug. 17, 2014, 11:01 a.m.)
 
 
 Review request for KDE Frameworks and Eike Hein.
 
 
 Repository: kio
 
 
 Description
 ---
 
 This was within libkonq as KonqMultiRestoreJob, public API 
 KonqOperations::restoreTrashedItems().
 
 REVIEW: 119809
 
 
 Diffs
 -
 
   autotests/fileundomanagertest.h 112657a1fcfea1e588e5fa53a1bbfccfe7a5de3d 
   autotests/fileundomanagertest.cpp c1f253b4e41bff6d96baa9017fe8d6c70386efb8 
   src/core/CMakeLists.txt 1e9ab98d2c0c27a696baab0bb8375c8cf81a3632 
   src/core/restorejob.h PRE-CREATION 
   src/core/restorejob.cpp PRE-CREATION 
 
 Diff: https://git.reviewboard.kde.org/r/119809/diff/
 
 
 Testing
 ---
 
 Added unittest; passes.
 
 It reminded me that this operation wasn't undoable yet, though.
 
 
 Thanks,
 
 David Faure
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 118775: Make KFileItem a Q_MOVABLE type

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118775/#review60179
---



src/core/kcoredirlister_p.h
https://git.reviewboard.kde.org/r/118775/#comment41931

using KFileItem::KFileItem; maybe?



src/core/kcoredirlister_p.h
https://git.reviewboard.kde.org/r/118775/#comment41932

{}


- Emmanuel Pescosta


On June 16, 2014, 9:58 a.m., Frank Reininghaus wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118775/
 ---
 
 (Updated June 16, 2014, 9:58 a.m.)
 
 
 Review request for KDE Frameworks and David Faure.
 
 
 Repository: kio
 
 
 Description
 ---
 
 This is my second attempt to make KFileItem a Q_MOVABLE_TYPE, after
 
 https://git.reviewboard.kde.org/r/111789/
 
 which I had to revert because it broke KDirLister, see
 
 http://mail.kde.org/pipermail/kde-frameworks-devel/2013-August/004135.html
 
 The motivation for this change is that this reduces the memory usage of a 
 QListKFileItem a.k.a. KFileItemList, which is used extensively in 
 KDirLister's API and in applications, by 32 bytes per item on a 64-bit 
 system, and that it may also improve the performance in some situations 
 because many memory allocations are saved (for details on why making a type 
 movable saves memory allocations when putting objects of that type into a 
 QList, see the discussion in the related request 
 https://git.reviewboard.kde.org/r/115739/ for UDSEntry).
 
 The problem with the first attempt was that KDirListerCache actually relies 
 on the fact that KFileItem is NOT movable in memory - it keeps pointers to 
 KFileItems in a QList and expects that these pointers remain valid even if 
 the list is resized, and the location of its contiguous data storage with 
 size ~number of items in the list in memory changes. This is the case right 
 now because QList only keeps pointers to the KFileItems, and moving the 
 pointers when the list is resized does not change the location of the actual 
 KFileItems. For movable types, QList stores the objects directly, such that 
 resizing the list may move the actual KFileItems. This conflicts with 
 KDirListerCache's expectation that the KFileItems do not move.
 
 David suggested to change the internal data storage of KDirListerCache to, 
 e.g., a QLinkedList to circumvent this problem, see
 
 http://mail.kde.org/pipermail/kde-frameworks-devel/2013-September/004845.html
 
 I have a less intrusive proposal now: Make KFileItem movable, but replace all 
 places where KDirListerCache expects a non-movable KFileItem with 
 NonMovableFileItem, which is a class that inherits KFileItem, but does not 
 have the movable property.
 
 That way, the data storage inside KDirListerCache remains exactly the same, 
 and everything outside that class benefits from the movability of KFileItems. 
 Most changes in this patch are straightforward subsitutions.
 
 The only place where performance might suffer is 
 KCoreDirLister::itemsForDir(const QUrl dir, WhichItems which) in the which 
 == AllItems case. The current code simply returns a shallow copy of the 
 internal KFileItemList, but with this patch, the list has to be copied item 
 by item (this happens in NonMovableFileItemList::operator KFileItemList()). 
 However, the QLinkedList idea or any other approach which makes KFileItem 
 movable, but keeps the KFileItems in KDirListerCache at fixed memory 
 locations would suffer from the same problem.
 
 I'm not sure if that function is used much in the AllItems case though. I put 
 a Q_ASSERT(0);  into NonMovableFileItemList::operator KFileItemList() and 
 was unable to trigger that assert with Dolphin.
 
 Ideally, one would do some benchmarking and memory profiling of this patch 
 and alternatives, such as the QLinkedList idea. However, I'm running out of 
 time because the release schedule is progressing fast, and even though this 
 change is quite straightforward, it is binary incompatible. This is why I am 
 creating this review request right now.
 
 
 Diffs
 -
 
   src/core/kcoredirlister.cpp fef28db 
   src/core/kcoredirlister_p.h 2660e99 
   src/core/kfileitem.h bc2f90c 
 
 Diff: https://git.reviewboard.kde.org/r/118775/diff/
 
 
 Testing
 ---
 
 Unit tests still pass. I verified that the memory usage of a KFileItemList 
 with many items decreases as expected.
 
 
 Thanks,
 
 Frank Reininghaus
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Review Request 118313: [Konqlib] KUrl - QUrl

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118313/
---

Review request for KDE Base Apps, KDE Frameworks and David Faure.


Repository: kde-baseapps


Description
---

Ported Konqlib from KUrl to QUrl (convert-kurl.pl)

Still some ToDos left in the code (I'll mark it inline)


Diffs
-

  lib/konq/favicons/favicons.h 30f0272 
  lib/konq/favicons/favicons.cpp 4646fa5 
  lib/konq/konq_copytomenu.h 0958e20 
  lib/konq/konq_copytomenu.cpp 7ee5131 
  lib/konq/konq_copytomenu_p.h eb6649a 
  lib/konq/konq_dndpopupmenuplugin.h 2417080 
  lib/konq/konq_historyentry.h 6b33b98 
  lib/konq/konq_historyentry.cpp bfc4a5d 
  lib/konq/konq_historyprovider.h 91b5b87 
  lib/konq/konq_historyprovider.cpp 10a5e74 
  lib/konq/konq_nameandurlinputdialog.h 3a943f7 
  lib/konq/konq_nameandurlinputdialog.cpp 2fb06f9 
  lib/konq/konq_operations.h b51fa3f 
  lib/konq/konq_operations.cpp 0cbff08 
  lib/konq/konq_popupmenu.h b5f5e39 
  lib/konq/konq_popupmenu.cpp caa37e7 
  lib/konq/konq_popupmenuinformation.h 1454073 
  lib/konq/konq_popupmenuinformation.cpp 6e1033b 
  lib/konq/konq_sound.h 8063073 
  lib/konq/konq_sound.cc f8a80a9 
  lib/konq/konqmimedata.h a2640a2 
  lib/konq/konqmimedata.cpp 8f9ecf1 
  lib/konq/tests/favicontest.cpp d48b03d 
  lib/konq/tests/konqmimedatatest.cpp 6a2be4f 
  lib/konq/tests/konqpopupmenutest.cpp 46851c5 

Diff: https://git.reviewboard.kde.org/r/118313/diff/


Testing
---

Tests fail

konqmimedatatest: (todos)
FAIL!  : KonqMimeDataTest::testPopulate() 'mimeData-hasUrls()' returned FALSE.
FAIL!  : KonqMimeDataTest::testCut() 'mimeData-hasUrls()' returned FALSE.

konqpopupmenutest: (local setup problem maybe?!)
QFATAL : KonqPopupMenuTest::testFile() ASSERT failure in 
KServiceTypeFactory::KServiceTypeFactory()

favicontest: (local setup problem maybe?!)
QFATAL : FavIconTest::testSetIconForURL() ASSERT failure in 
KServiceTypeFactory::KServiceTypeFactory()


Thanks,

Emmanuel Pescosta

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 118313: [Konqlib] KUrl - QUrl

[Emmanuel Pescosta]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118313/#review58416
---



lib/konq/konq_popupmenu.cpp
https://git.reviewboard.kde.org/r/118313/#comment40624

KUrl::cleanPath() - QUrl::???



lib/konq/konq_popupmenu.cpp
https://git.reviewboard.kde.org/r/118313/#comment40625

KUrl::cleanPath() - QUrl::???



lib/konq/konqmimedata.cpp
https://git.reviewboard.kde.org/r/118313/#comment40626

Is mimeData-setUrls(kdeUrls); correct?



lib/konq/konqmimedata.cpp
https://git.reviewboard.kde.org/r/118313/#comment40627

Is mimeData-setUrls(mostLocalURLs); correct?


- Emmanuel Pescosta


On May 25, 2014, 3:27 p.m., Emmanuel Pescosta wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118313/
 ---
 
 (Updated May 25, 2014, 3:27 p.m.)
 
 
 Review request for KDE Base Apps, KDE Frameworks and David Faure.
 
 
 Repository: kde-baseapps
 
 
 Description
 ---
 
 Ported Konqlib from KUrl to QUrl (convert-kurl.pl)
 
 Still some ToDos left in the code (I'll mark it inline)
 
 
 Diffs
 -
 
   lib/konq/favicons/favicons.h 30f0272 
   lib/konq/favicons/favicons.cpp 4646fa5 
   lib/konq/konq_copytomenu.h 0958e20 
   lib/konq/konq_copytomenu.cpp 7ee5131 
   lib/konq/konq_copytomenu_p.h eb6649a 
   lib/konq/konq_dndpopupmenuplugin.h 2417080 
   lib/konq/konq_historyentry.h 6b33b98 
   lib/konq/konq_historyentry.cpp bfc4a5d 
   lib/konq/konq_historyprovider.h 91b5b87 
   lib/konq/konq_historyprovider.cpp 10a5e74 
   lib/konq/konq_nameandurlinputdialog.h 3a943f7 
   lib/konq/konq_nameandurlinputdialog.cpp 2fb06f9 
   lib/konq/konq_operations.h b51fa3f 
   lib/konq/konq_operations.cpp 0cbff08 
   lib/konq/konq_popupmenu.h b5f5e39 
   lib/konq/konq_popupmenu.cpp caa37e7 
   lib/konq/konq_popupmenuinformation.h 1454073 
   lib/konq/konq_popupmenuinformation.cpp 6e1033b 
   lib/konq/konq_sound.h 8063073 
   lib/konq/konq_sound.cc f8a80a9 
   lib/konq/konqmimedata.h a2640a2 
   lib/konq/konqmimedata.cpp 8f9ecf1 
   lib/konq/tests/favicontest.cpp d48b03d 
   lib/konq/tests/konqmimedatatest.cpp 6a2be4f 
   lib/konq/tests/konqpopupmenutest.cpp 46851c5 
 
 Diff: https://git.reviewboard.kde.org/r/118313/diff/
 
 
 Testing
 ---
 
 Tests fail
 
 konqmimedatatest: (todos)
 FAIL!  : KonqMimeDataTest::testPopulate() 'mimeData-hasUrls()' returned 
 FALSE.
 FAIL!  : KonqMimeDataTest::testCut() 'mimeData-hasUrls()' returned FALSE.
 
 konqpopupmenutest: (local setup problem maybe?!)
 QFATAL : KonqPopupMenuTest::testFile() ASSERT failure in 
 KServiceTypeFactory::KServiceTypeFactory()
 
 favicontest: (local setup problem maybe?!)
 QFATAL : FavIconTest::testSetIconForURL() ASSERT failure in 
 KServiceTypeFactory::KServiceTypeFactory()
 
 
 Thanks,
 
 Emmanuel Pescosta
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel