patterncppMinor
QPlainTextEdit subclass function to indent lines in selection
Viewed 0 times
selectionfunctionsubclassindentlinesqplaintextedit
Problem
I am, primarily for learning purposes, working on creating an editor component in C++ with Qt. As a first step, to begin getting familiar with editing a document via
I'm interested in any feedback on whether or not I'm following "best practices" with
QTextCursor, I wrote this function on my subclass of QPlainTextEdit to indent multiple selected lines:void MyEditor::increaseSelectionIndent()
{
QTextCursor curs = textCursor();
// Do nothing if we don't have a selection.
if(!curs.hasSelection())
return;
// Get the first and count of lines to indent.
int spos = curs.anchor(), epos = curs.position();
if(spos > epos)
{
int hold = spos;
spos = epos;
epos = hold;
}
curs.setPosition(spos, QTextCursor::MoveAnchor);
int sblock = curs.block().blockNumber();
curs.setPosition(epos, QTextCursor::MoveAnchor);
int eblock = curs.block().blockNumber();
// Do the indent.
curs.setPosition(spos, QTextCursor::MoveAnchor);
curs.beginEditBlock();
for(int i = 0; i <= (eblock - sblock); ++i)
{
curs.movePosition(QTextCursor::StartOfBlock, QTextCursor::MoveAnchor);
curs.insertText("\t");
curs.movePosition(QTextCursor::NextBlock, QTextCursor::MoveAnchor);
}
curs.endEditBlock();
// Set our cursor's selection to span all of the involved lines.
curs.setPosition(spos, QTextCursor::MoveAnchor);
curs.movePosition(QTextCursor::StartOfBlock, QTextCursor::MoveAnchor);
while(curs.block().blockNumber() < eblock)
{
curs.movePosition(QTextCursor::NextBlock, QTextCursor::KeepAnchor);
}
curs.movePosition(QTextCursor::EndOfBlock, QTextCursor::KeepAnchor);
// Done!
setTextCursor(curs);
}I'm interested in any feedback on whether or not I'm following "best practices" with
QTextCursor - am I doing anything weird, or that can be simplified or improved?Solution
-
Some of these comments can be removed:
Too obvious:
Noisy:
-
These initialized variables:
should be on separate lines for better maintenance:
-
No need to do a manual swap:
Just use
-
No need calculate the difference each time:
Define a constant before the loop and use it instead:
Some of these comments can be removed:
Too obvious:
// Do nothing if we don't have a selection.Noisy:
// Done!-
These initialized variables:
int spos = curs.anchor(), epos = curs.position();should be on separate lines for better maintenance:
int spos = curs.anchor();
int epos = curs.position();-
No need to do a manual swap:
if(spos > epos)
{
int hold = spos;
spos = epos;
epos = hold;
}Just use
std::swap to keep it idiomatic:if (spos > epos)
{
std::swap(spos, epos);
}-
No need calculate the difference each time:
for(int i = 0; i <= (eblock - sblock); ++i)Define a constant before the loop and use it instead:
const int blockDifference = eblock - sblock;
for (int i = 0; i <= blockDifference; ++i)Code Snippets
// Do nothing if we don't have a selection.int spos = curs.anchor(), epos = curs.position();int spos = curs.anchor();
int epos = curs.position();if(spos > epos)
{
int hold = spos;
spos = epos;
epos = hold;
}if (spos > epos)
{
std::swap(spos, epos);
}Context
StackExchange Code Review Q#33899, answer score: 6
Revisions (0)
No revisions yet.