Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Option to display history in reverse order #150

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

mhtvsSFrpHdE
Copy link

@mhtvsSFrpHdE mhtvsSFrpHdE commented Aug 12, 2023

Look at "Unsolved concern" section, there's a change may break current stable release.
close #149

QXT

NO_QXT

This code come with no Qxt library.
So every code wrapped inside #ifndef will compile and run.

#ifndef NO_QXT
    // Code...
#endif

System tray menu data

qlippersystemtray.cpp

QlipperModel::clearHistory() is at qlippermodel.cpp.
When clearing history, a sample clipboard item will created like this:

ClipboardContent tmp;
tmp["text/plain"] = tr("Welcome to the Qlipper clipboard history applet").toUtf8();
QlipperItem item(QClipboard::Clipboard, QlipperItem::PlainText, tmp);

beginInsertRows(QModelIndex(), sticky_count, sticky_count);
m_dynamic.append(item);    // <- here
endInsertRows();

History is stored in m_dynamic, and beginInsertRows seems to be able to decide where to insert a clipboard item.

System tray menu interface

qlippersystemtray.cpp

There is m_shortcutMenu = new QMenuView();, and related code to bind events.
m_shortcutMenu->setModel(m_model); tells the menu where to get data.

m_model is created by m_model = new QlipperModel(this);

How the data goes in system tray menu

qlippermodel.cpp

There are two types of data in system tray menu,
one is m_sticky and another is m_dynamic.
m_dynamic is the list that stores copied data,
m_sticky can be ignored right now.

The program is hard coded to put data on top of the list by use
m_dynamic.prepend(item);, to reverse the order, change this code to
m_sticky.append(item);

When it come to interface

In QCreator, double click to open qlipperpreferencesdialog.ui
so "Check box" could be dragged from "Design" tab into setting interface.
UI is layout based, multiple checkbox could stay inside same line,
if thier width is smaller than certain amout.
Otherwise, force two wide checkbox into same line,
setting window width will increase.

Get interface value in code

"qlipperpreferences.h"

There is "bool clearItemsOnExit() const;",
write similar code aside to get user interface value.
Typically a checkbox definiation looks like:

bool QlipperPreferences::clearItemsOnExit() const
{
    return value("clearItemsOnExit", false).toBool();
}

To use the function, call from other place:

auto clearItemsOnExit = QlipperPreferences::Instance()->clearItemsOnExit();

Old data

qlippermodel.cpp

When copy content same as exist value, it will picked from data list,
and put to top of the menu, then do font bold.
By doing test, I should put this data to end of the menu,
change bold position from sticky_count + 1 to end of the menu.

The variable to determine a font is bold or not seems to be m_currentIndex.

The code to move data to top seems to be

void QlipperModel::setCurrentDynamic(int ix)

The code use erase to remove range of element at once instead of one by one.

At the beginning, I guess this ensure that all old data will be deleted in signle event.
But later the code use -1 to determine how many data to be removed.
I should update exist code to use ranged remove,
but exist behavior is stable, best not to touch it.

Normal order:

beginRemoveRows(QModelIndex(), sticky_count + max_history - 1, sticky_count + m_dynamic.count() - 1);
m_dynamic.erase(m_dynamic.begin() + (max_history - 1), m_dynamic.end());
endRemoveRows();

Reverse order:

auto removeRowsBeginPosition = sticky_count;
auto removeRowsEndPosition = removeRowsBeginPosition + (m_dynamic.count() - max_history);
beginRemoveRows(QModelIndex(), removeRowsBeginPosition, removeRowsEndPosition);
auto dataBeginPosition = m_dynamic.begin();
auto dataEndPosition = dataBeginPosition + (m_dynamic.count() - max_history);
m_dynamic.erase(dataBeginPosition, dataEndPosition);
endRemoveRows();

Move data

qlippermodel.cpp

The code with reverse order:

    if (reverseOrder)
    {
        auto m_dynamic_lastIndex = m_dynamic_count - 1;

        // move if not already on end
        if (ix != m_dynamic_lastIndex)
        {
            beginMoveRows(QModelIndex(), sticky_count + ix, sticky_count + ix, QModelIndex(), sticky_count + m_dynamic_count);
            m_dynamic.move(ix, m_dynamic_lastIndex);
            endMoveRows();
        }
    }
    else
    {
        // move if not already on top
        if (ix != 0)
        {
            beginMoveRows(QModelIndex(), sticky_count + ix, sticky_count + ix, QModelIndex(), sticky_count);
            m_dynamic.move(ix, 0);
            endMoveRows();
        }
    }

However, Qt API beginMoveRows seems to have a index start from 0,
but the above code send count as parameter instead of count - 1.
If I use count - 1, endMoveRows() will crash the program.
I must assume that this code is based on count and have code hack in somewhere else.

Bold it

qlippermodel.cpp

Look all references to m_currentIndex, one of them located at

    switch (role)
    {
    case Qt::DisplayRole:
        return list.at(row).displayRole();
    case Qt::DecorationRole:
        return list.at(row).decorationRole();
    case Qt::ToolTipRole:
        return list.at(row).tooltipRole();
    case Qt::FontRole:
        return m_currentIndex == index ? m_boldFont : m_normalFont;    // <- here
    }

This code not need to update.
To update it use reverseOrder:

if (reverseOrder)
{
    m_currentIndex = index(m_sticky.count() + m_dynamic_lastIndex);
    m_network->sendData(m_dynamic.at(m_dynamic_lastIndex).content());
}

Unsolved concern

Debug mode save setting crash

An unknwon exception will crash the program when the code runs to

void QlipperModel::resetPreferences()
{
    beginRemoveRows(QModelIndex(), 0, m_sticky.count() - 1);    // <- here
    m_sticky.clear();
    endRemoveRows();
    QList<QlipperItem> sticky = QlipperPreferences::Instance()->getStickyItems();
    beginInsertRows(QModelIndex(), 0, sticky.count() - 1);
    m_sticky = sticky;
    endInsertRows();
}

This crash only happens in Debug mode but not Release mode.
A quick fix to avoid this suggested by code AI:

void QlipperModel::resetPreferences()
{
    // If sticky items is empty, code will crash in Debug mode for unknown reason
    // It will not crash in Release mode
    if (m_sticky.count() == 0){
        return;
    }

    beginRemoveRows(QModelIndex(), 0, m_sticky.count() - 1);
    m_sticky.clear();
    endRemoveRows();
    QList<QlipperItem> sticky = QlipperPreferences::Instance()->getStickyItems();
    beginInsertRows(QModelIndex(), 0, sticky.count() - 1);
    m_sticky = sticky;
    endInsertRows();
}

When click OK to save Qlipper Preferences, program will crash in debug mode, but runs fine in release mode.
This commit is questionable and could break release mode behavior. The commit author have no desire to do further investigation.
@mhtvsSFrpHdE mhtvsSFrpHdE changed the title Option to display history in reverse order close #149 Option to display history in reverse order Aug 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Option to display history in reverse order
1 participant