Differential D19131

[RFC] DocumentPrivate: Fix "auto-rewrap" in static wrap mode
Needs ReviewPublic
Actions

Authored by loh.tar on Feb 18 2019, 8:01 PM.

Details

Reviewers
cullmann
Group Reviewers
KTextEditor
Summary

This patch is very similar to D17949 but in this state not a full replacement for that.

Here is some core function touched which need some watchfulness to make it not even worse.
Here is the existing behavior what will wrapped without a selection not changed.
With this patch could be D17949 simplified or perhaps their extra features added here.

This patch has in static wrap mode the potential to re-wrap the paragraph nicely on-the-fly, similar to dynamic wrap, whereas without this patch only the current line is wrapped, but not the following, with the result that the paragraph became cluttered or the preceding line with the result that the paragraph became not optimal wrapped in some situations.

Looking at other code parts indicate that this behavior is/was desired but seems to got lost at sometime. See also D19051, which looks with this patch obsolete.

  • The "auto-rewrap" can only work when the text is fresh entered, not after some load from disk. Doing a join of the paragraph should trigger the needed change
  • The old wrapText function is now split in three functions
  • The new wrapLine function is very close to old code but some variables are renamed

Issues which needs to be fixed

  • Based on TextLine::isAutoWrapped. This info gets lost when you undo some change
  • There may cases where the user wants to remove the isAutoWrapped flag
  • The cursor is sometimes misplaced, see pic where the x was DELeted and should be now in line 1 in the middle of the bbbb Hint: It was not DEL key but BACKSPACE

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped
loh.tar created this revision.Feb 18 2019, 8:01 PM
Restricted Application added projects: Kate, Frameworks. · View Herald TranscriptFeb 18 2019, 8:01 PM
Restricted Application added subscribers: kde-frameworks-devel, kwrite-devel. · View Herald Transcript
loh.tar requested review of this revision.Feb 18 2019, 8:01 PM
loh.tar updated this revision to Diff 52072.Feb 19 2019, 2:58 PM
loh.tar edited the summary of this revision. (Show Details)

Fix mentioned misplaced cursor.
The reason was a special treatment for past-end-of-line cursors. The fix changed/fixed also that now the cursor is moved by one not two positions in this case.

loh.tar updated this revision to Diff 52158.Feb 20 2019, 4:26 PM
loh.tar edited the summary of this revision. (Show Details)
loh.tar set the repository for this revision to R39 KTextEditor.
  • Do not wrap in some cases with block selection
  • Auto wrap once wrapped line again also when static wrap is disabled

Autotest looks BTW not so bad

The following tests FAILED:
62 - vimode_modes (Failed)
65 - vimode_keys (Failed)
cullmann added a comment.Mar 4 2019, 6:26 PM

:=) Perhaps I read this diff to fast, but I can not really tell if it does what it should.

I can understand from the description it tries to keep auto-wrapped things in balance during editing.

I think to better understand the behavior, one needs to have tests for the different cases it kicks in.

For the implementation: I think that code shall not be triggered ever if the user has the static wrap off, I think that would be very confusing.

I am even not sure if that should be the default behavior or if one should not be able to turn that off to have a "dumb" but deterministic wrapping.

But I use static word wrap "never", therefore I might be off with my opinion.

Revision Contents
Changeset List

Diff 52158

View Options

src/document/katedocument.h

Loading...
View Options

src/document/katedocument.cpp

Loading...