D20852: Fix to show folding preview when move the mouse from bottom to top

[loh tar]

loh.tar updated this revision to Diff 57082.
loh.tar edited the summary of this revision.
loh.tar edited the test plan for this revision.
loh.tar added a comment.


  - Fix missing cleanup in case of unvalid mouse position
  - Give members more fitting names
  - Remove unneeded m_nextHighlightBlock
  - Remove repaint()
  - Remove commented event filter hint

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20852?vs=57064=57082

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

AFFECTED FILES
  src/view/kateviewhelpers.cpp
  src/view/kateviewhelpers.h

To: loh.tar, #ktexteditor
Cc: brauch, kwrite-devel, kde-frameworks-devel, #ktexteditor, domson, michaelh, 
ngraham, bruns, demsking, cullmann, sars, dhaumann

D20748: Fix wrong "Unable to find service type" warnings

[Anthony Fieroni]

anthonyfieroni added inline comments.

INLINE COMMENTS

> desktopfileparser.cpp:402
>  // in cache but we still must make our own copy
>  m_definitions << *def;
> +} else {

Where def is deleted?

REPOSITORY
  R244 KCoreAddons

BRANCH
  master

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

To: dfaure, mart, arichardson, davidedmundson, mpyne, apol
Cc: anthonyfieroni, aacid, kde-frameworks-devel, michaelh, ngraham, bruns

D20748: Fix wrong "Unable to find service type" warnings

[Aleix Pol Gonzalez]

apol accepted this revision.
apol added a comment.
This revision is now accepted and ready to land.


  Thanks!

REPOSITORY
  R244 KCoreAddons

BRANCH
  master

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

To: dfaure, mart, arichardson, davidedmundson, mpyne, apol
Cc: aacid, kde-frameworks-devel, michaelh, ngraham, bruns

D19812: Add a web page to view and compare icons of different sizes

[Nathaniel Graham]

ngraham added a comment.


  If you want to use a library, then it would be better to mark it as a 
dependency in CMake rather than bundling it here.

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: ltoscano, filipf, trickyricky26, GB_2, pino, bcooksley, ngraham, 
kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Yunhe Guo]

guoyunhe abandoned this revision.
guoyunhe added a comment.


  Using a library can save a lot of time. Just like many simple web pages use 
jQuery, D3 , etc.
  
  I don't have the ability to write it all by hands from scratch...

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: ltoscano, filipf, trickyricky26, GB_2, pino, bcooksley, ngraham, 
kde-frameworks-devel, michaelh, bruns

D20852: Fix to show folding preview when move the mouse from bottom to top

[Sven Brauch]

brauch added a comment.


  I guess the intention of the highlight delay is that when you move your mouse 
across the border, the view doesn't flicker. The 150ms does this well enough 
for me, I never see a flicker at least ;)

REPOSITORY
  R39 KTextEditor

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

To: loh.tar, #ktexteditor
Cc: brauch, kwrite-devel, kde-frameworks-devel, #ktexteditor, domson, michaelh, 
ngraham, bruns, demsking, cullmann, sars, dhaumann

D20852: Fix to show folding preview when move the mouse from bottom to top

[loh tar]

loh.tar created this revision.
loh.tar added a reviewer: KTextEditor.
Herald added projects: Kate, Frameworks.
Herald added subscribers: kde-frameworks-devel, kwrite-devel.
loh.tar requested review of this revision.

REVISION SUMMARY
  Without this patch happens in that case nothing. You have to leave the area
  an hover again the folding begin to trigger the popup.
  
  - Tried to simplify the code

TEST PLAN
  TODO/QUESTIONS
  
  - Increase m_delayFoldingHlTimer to 500ms just for interest, 150 is not to 
notice
  - The working of these timer looks to me broken. I guess the intend was to 
delay only the popup, or maybe always, but currently it looks somehow pointless.
  - With this new value you have at least a flicker protection when you try to 
reach the areas behind the folding area. Perhaps is that the only intend(?)

REPOSITORY
  R39 KTextEditor

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

AFFECTED FILES
  src/view/kateviewhelpers.cpp

To: loh.tar, #ktexteditor
Cc: kwrite-devel, kde-frameworks-devel, #ktexteditor, domson, michaelh, 
ngraham, bruns, demsking, cullmann, sars, dhaumann

D19812: Add a web page to view and compare icons of different sizes

[Nathaniel Graham]

ngraham added a comment.


  At the minimum, I'd like to see the original/un-minified version rather than 
the illegible version here that's vulnerable to attack. But why do we even need 
it in the first place? The original is 12,000 lines of Javascript. Is it 
actually necessary to have such a thing just for building a simple webpage of 
icons? Seems like overkill to me.

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: ltoscano, filipf, trickyricky26, GB_2, pino, bcooksley, ngraham, 
kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Yunhe Guo]

guoyunhe added a comment.


  In D19812#456869 , @ltoscano wrote:
  
  > The documentation website of krita is a different type of repository. This 
project is going to be distributed and compiled by the distributions, and many 
of them will have to patch out the minified javascript and replace it with a 
dependency on a proper packaged version. The idea is that the minified 
(obscured) version is not the preferred form of distribution (not the "source 
code"), more or less. So special care should be taken here.
  
  
  The web page is only used by developer/designer. End users don't need it. It 
won't be executed during packaging or installation. It won't be installed, 
either. Here isn't cmake rule to execute or install it.

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: ltoscano, filipf, trickyricky26, GB_2, pino, bcooksley, ngraham, 
kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Luigi Toscano]

ltoscano added a comment.


  The documentation website of krita is a different type of repository. This 
project is going to be distributed and compiled by the distributions, and many 
of them will have to patch out the minified javascript and replace it with a 
dependency on a proper packaged version. The idea is that the minified 
(obscured) version is not the preferred form of distribution (not the "source 
code"), more or less. So special care should be taken here.

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: ltoscano, filipf, trickyricky26, GB_2, pino, bcooksley, ngraham, 
kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Yunhe Guo]

guoyunhe added a comment.


  The code is stable version Vue.js. Source is here 
https://github.com/vuejs/vue/blob/v2.6.10/dist/vue.min.js
  Uncompiled version is here 
https://github.com/vuejs/vue/blob/v2.6.10/dist/vue.js
  
  I used to include it from remote URL. But someone think it is not trusted. So 
I changed it to local file.
  
  Here are some KDE projects also including compiled JavaScript files 
https://cgit.kde.org/websites/docs-krita-org.git/tree/theme/static/js/modernizr.min.js

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: filipf, trickyricky26, GB_2, pino, bcooksley, ngraham, 
kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Filip Fila]

filipf added a comment.


  In D19812#456848 , @ngraham wrote:
  
  > What is this giant javascript file with no whitespace that you're adding? I 
really don't like how it's formatted in such a way that makes the code 
practically impossible to read. This would be the perfect place to sneak in 
malware (not saying you or the author have done this, but that this kind of 
obfuscated code is a perfect vector for it).
  
  
  I have to strongly second this sentiment. What's the source of the 
`vue.min.js` file? At the very least code shouldn't be formatted this way, and 
if this was the base file I'm seeing it's not in its original form: 
https://github.com/vuejs/vue/blob/dev/dist/vue.js

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: filipf, trickyricky26, GB_2, pino, bcooksley, ngraham, 
kde-frameworks-devel, michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Yunhe Guo]

guoyunhe added a comment.


  In D19812#456848 , @ngraham wrote:
  
  > What is this giant javascript file with no whitespace that you're adding? I 
really don't like how it's formatted in such a way that makes the code 
practically impossible to read. This would be the perfect place to sneak in 
malware (not saying you or the author have done this, but that this kind of 
obfuscated code is a perfect vector for it).
  
  
  It is Vue, a MVC library to make interactive single page application. I can 
provide some examples:
  
  1. WordPress https://github.com/WordPress/WordPress/tree/master/wp-includes/js
  2. Bootstrap https://github.com/twbs/bootstrap/tree/master/dist/js
  3. Phabricator (this website) 
https://github.com/phacility/phabricator/blob/master/webroot/rsrc/externals/d3/d3.min.js
  
  They all have some minified JavaScript in Git repository.

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: trickyricky26, GB_2, pino, bcooksley, ngraham, kde-frameworks-devel, 
michaelh, bruns

D19812: Add a web page to view and compare icons of different sizes

[Nathaniel Graham]

ngraham added a comment.


  What is this giant javascript file with no whitespace that you're adding? I 
really don't like how it's formatted in such a way that makes the code 
practically impossible to read. This would be the perfect place to sneak in 
malware (not saying you or the author have done this, but that this kind of 
obfuscated code is a perfect vector for it).

REPOSITORY
  R266 Breeze Icons

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

To: guoyunhe, ngraham, #vdg, ndavis, #breeze
Cc: trickyricky26, GB_2, pino, bcooksley, ngraham, kde-frameworks-devel, 
michaelh, bruns

D20838: Allow to drop one file or one folder on KDirOperator

[Nathaniel Graham]

ngraham added a comment.


  In D20838#456831 , @meven wrote:
  
  > I have tested on my side, I don't understand why it does not work.
  >  From dolphin desktop:/ you can drag'n drop fine but not from the folder 
view.
  >  I receive events and desktop:/ urls but the drag is not accepted whatever 
I do, like always calling event->accept() in the "case QEvent::DragEnter".
  >  Could it be a because of the folder view filtering where it accepts to get 
dragged for instance ?
  
  
  Sounds like it. If it works from `desktop:/` in Dolphin, but not from Folder 
view, I bet the drag isn't being sent the right way.
  
  However, testing with a file in `desktop:/`, the drag and drop happens 
successfully, but the path listed in the filename field is invalid (e.g. 
`desktop:IMG_0713.JPG` and the file can't actually be opened:
  
  F6795316: Screenshot_20190426_111318.png 

  
  Looks like the protocol is missing a trailing slash before the file path 
part. Interestingly, I notice that if I drag the same file from 
`desktop:/` the filename field, the path is listed as 
`file:///home/dev/Desktop/IMG_0713.JPG` which is different, but also correct. 
Perhaps the path just needs to be sanitized in the same way when dragged to the 
file view?

REPOSITORY
  R241 KIO

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

To: meven, ngraham
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

[Méven Car]

meven added a comment.


  In D20838#456820 , @ngraham wrote:
  
  > Wow, it works and it's so nice. What an improvement!
  
  
  :)
  
  > Drop options are not needed here; this isn't a move/copy/symlink UI; it's 
simply a quick way to choose a file that you happen to have visible in Dolphin 
or on the desktop.
  >  On that subject, I notice that drops from the desktop don't work. Not sure 
if that needs work here or in Folder View.
  
  I have tested on my side, I don't understand why it does not work.
  From dolphin desktop:/ you can drag'n drop fine but not from the folder view.
  I receive events and desktop:/ urls but the drag is not accepted whatever I 
do, like always calling event->accept() in the "case QEvent::DragEnter".
  Could it be a because of the folder view filtering where it accepts to get 
dragged for instance ?
  I am a bit stuck here. I am missing something.

REPOSITORY
  R241 KIO

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

To: meven, ngraham
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

[Méven Car]

meven updated this revision to Diff 57056.
meven added a comment.


  Remove unnecessary comment

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20838?vs=57027=57056

BRANCH
  arcpatch-D20838

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

AFFECTED FILES
  src/filewidgets/kdiroperator.cpp
  src/filewidgets/kdiroperator.h
  tests/kfilewidgettest_gui.cpp

To: meven, ngraham
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

[Méven Car]

meven marked an inline comment as done.

REPOSITORY
  R241 KIO

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

To: meven, ngraham
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

[Nathaniel Graham]

ngraham added a comment.


  Wow, it works and it's so nice. What an improvement!
  
  Drop options are not needed here; this isn't a move/copy/symlink UI; it's 
simply a quick way to choose a file that you happen to have visible in Dolphin 
or on the desktop. On that subject, I notice that drops from the desktop don't 
work. Not sure if that needs work here or in Folder View.

INLINE COMMENTS

> kdiroperator.cpp:1410
> +if (entry.isDir()) {
> +// if this was a directory
> +setUrl(url, false);

Comment seems unnecessary; the code is clear enough

REPOSITORY
  R241 KIO

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

To: meven, ngraham
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20301: Ensure to add a trailing / at the end of fstab mounted devices.

[Méven Car]

meven added a comment.


  In D20301#453031 , @ngraham wrote:
  
  > I've been told that @bruns can review this sometime next week. :)
  
  
  Can't wait, the bug annoys me.

REPOSITORY
  R245 Solid

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

To: meven, bruns, lukas, broulik
Cc: ngraham, kde-frameworks-devel, michaelh, bruns

D20748: Fix wrong "Unable to find service type" warnings

[David Faure]

dfaure added a reviewer: apol.

REPOSITORY
  R244 KCoreAddons

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

To: dfaure, mart, arichardson, davidedmundson, mpyne, apol
Cc: aacid, kde-frameworks-devel, michaelh, ngraham, bruns

D20844: KTar::KTarPrivate::readLonglink: Fix crash in malformed files

[Albert Astals Cid]

aacid added inline comments.

INLINE COMMENTS

> apol wrote in ktar.cpp:289
> Doesn't QString have the same size limitation as QByteArray?

I'll double check, also i now see that decodeName char creates a temporary 
qbytearray so that may be problematic too :/

REPOSITORY
  R243 KArchive

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

To: aacid
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D19850: Make AppStream generation opt-in

[Aleix Pol Gonzalez]

apol abandoned this revision.

REPOSITORY
  R290 KPackage

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

To: apol, #plasma, ngraham
Cc: sitter, ngraham, kde-frameworks-devel, michaelh, bruns

KDE CI: Frameworks » kpackage » kf5-qt5 FreeBSDQt5.12 - Build # 24 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kpackage/job/kf5-qt5%20FreeBSDQt5.12/24/
 Project:
kf5-qt5 FreeBSDQt5.12
 Date of build:
Fri, 26 Apr 2019 14:34:16 +
 Build duration:
57 sec and counting
   JUnit Tests
  Name: projectroot Failed: 1 test(s), Passed: 10 test(s), Skipped: 0 test(s), Total: 11 test(s)Failed: projectroot.autotests.plasma_packagestructuretest

D20839: AppStream generation: make sure we look up for the package structure on packages that have metadata.desktop/json too

[Aleix Pol Gonzalez]

This revision was automatically updated to reflect the committed changes.
Closed by commit R290:bf261881f6fe: AppStream generation: make sure we look up 
for the package structure on… (authored by apol).

REPOSITORY
  R290 KPackage

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20839?vs=57028=57048

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

AFFECTED FILES
  src/kpackagetool/kpackagetool.cpp

To: apol, #plasma, mart
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D14910: Pass a dedicated fd to each keyboard for the xkb keymap

[Vlad Zagorodniy]

zzag accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R127 KWayland

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

To: graesslin, #kwin, #frameworks, davidedmundson, zzag
Cc: plasma-devel, kde-frameworks-devel, michaelh, ngraham, bruns

D20839: AppStream generation: make sure we look up for the package structure on packages that have metadata.desktop/json too

[Marco Martin]

mart accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R290 KPackage

BRANCH
  master

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

To: apol, #plasma, mart
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D20844: KTar::KTarPrivate::readLonglink: Fix crash in malformed files

[Aleix Pol Gonzalez]

apol added inline comments.

INLINE COMMENTS

> ktar.cpp:289
>  case 'L':
> -name = QFile::decodeName(longlink.constData());
> +name = QFile::decodeName(longlink);
>  break;

Doesn't QString have the same size limitation as QByteArray?

REPOSITORY
  R243 KArchive

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

To: aacid
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20844: KTar::KTarPrivate::readLonglink: Fix crash in malformed files

[Albert Astals Cid]

aacid created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
aacid requested review of this revision.

REVISION SUMMARY
  QByteArray::resize doesn't work for std::numeric_limits::max()
  https://bugreports.qt.io/browse/QTBUG-75470
  rumour is that it only works up to 2^31 but since there's no define or
  similar better just use plain old new[]/delete[]
  
  oss-fuzz #14397

REPOSITORY
  R243 KArchive

BRANCH
  master

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

AFFECTED FILES
  src/ktar.cpp

To: aacid
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D20627: Introduce CriticalNotificationType

[David Edmundson]

davidedmundson accepted this revision.

REPOSITORY
  R278 KWindowSystem

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

To: broulik, #kwin, zzag, davidedmundson
Cc: zzag, graesslin, kde-frameworks-devel, michaelh, ngraham, bruns

D20627: Introduce CriticalNotificationType

[Vlad Zagorodniy]

zzag added subscribers: graesslin, zzag.
zzag accepted this revision.
zzag added a comment.
This revision is now accepted and ready to land.


  Hmm, in general this is quite delicate case because KWin/X11 is feature 
frozen...
  
  Given that there is no other way around (except using above state for 
critical notifications or letting plasmashell keep track of fullscreen state), 
the only option is to break the rule. :/
  
  But you may still want to wait for input from other #KWin 
 folks as well, more specifically from 
@graesslin.

REPOSITORY
  R278 KWindowSystem

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

To: broulik, #kwin, zzag
Cc: zzag, graesslin, kde-frameworks-devel, michaelh, ngraham, bruns

D20628: Add CriticalNotification window type to PlasmaShellSurface protocol

[Vlad Zagorodniy]

zzag added a comment.


  Please land together with other revisions.

REPOSITORY
  R127 KWayland

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

To: broulik, #kwin, zzag
Cc: zzag, davidedmundson, kde-frameworks-devel, michaelh, ngraham, bruns

D20628: Add CriticalNotification window type to PlasmaShellSurface protocol

[Vlad Zagorodniy]

zzag accepted this revision.
zzag added inline comments.
This revision is now accepted and ready to land.

INLINE COMMENTS

> davidedmundson wrote in plasma-shell.xml:194
> since="6"
> 
> See
> https://github.com/wayland-project/wayland/blob/master/protocol/wayland.dtd

Also, folks usually put

  

:-)

REPOSITORY
  R127 KWayland

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

To: broulik, #kwin, zzag
Cc: zzag, davidedmundson, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

[Méven Car]

meven added inline comments.

INLINE COMMENTS

> apol wrote in kdiroperator.cpp:418
> This default change should be documented.

Not sure I have done this correctly.

REPOSITORY
  R241 KIO

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

To: meven, ngraham
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20839: AppStream generation: make sure we look up for the package structure on packages that have metadata.desktop/json too

[Aleix Pol Gonzalez]

apol created this revision.
apol added a reviewer: Plasma.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
apol requested review of this revision.

TEST PLAN
  Generated the appstream for plasmoids in kdeplasma addons and they properly 
extend its X-KDE-ParentApp

REPOSITORY
  R290 KPackage

BRANCH
  master

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

AFFECTED FILES
  src/kpackagetool/kpackagetool.cpp

To: apol, #plasma
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

[Méven Car]

meven updated this revision to Diff 57027.
meven added a comment.


  Add a since in documentation

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20838?vs=57026=57027

BRANCH
  drop-kdiroperator

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

AFFECTED FILES
  src/filewidgets/kdiroperator.cpp
  src/filewidgets/kdiroperator.h
  tests/kfilewidgettest_gui.cpp

To: meven, ngraham
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

[Méven Car]

meven marked an inline comment as done.

REPOSITORY
  R241 KIO

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

To: meven, ngraham
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

[Méven Car]

meven marked 4 inline comments as done.

REPOSITORY
  R241 KIO

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

To: meven, ngraham
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

[Méven Car]

meven updated this revision to Diff 57026.
meven added a comment.


  Review feedback

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20838?vs=57024=57026

BRANCH
  drop-kdiroperator

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

AFFECTED FILES
  src/filewidgets/kdiroperator.cpp
  src/filewidgets/kdiroperator.h
  tests/kfilewidgettest_gui.cpp

To: meven, ngraham
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20748: Fix wrong "Unable to find service type" warnings

[Albert Astals Cid]

aacid added a comment.


  It looks good to me, but i'm not sure i know enough about this code to give 
you an approved, but if noone else does, let's assume i did :D

REPOSITORY
  R244 KCoreAddons

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

To: dfaure, mart, arichardson, davidedmundson, mpyne
Cc: aacid, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

[Aleix Pol Gonzalez]

apol added inline comments.

INLINE COMMENTS

> kdiroperator.cpp:418
>  setFocusPolicy(Qt::WheelFocus);
> +setAcceptDrops(true);
>  }

This default change should be documented.

> kdiroperator.cpp:1401
> +} else {
> +QUrl url = urls.at(0);
> +

.constFirst() maybe reads better?

> kdiroperator.cpp:1419
> +QMetaObject::Connection connection;
> +auto fun = [this, url, connection](){
> +this->setCurrentItem(url);

maybe call the function urlSetter?

> kdiroperator.cpp:1523
> +if (acceptsDrops) {
> +this->view()->installEventFilter(this);
> +} else {

Initial this-> isn't necessary, just `view()`.

> kfilewidgettest_gui.cpp:34
>  
> +app.connect(fileWidget, ::destroyed, fileWidget, []() {
> +app.exit();

No need to lambda:
`QObject::connect(fileWidget, ::destroyed, , 
::exit);`

REPOSITORY
  R241 KIO

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

To: meven, ngraham
Cc: apol, kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

[Méven Car]

meven added a reviewer: ngraham.

REPOSITORY
  R241 KIO

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

To: meven, ngraham
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D20838: Allow to drop one file or one folder on KDirOperator

[Méven Car]

meven created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
meven requested review of this revision.

REVISION SUMMARY
  Open questions:
  
  - drop options
  
  Todo:
  
  - Add automated tests
  
  BUG: 45154
  FIXED-IN: 5.58

TEST PLAN
  Manual testing using kfilewidgettestgui.cpp and kfilewidgettest_saving_gui.cpp

REPOSITORY
  R241 KIO

BRANCH
  drop-kdiroperator

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

AFFECTED FILES
  src/filewidgets/kdiroperator.cpp
  tests/kfilewidgettest_gui.cpp

To: meven
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D20833: Simpler check for flatpak

[Aleix Pol Gonzalez]

apol updated this revision to Diff 57023.
apol added a comment.


  No need for inSandbox to be a class member

REPOSITORY
  R289 KNotifications

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D20833?vs=56999=57023

BRANCH
  master

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

AFFECTED FILES
  src/knotificationmanager.cpp

To: apol, #frameworks
Cc: broulik, kde-frameworks-devel, michaelh, ngraham, bruns

D20833: Simpler check for flatpak

[Aleix Pol Gonzalez]

apol added a comment.


  We still use it to check whether to initialise `portalDBusServiceExists`. 
I'll simplify it a bit.

REPOSITORY
  R289 KNotifications

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

To: apol, #frameworks
Cc: broulik, kde-frameworks-devel, michaelh, ngraham, bruns

D20833: Simpler check for flatpak

[Kai Uwe Broulik]

broulik added inline comments.

INLINE COMMENTS

> knotificationmanager.cpp:94
>  
>  if (!qEnvironmentVariableIsEmpty("XDG_RUNTIME_DIR")) {
> +d->inSandbox = QFileInfo::exists(QLatin1String("/.flatpak-info"));

Do you need this check now, even, since you're not using it anymore?

REPOSITORY
  R289 KNotifications

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

To: apol, #frameworks
Cc: broulik, kde-frameworks-devel, michaelh, ngraham, bruns