From da34da69b6edd204e2d4c7ce1eec0231b4df9846 Mon Sep 17 00:00:00 2001 From: shenleban tongying Date: Thu, 10 Oct 2024 03:00:13 -0400 Subject: [PATCH] clean: delete or rewrite weird things in `history.cc` (#1815) --- src/history.cc | 77 ++++++++++++++++---------------------------- src/history.hh | 41 ++++++----------------- src/ui/mainwindow.cc | 8 ++--- 3 files changed, 42 insertions(+), 84 deletions(-) diff --git a/src/history.cc b/src/history.cc index 8a4662c8..1c9d559f 100644 --- a/src/history.cc +++ b/src/history.cc @@ -13,53 +13,34 @@ History::History( unsigned size, unsigned maxItemLength_ ): addingEnabled( true ), dirty( false ), timerId( 0 ) -{ -} - -History::History( Load, unsigned size, unsigned maxItemLength_ ): - maxSize( size ), - maxItemLength( maxItemLength_ ), - addingEnabled( true ), - dirty( false ), - timerId( 0 ) { QFile file( Config::getHistoryFileName() ); if ( !file.open( QFile::ReadOnly | QIODevice::Text ) ) return; // No file -- no history - for ( unsigned count = 0; count < maxSize; ++count ) { - QByteArray lineUtf8 = file.readLine( 4096 ); + QTextStream in( &file ); + while ( !in.atEnd() && items.size() <= maxSize ) { + QString line = in.readLine( 4096 ); - if ( lineUtf8.endsWith( '\n' ) ) - lineUtf8.chop( 1 ); + auto firstSpace = line.indexOf( ' ' ); - if ( lineUtf8.isEmpty() ) + if ( firstSpace < 0 || firstSpace == line.size() ) { break; + } - QString line = QString::fromUtf8( lineUtf8 ); + QString t = line.right( line.size() - firstSpace - 1 ).trimmed(); - int firstSpace = line.indexOf( ' ' ); - - if ( firstSpace < 0 || firstSpace + 1 == line.size() ) - // No spaces or value? Bad line. End this. - break; - - bool isNumber; - - unsigned groupId = line.left( firstSpace ).toUInt( &isNumber, 10 ); - - if ( !isNumber ) - break; // That's not right - - items.push_back( Item( groupId, line.right( line.size() - firstSpace - 1 ) ) ); + if ( !t.isEmpty() ) { + items.push_back( Item{ line.right( line.size() - firstSpace - 1 ).trimmed() } ); + } } } History::Item History::getItem( int index ) { if ( index < 0 || index >= items.size() ) { - return Item(); + return {}; } return items.at( index ); } @@ -69,7 +50,7 @@ void History::addItem( Item const & item ) if ( !enabled() ) return; - if ( item.word.isEmpty() ) { + if ( item.word.isEmpty() || item.word.size() > maxItemLength ) { // The search looks bogus. Don't save it. return; } @@ -78,9 +59,14 @@ void History::addItem( Item const & item ) if ( items.contains( item ) ) items.removeOne( item ); - //TODO : The groupid has not used at all. items.push_front( item ); + Item & addedItem = items.first(); + + // remove \n and \r to avoid destroying the history file + addedItem.word.replace( QChar::LineFeed, QChar::Space ); + addedItem.word.replace( QChar::CarriageReturn, QChar::Space ); + ensureSizeConstraints(); dirty = true; @@ -119,22 +105,14 @@ bool History::save() return true; QSaveFile file( Config::getHistoryFileName() ); - - if ( !file.open( QFile::WriteOnly | QIODevice::Text ) ) + if ( !file.open( QFile::WriteOnly | QIODevice::Text ) ) { return false; + } - for ( QList< Item >::const_iterator i = items.constBegin(); i != items.constEnd(); ++i ) { - QByteArray line = i->word.toUtf8(); - - // Those could ruin our format, so we replace them by spaces. They shouldn't - // be there anyway. - line.replace( '\n', ' ' ); - line.replace( '\r', ' ' ); - - line = QByteArray::number( i->groupId ) + " " + line + "\n"; - - if ( file.write( line ) != line.size() ) - return false; + QTextStream out( &file ); + for ( const auto & i : items ) { + // "0 " is to keep compatibility with the original GD (an unused number) + out << "0 " << i.word.trimmed() << '\n'; } if ( file.commit() ) { @@ -156,13 +134,14 @@ void History::clear() void History::setSaveInterval( unsigned interval ) { - if ( timerId ) { + if ( timerId != 0 ) { killTimer( timerId ); timerId = 0; } - if ( interval ) { - if ( dirty ) + if ( interval != 0 ) { + if ( dirty ) { save(); + } timerId = startTimer( interval * 60000 ); } } diff --git a/src/history.hh b/src/history.hh index 2fb924bc..e2dfda33 100644 --- a/src/history.hh +++ b/src/history.hh @@ -8,9 +8,14 @@ #include #include -#define DEFAULT_MAX_HISTORY_ITEM_LENGTH 256 +constexpr unsigned DEFAULT_MAX_HISTORY_ITEM_LENGTH = 256; + /// Search history +/// +/// To work with the original GD's history file, +/// every entry starts with a number like "123 word" +/// class History: public QObject { Q_OBJECT @@ -20,43 +25,17 @@ public: /// An item in history struct Item { - /// Group the search was performed in - unsigned groupId; - /// The word that was searched QString word; - Item(): - groupId( 0 ) - { - } - - Item( unsigned groupId_, QString const & word_ ): - groupId( groupId_ ), - word( word_ ) - { - } - + // For assisting QList::contains & QList::removeOne bool operator==( Item const & other ) const { - return QString::compare( word, other.word, Qt::CaseInsensitive ) == 0 && groupId == other.groupId; - } - - bool operator!=( Item const & other ) const - { - return !operator==( other ); + return QString::compare( word, other.word, Qt::CaseInsensitive ) == 0; } }; - /// Indicates an intention to load -- see the relevant History constructor. - struct Load - {}; - - /// Constructs an empty history which can hold at most "size" items. - History( unsigned size = 20, unsigned maxItemLength = DEFAULT_MAX_HISTORY_ITEM_LENGTH ); - - /// Loads history from its file. If load fails, the result would be an empty - /// history. The size parameter is same as in other constructor. - explicit History( Load, unsigned size = 20, unsigned maxItemLength = DEFAULT_MAX_HISTORY_ITEM_LENGTH ); + /// Loads history from its file. If the loading fails, the result would be an empty history. + explicit History( unsigned size = 256, unsigned maxItemLength = DEFAULT_MAX_HISTORY_ITEM_LENGTH ); /// Adds new item. The item is always added at the beginning of the list. /// If there was such an item already somewhere on the list, it gets removed diff --git a/src/ui/mainwindow.cc b/src/ui/mainwindow.cc index e1e93d2c..ec1e91d2 100644 --- a/src/ui/mainwindow.cc +++ b/src/ui/mainwindow.cc @@ -154,7 +154,7 @@ MainWindow::MainWindow( Config::Class & cfg_ ): trayIconMenu( this ), addTab( this ), cfg( cfg_ ), - history( History::Load(), cfg_.preferences.maxStringsInHistory, cfg_.maxHeadwordSize ), + history( cfg_.preferences.maxStringsInHistory, cfg_.maxHeadwordSize ), dictionaryBar( this, configEvents, cfg.preferences.maxDictionaryRefsInContextMenu ), articleMaker( dictionaries, groupInstances, cfg.preferences ), articleNetMgr( this, @@ -3800,7 +3800,7 @@ void MainWindow::on_importHistory_triggered() history.enableAdd( true ); for ( QList< QString >::const_iterator i = itemList.constBegin(); i != itemList.constEnd(); ++i ) - history.addItem( History::Item( 1, *i ) ); + history.addItem( { *i } ); history.enableAdd( cfg.preferences.storeHistory ); @@ -3970,13 +3970,13 @@ void MainWindow::addWordToHistory( const QString & word ) // skip epwing reference link. epwing reference link has the pattern of r%dAt%d if ( QRegularExpressionMatch m = RX::Epwing::refWord.match( word ); m.hasMatch() ) return; - history.addItem( History::Item( 1, word.trimmed() ) ); + history.addItem( { word.trimmed() } ); } void MainWindow::forceAddWordToHistory( const QString & word ) { history.enableAdd( true ); - history.addItem( History::Item( 1, word.trimmed() ) ); + history.addItem( { word.trimmed() } ); history.enableAdd( cfg.preferences.storeHistory ); }