D12295: Supporting nested brackets for Kate autobrackets

[loh tar]

loh.tar added a comment.


  His original post with some more text/hints/questions
  https://mail.kde.org/pipermail/kwrite-devel/2018-April/000345.html
  He got the advice from @brauch to post here.
  
  For me a little bit hard to follow, not only there but also here.
  I could apply this patch almost without trouble. Why some stuff was rejected 
I have no idea. Just added manually and it seems to work.
  
  So far I understood ask he how to add in some cases the proper cursor 
positions to the stack, and not only an "approximation". But the noted code 
lines are not so easy to find for me.
  
  The main issue with this patch is (as he had explained (I think)) that it 
does not work properly when you move/edit around inside some new created "brace 
range". To fix this he ask for help.
  
  My feeling is that this patch may cause in the actual condition more hassle 
than it solves due to the noted missing fixes.
  
  Currently I have not the mood to think more about this, but you smart guys 
may have no problem to fix the issues :-)

REPOSITORY
  R39 KTextEditor

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

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

D17730: [WIP] Review KateStatusBar (based on D17729)

[loh tar]

loh.tar added a comment.


  > For the "should we use buttons": I think I tried that in the past and it 
did look horrible in many styles, but perhaps I am wrong ;)
  
  I see. May it in the meantime not an issue anymore, due to new designs 
elsewhere? And what's up with the existing buttons, they should look also ugly 
there.
  
  > I am a bit confused which questions still need answers ;=)
  
  
  
  - Most of them have an answer now ;-) Will add more stuff here
  - Your comment to @zetazeta request ? I have tried to add a QScrollArea but 
without success :-/ Ideas? Perhaps is a second level of layout/widget needed(?)
  
 updateStatus();
 wordCountChanged(0, 0, 0, 0);
+
+QScrollArea *scrollArea = new QScrollArea; // No status bar
+// QScrollArea *scrollArea = new QScrollArea(this); // No status bar
+// QScrollArea *scrollArea = new QScrollArea(centralWidget());// No 
Kate window appears!
+scrollArea->setWidget(centralWidget());
 }
  
  
  
  > I think having the dictionary picker per default there takes to much space
  
  Why do you think so? My status bar is almost empty, but I did not show "word 
count" and such. The extra space is hardly worth mentioning. Add an new pic 
with my typical window size. The "German version" need some more space but is 
also hardly worth mentioning.
  F6511683: 1545915793.png 
  
  > ..and one should have some "show XXX" in the context menu for it.
  
  Doing it like the other stuff need some effort, like to change the config 
dialog :-/ *hmpf*

REPOSITORY
  R39 KTextEditor

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

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

D17729: KateStatusBar: Reformatted by astyle command to follow coding style

[loh tar]

loh.tar added a comment.


  @cullmann wrote:
  
  > Actually, if there is a astyle commandline that enforces the right style, 
could you paste it here?
  
  Um (?) I had in mind it was on the Kate HP but couln'd find it now. But it's 
there 
https://community.kde.org/Policies/Kdelibs_Coding_Style#Artistic_Style_.28astyle.29_automatic_code_formatting
  
  As said I add "--align-pointer=name" but don't like to change this wiki, as 
noob :-)
  Furthermore I noticed that it is still not full coding style conform due to 
not fixed "one line conditional statements", so "--add-braces" may an option 
but I didn't try it
  
  > I am in favor of some script that runs that over the repo and just applying 
that regularly to enforce it stays sane, that is much more useful than to 
review each ( placement in all commits ;=)
  
  In generally a good idea. But sometimes it's nice to break the coding style 
to achieve some more lovely look

REPOSITORY
  R39 KTextEditor

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

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

D17693: DocumentPrivate: Treat angle bracket < and backtick ` also as "auto bracket" when we have a selection

[loh tar]

loh.tar added a comment.


  > Btw., have you perhaps played a bit with the 
https://phabricator.kde.org/D12295 patch?
  
  No, but noticed it sometime in the past
  
  > If you use it, could you try that patch, too?
  
  Will try it

REPOSITORY
  R39 KTextEditor

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

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

D17730: [WIP] Review KateStatusBar (based on D17729)

[loh tar]

loh.tar updated this revision to Diff 48237.
loh.tar edited the test plan for this revision.
loh.tar set the repository for this revision to R39 KTextEditor.
loh.tar added a comment.


  - All in one diff
  - Rebased on master
  - Removed test plan
  - Will update Summary/Title before you commit

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17730?vs=48236=48237

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

AFFECTED FILES
  src/view/katestatusbar.cpp

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

D17730: [WIP] Review KateStatusBar (based on D17729)

[loh tar]

loh.tar updated this revision to Diff 48236.
loh.tar edited the summary of this revision.
loh.tar added a comment.


  - Only show lineColLabel context menu when click on it and not somewhere on 
the bar
  
  As "usual" without all other patches.
  
  > No idea how the spell-checking code handles that.
  
  I have a working solution in the meantime. I may open a new Diff for that.
  
  > I think it is useful, but now the statusbar is very full.
  >  Could we make that configurable via context menu? Like we have "Show line 
count", ... there?
  
  Should be possible. Will look at it. Your comment to the next question?
  
  > I would like the status bar to get cut (if the status bar is scrollable or 
not, I personally dont care)
  
  About "cut" I have ATM no idea how to do that, and I guess nobody would like 
that. A Scrollbar may possible and a somehow
   "auto-wrap" behaviour too
  
  > will it be hard to implement?
  
  Hard to say ;-) The auto-wrap should be the most tricky
  
  @cullmann Let me know how much stuff I can add here or have remove. There are 
many possibilities left, e.g.
  
  > How about to change the "action-labels" to buttons? Thisway would it be 
less "quirky" and we would have a feedback that there is an action available 
when hovered by mouse
  
  I will upload a "all in one" diff for a better overview

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17730?vs=48000=48236

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

AFFECTED FILES
  src/view/katestatusbar.cpp

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

D17730: [WIP] Review KateStatusBar (based on D17729)

[loh tar]

loh.tar added a comment.


  > My main intent is to add somehow a spell check button/hint
  
  Here a snapshot how it looks currently
  F6503235: 1545570321.png 
  
  Sadly stuck I now, every help is appreciated
  
  I like to show on the button the current used dictionary (code), like 
"en_US". Because Kate support multible dictionary settings on a document, I 
miss a signal that the current dictionary has changed when the cursor moved 
into a new range. Note the few misspelled words in the pic.
  
  Is there a way to achieve this without to add new signal to somewhere, 
probably KTextEditor::DocumentPrivate ?
  
  Merry Christmas everybody!

REPOSITORY
  R39 KTextEditor

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

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

D17729: KateStatusBar: Reformatted by astyle command to follow coding style

[loh tar]

loh.tar updated this revision to Diff 48054.
loh.tar added a comment.


  - Run astyle again, this time with "--align-pointer=name"
  - Omit changes done in the signal/slot connect code because these need a port 
to the new style. Well, was somehow pointless because these code is changed 
anyway but I noticed that too late :-/

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17729?vs=47994=48054

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

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

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

D17729: KateStatusBar: Reformatted by astyle command to follow coding style

[loh tar]

loh.tar added a comment.


  - It's a prearrangement for more changes D17730 

  - In the meantime I noticed that these astyle command did not move/fix to 
place "*&" to the right. I will update/fix this before someone like to commit

REPOSITORY
  R39 KTextEditor

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

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

D17733: ViewPrivate: Don't hide 'Clear Dictionary Ranges' action

[loh tar]

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

REVISION SUMMARY
  Toggle the action instead enable/disable ensure the user knows that
  there is some interesting feature available

TEST PLAN
  No pic, guess not needed, you can imagine

REPOSITORY
  R39 KTextEditor

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

AFFECTED FILES
  src/view/kateview.cpp

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

D17730: [WIP] Review KateStatusBar (based on D17729)

[loh tar]

loh.tar updated this revision to Diff 48000.
loh.tar edited the summary of this revision.
loh.tar added a comment.


  - KateStatusBar: Move the modified-label from far right to far left
  
  Once again without all other patches

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17730?vs=47998=48000

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

AFFECTED FILES
  src/view/katestatusbar.cpp

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

D17730: [WIP] Review KateStatusBar (based on D17729)

[loh tar]

loh.tar added a comment.


  How about to change the "action-labels" to buttons? Thisway would it be less 
"quirky" and we would have a feedback that there is an action available when 
hovered by mouse

REPOSITORY
  R39 KTextEditor

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

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

D17730: [WIP] Review KateStatusBar (based on D17729)

[loh tar]

loh.tar added a comment.


  Oops, have now the first commit omitted in this update. Perhaps not too bad 
this way(?)

REPOSITORY
  R39 KTextEditor

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

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

D17730: [WIP] Review KateStatusBar (based on D17729)

[loh tar]

loh.tar updated this revision to Diff 47998.
loh.tar edited the summary of this revision.
loh.tar added a comment.


  - KateStatusBar: Don't treat click in empty area as click on mode-label

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17730?vs=47997=47998

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

AFFECTED FILES
  src/view/katestatusbar.cpp

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

D17730: [WIP] Review KateStatusBar (based on D17729)

[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
  - KateStatusBar: Do actions on labels by single click instead of double-click
  
This way is the behavior harmonized with the available buttons

TEST PLAN
  Give feedback which of these commits you will reject or like to have as an 
own patch
  My main intent is to add somehow a spell check button/hint

REPOSITORY
  R39 KTextEditor

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

AFFECTED FILES
  src/view/katestatusbar.cpp

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

D17729: KateStatusBar: Reformatted by astyle command to follow coding style

[loh tar]

loh.tar added a comment.


  Nothing else was done

REPOSITORY
  R39 KTextEditor

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

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

D17729: KateStatusBar: Reformatted by astyle command to follow coding style

[loh tar]

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

REPOSITORY
  R39 KTextEditor

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

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

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

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

[loh tar]

loh.tar updated this revision to Diff 47966.
loh.tar added a comment.


  Improve the readability and prevent unwanted lookup by caching d(this) as dd

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17459?vs=47963=47966

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: anthonyfieroni, brauch, cullmann, abetts, kwrite-devel, 
kde-frameworks-devel, #ktexteditor, hase, michaelh, ngraham, bruns, demsking, 
sars, dhaumann

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

[loh tar]

loh.tar added inline comments.

INLINE COMMENTS

> anthonyfieroni wrote in katesearchbar.cpp:860-865
> Maybe not an issue, but you can try to cache value preventing unwanted lookup
> 
>   auto dd = d(this);
>   dd->...

Only here or everywhere?
At this particular place may that optimized by the compiler(?)

REPOSITORY
  R39 KTextEditor

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

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

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

[loh tar]

loh.tar added a comment.


  I have tried to run two S jobs at the same time on the same document, seems 
to works nicely
  
  - view 1 -> S "tab" -> "-"
  - view 2 -> S "0" -> "+"
  
  I had canceled both jobs and then resume, that's why at the pic are already 
replacents to see while the job is still running. Job is almost finished, had 
taken a couple of minutes and ~2GB RAM
  F6496970: still-running-near-finish.png 
  After finish, scrolled down
  F6496972: finished-scrolled-down.png 
  
  My current status is: No crashes I'm aware of and everything seems to work.
  
  - "m_cancelFindOrReplace" may better renamed to "m_jobIsRuning" or similar
  - Is the "Cancel" perhaps better named "Stop"?
  - The new added, and now  "d->m_foo" variables by better renamed to "d->foo", 
but perhaps will never a true d-pointer added but the new variables moved back 
as member again (?)

REPOSITORY
  R39 KTextEditor

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

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

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

[loh tar]

loh.tar updated this revision to Diff 47963.
loh.tar set the repository for this revision to R39 KTextEditor.
loh.tar added a comment.


  - Fix crash when the document will closed while a S job is running
  - Fix crash when the view will closed while a S job is running
  
  These fixes seems to work so far but they do not block the user action which 
is somehow an unusual behaviour.

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17459?vs=47907=47963

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: anthonyfieroni, brauch, cullmann, abetts, kwrite-devel, 
kde-frameworks-devel, #ktexteditor, hase, michaelh, ngraham, bruns, demsking, 
sars, dhaumann

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

[loh tar]

loh.tar updated this revision to Diff 47907.
loh.tar added a comment.


  Fixed/Simplified as suggested
  
  - I noticed a reproducible crash when e.g. Kate will closed while a S is 
running. Happens also when a split view is closed (so far so good) and then 
Kate will closed . I will try to solve this later, but help is appreciated.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17459?vs=47891=47907

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: anthonyfieroni, brauch, cullmann, abetts, kwrite-devel, 
kde-frameworks-devel, #ktexteditor, hase, michaelh, ngraham, bruns, demsking, 
sars, dhaumann

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

[loh tar]

loh.tar updated this revision to Diff 47891.
loh.tar added a comment.


  - Workaround to keep binary compatibility
  
  > It's exported class you cannot...
  
  Argh! How can I see this quickly the next time? KTEXTEDITOR_EXPORT ?
  
  - Seams still to work so far, but looks not so lovely
  - Fixed (hopefully) as described there 
https://community.kde.org/Policies/Binary_Compatibility_Issues_With_C%2B%2B#Trouble_shooting
  - Some functions have now no implementation, is that a problem?
  - Some new function could be avoided and an old one used, but that would 
reduce the readability

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17459?vs=47843=47891

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: anthonyfieroni, brauch, cullmann, abetts, kwrite-devel, 
kde-frameworks-devel, #ktexteditor, hase, michaelh, ngraham, bruns, demsking, 
sars, dhaumann

D17693: DocumentPrivate: Treat angle bracket < and backtick ` also as "auto bracket" when we have a selection

[loh tar]

loh.tar added a comment.


  > the change from QChar to const QChar & does not improve the amount of 
copied data.
  
  Argh! OK. Was just an impulse to do it as usual and to follow 
isStartBracket(..) and isBracket(..)
  Will wait for further advice

REPOSITORY
  R39 KTextEditor

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

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

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

[loh tar]

loh.tar edited the summary of this revision.

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

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

D17693: DocumentPrivate: Treat angle bracket < and backtick ` also as "auto bracket" when we have a selection

[loh tar]

loh.tar added a comment.


  Should you dislike the auto bracket code changes can I remove them from this 
patch. They are not needed for this one and only be the byproduct of my code 
studying.
  
  As always, poor tested :-)

REPOSITORY
  R39 KTextEditor

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

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

D17693: DocumentPrivate: Treat angle bracket < and backtick ` also as "auto bracket" when we have a selection

[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
  The first attempt was to add these chars to the auto bracket code but that 
caused
  surprising odd behavour in the normal edit flow, so it's done in a special 
way which
  hopefully caused no trouble.
  
  - Improve readability of auto bracket code
  - Change arguments as reference
  - Remove unneeded code

REPOSITORY
  R39 KTextEditor

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

AFFECTED FILES
  src/document/katedocument.cpp

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

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

[loh tar]

loh.tar updated this revision to Diff 47843.
loh.tar added a comment.


  - Fix too much stretching of Cancel|Find/Replace button (I misunderstood 
probably Andres above :-)
  - Add S progress hint, Rename showInfoMessage(..) -> showResultMessage() 
and move logic in what text to show
  
  Notes:
  
  - The auto hide time of KTextEditor::Message seams to be buggy, stays 
sometimes much longer on display
  - The showResultMessage is always green/Positive (as it was) also on no 
matches. Blue/Info may always fit this way or I can add some more checks to 
display a red/Error message in such case

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17459?vs=47408=47843

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: brauch, cullmann, abetts, kwrite-devel, kde-frameworks-devel, #ktexteditor, 
hase, michaelh, ngraham, bruns, demsking, sars, dhaumann

D17661: ViewConfig: Add 'Dynamic Wrap At Static Marker' option

[loh tar]

loh.tar added a comment.


  - No sure if there must be something at other places considered
  - This is my second version where the wrap is done ether at the static 
setting or the view width, whatever is smaller. At first is was always wrapped 
at the static setting. Then I thought it may this way better. But I'm still 
undecided, the other behaviour has also charm. Somehow fit now the naming no 
more perfect.
  - At first I thought there is now an issue with the static wrap marker (the 
line has gaps) and the highlighting when the first word is selected (span the 
unused space at the right) but that seams to be "normal"
  - Only poor tested!

REPOSITORY
  R39 KTextEditor

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

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

D17661: ViewConfig: Add 'Dynamic Wrap At Static Marker' option

[loh tar]

loh.tar created this revision.
loh.tar added reviewers: KTextEditor, VDG.
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 is in "Dynamic Wrap Mode" the text always wrapped 
  at the outer end of the view, which may hinder the reading flow with 
  a wide screen.

TEST PLAN
  Normal behaviour
  F6487064: 1545150257.png 
  
  With patch and activated option
  F6487068: 1545150190.png 
  
  Config page, no old one (sorry, but looks almost the same just without the 
new option) 
  F6487072: 1545151316.png 

REPOSITORY
  R39 KTextEditor

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

AFFECTED FILES
  src/dialogs/katedialogs.cpp
  src/dialogs/textareaappearanceconfigwidget.ui
  src/utils/kateconfig.cpp
  src/utils/kateconfig.h
  src/view/kateviewinternal.cpp

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

D17631: class Message: Use inclass member initialization

[loh tar]

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


  I'm surprised. Obviously have you change something previous (?)

REPOSITORY
  R39 KTextEditor

BRANCH
  message_inclass_member_initializationa

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

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

D17624: KTextEditor::Message: Review documentation

[loh tar]

loh.tar added a comment.


  Sorry for the hassle, will revert the update
  
  > Further, I would prefer autoHideDelay instead of just delay.
  
  Are you refering here to the header or the code part?
  The old name in the header was "autoHideTimeR" which is pretty wrong. So have 
I to change this in the header/docu to "autoHideDelay"? Or perhaps 
"autoHideTime"? Then is the change minimal.

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

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

D17624: KTextEditor::Message: Review documentation

[loh tar]

loh.tar updated this revision to Diff 47678.
loh.tar added a comment.


  -Rename member autoHide->delay to fit accepted change in header file
  
  just an offer

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17624?vs=47673=47678

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

AFFECTED FILES
  src/include/ktexteditor/message.h
  src/utils/messageinterface.cpp

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

D17624: KTextEditor::Message: Review documentation

[loh tar]

loh.tar updated this revision to Diff 47673.
loh.tar added a comment.


  - Fix optionally/optional + be
  
  I like to suggest that some more eyes take a look at that :-)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17624?vs=47669=47673

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

AFFECTED FILES
  src/include/ktexteditor/message.h

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

D17624: KTextEditor::Message: Review documentation

[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
  - Reduce redundant text
  - Change some argument names in the hope it's an improvement
  - Update a code example to fit new style
  - Try to improve some wording
  - Add some @see, @p and such, but probably not all will be needed due to 
"auto reference"
  - Fix some outdated info

REPOSITORY
  R39 KTextEditor

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

AFFECTED FILES
  src/include/ktexteditor/message.h

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

D17310: Improve Kile icon with LaTeX font

[loh tar]

loh.tar added a comment.


  The used traffic cone by VLC is for me also nondescript, but that's their 
icon and therefore we know it.
  My point is only that your new icon should be somehow similar to the stuff 
used by Kile, and not to try to describe what we can do with Kile.
  
  Should I miss that your new icon will be the new Kile icon, I'm sorry for the 
noise :-)

REPOSITORY
  R266 Breeze Icons

BRANCH
  improve-kile-icon (branched from master)

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

To: trickyricky26, #vdg, ngraham, #kile
Cc: loh.tar, mludwig, ndavis, ngraham, kde-frameworks-devel, michaelh, bruns

D17609: KTextEditor: Add action for static word wrap

[loh tar]

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

REVISION SUMMARY
  - Move "Show Static Word Wrap Marker" code some lines up to group related 
actions
  
  BUG:141946

TEST PLAN
  F6478656: 1544896348.png 

REPOSITORY
  R39 KTextEditor

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

AFFECTED FILES
  src/data/katepart5ui.rc
  src/view/kateview.cpp

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

D17609: KTextEditor: Add action for static word wrap

[loh tar]

loh.tar added a comment.


  - Not well tested
  - Slot code may not fit coding style but looks not bad to me
  - No Ampersand & short cut set as done by other actions

REPOSITORY
  R39 KTextEditor

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

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

D17310: Improve Kile icon with LaTeX font

[loh tar]

loh.tar added a comment.


  For me is these root sign nondescript. When the orig old blue symbol is not 
desired, how about to freshen it up? How? I don't know.
  Perhaps Black & White just as all your other offers on these rectangular 
sheet of paper?
  Furthermore exist these splash-screen. So maybe in yellow on these fuzzy gray 
paper?
  
  F6477111: kile_splash.png 

REPOSITORY
  R266 Breeze Icons

BRANCH
  improve-kile-icon (branched from master)

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

To: trickyricky26, #vdg, ngraham, #kile
Cc: loh.tar, mludwig, ndavis, ngraham, kde-frameworks-devel, michaelh, bruns

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

[loh tar]

loh.tar updated this revision to Diff 47408.
loh.tar added a comment.


  - Use new (dis)connect style in touched functions
  - Clean-Up no longer needed MovingRange
  
  Thanks for the explaining. Sounds like QScopedPointer(?)
  I have not used any of them now because it looked to me to have not much 
benefit in this case(?)
  
  BTW: I Always try to avoid a "Standard" version when there is a "Cute" 
available ;-) just only because std stuff looks for me ugly.
  
  The noted odd highlighting still happens when you do successive S() tasks. 
Is it a feature? :-)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17459?vs=47380=47408

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: brauch, cullmann, abetts, kwrite-devel, kde-frameworks-devel, #ktexteditor, 
hase, michaelh, ngraham, bruns, demsking, sars, dhaumann

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

[loh tar]

loh.tar added a comment.


  In D17459#375429 , @cullmann wrote:
  
  > I am atm a bit busy
  
  
  Thanks for the hint
  
  > For the input range, I think it would make sense to store it as 
std::unique_ptr to have it taking care of "edits" 
in-between the calls (even if that is no good idea, but that could avoid some 
issues, and saves the re-creation of the range in each call)
  
  Um, have updated before read this. have droped your std::unique_ptr because 
im not familar with, no idea if my solution has ugly drawbacks. Is there a need 
to delete the hold object now?
  Will do other things for 1-2 days, so please look closer at it and give 
advices in DETAIL ;-)
  
  Ah, I noticed that after a some replace of the highlight looks a little 
strage(?) Could it currently not reproduce

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

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

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

[loh tar]

loh.tar updated this revision to Diff 47380.
loh.tar added a comment.


  - Fix to remember loop state

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17459?vs=47379=47380

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: brauch, cullmann, abetts, kwrite-devel, kde-frameworks-devel, #ktexteditor, 
hase, michaelh, ngraham, bruns, demsking, sars, dhaumann

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

[loh tar]

loh.tar updated this revision to Diff 47379.
loh.tar added a comment.


  - Fix crash in replace mode

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17459?vs=47362=47379

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: brauch, cullmann, abetts, kwrite-devel, kde-frameworks-devel, #ktexteditor, 
hase, michaelh, ngraham, bruns, demsking, sars, dhaumann

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

[loh tar]

loh.tar updated this revision to Diff 47362.
loh.tar edited the summary of this revision.
loh.tar added a comment.


  - Code cosmetic
  - Add singleShot stuff
  
  That's my first draft of that requested singleShot approach, help is now 
appreciated.
  
  - Replace crashes due to (I think) these pointer instead of reference 
argument mismatch.
  - Find works odd. Most of the time looks good, searching "5" or "A" seams to 
run endless in our fat file
  - Can't see how that loop works, "line" is not incremented and I'm not sure 
if "done" is sufficient set

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17459?vs=47275=47362

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: brauch, cullmann, abetts, kwrite-devel, kde-frameworks-devel, #ktexteditor, 
hase, michaelh, ngraham, bruns, demsking, sars, dhaumann

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

[loh tar]

loh.tar added a comment.


  > everything could have happened, e.g. the search bar got deleted because the 
view got delete
  
  OK, but I still do not see why that will be different with that singleShot 
approach.

REPOSITORY
  R39 KTextEditor

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

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

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

[loh tar]

loh.tar added a comment.


  Hmpf! Some googling didn't help. Just some thoughts.
  
  - Adding a couple of checks where ever just to block in case of a running S 
could be a little cumbersome
  - What we need would be an QObject::processEvents, so that only the Cancle 
button is a live but no other stuff
  - Can we install an eventFilter and ignore all except our Cancel button?
  - QCoreApplication::processEvents take an optional argument e.g. 
QEventLoop::ExcludeUserInputEvents, nice but didn't help
  
  I have tried the S plugin with the fat file. That's smart enough to see 
that the task is too heavy and stopped after a couple of hits when searching. 
The Cancel button there (almost) didn't appear in this use case when invoke the 
"replace all" action. Obviously is it only updated between file changes or so. 
Hence, we can not profit from his functionality.

REPOSITORY
  R39 KTextEditor

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

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

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

[loh tar]

loh.tar added a comment.


  
  
  > That always leads to evil things, like e.g. what happens if you press the X 
button of the view/window during that.
  
  I assume you have read that article a longer time ago and have these 
paragraph regarding `Manual event processing` in mind.
  
  > This approach has significant drawbacks. For example...
  
  There are three listed
  
  1. you can't distribute computing power among different tasks
  2. makes the application react with delays to events
  3. the code is difficult to read and analyze
  
  I can't see any hint in the whole article that would avoid to choose some 
action. It's all about keeping the GUI snappy. And that is always done by enter 
the event loop.
  
  I think the points 1+2 are not important here. Point 3 do I not understand, 
at least the "read" part, but I think we have here not much more to analyze 
than if the Cancel-button works or not.
  
  The S process is not broken in some unclear state. There will only 
probably remaining matches not replaced.
  
  You mean what happens e.g. when the user continue to edit while the S is 
running. I have just tried it with our good known fat file. It's funny. You can 
scroll, edit and almost watch how it progress.
  
  There may the need to add some blocking for that. Is it possible to flag the 
document as "read only" without to break the started S?
  
  There may the need to block other things too, like your mentioned view/window 
close.
  
  One hint there was that timers are not fire without the event loop. I have 
tried this, and yes, no effect to check if a single shot timer isActive() or 
not. So I updated/fix the modulo check slightly because I noticed a lag in some 
cases. But may still not the best solution.
  
  ATM is my conclusion: Your suggested re-design may fix probably responsive 
lags but not the problem to block ugly actions. Because this whole patch is 
only in rare cases really needed I would like avoid the effort to re-design the 
logic.

REPOSITORY
  R39 KTextEditor

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

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

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

[loh tar]

loh.tar updated this revision to Diff 47275.
loh.tar added a comment.


  - Check every 500 lines or 100 machtes, whatever is first
  - Fix false check due to wrong math as long no match was found

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17459?vs=47208=47275

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: cullmann, abetts, kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, 
michaelh, ngraham, bruns, demsking, sars, dhaumann

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

[loh tar]

loh.tar added a comment.


  
  
  > I would probably not make the button that big
  
  There are two reasons why it is so big:
  
  1. It's handy. You do not have to move the mouse, no matter which button was 
clicked
  2. It "covers" both buttons to avoid a re-start of any such action without 
the need to disable them
  
  The first is of course the main reason.
  
  > That always leads to evil things, like e.g. what happens if you press the X 
button of the view/window during that.
  
  Assumed the changes you want are done, I have problems to understand in which 
way this will avoid that the Close-Window button can be clicked. For a working 
Cancel button you have to call in some way the event loop, even direct as 
currently, or indirect due to some suspend from the current work.
  
  > It would be better to refactor the replaceAll method in a way that it does
  > 
  > 1. setup
  > 2. trigger the search part wise via e.g. single shot timer
  > 3. some finalize phase at the end
  
  As you know, I'm a bit "slow" at times, Your instructions are not sufficient 
for me how to do that.

REPOSITORY
  R39 KTextEditor

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

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

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

[loh tar]

loh.tar added a comment.


  Not well tested but looks so far pretty promising.
  
  The only thing what I not really like is these modulo check when to ask for 
user input. How about a QTimer with 200/300ms?

REPOSITORY
  R39 KTextEditor

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

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

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

[loh tar]

loh.tar created this revision.
loh.tar added reviewers: KTextEditor, VDG.
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 could it happens that Kate run an almost endless S
  job which could lead up to an unusable system due to a run out of memory
  with no possibility to cancel the job other than kill Kate, See...
  

  
  CCBUG: 333517

TEST PLAN
  Before
  F6466371: 154434.png 
  
  New waiting for action
  F6466378: 1544380395.png 
  
  New while running job
  F6466380: 1544381064.png 

REPOSITORY
  R39 KTextEditor

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
Cc: kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, michaelh, ngraham, 
bruns, demsking, cullmann, sars, dhaumann

D17030: DictionaryComboBox: Keep user preferred dictionaries on top

[loh tar]

loh.tar added a comment.


  Um, I'm bit confused when I now look at the diff 1/2 here???

REPOSITORY
  R246 Sonnet

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

To: loh.tar, #frameworks, #vdg, ngraham, cullmann
Cc: cullmann, ngraham, kde-frameworks-devel, michaelh, bruns

D17030: DictionaryComboBox: Keep user preferred dictionaries on top

[loh tar]

loh.tar updated this revision to Diff 47194.
loh.tar added a comment.


  - Add @since hint to speller.h
  - Add some blank lines to speller.h to be looking a little bit nicer

REPOSITORY
  R246 Sonnet

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17030?vs=45842=47194

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

AFFECTED FILES
  src/core/settings.cpp
  src/core/settings_p.h
  src/core/speller.cpp
  src/core/speller.h
  src/ui/configui.ui
  src/ui/configwidget.cpp
  src/ui/dictionarycombobox.cpp

To: loh.tar, #frameworks, #vdg, ngraham, cullmann
Cc: cullmann, ngraham, kde-frameworks-devel, michaelh, bruns

D17030: DictionaryComboBox: Keep user preferred dictionaries on top

[loh tar]

loh.tar added a comment.


  In D17030#373912 , @cullmann wrote:
  
  > Ah, one thing: Could you add some @since 5.xxx to the new function in 
speller.h?
  
  
  Only in speller? In settings_p.h are also two new, but there was no docu, so 
I also wrote nothing, but could
  
  > I assume 5.54 would be the right one.
  
  In CMake it's currently `KF5_VERSION 5.53.0` so 5.54 is the right one?

REPOSITORY
  R246 Sonnet

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

To: loh.tar, #frameworks, #vdg, ngraham, cullmann
Cc: cullmann, ngraham, kde-frameworks-devel, michaelh, bruns

D17441: tune editing actions for large number of small edits

[loh tar]

loh.tar added a comment.


  > Yes, there is now a limit for how many highlightings are done.
  >  If its too much, we skip that, else the rendering will break everthing 
down ;=)
  
  Doing these highliting "on the fly" (only what is/became visible) is probably 
not possible(?)
  
  BTW: Thanks a lot for your detailed perf explanations! Unfortunately didn't 
see I more clear, these percent data are not very self explanatory

REPOSITORY
  R39 KTextEditor

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

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

D17437: SearchBar: Avoid malfunction with very large files

[loh tar]

loh.tar added a comment.


  In this state was this patch only a simple attempt to avoid the worst case. 
  With your new improvements is it much less likely to happen, but still 
possible. I do not like to insist of this patch it's only an offer, which could 
be improved.
  
  BTW I'm working on your suggestion to add a (functioning) Cancel button. With 
that, this patch may become more obsolete ;-)

REPOSITORY
  R39 KTextEditor

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

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

D17441: tune editing actions for large number of small edits

[loh tar]

loh.tar added a comment.


  > This needs now some user testing ;=)
  
  Done as previous, with that testfile. Seams to work so far. Find is still 
quick, these TAB S takes around 2min.
  
  The only curiosity I noticed is that no highliting take effect when there are 
too much hits(?) Search some number like 5 "142926 matches found" no highlight. 
Char A "10565 matches found" is highlighted

REPOSITORY
  R39 KTextEditor

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

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

D17441: tune editing actions for large number of small edits

[loh tar]

loh.tar added inline comments.

INLINE COMMENTS

> cullmann wrote in katetextblock.cpp:290
> Ok, doesn't work due to the issue that we might delete the range via notifier 
> in checkValidity() :P

Noob question: Would QPointer help?

  if (range && range->isValidityCheckRequired()) {

REPOSITORY
  R39 KTextEditor

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

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

D17441: tune editing actions for large number of small edits

[loh tar]

loh.tar added a comment.


  I have tried this one. Yes, works much more better! 
  Nice to see that my own (unloved) patch wake your interest to dig into that 
issue :-)
  
  As usual understand I most of your talk here not really, and these perf 
protocols give me no light on the first sight. But thank you that you let me 
participate.
  
  - Now, just find all of a char in that fat file is done very quickly
  - Doing these tab->semicolon S still takes quite a while but works here. 
System monitor told 1.8G memory was eaten
  
  I think with this patch need KateSearchBar still some improvement. Like to 
give a hint that S is running and the possibility to abort. Would be good to 
add some test if a S may run out of memory. Not everyone has a lot of RAM 
available.
  
  Um, just a stupid question: Is there a need to do the S just as it is done 
currently? So far I understand that, is the only reason why that is so 
f**antastic slow caused by tons off calls which are intend to track a normal 
editing.
  
  Why not simple do the S on the raw file, make a diff, highlight the changes 
and we are done(?) 
  I did the tab->semicolon S by sed, just for interest, and it takes 2-3 
seconds :-/

REPOSITORY
  R39 KTextEditor

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

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

D17437: SearchBar: Avoid malfunction with very large files

[loh tar]

loh.tar added a comment.


  Would be good you take the time and do that test @cullmann When you then 
think it is still useful to soften that limit let me know how.
  To become that code more smart as you suggest I have right now not the mood, 
sorry.
  I think this patch is good enough to avaoid the biggest trouble.

REPOSITORY
  R39 KTextEditor

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

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

D17437: SearchBar: Avoid malfunction with very large files

[loh tar]

loh.tar added a comment.


  - The mentioned crash could I not reproduce, but after 5 million hits and a 
felt eternity I had killed Kate
  - There is an issue now with the wrap message, where is now the same close 
button. Any ideas what's wrong?
  - I like to suggest to get rid of most of that S code and use instead code 
from the Search plugin. There is the S done in an own thread and did not 
block the UI.

REPOSITORY
  R39 KTextEditor

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

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

D17437: SearchBar: Avoid malfunction with very large files

[loh tar]

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

REVISION SUMMARY
  This is only a workaround to avoid cases where Kate can crash due to a
  run out of memory.
  

  
  CCBUG: 333517

TEST PLAN
  Search warning
  F6464367: 1544290032.png 
  Replace warning
  F6464368: 1544290055.png 

REPOSITORY
  R39 KTextEditor

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

AFFECTED FILES
  src/search/katesearchbar.cpp

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

D17128: DocumentPrivate: Remove comment mark when joining lines

[loh tar]

loh.tar added a comment.


  Thanks for your input Dominik. 
  In the meanwhile I noticed (again) these "Smart-Return" function (I noticed, 
and used that a couple of years for a short time period, but forgot that 
somehow) A reverse version of that should be nice. And somehow the possibility 
to use that automatically without to todo some special key-stroke when join 
lines by simple remove the newline character by Del-Key.
  
  But how that all could be done in sane way I have no idea. Currently I tend 
to make a new "Smart-Wrap-Paragraph" function, as suggested by Christoph. 
Because that's new, I can try to be as smart as I like without to change old 
behaviour.
  
  As said is from my point of view the existing wrap function broken, not only 
due to the behavior described in that bug report 135737 but also because the 
whole file is wrapped when no selection is done. That's not handy. This way you 
can't wrap quickly only a paragraph by shortcut without to select them 
previous. I have often scrambled a code file when I only want to warp some 
comment block.

REPOSITORY
  R39 KTextEditor

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

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

D17382: KateViewInternal: Rename getMouse/Cursor() => mouse/cursorPosition() to fit coding style

[loh tar]

loh.tar added a comment.


  In D17382#372925 , @dhaumann wrote:
  
  > sorry for the delay :(
  
  
  What? No, it was pretty fast :-)
  But I have a couple of Sonnet stuff where no progress happens :-/

REPOSITORY
  R39 KTextEditor

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

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

D9077: Bug 377342 Question

[loh tar]

loh.tar added subscribers: cullmann, loh.tar.
loh.tar added a comment.
Herald edited subscribers, added: kde-frameworks-devel, kwrite-devel; removed: 
Frameworks.


  @cullmann I thing you could give a short hint
  
  But perhaps is @christofg no longer interested to work on this issue

REPOSITORY
  R39 KTextEditor

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

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

D17030: DictionaryComboBox: Keep user preferred dictionaries on top

[loh tar]

loh.tar edited the summary of this revision.

REPOSITORY
  R246 Sonnet

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

To: loh.tar, #frameworks, #vdg, ngraham
Cc: ngraham, kde-frameworks-devel, michaelh, bruns

D17030: DictionaryComboBox: Keep user preferred dictionaries on top

[loh tar]

loh.tar added a comment.


  #Frameworks  Ping! (?)

REPOSITORY
  R246 Sonnet

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

To: loh.tar, #frameworks, #vdg, ngraham
Cc: ngraham, kde-frameworks-devel, michaelh, bruns

D17382: KateViewInternal: Rename getMouse/Cursor() => mouse/cursorPosition() to fit coding style

[loh tar]

loh.tar added a comment.


  Without any special "Oh, that would be nice", I will stop now with further S
  
  - I have tried to clean up a little bit the two header files, but that's more 
difficult than thought
  - m_cursor => cursorPosition() is also not so easy
  - There was no comment regarding m_startPos => startPos()
  - m_mouse => mousePosition(), not investigated
  - Who can I bother regarding stuff on kate-editor.org ?

REPOSITORY
  R39 KTextEditor

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

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

D17382: KateViewInternal: Rename getMouse/Cursor() => mouse/cursorPosition() to fit coding style

[loh tar]

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

REPOSITORY
  R39 KTextEditor

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

AFFECTED FILES
  src/view/kateview.cpp
  src/view/kateviewaccessible.h
  src/view/kateviewinternal.h
  src/vimode/inputmodemanager.cpp

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

D17257: KateViewInternal: S

[loh tar]

loh.tar added a comment.


  @cullmann Did you notice this? The other patches did you apply very quick, so 
I hope I didn't unsettle you with my comments.

REPOSITORY
  R39 KTextEditor

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

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

D17028: Loader: Avoid Q_FOREACH

[loh tar]

loh.tar added a comment.


  >> @cfeck wrote:
  > 
  > It looked strange on the list of names of all committers.
  
  A look at your page indicates that you are doing some public relations? 
Should you copy some data from an auto generated list, feel free to 
remove me.

REPOSITORY
  R246 Sonnet

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

To: loh.tar, davidedmundson
Cc: cfeck, smartins, kde-frameworks-devel, michaelh, ngraham, bruns

D17028: Loader: Avoid Q_FOREACH

[loh tar]

loh.tar added a comment.


  >   ..but I cannot remember we didn't ask contributors for their real name 
for contributions.
  
  Um, what? I was asked. I hope it's ok as it is.

REPOSITORY
  R246 Sonnet

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

To: loh.tar, davidedmundson
Cc: cfeck, smartins, kde-frameworks-devel, michaelh, ngraham, bruns

D17028: Loader: Avoid Q_FOREACH

[loh tar]

loh.tar added a comment.


  Well, neither of that. My "online me" is loh.tar but that was rejected by 
Phabricator, so the dot had to go. Lastly used ngraham uppercase letters, what 
looks even more strange to me :-)
  
  Should you intend to commit this, thanks in advance! May you look at D17055 
 too?

REPOSITORY
  R246 Sonnet

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

To: loh.tar, davidedmundson
Cc: cfeck, smartins, kde-frameworks-devel, michaelh, ngraham, bruns

D17028: Loader: Avoid Q_FOREACH

[loh tar]

loh.tar added a comment.


  @davidedmundson  
  May I ask how does this here will progress? You seemed to request a change 
which was not clear to me, but it's anyway "green flagged".

REPOSITORY
  R246 Sonnet

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

To: loh.tar, davidedmundson
Cc: smartins, kde-frameworks-devel, michaelh, ngraham, bruns

D17257: KateViewInternal: S

[loh tar]

loh.tar added a comment.


  Only 'm_view->' => 'view()' has plenty of hits, the others only a few.
  
  Please do a grep for m_startPos. To replace these few would  require a 
changed or added function.
  
  Last note: I hope you test all these changes thoroughly. At the last 
mentioned m_startPos S was suddenly the scrolling broken: Hence undo to only 
one change.

REPOSITORY
  R39 KTextEditor

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

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

D17257: KateViewInternal: S

[loh tar]

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

REVISION SUMMARY
  - 'm_view->' => 'view()'
  - 'm_startPos.line()' => 'startLine()'
  - 'm_startX' => 'startX()'
  - 'startPos().line()' => 'startLine()'
  - 'm_startPos' => 'startPos()'

REPOSITORY
  R39 KTextEditor

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

AFFECTED FILES
  src/view/kateviewinternal.cpp

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

D17237: KTextEditor::ViewPrivate: S

[loh tar]

loh.tar added a comment.


  I can split it if you like in two diff
  
  - 'm_doc->' => 'doc()->' ~100 hits, perhaps not the best idea? A search for 
doc() has 24 hits (without this patch)
  - 'm_viewInternal->m_cursor' => 'cursorPosition()' ~10 hits
  
  Next would be look for inline candidates, look for S in KateViewInternal

REPOSITORY
  R39 KTextEditor

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

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

D17237: KTextEditor::ViewPrivate: S

[loh tar]

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

REVISION SUMMARY
  - 'm_doc->' => 'doc()->'
  - 'm_viewInternal->m_cursor' => 'cursorPosition()'

REPOSITORY
  R39 KTextEditor

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

AFFECTED FILES
  src/view/kateview.cpp

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

D17193: KateViewInternal: Remove unneeded functions

[loh tar]

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


  - Prefer function calls for member excess
  - Merge KateViewInternal::updateView with KateViewInternal::doUpdateView
  - Add documentation to related functions and some near by cosmetic. Well, 
"Better no docu than bad docu", so if that is to difficult to check now, I can 
remove it and ship as own diff
  - Complete commit message
  - Remove Testplan, I'm always surpsised to see that later in the commit 
message

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17193?vs=46411=46473

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

AFFECTED FILES
  src/view/kateview.cpp
  src/view/kateview.h
  src/view/kateviewinternal.cpp
  src/view/kateviewinternal.h

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

D17030: DictionaryComboBox: Keep user preferred dictionaries on top

[loh tar]

loh.tar added a comment.


  Bug302689 - Impossible to delete unwanted dictionaries
  https://bugs.kde.org/show_bug.cgi?id=302689
  
  Does not quite fit, but relieves the symptoms. Enough to close that?

REPOSITORY
  R246 Sonnet

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

To: loh.tar, #frameworks, #vdg, ngraham
Cc: ngraham, kde-frameworks-devel, michaelh, bruns

D17026: DictionaryComboBoxTest: Add stretch to avoid expanding Dump button

[loh tar]

loh.tar added a comment.


  No, but that's fine. One thing less where I can make mistakes :-)
  Should your next question be regarding author ship, take a look at 
edbf2e36c805 

  Thanks.

REPOSITORY
  R246 Sonnet

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

To: loh.tar, #frameworks, #vdg, ngraham
Cc: ngraham, kde-frameworks-devel, michaelh, bruns

D17026: DictionaryComboBoxTest: Add stretch to avoid expanding Dump button

[loh tar]

loh.tar added a comment.


  Um, no(?) In that case I would have named this patch "Add stretch to ensure 
Dump button is expanding"
  Sorry, next time I try to describe the pics more precise.

REPOSITORY
  R246 Sonnet

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

To: loh.tar, #frameworks, #vdg
Cc: ngraham, kde-frameworks-devel, michaelh, bruns

D17026: DictionaryComboBoxTest: Add stretch to avoid expanding Dump button

[loh tar]

loh.tar edited the test plan for this revision.
loh.tar added a reviewer: VDG.

REPOSITORY
  R246 Sonnet

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

To: loh.tar, #frameworks, #vdg
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17193: KateViewInternal: Remove unneeded functions

[loh tar]

loh.tar updated this revision to Diff 46411.
loh.tar added a comment.


  I think that's all. 
  Well, it looks less beneficial as previous anticipated by me, but I think 
it's still the right direction.
  
  Just a view questions regarding the mix in the KTextEditor::ViewPrivate code 
how to access some data:
  
  - doc() vs m_doc: doc() is inline
  - cursorPosition() vs m_viewInternal->m_cursor: cursorPosition() is NOT inline
  
  I may have used in my patch also a mix due to the variety of existing code. 
Let me know which one to use.
  Furthermore I offer a S to unify the existing code, as an own DIFF/ticket 
or if you like in this one.
  
  Normally I would prefer the function calls, but because the highliting looks 
so nice :-) and cursorPosition() is not inline I'm undecided.
  Would it be make sense to change them to inline?
  
  Some more things I noticed and offer to change.
  
  - KateViewInternal::updateView (slot) calls KateViewInternal::doUpdateView 
(nowhere else called). Any benefit from that construct? Think its only a 
leftover from the ancient. (would make sense to include in this patch)
  
  - KateViewInternal::renderer() (often used) just returns 
KTextEditor::ViewPrivate::m_renderer but is NOT inline. When removed, could the 
call "renderer()" replaced by "m_view->m_renderer" or "m_view->renderer()" (the 
latter is also NOT inline)
  
  KateViewInternal contains some additional classes: CalculatingCursor, 
BoundedCursor, WrappingCursor, together ~275 lines
  ZoomEventFilter ~53 lines
  
  At least the cursor stuff looks worth to move into an own file.
  
  Some out-commented FIXME KF5 stuff (4 years old) encapsulated in #ifndef 
QT_NO_ACCESSIBILITY, lines ~1921 and ~747.
  May removed in above S offer when commit is named "Cleanup" or similar.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17193?vs=46327=46411

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

AFFECTED FILES
  src/view/kateview.cpp
  src/view/kateviewinternal.cpp
  src/view/kateviewinternal.h

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

D17013: Show menu bar, how to re-enable, common shortcut dialog

[loh tar]

loh.tar added a comment.


  Hi, I accidentally went by.
  How about to add an icon to some (main) toolbar, if any is present? 
  That would the need to show such a message reduce to cases where no tool bar 
is available. If that icon was added, and not already there, could that pop up 
a small tool tip to get some attention.
  A quick search here in my browser give no hit for "isVisible". There should 
be no need to show such message when the action is somewhere seen.
  
  Sorry for the noise, if I should completely wrong :-)

REPOSITORY
  R236 KWidgetsAddons

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

To: lsartorelli, ngraham, #frameworks, #kde_applications
Cc: loh.tar, aacid, cfeck, broulik, kde-frameworks-devel, michaelh, ngraham, 
bruns

D17193: KateViewInternal: Remove unneeded functions

[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.

TEST PLAN
  This is only an example snippet to show that obviously something could be 
simplified.
  
  Let me know if it is desired to go further or if there is a good reason why 
it is so complicated. 
  Depend of your answer I will enhance this patch or abandon.

REPOSITORY
  R39 KTextEditor

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

AFFECTED FILES
  src/view/kateview.cpp
  src/view/kateviewinternal.cpp
  src/view/kateviewinternal.h

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

D17030: DictionaryComboBox: Keep user preferred dictionaries on top

[loh tar]

loh.tar edited the test plan for this revision.
loh.tar added a reviewer: VDG.

REPOSITORY
  R246 Sonnet

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

To: loh.tar, #frameworks, #vdg
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17128: DocumentPrivate: Remove comment mark when joining lines

[loh tar]

loh.tar added a comment.


  > I am not sure if the normal join lines should try to be that clever.
  
  But consider the "auto comment" feature after you hit "Return", it's also 
somehow "special clever" but very useful. This tries to add the corresponding 
functionality. Well, the true opposite would be when you hit "Del" at the on of 
a line. Yeah, sounds good, will look at that too but it is (almost, see next 
note) independent from this aim.
  
  I'm currently digging into the wrap functionally which is broken since a 
decade BUG135737 . Sometimes it 
works already pretty well (with normal text) but sometimes it drives nuts 
(without selection in a code file). Very strange. Why I mention this? There I 
have follow a suggestion posted in that bug report, just o trigger a "Return" 
input at the right place. Now, while writing this text I came up with this 
"Del" idea. So, it may a good idea to re-design this patch (?)
  
  > It would make more sense to have an extra action for such a "smart" variant 
in my eyes.
  
  An extra action looks to me at first sight a little overdone, but an option 
how join behaves may appropriate. Let me know where to place that in the Edit 
Options page if you (and other) like that idea

REPOSITORY
  R39 KTextEditor

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

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

D17056: Dialog: Use DictionaryComboBox instead of plain QComboBox

[loh tar]

loh.tar added a reviewer: Frameworks.

REPOSITORY
  R246 Sonnet

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

To: loh.tar, #frameworks
Cc: mludwig, kde-frameworks-devel, michaelh, ngraham, bruns

D17055: Avoid Q_FOREACH

[loh tar]

loh.tar added a reviewer: davidedmundson.

REPOSITORY
  R246 Sonnet

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

To: loh.tar, davidedmundson
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17030: DictionaryComboBox: Keep user preferred dictionaries on top

[loh tar]

loh.tar added a reviewer: Frameworks.

REPOSITORY
  R246 Sonnet

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

To: loh.tar, #frameworks
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17026: DictionaryComboBoxTest: Add stretch to avoid expanding Dump button

[loh tar]

loh.tar added a reviewer: Frameworks.

REPOSITORY
  R246 Sonnet

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

To: loh.tar, #frameworks
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17128: DocumentPrivate: Remove comment mark when joining lines

[loh tar]

loh.tar added a comment.


  This works so far in most cases I have tried.
  
  - C/C++, // and /* */ works, but sadly not Oxygen style comments. Guess the 
single star is not as special marked. Could that be fixed in the highlight 
stuff?
  - Bash, # works
  - Ruby, # works
  - Python, # works
  - PHP, // works, # works NOT which surprised me because they are as comment 
colored
  - Tcl, # works, backslash comment continuation works not but it's not 
surprising

REPOSITORY
  R39 KTextEditor

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

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

D17128: DocumentPrivate: Remove comment mark when joining lines

[loh tar]

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

REVISION SUMMARY
  ...from the next line when current line is comment.

REPOSITORY
  R39 KTextEditor

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

AFFECTED FILES
  src/document/katedocument.cpp

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

D17123: KTextEditor::ViewPrivate: Avoid warning "Text requested for invalid range"

[loh tar]

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

REVISION SUMMARY
  ...when cut or copy a line without selection

REPOSITORY
  R39 KTextEditor

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

AFFECTED FILES
  src/view/kateview.cpp

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

D17103: SpellCheckBar: Use DictionaryComboBox instead of plain QComboBox

[loh tar]

loh.tar added a comment.


  It would be nice someone could explain, or point me where to read, why there 
is no (more) direct use of Sonnet stuff in KTextEditor/Kate.
  
  1. KTextEditor::SpellCheckBar looks (almost) the same as Sonnet::Dialog but 
has differently ingredients. I had expect, after my first poke around in 
Sonnet, that these are appear in Kate. Yes some, but not all.
  2. I was even more surprised to see that Sonnet ships some demo/test 
applications where the spellcheck, respectively correction by mouse, works like 
a charm with a morphed context menu shown only what is of interest, where as 
Kate shows the full menu where you have to fiddle down/around which is pretty 
annoying, almost unusable.
  
  To 1., if it is desired to get rid of KTextEditor::SpellCheckBar and use 
Sonnet::Dialog, I would try to do it. The usability of both are not so nice. I 
use the Nano editor for some tasks and like there spell check user interface, 
very handy. Well, it's for me indeed a major reason to use Nano at all. So, I 
would try to "copy" that interface.
  
  To 2., needless to emphasize, I like to see that in Kate too

REPOSITORY
  R39 KTextEditor

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

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

D17103: SpellCheckBar: Use DictionaryComboBox instead of plain QComboBox

[loh tar]

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

REVISION SUMMARY
  This patch alone is reasonable but become really important with the aim of 
D17030 
  and complies to D17056 

REPOSITORY
  R39 KTextEditor

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

AFFECTED FILES
  src/spellcheck/spellcheckbar.cpp
  src/spellcheck/spellcheckbar.ui

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

D17028: Loader: Avoid Q_FOREACH

[loh tar]

loh.tar added inline comments.

INLINE COMMENTS

> davidedmundson wrote in loader.cpp:273
> > Probably, yes. But Qt docu always says, "thanks to implicit sharing copying 
> > a container is very fast"
> 
> That's mixing up docs.
> 
> foreach() always does a const copy of the list. This is a cheap implicitly 
> shared copy.
> when we loop we're never detaching this list because it's const.
> 
> for() does not
> So when we loop if we call begin() we might get the non-const version. If 
> this list is used elsewhere that means we detach and that causes a deep-copy. 
> This is expensive.
> 
> In this specific case language() returns a new QStringList (.keys() generates 
> a new list) so I /think/ it is fine,
> 
> but it's definitely ambiguous. A qAsConst would make sense here.

> A qAsConst would make sense here.

In which way? This is not working:

  for (const QString  : qAsConst(languages()))

So you like to see this?

  const QStringList constLanguages = languages();
  for (const QString  : constLanguages) {

REPOSITORY
  R246 Sonnet

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

To: loh.tar, davidedmundson
Cc: smartins, kde-frameworks-devel, michaelh, ngraham, bruns

D17056: Dialog: Use DictionaryComboBox instead of plain QComboBox

[loh tar]

loh.tar created this revision.
Herald added a project: Frameworks.
loh.tar requested review of this revision.

REVISION SUMMARY
  This patch alone is reasonable but become really important with the aim of 
D17030 

REPOSITORY
  R246 Sonnet

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

AFFECTED FILES
  src/ui/dialog.cpp
  src/ui/sonnetui.ui

To: loh.tar
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17055: Avoid Q_FOREACH

[loh tar]

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

REVISION SUMMARY
  VoikkoDict, HunspellClient, HSpellDict, GuessLanguage, parsetrigrams

REPOSITORY
  R246 Sonnet

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

AFFECTED FILES
  data/parsetrigrams.cpp
  src/core/guesslanguage.cpp
  src/plugins/hspell/hspelldict.cpp
  src/plugins/hunspell/hunspellclient.cpp
  src/plugins/voikko/voikkodict.cpp

To: loh.tar
Cc: kde-frameworks-devel, michaelh, ngraham, bruns