D17949: ViewPrivate: Make applyWordWrap() more comfortable

[loh tar]

loh.tar added a comment.


  Interesting. Didn't know that such function exist. Well, it may help to read 
the handbook...
  Is there no GUI way to access such stuff? "Read the handbook" lala...
  
  Well, word/text wrapping is a field which has many room for improvements.
  
  - Adjust as block by inserting spaces at "smart" places to avoid ugly holes 
in the overall appearance
  - Wrap/split the word, not only text at a space. IIRC Is that somewhere done 
in Kate, or was there a function in Sonnet which offer that(?)
  - Your smart solution to allow in rare cases to extend the desired width
  - Probably some more I didn't know
  - Keep the indent: https://bugs.kde.org/show_bug.cgi?id=135737
  - Keep the indent and the "semantic", e.g. "Is comment" or "Is citation (like 
in e-mails)" https://bugs.kde.org/show_bug.cgi?id=369049
  
  As some of you know, I talk often about the latter two issues and I'm willing 
to fix that.
  The first three are really nice but more like the candy after the necessary 
lunch.

REPOSITORY
  R39 KTextEditor

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

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

D17949: ViewPrivate: Make applyWordWrap() more comfortable

[Dominik Haumann]

dhaumann added a comment.


  @loh.tar Slightly related: You may want to also have a look at this: 
https://phabricator.kde.org/source/ktexteditor/browse/master/src/script/data/commands/utils.js$271
  In short, there is a command line command (F7) called 'rewrap' that rewraps 
the selection in a "smart" way. My idea when writing this once was:
  
  1. there is a target wrap column, say 80
  2. there is a soft violation wrap column, say 82
  3. there is a minimum wrap column, say 70
  
  Now, lets take the following example:
  
 1 2 3 4 5 6 7  
   8

1234567890123456789012345678901234567890123456789012345678901234567890123456789012345
the quick brown fox jumps of the lazy dog. The bla bla bla bla bla 
aRatherLongWord
  
  Now, usually, aRatherLongWord would wrap to the next line, since it is longer 
that column 80. However, it leaves a "big gap" starting at column <70 (cf. 3. 
soft wrap above) in the end of the line, which is visually not so appealing ;) 
So in this case, we allow violation of the hard-wrap at 80, and instead allow a 
violation up to column 82 (cf. 2. above).
  
  With this, you can for instance nicely rewrap a TeX document so that you can 
nicely read it without too big gaps at the end of a line.
  
  However, to be honest, I don't even know whether I ever finished the 
implementation, so maybe rewrap even does something different...

REPOSITORY
  R39 KTextEditor

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

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

D17949: ViewPrivate: Make applyWordWrap() more comfortable

[loh tar]

loh.tar added subscribers: cullmann, dhaumann.
loh.tar added a comment.


  > please add proper tests for this functionality
  
  No idea how
  
  Because  there are a couple of bug reports where often the response was like 
"No, can't be done" need this a closer look from @dhaumann and @cullmann

INLINE COMMENTS

> mwolff wrote in kateview.cpp:2355
> store in a std::unique_ptr and remove the manual `delete` further down below

fine, will do it

> mwolff wrote in kateview.cpp:2359
> bool shouldWrap = true;

I dislike that. I hope someone else like my goto way

REPOSITORY
  R39 KTextEditor

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

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

D17949: ViewPrivate: Make applyWordWrap() more comfortable

[Milian Wolff]

mwolff requested changes to this revision.
mwolff added a comment.
This revision now requires changes to proceed.


  I like what I'm seeing in the screenshot, but please add proper tests for 
this functionality

INLINE COMMENTS

> kateview.cpp:2355
> +// Because we shrink and expand lines, we need a powerful "Moving Cursor"
> +KTextEditor::MovingCursor *curr = 
> doc()->newMovingCursor(KTextEditor::Cursor(selectionRange().start()));
> +

store in a std::unique_ptr and remove the manual `delete` further down below

> kateview.cpp:2359
> +for (int line = first; line <= selectionRange().end().line(); ++line) {
> +// Is our first line a somehow filled line?
> +while(doc()->plainKateTextLine(first)->firstChar() < 0) {

bool shouldWrap = true;

> kateview.cpp:2366
> +// C++ can only continue the inner loop, but we need here to 
> continue our "for" loop
> +goto NextLine;
> +}

shouldWrap = false;
break;

> kateview.cpp:2370
> +// Is our current line a somehow filled line? If not, wrap the 
> paragraph
> +if (doc()->plainKateTextLine(line)->firstChar() < 0) {
> +curr->setPosition(line, 0); // Set on empty line

if (!shouldWrap) {
  continue;
  }

REPOSITORY
  R39 KTextEditor

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

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

D17949: ViewPrivate: Make applyWordWrap() more comfortable

[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
  - Don't wrap entire document when nothing is selected but wrap current line
  - Join paragraph before wrap to avoid odd result
  
  TODO Update "What this" hint
  This patch may conflict with D17128 , but 
because that need anyway some more love
  I like to offer this one now, to fetch some feedback.
  
  As always fresh and poor tested. But I think it is a handy improvement and 
seems
  to work nicely. As you know I have often mention not so lovely things about 
the
  wrap feature and there are a couple of bug reports.
  
  This patch address not all aspects but should be a step in the right 
direction.

TEST PLAN
  My arbitrary choose test data.
  
  1a    aaa
  2aa aaa aa aaa aaa    aaa aa
  3 aaa aaa aa aaa a   a 
a   aaa aa
  4a    aaa aa
  5aaa    aaa aa  aaa aa aaa
  
  1-    ---
  2-- --- -- --- ---    --- --
  3 --- --- -- --- -   - 
-   --- --
  4-    --- --
  5---    --- --  --- -- ---
  
  1x    xxx
  2xx xxx xx xxx xxx    xxx xx
  3 xxx xxx xx xxx x   x 
x   xxx xx
  4x    xxx xx
  5xxx    xxx xx  xxx xx xxx
  
  Before
  F6523470: 1546546614.png 
  With Patch
  F6523471: 1546546403.png 

REPOSITORY
  R39 KTextEditor

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

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

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