[NestedListHelper] Improve indentation code
ClosedPublic

Authored by poboiko on Apr 26 2020, 4:10 PM.

Details

Summary

The patch includes following improvements:

  1. handleAfterKeyPressEvent was only used to adjust margins. We don't do it anymore, so get rid of it.
  2. Add support for Tab key to increase indentation level (either at the beginning of a list item or with multiple lines selected)
  3. Fix canIndent / canDedent logic when cursor has a selection. canIndent should work with topOfSelection, and canDedent --- with bottomOfSelection.
  4. Enclose handleOnIndentMore / Less in beginEditBlock / endEditBlock, so they appear as a single event in Undo stack.
  5. A Return on an empty list element decreases the indentation (so double-Return terminates the list)
Test Plan
  1. Tab / Return keys now work as explained
  2. IndentMore / Less are undoable with single Ctrl+Z

(this leads to cursor jumping around, which is probably beyond the scope of this patch)

  1. Increase / Decrease Indent actions now become enabled when they should if the selection is present

(the behavior of handleOnIndentMore / Less when selection is present is slightly broken, which is also beyond the scope)

Diff Detail

Repository
R310 KTextWidgets
Branch
unused (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 26230
Build 26248: arc lint + arc unit
poboiko created this revision.Apr 26 2020, 4:10 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptApr 26 2020, 4:10 PM
poboiko requested review of this revision.Apr 26 2020, 4:10 PM
dfaure added inline comments.Apr 29 2020, 10:45 PM
src/widgets/nestedlisthelper.cpp
87 ↗(On Diff #81256)

So the last block cannot be unindented? How come?

poboiko added inline comments.Apr 30 2020, 1:22 PM
src/widgets/nestedlisthelper.cpp
87 ↗(On Diff #81256)

That's the current block being checked, not the next one. I've just checked to be sure, last block can be unindented :)

TBH, I don't really know if it's even possible for the current block to be invalid (that would probably mean that textCursor() returned by QTextEdit is invalid?).
I've just borrowed this particular check from the old code...

dfaure added inline comments.May 2 2020, 10:40 AM
src/widgets/nestedlisthelper.cpp
87 ↗(On Diff #81256)

Oh. I see. Well, it would be much more readable to move that if() *before* the line that declares and sets nextBlock.

And yes I can't really see how this can happen, either.

93 ↗(On Diff #81256)

nextBlock is only used here so I would move its definition to just before this line.

(same thing in the previous method, about prevBlock)

poboiko updated this revision to Diff 81738.May 2 2020, 12:06 PM

Improve readability as suggested, also const'ify

dfaure accepted this revision.May 2 2020, 12:14 PM
This revision is now accepted and ready to land.May 2 2020, 12:14 PM
This revision was automatically updated to reflect the committed changes.