D18424: Instantiate QApplication before KCrash/KCatalog

2019-01-20 Thread Laurent Montel
mlaurent accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R293 Baloo

BRANCH
  master

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

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


D18424: Instantiate QApplication before KCrash/KCatalog

2019-01-20 Thread Luca Beltrame
lbeltrame added reviewers: bruns, poboiko.

REPOSITORY
  R293 Baloo

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

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


D13216: Overhaul the file index scheduler.

2019-01-20 Thread James Smith
smithjd added a comment.


  "Locking" the scheduler is simpler here than identifying that it "has gone 
idle". "Locking" only happens here for a small number of runnables (two) that 
can't run concurrently.

REPOSITORY
  R293 Baloo

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

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


D18125: KateStatusBar: Add dictionary button

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


  > m_dictionaryGroup->checkedAction() might be null.
  
  Oops! Thanks.
  
  > For the naming: Given we show the encodings with their short-names, too, I 
can live with de_DE or stuff like that.
  
  Is it OK to add some cutting like that? Currently is that extra stuff shown
  
  - en-wo_accents -> en
  - en_AU-variant_0 -> en_AU
  
  > For the overhead: I would like to have this optional and disabled per 
default, that avoids all that work in cursorPositionChanged()
  >  Just like we do for word count/...
  
  Hm, would be nice I could do this in an extra patch. At best after my 
"general config interface idea" (where I have nothing tried to do as of now)
  Did you do some perf tests or simiar, is it so much extra work?
  Ideas to reduce the overhead?

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

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


[sysadmin/ci-tooling] build-specs/Plasma: Disable execution of tests for plasma-integration.

2019-01-20 Thread Ben Cooksley
Git commit f6c79ff4787148459aa91c17d683e4fd6a57c323 by Ben Cooksley.
Committed on 21/01/2019 at 05:46.
Pushed by bcooksley into branch 'master'.

Disable execution of tests for plasma-integration.
This is necessary to ensure CI nodes do not become blocked due to hanging tests 
withing plasma-integration.

Currently, plasma-integration has several tests that make use of KIO slaves 
directly (skipping KLauncher).
Unfortunately, they do not terminate the slaves prior to the conclusion of the 
test, resulting in the kioslave processes being left around afterwards.
This is a condition that CTest will not tolerate, leading to it waiting 
indefinitely for these processes to exit - and in turn blocking all other 
builds on the CI node in question.
While this is not a major issue in the case of Linux builds, it can quickly 
become a severe condition in the case of FreeBSD and Windows builds due to 
those builders being fixed rather than dynamically allocated.

This class of issue (CTest waiting due to resident processes being left behind) 
has been a major issue as of late and is quickly leading to the CI system 
becoming unmaintainable due to the level of breakage.
Should it be necessary to ensure the maintainability of the system, withdrawal 
of execution of tests for all projects is an option currently under 
consideration.

CCMAIL: plasma-de...@kde.org
CCMAIL: kde-frameworks-devel@kde.org
CCMAIL: release-t...@kde.org
CCMAIL: kdevelop-de...@kde.org
CCMAIL: sysad...@kde.org

M  +2-0build-specs/Plasma/plasma-integration.yaml

https://invent.kde.org/sysadmin/ci-tooling/commit/f6c79ff4787148459aa91c17d683e4fd6a57c323

diff --git a/build-specs/Plasma/plasma-integration.yaml 
b/build-specs/Plasma/plasma-integration.yaml
index 3d39455..159546c 100644
--- a/build-specs/Plasma/plasma-integration.yaml
+++ b/build-specs/Plasma/plasma-integration.yaml
@@ -1,5 +1,7 @@
 kf5-qt5:
+  run-tests: False
   force-inject-asan: True
 
 stable-kf5-qt5:
+  run-tests: False
   force-inject-asan: True


D17459: SearchBar: Add Cancel button to stop long running tasks

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


  > are you interested in that challenge, too?
  
  Um...atm? No.

REPOSITORY
  R39 KTextEditor

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

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


D17857: DocumentPrivate: Don't scroll view when add auto-bracket at end of file

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


  > But on the other side: how does one reproduce this?
  
  Hm, I had just follow the descriptions in the report.

REPOSITORY
  R39 KTextEditor

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

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


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

2019-01-20 Thread Martin Flöser
graesslin added a comment.


  For done code this warning is pointless and negative. I invite you to work 
with a code base like KWin where it is more important to have a working git 
blame than protection for theoretical problems. Nobody will be able to 
guarantee that a 500+ change to add override won't break. Human errors happen, 
nobody will be able to review something like that. Addressing this warning by 
adding override all over our legacy code base has a serious risk.
  
  I'm seriously pissed that this is forced on us and we have to change code.
  
  I'm totally fine with this warning for new code and new projects. But let 
projects opt in for it instead of forcing it on legacy code bases.

REPOSITORY
  R240 Extra CMake Modules

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

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


D18424: Instantiate QApplication before KCrash/KCatalog

2019-01-20 Thread Simon Depiets
sdepiets edited the summary of this revision.
sdepiets added a reviewer: Baloo.

REPOSITORY
  R293 Baloo

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

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


D18424: Instantiate QApplication before KCrash/KCatalog

2019-01-20 Thread Simon Depiets
sdepiets created this revision.
Herald added projects: Frameworks, Baloo.
Herald added subscribers: Baloo, kde-frameworks-devel.
sdepiets requested review of this revision.

REPOSITORY
  R293 Baloo

BRANCH
  master

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

AFFECTED FILES
  src/file/extractor/main.cpp

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


D13216: Overhaul the file index scheduler.

2019-01-20 Thread Stefan Brüns
bruns requested changes to this revision.
bruns added a comment.
This revision now requires changes to proceed.


  STOP IT!
  You are reverting recent, required changes.

REPOSITORY
  R293 Baloo

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

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


D13216: Overhaul the file index scheduler.

2019-01-20 Thread James Smith
smithjd updated this revision to Diff 49977.
smithjd added a comment.


  - Re-write the file index scheduler. Combine content indexer suspend logic.
  - Update the balooctl tool with the changed suspend/resume behaviour.
  - Re-order and use IndexerState to prioritize the indexer thread pool.
  - Newline and tab fixes.
  - Prevent a scheduler race condition.
  - Separate runnableStateChanged into two functors.
  - Allow the first run indexer to complete before running any other runnables. 
Prevent the new file and unindexed file runnables from running at the same time.
  - Revert "Re-order and use IndexerState to prioritize the indexer thread 
pool."
  - Simplify locking the scheduler.
  - Adapt index cleaner to scheduler.
  - Rebase

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13216?vs=43623&id=49977

BRANCH
  master-scheduler (branched from master)

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

AFFECTED FILES
  src/file/fileindexscheduler.cpp
  src/file/fileindexscheduler.h
  src/tools/balooctl/main.cpp

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


D18380: KIO: make file dialog columns resizable again (and movable)

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


  >   I don't see the squeezed text when using Breeze and Noto Sans. Am I 
missing something?
  
  No, I don't think so. This must correspond to the 2nd pair of images I 
attached.
  
  >   I kind of hope so, because I predict that if we ship with text that gets 
squeezed at various sizes, we'll totally get bug reports about it. :-)
  
  No, this is mostly a font-specific thing; Noto Sans apparently doesn't 
stretch (nor squeeze) at typical UI sizes.
  And do you know of examples where this widget does NOT use the generic text 
font for the directory list? IOW, what are the chances that a single user can 
get instances of this widget where the font doesn't stretch and other instances 
where the font does stretch (because of being at another size or family)? Would 
they even notice?
  
  I kind of agree with the principle of not complicating things overly, but I 
also kind of like the effect I created here. Who could we add to this review to 
get some more opinions?
  
  I have a little font tinker app at github:RJVB/fontweightissue-qt5 (and a 
plasma-integration patch that adds support for alternative "kdeglobals" 
configurations at 
https://github.com/RJVB/macstrop/blob/master/Linux/kf5/kf5-plasma-integration/files/patch-support-alt-config.diff
 ;) )
  
  FWIW, there's also a possibility to reduce letter spacing which can help 
here, but there the margin for error is a lot smaller.

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

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


D18421: [autotests] Stabilize testWindowmanagement

2019-01-20 Thread Vlad Zagorodniy
zzag created this revision.
zzag added a reviewer: KWin.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
zzag requested review of this revision.

REVISION SUMMARY
  In rare cases testWindowmanagement fails because the initial iconChanged
  signal gets emitted after setIcon is called.

REPOSITORY
  R127 KWayland

BRANCH
  stabilize-test-windowmanagement

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

AFFECTED FILES
  autotests/client/test_wayland_windowmanagement.cpp

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


D11236: [KCrash] Establish socket to allow change of ptracer

2019-01-20 Thread Christoph Roick
croick added a comment.


  > the 3rd line is still wrong ...
  
  So I used the word "tracer" now. Otherwise I don't know what you mean.

REPOSITORY
  R285 KCrash

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

To: croick, #frameworks, ossi
Cc: dfaure, lepagevalleeemmanuel, kde-frameworks-devel, sitter, michaelh, 
ngraham, bruns


D11235: [DrKonqi] Request change of ptrace scope from KCrash

2019-01-20 Thread Christoph Roick
croick edited the summary of this revision.

REPOSITORY
  R871 DrKonqi

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

To: croick, #plasma_workspaces, #frameworks, ossi
Cc: ossi, lepagevalleeemmanuel, maximilianocuria, adridg, plasma-devel, 
jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart


D11236: [KCrash] Establish socket to allow change of ptracer

2019-01-20 Thread Christoph Roick
croick edited the summary of this revision.

REPOSITORY
  R285 KCrash

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

To: croick, #frameworks, ossi
Cc: dfaure, lepagevalleeemmanuel, kde-frameworks-devel, sitter, michaelh, 
ngraham, bruns


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

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


  In D18167#397020 , @graesslin 
wrote:
  
  > This causes in KWin 500+ new warnings. Do you really think it's a good idea 
to spam all of KDE with new compiler warnings. KDE has an old code base. We 
cannot enable warnings for the way you developed C++ for 20 years.
  
  
  These warnings are a good, thing, it's too easy to break an interface and 
never realize if you did not mark your functions as override.
  
  Anyway, if you don't want the warnings just disable them for gcc, you already 
made the decision to disable it for clang, so just extend it to gcc.

REPOSITORY
  R240 Extra CMake Modules

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

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


D18369: Create tel: links for phone numbers

2019-01-20 Thread Sune Vuorela
svuorela added inline comments.

INLINE COMMENTS

> ktexttohtml.cpp:159
> +// this isn't 100% accurate, we filter stuff below that is too hard to 
> capture with a regexp
> +static const QRegularExpression telPattern(QStringLiteral(R"([+0](( |( 
> ?[/-] ?)?)\(?\d+\)?+){6,30})"));
> +const auto match = telPattern.match(mText, mPos, 
> QRegularExpression::NormalMatch, QRegularExpression::AnchoredMatchOption);

How are the thread safety of the KTextToHTML functions?

And does that match the thread safety of the QRegularExpression and the const 
methods in there?

REPOSITORY
  R244 KCoreAddons

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

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


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

2019-01-20 Thread Martin Flöser
graesslin added a comment.


  This causes in KWin 500+ new warnings. Do you really think it's a good idea 
to spam all of KDE with new compiler warnings. KDE has an old code base. We 
cannot enable warnings for the way you developed C++ for 20 years.

REPOSITORY
  R240 Extra CMake Modules

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

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


D18380: KIO: make file dialog columns resizable again (and movable)

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


  I don't see the squeezed text when using Breeze and Noto Sans. Am I missing 
something?
  
  I kind of hope so, because I predict that if we ship with text that gets 
squeezed at various sizes, we'll totally get bug reports about it. :-) No need 
to overcomplicate things IMHO. The prior state of the patch seemed just about 
perfect to me with the removal of the Size column shrinking.

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

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


D18380: KIO: make file dialog columns resizable again (and movable)

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


  My QtCurve-based theme, "narrow mode" just activated:
  F6558463: kdialog-narrow-mode.png 
  Idem, "narrow mode" just deactivated
  F6558464: kdialog-regular-mode.png 
  
  Default theme, narrow mode just activated
  F6558466: kdialog-narrow-mode-default.png 

  Idem, narrow mode just deactivated
  F6558467: kdialog-regular-mode-default.png 


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

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


D18380: KIO: make file dialog columns resizable again (and movable)

2019-01-20 Thread René J . V . Bertin
rjvbb updated this revision to Diff 49947.
rjvbb added a comment.


  Final attempt at being a bit clever :)
  
  Entering "narrow mode" now also sets the font stretch (exitting the mode will 
reset the stretch), the same as the Mac Finder will do. I use  factor 83, which 
corresponds roughly to making a 12pt font as wide as a 10pt font.
  Setting stretch does not seem to have any effect when done in the handler 
slot, so it has to be done together with the width adaptation. That means the 
size and date columns need to be stretched too otherwise they will be too wide; 
I find that condensing them by half as much as the font gives a satisfactory 
result. The gained pixels are added to the name column.
  
  Tested with a range of fonts and font sizes I find this to work pretty well. 
NB: the default UI font doesn't seem to support stretch; the code detect this 
by comparing `QFontMetrics::avCharWidth()` before and after setting the stretch.
  
  The look *is* slightly different with the condensed font, but theoretically 
it's only a quantitative difference ;)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18380?vs=49908&id=49947

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

AFFECTED FILES
  src/filewidgets/kdiroperatordetailview.cpp
  src/filewidgets/kdiroperatordetailview_p.h

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


D18384: Allow creating directory named '~' and throw a warning before creating it.

2019-01-20 Thread Shubham
shubham retitled this revision from "Create directory named '~' and throw a 
warning before creating it successfully." to "Allow creating directory named 
'~' and throw a warning before creating it.".

REPOSITORY
  R241 KIO

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

To: shubham, ngraham
Cc: pino, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns


D18384: Create directory named '~' and throw a warning before creating it successfully.

2019-01-20 Thread Shubham
shubham retitled this revision from "Allow creating directory named '~' and 
throw a warning before creating it." to "Create directory named '~' and throw a 
warning before creating it successfully.".

REPOSITORY
  R241 KIO

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

To: shubham, ngraham
Cc: pino, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns


D18384: Allow creating directory named '~' and throw a warning before creating it.

2019-01-20 Thread Shubham
shubham updated this revision to Diff 49943.
shubham marked an inline comment as done.
shubham added a comment.


  Remove redundant setting of window title.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18384?vs=49935&id=49943

BRANCH
  dir

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

AFFECTED FILES
  src/filewidgets/knewfilemenu.cpp
  src/filewidgets/knewfilemenu.h

To: shubham, ngraham
Cc: pino, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns


D18408: Set new PlasmaShellSurface::PopupBehavior flags

2019-01-20 Thread David Edmundson
davidedmundson created this revision.
davidedmundson added a reviewer: Plasma.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
davidedmundson requested review of this revision.

REVISION SUMMARY
  Set on surface creation and on relevant setFlags API of
  Dialog

TEST PLAN
  Used with relevant kwin patch, the "pin" icon
  on the calendar/system tray works

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  master

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

AFFECTED FILES
  src/plasmaquick/dialog.cpp

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


D18384: Allow creating directory named '~' and throw a warning before creating it.

2019-01-20 Thread Pino Toscano
pino added a comment.


  Ah yes, now I see it better, the whole KMessageBox::shouldBeShownContinue() 
check is bogus, since that key is not set by anything.

INLINE COMMENTS

> knewfilemenu.cpp:422
>  QDialog *confirmDialog = new QDialog(m_parentWidget);
>  confirmDialog->setWindowTitle(i18n("Create hidden directory?"));
>  confirmDialog->setModal(m_modal);

this is redundant now, as done in both the if branches below

REPOSITORY
  R241 KIO

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

To: shubham, ngraham
Cc: pino, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns


D18384: Allow creating directory named '~' and throw a warning before creating it.

2019-01-20 Thread Shubham
shubham added a comment.


  @pino The test case you told is not applicable(I confirmed).
  My test case:
  
  1. Create directory named .foo (Results in a warning)
  2. User checks "Do not ask again".
  3. User tries to create directory named .xyz (This time NO WARNING appears, 
as it is obvious)
  4. User tries to create directory named ~ (A warning is shown)
  
  So the conclusion is that the 2 warning messageboxes are treated 
individually, and their key is also set individually.

REPOSITORY
  R241 KIO

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

To: shubham, ngraham
Cc: pino, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns


D18384: Allow creating directory named '~' and throw a warning before creating it.

2019-01-20 Thread Pino Toscano
pino added a comment.


  In D18384#396895 , @shubham wrote:
  
  > @pino got it what you meant.
  
  
  Definitely not. Let me explain it again:
  
  - so far, KNewFileMenuPrivate::confirmCreatingHiddenDir is used only to ask 
to the user whether choice of a directory starting with '.' is wanted; 
KNewFileMenuPrivate::confirmCreatingHiddenDir checks whether ask by using the 
message box "do not ask again" key "confirm_create_hidden_tilde_dir"
  - a user tries to create a directory starting with '.':
1. they get the messagebox that confirmCreatingHiddenDir shows
2. they tick the "do not ask again"
3. they proceed
  - as result of the point above, the key "confirm_create_hidden_tilde_dir" is 
set
  - your changes rename KNewFileMenuPrivate::confirmCreatingHiddenDir to 
confirmCreatingDir, and make it used also when a directory starts with '~'
  - the same user tries to create a directory starting with '~':
1. confirmCreatingDir is called
2. the "confirm_create_hidden_tilde_dir" key is set, so no message box is 
shown
3. the directory is created directly, without user confirmation

REPOSITORY
  R241 KIO

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

To: shubham, ngraham
Cc: pino, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns


D18384: Allow creating directory named '~' and throw a warning before creating it.

2019-01-20 Thread Shubham
shubham updated this revision to Diff 49935.
shubham added a comment.


  1. Use ==
  2. Rename slot's name

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D18384?vs=49914&id=49935

BRANCH
  dir

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

AFFECTED FILES
  src/filewidgets/knewfilemenu.cpp
  src/filewidgets/knewfilemenu.h

To: shubham, ngraham
Cc: pino, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns


D18384: Allow creating directory named '~' and throw a warning before creating it.

2019-01-20 Thread Shubham
shubham marked 3 inline comments as done.

REPOSITORY
  R241 KIO

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

To: shubham, ngraham
Cc: pino, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns


D18384: Allow creating directory named '~' and throw a warning before creating it.

2019-01-20 Thread Shubham
shubham added a comment.


  @pino got it what you meant. Also do I need to change the  
_k_slotCreateHiddenDirectory( ) slot name?

REPOSITORY
  R241 KIO

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

To: shubham, ngraham
Cc: pino, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns


D18384: Allow creating directory named '~' and throw a warning before creating it.

2019-01-20 Thread Pino Toscano
pino added inline comments.

INLINE COMMENTS

> shubham wrote in knewfilemenu.cpp:439
>   == 
> 
> internally calls
> 
>   operator==()

The compiler automatically replaces the operators into the calls to the 
appropriate functions. There is no speed penalty.

REPOSITORY
  R241 KIO

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

To: shubham, ngraham
Cc: pino, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns


D18384: Allow creating directory named '~' and throw a warning before creating it.

2019-01-20 Thread Shubham
shubham added a comment.


  @pino Sorry for inconvenience caused. BTW I did not get what you meant?

REPOSITORY
  R241 KIO

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

To: shubham, ngraham
Cc: pino, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns


D18384: Allow creating directory named '~' and throw a warning before creating it.

2019-01-20 Thread Shubham
shubham added inline comments.

INLINE COMMENTS

> dhaumann wrote in knewfilemenu.cpp:439
> Really? But isn't == not exactly the same? Can you elaborate?

== 

internally calls

  operator==()

REPOSITORY
  R241 KIO

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

To: shubham, ngraham
Cc: pino, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns


D18384: Allow creating directory named '~' and throw a warning before creating it.

2019-01-20 Thread Pino Toscano
pino added a comment.


  Also, not related to the code: @shubham, you seem to often remove your own 
comments. This is a bad practice for many POV of views (transparency, breaks 
the logic of a conversation, etc). As these reviews send notification emails to 
mailing lists usually, then your removed messages are archived, and thus 
removing them is useless.
  Please stop doing this anti-social practice, thank you.

INLINE COMMENTS

> knewfilemenu.cpp:408-413
> +void KNewFileMenuPrivate::confirmCreatingDir(const QString &name)
>  {
>  if 
> (!KMessageBox::shouldBeShownContinue(QStringLiteral("confirm_create_hidden_dir")))
>  {
>  _k_slotCreateHiddenDirectory();
>  return;
>  }

renaming confirmCreatingHiddenDir and using it also for another case means that 
the "confirm_create_hidden_dir" messagebox confirmation also applies to the 
other cases (filename == '~' in this case)

> knewfilemenu.cpp:911
>  }
> +
>  url = baseUrl;

extra line added

REPOSITORY
  R241 KIO

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

To: shubham, ngraham
Cc: pino, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns


D18384: Allow creating directory named '~' and throw a warning before creating it.

2019-01-20 Thread Dominik Haumann
dhaumann added inline comments.

INLINE COMMENTS

> shubham wrote in knewfilemenu.cpp:439
> This is faster than simple ==

Really? But isn't == not exactly the same? Can you elaborate?

REPOSITORY
  R241 KIO

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

To: shubham, ngraham
Cc: dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns


D17241: WIP:Disable highlighting after 512 characters on a line.

2019-01-20 Thread Christoph Cullmann
cullmann added a comment.


  Sorry to chime in that late ;=)
  
  Wouldn't it make more sense, to limit the number of ranges per line?
  I think the real issue is that we can't handle  highlightings per line, 
not that the raw number of letters is that much an issue.
  
  This would relax the constraint for most use cases.

REPOSITORY
  R39 KTextEditor

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

To: sars, cullmann, vkrause, dhaumann, mwolff
Cc: zetazeta, mwolff, brauch, kwrite-devel, kde-frameworks-devel, hase, 
michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann


D18125: KateStatusBar: Add dictionary button

2019-01-20 Thread Christoph Cullmann
cullmann requested changes to this revision.
cullmann added a comment.
This revision now requires changes to proceed.


  I played with this patch now ;)
  
  First: to now have KWrite crash on start, one need to modify one line to:
  
  if (!m_dictionaryGroup->checkedAction() || 
(m_dictionaryGroup->checkedAction()->data().toString() != newDict) || 
m_dictionary->text().isEmpty()) {
  
  m_dictionaryGroup->checkedAction() might be null.
  
  For the naming: Given we show the encodings with their short-names, too, I 
can live with de_DE or stuff like that.
  
  For the overhead: I would like to have this optional and disabled per 
default, that avoids all that work in cursorPositionChanged()
  Just like we do for word count/...

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

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


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

2019-01-20 Thread CI System
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/ktexteditor/job/kf5-qt5%20FreeBSDQt5.12/18/
 Project:
kf5-qt5 FreeBSDQt5.12
 Date of build:
Sun, 20 Jan 2019 12:48:09 +
 Build duration:
6 min 45 sec and counting
   JUnit Tests
  Name: projectroot Failed: 1 test(s), Passed: 60 test(s), Skipped: 0 test(s), Total: 61 test(s)Failed: projectroot.autotests.kateindenttest_testCppstyleName: projectroot.autotests.src Failed: 0 test(s), Passed: 5 test(s), Skipped: 0 test(s), Total: 5 test(s)

D17459: SearchBar: Add Cancel button to stop long running tasks

2019-01-20 Thread Christoph Cullmann
This revision was automatically updated to reflect the committed changes.
Closed by commit R39:7cf6644e8017: SearchBar: Add Cancel button to stop long 
running tasks (authored by loh.tar, committed by cullmann).

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17459?vs=49754&id=49928

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

AFFECTED FILES
  src/search/katesearchbar.cpp
  src/search/katesearchbar.h
  src/search/searchbarpower.ui

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


D17459: SearchBar: Add Cancel button to stop long running tasks

2019-01-20 Thread Christoph Cullmann
cullmann accepted this revision.
cullmann added a comment.
This revision is now accepted and ready to land.


  I played with the current state.
  
  I like it ;=)
  
  That the button "Cancel" spans over both "Replace/Find All" things is not bad 
in my eyes,
  
  This fixes the issue of "Kate/KWrite/... must be killed on long replace 
operations".
  
  What I would like to have further, now that some "batching" code is there: 
Try to apply it for the normal search, too.
  
  e.g. if you search for some word in a large file, the GUI might block "long" 
even during typing, if no early match is found.
  
  But that is a separate issue.
  
  loh.tar, are you interested in that challenge, too?
  Given you are now more "into" the searching code.
  
  Btw., here all unit tests pass (beside the Qt 5.12 breackage of 
kateindenttest_testCppstyle)
  
  I will land this, to have it tested by "all" people :=)

REPOSITORY
  R39 KTextEditor

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

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


D17693: DocumentPrivate: Treat some chars also as "auto bracket" only when we have a selection

2019-01-20 Thread Christoph Cullmann
cullmann requested changes to this revision.
cullmann added a comment.
This revision now requires changes to proceed.


  Played a bit with it, works reasonable.
  Just replace const QChar &c with const QChar and we let this in.

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

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


D17857: DocumentPrivate: Don't scroll view when add auto-bracket at end of file

2019-01-20 Thread Christoph Cullmann
cullmann added a comment.


  Extra setCursor calls are always fishy.
  
  But on the other side: how does one reproduce this?
  
  For me, if I scroll behind the last line, close to any editing action 
triggers scrolling, with or without this patch.

REPOSITORY
  R39 KTextEditor

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

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


D18384: Allow creating directory named '~' and throw a warning before creating it.

2019-01-20 Thread Shubham
shubham added inline comments.

INLINE COMMENTS

> dhaumann wrote in knewfilemenu.cpp:439
> Please simply write name == QStringLiteral (...) instead of operator==. Same 
> below.

This is faster than simple ==

REPOSITORY
  R241 KIO

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

To: shubham, ngraham
Cc: dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns


D17956: DocumentPrivate: Fix broken doc links in qCWarning

2019-01-20 Thread Christoph Cullmann
cullmann closed this revision.
cullmann added a comment.


  Fixed links got commited in meantime:
  
  commit b64681408d840880c7625543992cc6a75535944a 

  Author: Grzegorz Szymaszek 
  Date:   Thu Jan 10 08:42:46 2019 +0100
  
Use HTTPS, if available, in links visible to users

Reviewers: #ktexteditor, dhaumann

Reviewed By: #ktexteditor, dhaumann

Subscribers: lueck, kwrite-devel, kde-frameworks-devel

Tags: #kate, #frameworks

Differential Revision: https://phabricator.kde.org/D18148

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

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


D18380: KIO: make file dialog columns resizable again (and movable)

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


  One thing we might be able to do is use QFont::setStretch() to make the text 
a bit more compact. I'll follow that thought for a bit.

REPOSITORY
  R241 KIO

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

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


D18380: KIO: make file dialog columns resizable again (and movable)

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


  I agree that reducing the size column width isn't as easy as I thought. I 
find it takes up more space than necessary when horizontal space is at a 
premium and I was hoping to get a more compact read-out that can still be 
interpreted ... and that would show itself in full in a tooltip.
  
  Maybe something can be done at a higher level, just like some systems will 
switch from full to compact date format when available space requires it (Mac 
OS will even switch to a narrow font). I'm not very motivated though to spend 
much time in figuring out how and where to do such a thing so if it's not 
straightforward I'll be leaving that approach to someone else.

REPOSITORY
  R241 KIO

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

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


D18390: Build without D-Bus on Android

2019-01-20 Thread Volker Krause
vkrause added a comment.


  In D18390#396771 , @dhaumann wrote:
  
  > You likely get a warning about an unused variable now in 
KBookmarkManager::init(const QString &dbusPath), since the function is empty.
  
  
  Yep (and same for the other frameworks). I don't expect this to be the final 
state though, it's a first step to get things building without the D-Bus stub 
on Android at all, and get an overview of the gaps. Some cases will need a 
proper Android implementation, some might end up being mere porting aids that 
make no sense on Android.

REPOSITORY
  R294 KBookmarks

BRANCH
  master

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

To: vkrause, dhaumann
Cc: dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns


D18385: Build without KAuth and D-Bus on Android

2019-01-20 Thread Dominik Haumann
dhaumann accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R265 KConfigWidgets

BRANCH
  master

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

To: vkrause, dhaumann
Cc: kde-frameworks-devel, michaelh, ngraham, bruns


D18386: Build without D-Bus on Android

2019-01-20 Thread Dominik Haumann
dhaumann accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R309 KService

BRANCH
  master

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

To: vkrause, dhaumann
Cc: kde-frameworks-devel, michaelh, ngraham, bruns


D18387: Build without D-Bus on Android

2019-01-20 Thread Dominik Haumann
dhaumann accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R302 KIconThemes

BRANCH
  master

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

To: vkrause, dhaumann
Cc: kde-frameworks-devel, michaelh, ngraham, bruns


D18388: Build without D-Bus on Android

2019-01-20 Thread Dominik Haumann
dhaumann accepted this revision.
dhaumann added a comment.
This revision is now accepted and ready to land.


  Lgtm.

REPOSITORY
  R263 KXmlGui

BRANCH
  master

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

To: vkrause, dhaumann
Cc: dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns


D18389: Build without D-Bus on Android

2019-01-20 Thread Dominik Haumann
dhaumann accepted this revision.
dhaumann added a comment.
This revision is now accepted and ready to land.


  Looks good to me. Same here: do you get warnings about unused variables now?

REPOSITORY
  R245 Solid

BRANCH
  master

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

To: vkrause, dhaumann
Cc: dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns


D18390: Build without D-Bus on Android

2019-01-20 Thread Dominik Haumann
dhaumann accepted this revision.
dhaumann added a comment.
This revision is now accepted and ready to land.


  Looks good to me.
  
  You likely get a warning about an unused variable now in 
KBookmarkManager::init(const QString &dbusPath), since the function is empty.

REPOSITORY
  R294 KBookmarks

BRANCH
  master

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

To: vkrause, dhaumann
Cc: dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns


D18384: Allow creating directory named '~' and throw a warning before creating it.

2019-01-20 Thread Dominik Haumann
dhaumann added inline comments.

INLINE COMMENTS

> knewfilemenu.cpp:439
> KMessageBox::NoExec);
> +} else if (name.operator==(QLatin1String("~"))) {
> +confirmDialog->setWindowTitle(i18n("Create directory named ~?"));

Please simply write name == QStringLiteral (...) instead of operator==. Same 
below.

REPOSITORY
  R241 KIO

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

To: shubham, ngraham
Cc: dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns


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

2019-01-20 Thread CI System
BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/ktexteditor/job/kf5-qt5%20FreeBSDQt5.12/17/
 Project:
kf5-qt5 FreeBSDQt5.12
 Date of build:
Sun, 20 Jan 2019 07:07:49 +
 Build duration:
1 hr 6 min and counting
   JUnit Tests
  Name: projectroot Failed: 1 test(s), Passed: 60 test(s), Skipped: 0 test(s), Total: 61 test(s)Failed: projectroot.autotests.kateindenttest_testCppstyleName: projectroot.autotests.src Failed: 0 test(s), Passed: 5 test(s), Skipped: 0 test(s), Total: 5 test(s)