clean: delete or rewrite weird things in history.cc (#1815)

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 );
-
-    int firstSpace = line.indexOf( ' ' );
-
-    if ( firstSpace < 0 || firstSpace + 1 == line.size() )
-      // No spaces or value? Bad line. End this.
-      break;
+    QString t = line.right( line.size() - firstSpace - 1 ).trimmed();
 
-    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 );
   }
 }

src/history.hh

@@ -8,9 +8,14 @@
 #include <QList>
 #include <QString>
 
-#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

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 );
 }