D17730: Review KateStatusBar

2019-01-12 Thread loh tar 
loh.tar added a comment.


  > I like the new status bar.
  
  Thanks :-)
  
  > In "modified" state it would be cool to be able to click to save the 
document
  
  Yes, that was also my idea. IIRC I had somewhere asked but got no response. 
Will do it soon.
  
  > Move the modified label from far right to far left
  
  Sadly is now the Close button e.g. of the goto bar at the same position. So 
closing the goto bar may then unintended save the document when you have some 
mouse trouble
  
  > In "unedited" state the purpose is (still) hard to guess from the icon (and 
there is the convention of not having tooltips for the status bar which 
requires a creative solution).
  
  I agree, I'm also not so happy with that. My idea was to show a tool-tip on 
click. Sadly is that not a "one-liner" in Qt, need some quirks.

REPOSITORY
  R39 KTextEditor

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

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

D17137: KTextEditor: File menu: Put Save, Print and Export in submenus

2019-01-12 Thread loh tar 
loh.tar added a comment.


  Sadly is the benefit, to reduce the menu entries, not "optimal". In the pics 
you have now 3 sub-entries for one "Variant" entry, effectively only 2 less per 
topic.
  
  I may like it if these "Variant-Action" could be avoided and merged with one 
of the "Normal-Action", but guess that will find someone too 
progressive/uncommon and I'm not sure if that is technically possible by Qt.
  What? Well, e.g hovering "Save As..." offer entries from "Save Variants" but 
click on it still do "Save As..."

REPOSITORY
  R39 KTextEditor

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

To: gregormi, #kate, #kdevelop
Cc: loh.tar, anthonyfieroni, ngraham, cullmann, flherne, dhaumann, 
kwrite-devel, kde-frameworks-devel, hase, michaelh, bruns, demsking, sars

D17797: Include applets/22

2019-01-12 Thread Noah Davis 
ndavis accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R266 Breeze Icons

BRANCH
  fix

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

To: nicolasfella, #breeze, ndavis
Cc: ndavis, kde-frameworks-devel, trickyricky26, michaelh, ngraham, bruns

D17137: KTextEditor: File menu: Put Save, Print and Export in submenus

2019-01-12 Thread Nathaniel Graham 
ngraham added a comment.


  Lookin' pretty good!

REPOSITORY
  R39 KTextEditor

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

To: gregormi, #kate, #kdevelop
Cc: anthonyfieroni, ngraham, cullmann, flherne, dhaumann, kwrite-devel, 
kde-frameworks-devel, hase, michaelh, bruns, demsking, sars

D17137: KTextEditor: File menu: Put Save, Print and Export in submenus

2019-01-12 Thread gregormi 
gregormi added a comment.


  What I wonder: during the work with the .rc files I noticed that even the 
Kate which is installed in root got affected by the changes I made in the 
development version and I had to regularly change the version number to make 
changes take effect. Is this normal?

REPOSITORY
  R39 KTextEditor

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

To: gregormi, #kate, #kdevelop
Cc: anthonyfieroni, ngraham, cullmann, flherne, dhaumann, kwrite-devel, 
kde-frameworks-devel, hase, michaelh, bruns, demsking, sars

D17137: KTextEditor: File menu: Put Save, Print and Export in submenus

2019-01-12 Thread gregormi 
gregormi marked an inline comment as done.
gregormi added a comment.


  This is what is looks like now (together with latest changes from 
https://phabricator.kde.org/D17138):
  
  F6547077: grafik.png 

REPOSITORY
  R39 KTextEditor

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

To: gregormi, #kate, #kdevelop
Cc: anthonyfieroni, ngraham, cullmann, flherne, dhaumann, kwrite-devel, 
kde-frameworks-devel, hase, michaelh, bruns, demsking, sars

D17137: KTextEditor: File menu: Put Save, Print and Export in submenus

2019-01-12 Thread gregormi 
gregormi updated this revision to Diff 49369.
gregormi added a comment.


  - Move "Save As..." out of the submenu

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17137?vs=46132=49369

BRANCH
  arcpatch-D17137

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

AFFECTED FILES
  src/data/katepart5ui.rc

To: gregormi, #kate, #kdevelop
Cc: anthonyfieroni, ngraham, cullmann, flherne, dhaumann, kwrite-devel, 
kde-frameworks-devel, hase, michaelh, bruns, demsking, sars

D17730: Review KateStatusBar

2019-01-12 Thread gregormi 
gregormi added a comment.


  @loh.tar: I like the new status bar.
  
  > Move the modified label from far right to far left This way have it a more 
prominent position
  
  Now, the "modified label" behaves like a button but with no visible action 
when it is clicked. Two general observations:
  
  - In "unedited" state the purpose is (still) hard to guess from the icon (and 
there is the convention of not having tooltips for the status bar which 
requires a creative solution).
  - In "modified" state it would be cool to be able to click to save the 
document

REPOSITORY
  R39 KTextEditor

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

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

D17442: KTextEditor: Tweak keyboard shortcuts to free F keys

2019-01-12 Thread gregormi 
gregormi retitled this revision from "KTextEditor: Tweak keyboard shortcuts to 
prepare for F6/Shift+F6 in Kate" to "KTextEditor: Tweak keyboard shortcuts to 
free F keys".

REPOSITORY
  R39 KTextEditor

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

To: gregormi, #kate
Cc: cullmann, dhaumann, ngraham, kwrite-devel, kde-frameworks-devel, hase, 
michaelh, bruns, demsking, sars

D17442: KTextEditor: Tweak keyboard shortcuts to prepare for F6/Shift+F6 in Kate

2019-01-12 Thread gregormi 
gregormi added a comment.


  With respect to bringing this https://phabricator.kde.org/T10279 forward, how 
should I proceed here? Can this go in as first step (after I removed F10 and 
F11)? (see also https://phabricator.kde.org/D17443)

INLINE COMMENTS

> kateview.cpp:700
>  ac->addAction(QStringLiteral("view_dynamic_word_wrap"), a);
> -ac->setDefaultShortcut(a, QKeySequence(Qt::Key_F10));
> +ac->setDefaultShortcuts(a, { QKeySequence(Qt::Key_F10), 
> QKeySequence(Qt::CTRL + Qt::Key_E, Qt::CTRL + Qt::Key_W) });
>  a->setWhatsThis(i18n("If this option is checked, the text lines will be 
> wrapped at the view border on the screen."));

todo: Remove F10

> kateview.cpp:727
>  ac->addAction(QStringLiteral("view_line_numbers"), a);
> -ac->setDefaultShortcut(a, QKeySequence(Qt::Key_F11));
> +ac->setDefaultShortcuts(a, { QKeySequence(Qt::Key_F11), 
> QKeySequence(Qt::CTRL + Qt::Key_E, Qt::CTRL + Qt::Key_L) });
>  a->setWhatsThis(i18n("Show/hide the line numbers on the left hand side 
> of the view."));

todo: remove F11

REPOSITORY
  R39 KTextEditor

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

To: gregormi, #kate
Cc: cullmann, dhaumann, ngraham, kwrite-devel, kde-frameworks-devel, hase, 
michaelh, bruns, demsking, sars

D18199: Add Kotlin (.kt) mimetype icons

2019-01-12 Thread TrickyRicky 
trickyricky26 planned changes to this revision.
trickyricky26 added a comment.


  I'm going to add the gradient to the `32px` as well

REPOSITORY
  R266 Breeze Icons

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

To: trickyricky26, #vdg, ngraham
Cc: alex-l, ngraham, kde-frameworks-devel, michaelh, bruns

D18215: Improve the preferences-desktop-cryptography icon

2019-01-12 Thread Nathaniel Graham 
ngraham requested changes to this revision.
ngraham added a comment.
This revision now requires changes to proceed.


  Much better than the original, but if we're trying to emulate the existing 
style, we should come as close to pixel-perfect as we can. :)
  
  Compare your new icon to the existing `preferences-system-network-iscsi` 
icon, which uses the same style:
  
  F6546965: Crypto.png 
  
  F6546966: iscsi.png 

REPOSITORY
  R266 Breeze Icons

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

To: trickyricky26, #vdg, ngraham
Cc: ngraham, kde-frameworks-devel, michaelh, bruns

D18199: Add Kotlin (.kt) mimetype icons

2019-01-12 Thread Nathaniel Graham 
ngraham accepted this revision.
ngraham added a comment.
This revision is now accepted and ready to land.


  That's much better! I like the subtle gradient.
  
  #VDG  folks, is everyone else good with 
this?

REPOSITORY
  R266 Breeze Icons

BRANCH
  add-kotlin-icons (branched from master)

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

To: trickyricky26, #vdg, ngraham
Cc: alex-l, ngraham, kde-frameworks-devel, michaelh, bruns

D18213: Fill lock in preferences-desktop-user-password

2019-01-12 Thread Nathaniel Graham 
This revision was automatically updated to reflect the committed changes.
Closed by commit R266:6af2171307e8: Fill lock in 
preferences-desktop-user-password (authored by trickyricky26, committed by 
ngraham).

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18213?vs=49348=49363

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

AFFECTED FILES
  icons-dark/preferences/32/preferences-desktop-user-password.svg
  icons/preferences/32/preferences-desktop-user-password.svg

To: trickyricky26, #vdg, ngraham
Cc: ngraham, kde-frameworks-devel, michaelh, bruns

D18213: Fill lock in preferences-desktop-user-password

2019-01-12 Thread Nathaniel Graham 
ngraham accepted this revision.
ngraham added a comment.
This revision is now accepted and ready to land.


  Thanks, much better.

REPOSITORY
  R266 Breeze Icons

BRANCH
  fill-lock-pref-password (branched from master)

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

To: trickyricky26, #vdg, ngraham
Cc: ngraham, kde-frameworks-devel, michaelh, bruns

D18215: Improve the preferences-desktop-cryptography icon

2019-01-12 Thread TrickyRicky 
trickyricky26 updated this revision to Diff 49360.
trickyricky26 added a comment.


  - widen the right part of the SATA connector to be more symmterical

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18215?vs=49356=49360

BRANCH
  improve-preferences-cryptography (branched from master)

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

AFFECTED FILES
  icons-dark/preferences/32/preferences-desktop-cryptography.svg
  icons/preferences/32/preferences-desktop-cryptography.svg

To: trickyricky26, #vdg
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D18215: Improve the preferences-desktop-cryptography icon

2019-01-12 Thread TrickyRicky 
trickyricky26 edited the test plan for this revision.

REPOSITORY
  R266 Breeze Icons

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

To: trickyricky26, #vdg
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D18215: Improve the preferences-desktop-cryptography icon

2019-01-12 Thread TrickyRicky 
trickyricky26 edited the test plan for this revision.

REPOSITORY
  R266 Breeze Icons

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

To: trickyricky26, #vdg
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D18215: Improve the preferences-desktop-cryptography icon

2019-01-12 Thread TrickyRicky 
trickyricky26 updated this revision to Diff 49356.
trickyricky26 added a comment.


  - Shift the lock upwards a bit, remove bottom left shadow

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18215?vs=49354=49356

BRANCH
  improve-preferences-cryptography (branched from master)

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

AFFECTED FILES
  icons-dark/preferences/32/preferences-desktop-cryptography.svg
  icons/preferences/32/preferences-desktop-cryptography.svg

To: trickyricky26, #vdg
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17851: Add Android notification backend

2019-01-12 Thread Volker Krause 
vkrause added inline comments.

INLINE COMMENTS

> broulik wrote in notifybyandroid.cpp:79
> Can there be multiple instances of this `NotifyByAndroid`?

KNotificationManager prevents that IIUC.

> broulik wrote in notifybyandroid.cpp:125
> Does this need a `FreeLocalRef` call?

Possible, I'll investigate.

REPOSITORY
  R289 KNotifications

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

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

D18215: Improve the preferences-desktop-cryptography icon

2019-01-12 Thread TrickyRicky 
trickyricky26 edited the summary of this revision.

REPOSITORY
  R266 Breeze Icons

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

To: trickyricky26, #vdg
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D18215: Improve the preferences-desktop-cryptography icon

2019-01-12 Thread TrickyRicky 
trickyricky26 edited the test plan for this revision.

REPOSITORY
  R266 Breeze Icons

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

To: trickyricky26, #vdg
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D18215: Improve the preferences-desktop-cryptography icon

2019-01-12 Thread TrickyRicky 
trickyricky26 updated this revision to Diff 49354.
trickyricky26 added a comment.


  - Remove Plasma logo and shadow of the white label

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18215?vs=49351=49354

BRANCH
  improve-preferences-cryptography (branched from master)

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

AFFECTED FILES
  icons-dark/preferences/32/preferences-desktop-cryptography.svg
  icons/preferences/32/preferences-desktop-cryptography.svg

To: trickyricky26, #vdg
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D18199: Add Kotlin (.kt) mimetype icons

2019-01-12 Thread TrickyRicky 
trickyricky26 added a subscriber: alex-l.

REPOSITORY
  R266 Breeze Icons

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

To: trickyricky26, #vdg
Cc: alex-l, ngraham, kde-frameworks-devel, michaelh, bruns

D18199: Add Kotlin (.kt) mimetype icons

2019-01-12 Thread TrickyRicky 
trickyricky26 edited the test plan for this revision.

REPOSITORY
  R266 Breeze Icons

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

To: trickyricky26, #vdg
Cc: ngraham, kde-frameworks-devel, michaelh, bruns

D18199: Add Kotlin (.kt) mimetype icons

2019-01-12 Thread TrickyRicky 
trickyricky26 updated this revision to Diff 49353.
trickyricky26 added a comment.


  - Use a nicer purple color, similar to the official logo

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18199?vs=49301=49353

BRANCH
  add-kotlin-icons (branched from master)

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

AFFECTED FILES
  icons-dark/mimetypes/16/text-x-kotlin.svg
  icons-dark/mimetypes/22/text-x-kotlin.svg
  icons-dark/mimetypes/32/text-x-kotlin.svg
  icons-dark/mimetypes/64/text-x-kotlin.svg
  icons/mimetypes/16/text-x-kotlin.svg
  icons/mimetypes/22/text-x-kotlin.svg
  icons/mimetypes/32/text-x-kotlin.svg
  icons/mimetypes/64/text-x-kotlin.svg

To: trickyricky26, #vdg
Cc: ngraham, kde-frameworks-devel, michaelh, bruns

D17851: Add Android notification backend

2019-01-12 Thread Kai Uwe Broulik 
broulik added a comment.


  Pretty cool!

INLINE COMMENTS

> notifybyandroid.cpp:79
> +{
> +s_instance = this;
> +#if __ANDROID_API__ >= 23

Can there be multiple instances of this `NotifyByAndroid`?

> notifybyandroid.cpp:125
> +pixmap.save(, "PNG");
> +auto jIconData = env->NewByteArray(iconData.length());
> +env->SetByteArrayRegion(jIconData, 0, iconData.length(), 
> reinterpret_cast(iconData.constData()));

Does this need a `FreeLocalRef` call?

REPOSITORY
  R289 KNotifications

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

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

D18215: Improve the preferences-desktop-cryptography icon

2019-01-12 Thread TrickyRicky 
trickyricky26 edited the test plan for this revision.

REPOSITORY
  R266 Breeze Icons

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

To: trickyricky26, #vdg
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D18215: Improve the preferences-desktop-cryptography icon

2019-01-12 Thread TrickyRicky 
trickyricky26 edited the summary of this revision.
trickyricky26 edited the test plan for this revision.

REPOSITORY
  R266 Breeze Icons

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

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

D18215: Improve the preferences-desktop-cryptography icon

2019-01-12 Thread TrickyRicky 
trickyricky26 added a reviewer: VDG.

REPOSITORY
  R266 Breeze Icons

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

To: trickyricky26, #vdg
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D18215: Improve the preferences-desktop-cryptography icon

2019-01-12 Thread TrickyRicky 
trickyricky26 created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
trickyricky26 requested review of this revision.

REPOSITORY
  R266 Breeze Icons

BRANCH
  improve-preferences-cryptography (branched from master)

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

AFFECTED FILES
  icons-dark/preferences/32/preferences-desktop-cryptography.svg
  icons/preferences/32/preferences-desktop-cryptography.svg

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

D18213: Fill lock in preferences-desktop-user-password

2019-01-12 Thread TrickyRicky 
trickyricky26 edited the summary of this revision.
trickyricky26 added a reviewer: VDG.

REPOSITORY
  R266 Breeze Icons

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

To: trickyricky26, #vdg
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D18213: Fill lock in preferences-desktop-user-password

2019-01-12 Thread TrickyRicky 
trickyricky26 created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
trickyricky26 requested review of this revision.

REPOSITORY
  R266 Breeze Icons

BRANCH
  fill-lock-pref-password (branched from master)

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

AFFECTED FILES
  icons-dark/preferences/32/preferences-desktop-user-password.svg
  icons/preferences/32/preferences-desktop-user-password.svg

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

D17932: Improvements to completion

2019-01-12 Thread Thomas Schöps 
thomassc updated this revision to Diff 49343.
thomassc added a comment.


  Update according to Milian's comments

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17932?vs=48574=49343

BRANCH
  improvements_to_completion (branched from master)

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

AFFECTED FILES
  autotests/src/completion_test.cpp
  src/completion/katecompletionmodel.cpp
  src/completion/katecompletionmodel.h

To: thomassc, #ktexteditor, #kdevelop, mwolff
Cc: mwolff, cullmann, kwrite-devel, kde-frameworks-devel, hase, michaelh, 
ngraham, bruns, demsking, head7, kfunk, sars, dhaumann

D17932: Improvements to completion

2019-01-12 Thread Thomas Schöps 
thomassc marked 4 inline comments as done.
thomassc added a comment.


  Thanks for reviewing. Regarding the question about which models would have an 
insensitive exact match, and which ones have sensitive exact matches:
  
  - An example for case-insensitive exact matches might be plain text, or a 
hypothetical case-insensitive programming language. For example for plain text, 
one might want to treat words like "Question" and "question" as exact matches, 
which will make the completion widget close itself when it shows one of them 
and the user types the other. This is the current behavior for the word 
completion in KWrite / Kate.
  - An example for case-sensitive exact matches would be a case-sensitive 
programming language like C++. If the user typed "m_var" but the variable is 
actually called "m_Var", then the completion widget should not hide itself, 
since it might still be useful to replace the typed text with the completion 
item that has different case.

INLINE COMMENTS

> mwolff wrote in katecompletionmodel.cpp:2026
> maybe simplify this to:
> 
> if (matchCompletion && m_nameColumn.startsWith(match, 
> model->exactMatchCaseSensitivity())) {
> 
>   matchCompletion = PerfectMatch;
>   m_haveExactMatch = true;
> 
> }

Hmm, I guess the current version might be a tiny bit faster in general since it 
only does another startsWith() check if this would be a stricter check than the 
one done before ...

> mwolff wrote in katecompletionmodel.h:394
> should this comment be asserted in the setters or is it handled gracefully in 
> the logic below?

I added asserts to the setters. One should be aware then though that it 
restricts the order in which these must be called. For example, if both 
settings are case-insensitive at first and both should be changed to 
case-sensitive, then the exact-match-sensitivity must be changed before the 
match-sensitivity. Alternatively, one could make a single setter function that 
changes both properties.

REPOSITORY
  R39 KTextEditor

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

To: thomassc, #ktexteditor, #kdevelop, mwolff
Cc: mwolff, cullmann, kwrite-devel, kde-frameworks-devel, hase, michaelh, 
ngraham, bruns, demsking, head7, kfunk, sars, dhaumann

D18204: Make KCheckAccelerators less invasive for apps that don't directly link to KXmlGui

2019-01-12 Thread Albert Astals Cid 
This revision was automatically updated to reflect the committed changes.
Closed by commit R263:02b523bad09a: Make KCheckAccelerators less invasive for 
apps that dont directly link to… (authored by aacid).

REPOSITORY
  R263 KXmlGui

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18204?vs=49340=49341

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

AFFECTED FILES
  src/kcheckaccelerators.cpp

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

D18167: Move -Wsuggest-override -Wlogical-op to regular compiler settings

2019-01-12 Thread Albert Astals Cid 
aacid added a comment.


  Since i have two +1 i'll commit this next saturday unless someone shouts in 
disagreement

REPOSITORY
  R240 Extra CMake Modules

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

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

D18204: Make KCheckAccelerators less invasive for apps that don't directly link to KXmlGui

2019-01-12 Thread Anthony Fieroni 
anthonyfieroni accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R263 KXmlGui

BRANCH
  arcpatch-D18204

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

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

D18204: Make KCheckAccelerators less invasive for apps that don't directly link to KXmlGui

2019-01-12 Thread Albert Astals Cid 
aacid updated this revision to Diff 49340.
aacid added a comment.


  Change style to make Anthony happy

REPOSITORY
  R263 KXmlGui

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18204?vs=49319=49340

BRANCH
  arcpatch-D18204

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

AFFECTED FILES
  src/kcheckaccelerators.cpp

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

D17691: Add rows info to the plasma virtual desktop protocol

2019-01-12 Thread Eike Hein 
hein added a task: T4457: [kwayland] Virtual Desktop protocol.

REPOSITORY
  R127 KWayland

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

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

D18010: Allow zero altitude/longitude/latitude exiv gps data

2019-01-12 Thread Stefan Brüns 
bruns accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R286 KFileMetaData

BRANCH
  fix_zero_gps

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

To: astippich, bruns
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D18205: test empty and zero gps data

2019-01-12 Thread Stefan Brüns 
bruns added inline comments.

INLINE COMMENTS

> exiv2extractortest.cpp:62
> +
> +using namespace KFileMetaData::Property;
> +QCOMPARE(result.properties().value(PhotoGpsLatitude).toDouble(), 41.411);

move this up, as it is not specific to this test case

> exiv2extractortest.cpp:65
> +QCOMPARE(result.properties().value(PhotoGpsLongitude).toDouble(), 2.173);
> +QVERIFY(qAbs(result.properties().value(PhotoGpsAltitude).toDouble() - 
> 12.2) <  0.0001);
> +

although only copied, I think it should be written like the other two:
`QCOMPARE(result.properties().value(PhotoGpsAltiitude).toDouble(), 12.2);`

REPOSITORY
  R286 KFileMetaData

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

To: astippich, bruns
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D18038: Fix semantics for ghns_exclude

2019-01-12 Thread Dan Leinir Turthra Jensen 
This revision was automatically updated to reflect the committed changes.
Closed by commit R304:301fe73569ba: Fix semantics for ghns_exclude (authored by 
leinir).

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18038?vs=48859=49339

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

AFFECTED FILES
  autotests/knewstuffentrytest.cpp
  src/core/engine.cpp
  src/core/engine.h
  src/core/tagsfilterchecker.cpp
  src/core/tagsfilterchecker.h
  tests/testdata/entry.xml

To: leinir, ronaldv, #kde_store, ngraham
Cc: kde-frameworks-devel, michaelh, ZrenBot, ngraham, bruns, akiraohgaki, 
alexanderschmidt, siyuandong, ronaldv, mikesomov, starbuck

D18038: Fix semantics for ghns_exclude

2019-01-12 Thread Dan Leinir Turthra Jensen 
leinir edited the summary of this revision.

REPOSITORY
  R304 KNewStuff

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

To: leinir, ronaldv, #kde_store, ngraham
Cc: kde-frameworks-devel, michaelh, ZrenBot, ngraham, bruns, akiraohgaki, 
alexanderschmidt, siyuandong, ronaldv, mikesomov, starbuck

D18038: Fix semantics for ghns_exclude

2019-01-12 Thread Dan Leinir Turthra Jensen 
leinir added a comment.


  In D18038#391684 , @ngraham wrote:
  
  > Thanks, in addition to the testing tool working, this patch seems to 
actually fix the issue in production (e.g. "Tree on Island" is no longer 
visible in the wallpaper downloader), and as far as I can tell the code is 
sane. Thanks for the additional documentation and commenting too.
  >
  > Should this be marked as actually fixing 402888? If so, it should be `BUG: 
402888`
  
  
  Yay! :D i'm quite keen on documentation being solid, i know what it's like to 
arrive at something which is... less than well documented ;) Great stuff, nice 
to know it works for people not me :)
  
  I think it should probably fix said bug (which also will show you what i mean 
when i say it doesn't work for me ;) ).

REPOSITORY
  R304 KNewStuff

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

To: leinir, ronaldv, #kde_store, ngraham
Cc: kde-frameworks-devel, michaelh, ZrenBot, ngraham, bruns, akiraohgaki, 
alexanderschmidt, siyuandong, ronaldv, mikesomov, starbuck

D16882: [KDevelop/Shell] prevent duplicate added contextmenu actions

2019-01-12 Thread René J . V . Bertin 
rjvbb added a comment.


  Milian Wolff wrote on 20190112::12:35:11 re: "D16882 
<https://phabricator.kde.org/D16882>: [KDevelop/Shell] prevent duplicate added 
contextmenu actions"
  
  >> mwolff wrote in textdocument.cpp:691
  >>  can't you just add a slot here that removes the actions we added once the 
menu is closed? that would fix this issue with way less code
  > 
  > and with slot I mean an local lambda that takes a copy of the actions list
  
  I agree that would be more elegant, and there was code (yours, IIRC) which 
aimed to do this. The problem is that for some reason the aboutToClose signal 
is not reliable.

REPOSITORY
  R32 KDevelop

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

To: rjvbb, #kdevelop, kossebau, mwolff
Cc: mwolff, egospodinova, kossebau, kde-frameworks-devel, kdevelop-devel, 
glebaccon, hase, antismap, iodelay, geetamc, Pilzschaf, akshaydeo, surgenight, 
arrowd

D18204: Make KCheckAccelerators less invasive for apps that don't directly link to KXmlGui

2019-01-12 Thread Anthony Fieroni 
anthonyfieroni added inline comments.

INLINE COMMENTS

> kcheckaccelerators.cpp:87
> +static bool doCheckAccelerators = true;
> +
>  QCoreApplication *app = QCoreApplication::instance();

We can cheet here

  if (!doCheckAccelerators) {
  return;
  }

> kcheckaccelerators.cpp:114
> +
> +if (doCheckAccelerators) {
> +KCheckAcceleratorsInitializer *initializer = new 
> KCheckAcceleratorsInitializer(app);

Don't need check :)

REPOSITORY
  R263 KXmlGui

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

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

D16882: [KDevelop/Shell] prevent duplicate added contextmenu actions

2019-01-12 Thread Milian Wolff 
mwolff added inline comments.

INLINE COMMENTS

> mwolff wrote in textdocument.cpp:691
> can't you just add a slot here that removes the actions we added once the 
> menu is closed? that would fix this issue with way less code

and with slot I mean an local lambda that takes a copy of the actions list

REPOSITORY
  R32 KDevelop

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

To: rjvbb, #kdevelop, kossebau, mwolff
Cc: mwolff, egospodinova, kossebau, kde-frameworks-devel, kdevelop-devel, 
glebaccon, hase, antismap, iodelay, geetamc, Pilzschaf, akshaydeo, surgenight, 
arrowd

D16882: [KDevelop/Shell] prevent duplicate added contextmenu actions

2019-01-12 Thread Milian Wolff 
mwolff requested changes to this revision.
mwolff added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> textdocument.cpp:691
> -menu->addAction(action);
> -}
> -}

can't you just add a slot here that removes the actions we added once the menu 
is closed? that would fix this issue with way less code

REPOSITORY
  R32 KDevelop

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

To: rjvbb, #kdevelop, kossebau, mwolff
Cc: mwolff, egospodinova, kossebau, kde-frameworks-devel, kdevelop-devel, 
glebaccon, hase, antismap, iodelay, geetamc, Pilzschaf, akshaydeo, surgenight, 
arrowd

D18158: Fix elapsed time check

2019-01-12 Thread Elvis Angelaccio 
elvisangelaccio added a comment.


  What else is missing to fix #402665?

REPOSITORY
  R241 KIO

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

To: davidedmundson, chinmoyr
Cc: elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, bruns

D18204: Make KCheckAccelerators less invasive for apps that don't directly link to KXmlGui

2019-01-12 Thread Albert Astals Cid 
aacid updated this revision to Diff 49319.
aacid added a comment.


  Review comments update

REPOSITORY
  R263 KXmlGui

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18204?vs=49308=49319

BRANCH
  arcpatch-D18204

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

AFFECTED FILES
  src/kcheckaccelerators.cpp

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

D18010: Allow zero altitude/longitude/latitude exiv gps data

2019-01-12 Thread Alexander Stippich 
astippich added a dependency: D18205: test empty and zero gps data.

REPOSITORY
  R286 KFileMetaData

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

To: astippich, bruns
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D18205: test empty and zero gps data

2019-01-12 Thread Alexander Stippich 
astippich added a dependent revision: D18010: Allow zero 
altitude/longitude/latitude exiv gps data.

REPOSITORY
  R286 KFileMetaData

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

To: astippich, bruns
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D18010: Allow zero altitude/longitude/latitude exiv gps data

2019-01-12 Thread Alexander Stippich 
astippich updated this revision to Diff 49317.
astippich added a comment.


  - rebase on revision

REPOSITORY
  R286 KFileMetaData

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18010?vs=48789=49317

BRANCH
  fix_zero_gps

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

AFFECTED FILES
  src/extractors/exiv2extractor.cpp

To: astippich, bruns
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D18205: test empty and zero gps data

2019-01-12 Thread Alexander Stippich 
astippich created this revision.
astippich added a reviewer: bruns.
Herald added projects: Frameworks, Baloo.
Herald added subscribers: Baloo, kde-frameworks-devel.
astippich requested review of this revision.

REVISION SUMMARY
  Create a new test that more extensively
  checks gps data obtained via exiv. The test
  currently fails.

REPOSITORY
  R286 KFileMetaData

BRANCH
  gps_tests

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

AFFECTED FILES
  autotests/exiv2extractortest.cpp
  autotests/exiv2extractortest.h
  autotests/samplefiles/test_no_gps.jpg
  autotests/samplefiles/test_zero_gps.jpg

To: astippich, bruns
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

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

2019-01-12 Thread CI System 
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20FreeBSDQt5.12/13/
 Project:
kf5-qt5 FreeBSDQt5.12
 Date of build:
Sat, 12 Jan 2019 08:05:44 +
 Build duration:
5 min 36 sec and counting
   JUnit Tests
  Name: projectroot Failed: 4 test(s), Passed: 48 test(s), Skipped: 0 test(s), Total: 52 test(s)Failed: projectroot.autotests.kiocore_kmountpointtestFailed: projectroot.autotests.kiowidgets_dropjobtestFailed: projectroot.autotests.kiowidgets_kdirlistertestFailed: projectroot.autotests.kiowidgets_kdirmodeltestName: projectroot.autotests Failed: 0 test(s), Passed: 6 test(s), Skipped: 0 test(s), Total: 6 test(s)Name: projectroot.src.ioslaves.trash Failed: 1 test(s), Passed: 0 test(s), Skipped: 0 test(s), Total: 1 test(s)Failed: projectroot.src.ioslaves.trash.tests.testtrashName: projectroot.src.kpasswdserver Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)

D18204: Make KCheckAccelerators less invasive for apps that don't directly link to KXmlGui

2019-01-12 Thread David Edmundson 
davidedmundson added a comment.


  That's bloody clever! 
  +1

REPOSITORY
  R263 KXmlGui

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

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

D18158: Fix elapsed time check

2019-01-12 Thread David Edmundson 
This revision was automatically updated to reflect the committed changes.
Closed by commit R241:cd2f67c39b25: Fix elapsed time check (authored by 
davidedmundson).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18158?vs=49166=49314

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

AFFECTED FILES
  src/core/slaveinterface.cpp

To: davidedmundson, chinmoyr
Cc: kde-frameworks-devel, michaelh, ngraham, bruns