D17730: Review KateStatusBar

loh.tar added a comment. > I like the new status bar. Thanks :-) > In "modified" state it would be cool to be able to click to save the document Yes, that was also my idea. IIRC I had somewhere asked but got no response. Will do it soon. > Move the modified label from far

D17730: Review KateStatusBar

gregormi added a comment. @loh.tar: I like the new status bar. > Move the modified label from far right to far left This way have it a more prominent position Now, the "modified label" behaves like a button but with no visible action when it is clicked. Two general observations:

D17730: Review KateStatusBar

This revision was automatically updated to reflect the committed changes. Closed by commit R39:dce48b397818: Review KateStatusBar (authored by loh.tar, committed by cullmann). REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17730?vs=48813=49037 REVISION

D17730: Review KateStatusBar

cullmann added a comment. Btw., I can resize both Kate and KWrite like I want, like in the screenshots visible above. REVISION DETAIL https://phabricator.kde.org/D17730 To: loh.tar, #ktexteditor, cullmann Cc: dhaumann, zetazeta, cullmann, kwrite-devel, kde-frameworks-devel, #ktexteditor,

D17730: Review KateStatusBar

cullmann accepted this revision. cullmann added a comment. I think the current state can go in. Here with and without this patch two tests fail: The following tests FAILED: 24 - kateindenttest_testCppstyle (Failed) 67 - vimode_emulatedcommandbar (Failed) (perhaps due

D17730: Review KateStatusBar

loh.tar added a comment. > It was stated that it broke tests, should this be addressed now? As shown in the pic, the bar forces no width anymore (at least it seems so) Your comments at the bug report are interesting, but I'm not sure to do it. I had Christoph promised to add options to

D17730: Review KateStatusBar

dhaumann added a comment. Hm, what is the current state of this patch? It was stated that it broke tests, should this be addressed now? Btw, I added a detailed comment to https://bugs.kde.org/show_bug.cgi?id=402904 that explains a proper solution to shrinking the statusbar: 1.

D17730: Review KateStatusBar

loh.tar updated this revision to Diff 48813. loh.tar edited the summary of this revision. loh.tar added a comment. - Remove not supported Rich Text from [BLOCK] hint - Add word count info to line label F6530066: pic.png CHANGES SINCE LAST UPDATE

D17730: Review KateStatusBar

loh.tar updated this revision to Diff 48781. loh.tar added a comment. Allow StatusBarButton to shrink Ah! Strange! :-/ But somehow the more usual behavior, these "crunching" is unlikely. Couldn't find a hint where it is done in Kate, but after some digging around I have some solution

D17730: Review KateStatusBar

zetazeta added a comment. The problem I talk about happens on kwrite, no kate. I get the crunch as well on kate. Cheers :) REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17730 To: loh.tar, #ktexteditor, cullmann Cc: dhaumann, zetazeta, cullmann, kwrite-devel,

D17730: Review KateStatusBar

loh.tar added a comment. @zetazeta wrote > I find it annoying that the status bar imposes a limit on the minimum width of the window @dhaumann wrote > Maybe with this patch, the status bar grows and forces a different width me wrote > The bar can shrink almost endless

D17730: Review KateStatusBar

zetazeta added a comment. In D17730#382676 , @cullmann wrote: > > - 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(?) > > I would not

D17730: Review KateStatusBar

loh.tar added a comment. That can I not really imagine :-/ Wouldn't that not somehow to be notice in normal use? Let's try this. Shot without bar, Now choose "Show bar", The window didn't change. (Sorry, no tool at hand to paste pics together) The bar can shrink almost endless.

D17730: Review KateStatusBar

dhaumann added a comment. @loh.tar I think this test uses dynamic word wrap, and therefore requires a specific width of the view. Maybe with this patch, the status bar grows and forces a different width, so that the dynamic word wrap changes and the test fails? REPOSITORY R39 KTextEditor

D17730: Review KateStatusBar

loh.tar added a comment. > Do we loose this property by using real buttons instead of labels as this patch proposes? I don't think so. The buttons are not entire new, only some labels are now ALSO buttons. REPOSITORY R39 KTextEditor REVISION DETAIL

D17730: Review KateStatusBar

dhaumann added a comment. @cullmann I remember that you fiddled around with the status bar quite a lot to make it pixel perfect in height, i.e. that the height of the statusbar matches the hight of e.g. the search bar. Do we loose this property by using real buttons instead of labels as

D17730: Review KateStatusBar

loh.tar added a comment. Harr, one more issue :-/ F6516314: 1546186710.png Idea how to fix this? I had in the past often such trouble, but so far I remember no solution. So my fix would be to remove the emphasis. Is it OK to update this diff

D17730: Review KateStatusBar

loh.tar added a comment. One idea: Now you have only to click once to change some setting, not twice (double-click). So if these test simulate a clicks... REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17730 To: loh.tar, #ktexteditor, cullmann Cc: zetazeta,

D17730: Review KateStatusBar

loh.tar added a comment. I'm pretty baffled. What does the status bar which may break some input (mode) ? These report says nothing to me. Well, ok, the vi-mode can enabled/disabled :-/ REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17730 To: loh.tar,

D17730: Review KateStatusBar

cullmann added a comment. I reverted it ATM, to see if the tests pass then again. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17730 To: loh.tar, #ktexteditor, cullmann Cc: zetazeta, cullmann, kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, michaelh,

D17730: Review KateStatusBar

cullmann reopened this revision. cullmann added a comment. This revision is now accepted and ready to land. Hmm, this seems to break the unit tests :/ see

D17730: Review KateStatusBar

This revision was automatically updated to reflect the committed changes. Closed by commit R39:3b874e56b8dc: Review KateStatusBar (authored by loh.tar, committed by cullmann). REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17730?vs=48387=48395 REVISION

D17730: Review KateStatusBar

cullmann accepted this revision. cullmann added a comment. This revision is now accepted and ready to land. Yep, fine with this. Thanks! REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17730 To: loh.tar, #ktexteditor, cullmann Cc: zetazeta, cullmann,

D17730: Review KateStatusBar

loh.tar updated this revision to Diff 48387. loh.tar retitled this revision from "[WIP] Review KateStatusBar " to "Review KateStatusBar ". loh.tar edited the summary of this revision. loh.tar added a comment. - Remove seemingly unneeded spacing. Well, there is a change but tiny and perhaps