D27285: Add left/right indent fill (as opposed to left-only), extend indent lines to broken lines

[eudoxos]

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

REVISION SUMMARY
  The configuration UI is somewhat clumsy, some better arrangement suggestion 
would be welcome.
  There are still more options on the radar, such as showing the "broken-line" 
character also
  optionally indented (now is shown left with line numbers) and others.

REPOSITORY
  R39 KTextEditor

BRANCH
  dynwrap-marker (branched from master)

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

AFFECTED FILES
  src/dialogs/katedialogs.cpp
  src/dialogs/textareaappearanceconfigwidget.ui
  src/render/katelinelayout.cpp
  src/render/katelinelayout.h
  src/render/katerenderer.cpp
  src/utils/kateconfig.cpp
  src/utils/kateconfig.h

To: eudoxos, #vdg
Cc: kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, cblack, GB_2, 
domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D27285: Add left/right indent fill (as opposed to left-only), extend indent lines to broken lines

[eudoxos]

eudoxos added a comment.


  This is an example with right fill of 3 characters, left fill off, with 
indentation lines (note that they spend all visual lines of the dynamically 
wrapped line).
  
  F8095407: image.png 

REPOSITORY
  R39 KTextEditor

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

To: eudoxos, #vdg
Cc: kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, cblack, GB_2, 
domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D27285: Add left/right indent fill (as opposed to left-only), extend indent lines to broken lines

[eudoxos]

eudoxos added a comment.


  Note: extra space (right fill) is not computed correctly if tabs are used for 
line indentation.

REPOSITORY
  R39 KTextEditor

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

To: eudoxos, #vdg
Cc: kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, cblack, GB_2, 
domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D27285: Add left/right indent fill (as opposed to left-only), extend indent lines to broken lines

[eudoxos]

eudoxos updated this revision to Diff 75440.
eudoxos added a comment.


  Fix tab/space indentation, fix maximum width. No other glitches known at 
present.

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27285?vs=75361&id=75440

BRANCH
  dynwrap-marker (branched from master)

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

AFFECTED FILES
  src/dialogs/katedialogs.cpp
  src/dialogs/textareaappearanceconfigwidget.ui
  src/render/katelinelayout.cpp
  src/render/katelinelayout.h
  src/render/katerenderer.cpp
  src/utils/kateconfig.cpp
  src/utils/kateconfig.h

To: eudoxos, #vdg
Cc: kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, cblack, GB_2, 
domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D27285: Add left/right indent fill (as opposed to left-only), extend indent lines to broken lines

[eudoxos]

eudoxos updated this revision to Diff 75533.
eudoxos added a comment.


  Rename config keys as per discussion under D27238 


REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27285?vs=75440&id=75533

BRANCH
  dynwrap-marker

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

AFFECTED FILES
  src/dialogs/katedialogs.cpp
  src/dialogs/textareaappearanceconfigwidget.ui
  src/render/katelinelayout.cpp
  src/render/katelinelayout.h
  src/render/katerenderer.cpp
  src/utils/kateconfig.cpp
  src/utils/kateconfig.h

To: eudoxos, #vdg
Cc: kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, cblack, GB_2, 
domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D27285: Add left/right indent fill (as opposed to left-only), extend indent lines to broken lines

[eudoxos]

eudoxos added a comment.


  This is the current dialogue, with two new options at the bottom (the circle 
had horizontal spring before an unrelated option, I think it was there by 
accident...?!).
  
  F8099358: image.png 

REPOSITORY
  R39 KTextEditor

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

To: eudoxos, #vdg
Cc: kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, cblack, GB_2, 
domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D27285: Add left/right indent fill (as opposed to left-only), extend indent lines to broken lines

[Dominik Haumann]

dhaumann added a comment.


  No offense meant, but even with the screenshots I still have no idea what 
this is about :)
  
  Can you add a before/after screenshot so we can see the visual difference?
  
  Also, the "fill left" and "fill right" wording is new, and not intuitively 
understandable for me.
  
  Can we do better?

REPOSITORY
  R39 KTextEditor

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

To: eudoxos, #vdg
Cc: dhaumann, kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, cblack, 
GB_2, domson, michaelh, ngraham, bruns, demsking, cullmann, sars

D27285: Add left/right indent fill (as opposed to left-only), extend indent lines to broken lines

[eudoxos]

eudoxos added a comment.


  > No offense meant, but even with the screenshots I still have no idea what 
this is about :)
  
  Good point, I somehow missed to explain the rationale for the change. :) If 
you are familiar with vim, this would be the `breakindent` and `breakindentopt` 
 settings (that was my patch from 2007 which did not make it into vim until a 
few years back).
  
  > Can you add a before/after screenshot so we can see the visual difference?
  
  I will use this example from std::sort documentation 
 which is not the most 
convoluted code (those changes are the most useful for deeply nested code).
  
  This is the original rendering (with narrow window to make the point) and 
D27238  applied (breaking line anywhere 
rather than at whitespace-only):
  F8100695: image.png 
  Note how indentation blocks, manifest from empty space left, and also 
vertical indentation lines, are visually damaged by the dynamic wrap fill (I 
call this left fill, it is left of the indentation level). Dynamic wrap is 
something related to presentation only, not to overall code logic 
(indentation), and is line-local; so the option to enable/disable this fill is 
to make it possible to remove this globally-prominent visual cue about 
something very much local. This is without the left fill (note how indentation 
lines now also extend vertically):
  F8100700: image.png 
  Now, the line being wrapped does have some importance and it should be shown 
somewhere. It is shown already by the arrow in the left margin, but that is way 
too far from the break, even left from folding markers (global logic) (I 
understand the wrap markers are there probably from the age when dynamic 
wrapping did not respect indentation and it was a sin to write code beyond col 
80 ;) ).
  
  So the "right fill" is **right** of the indentation level on subsequent 
visual lines and gives visual cue about the line being wrapped; like this (2 
characters wide, that is adjustable):
  F8100703: image.png 
  Of course, there are many options how to mark the break where it occurs, this 
one was just the easiest one. Onec can clutter configuration more and think of 
options like moving the arrow in the left margin to the line beginning, like 
this (this is just GIMP, not in the code)
  F8100706: image.png 
  but I think it is better to start with something relatively simple.
  
  It is possible to enable both left and right fill (not sure why anybody would 
want that, but I was not sure even for the original (left) fill):
  F8100716: image.png 
  
  > Also, the "fill left" and "fill right" wording is new, and not intuitively 
understandable for me.
  
  That is a new "terminology" I am free to adjust. Left is simply left of 
indentation level, right is on the right.
  
  Hope this explains somewhat.

REPOSITORY
  R39 KTextEditor

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

To: eudoxos, #vdg
Cc: dhaumann, kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, cblack, 
GB_2, domson, michaelh, ngraham, bruns, demsking, cullmann, sars

D27285: Add left/right indent fill (as opposed to left-only), extend indent lines to broken lines

[eudoxos]

eudoxos added a comment.


  (side note: I am not sure whether to use phabricator.kde.org or 
invent.kde.org for this. Are they complementary or orthogonal?)

REPOSITORY
  R39 KTextEditor

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

To: eudoxos, #vdg
Cc: dhaumann, kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, cblack, 
GB_2, domson, michaelh, ngraham, bruns, demsking, cullmann, sars

D27285: Add left/right indent fill (as opposed to left-only), extend indent lines to broken lines

[Dominik Haumann]

dhaumann added a subscriber: cullmann.
dhaumann added a comment.


  Ok, I now fully got this, thanks for the explanation. And indeed without any 
left fill visual feedback is missing.
  
  What I wonder is how much of this needs to be configurable. We could also 
just make the left fill of 4 default or so.
  
  In any case, I think we should go forward with this patch. @cullmann your 
opinion?

REPOSITORY
  R39 KTextEditor

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

To: eudoxos, #vdg
Cc: cullmann, dhaumann, kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, 
cblack, GB_2, domson, michaelh, ngraham, bruns, demsking, sars

D27285: Add left/right indent fill (as opposed to left-only), extend indent lines to broken lines

[Milian Wolff]

mwolff added a comment.


  In D27285#611163 , @dhaumann wrote:
  
  > Ok, I now fully got this, thanks for the explanation. And indeed without 
any left fill visual feedback is missing.
  >
  > What I wonder is how much of this needs to be configurable. We could also 
just make the left fill of 4 default or so.
  >
  > In any case, I think we should go forward with this patch. @cullmann your 
opinion?
  
  
  You could use the current document's indent width instead of hardcoding 4?

REPOSITORY
  R39 KTextEditor

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

To: eudoxos, #vdg
Cc: mwolff, cullmann, dhaumann, kwrite-devel, kde-frameworks-devel, rrosch, 
LeGast00n, cblack, GB_2, domson, michaelh, ngraham, bruns, demsking, sars

D27285: Add left/right indent fill (as opposed to left-only), extend indent lines to broken lines

[eudoxos]

eudoxos added a comment.


  Note to myself: should handle the case when width percentage disallows the 
indent, e.g. here:
  F8104336: image.png 

REPOSITORY
  R39 KTextEditor

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

To: eudoxos, #vdg
Cc: mwolff, cullmann, dhaumann, kwrite-devel, kde-frameworks-devel, rrosch, 
LeGast00n, cblack, GB_2, domson, michaelh, ngraham, bruns, demsking, sars

D27285: Add left/right indent fill (as opposed to left-only), extend indent lines to broken lines

[eudoxos]

eudoxos updated this revision to Diff 75727.
eudoxos added a comment.


  Fix indentation lines shown spuriously when horizontal space was unsufficient 
for indent fill

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27285?vs=75533&id=75727

BRANCH
  dynwrap-marker

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

AFFECTED FILES
  src/dialogs/katedialogs.cpp
  src/dialogs/textareaappearanceconfigwidget.ui
  src/render/katelinelayout.cpp
  src/render/katelinelayout.h
  src/render/katerenderer.cpp
  src/utils/kateconfig.cpp
  src/utils/kateconfig.h

To: eudoxos, #vdg
Cc: mwolff, cullmann, dhaumann, kwrite-devel, kde-frameworks-devel, rrosch, 
LeGast00n, cblack, GB_2, domson, michaelh, ngraham, bruns, demsking, sars