From f919685797afc64ddaafc2e8e00567cd1c4aeb4f Mon Sep 17 00:00:00 2001 From: Igor Kushnir Date: Tue, 29 Jun 2021 11:59:16 +0300 Subject: [PATCH] Extract duplicated "gdfrom-" string manipulation into functions Such helper functions facilitate understanding of the code and simplify implementing new features. --- articleview.cc | 72 +++++++++++++++++++++++++++++++++++--------------- articleview.hh | 3 +++ mainwindow.cc | 10 +++---- mainwindow.hh | 4 +-- preferences.cc | 2 +- 5 files changed, 61 insertions(+), 30 deletions(-) diff --git a/articleview.cc b/articleview.cc index 7946ea66..ac2bdf6e 100644 --- a/articleview.cc +++ b/articleview.cc @@ -183,6 +183,30 @@ static QVariant evaluateJavaScriptVariableSafe( QWebFrame * frame, const QString .arg( variable ) ); } +namespace { + +char const * const scrollToPrefix = "gdfrom-"; + +bool isScrollTo( QString const & id ) +{ + return id.startsWith( scrollToPrefix ); +} + +QString dictionaryIdFromScrollTo( QString const & scrollTo ) +{ + Q_ASSERT( isScrollTo( scrollTo ) ); + const int scrollToPrefixLength = 7; + return scrollTo.mid( scrollToPrefixLength ); +} + +} // unnamed namespace + +QString ArticleView::scrollToFromDictionaryId( QString const & dictionaryId ) +{ + Q_ASSERT( !isScrollTo( dictionaryId ) ); + return scrollToPrefix + dictionaryId; +} + ArticleView::ArticleView( QWidget * parent, ArticleNetworkAccessManager & nm, AudioPlayerPtr const & audioPlayer_, std::vector< sptr< Dictionary::Class > > const & allDictionaries_, @@ -550,11 +574,14 @@ void ArticleView::loadFinished( bool ) } } else - if ( Qt4x5::Url::queryItemValue( url, "scrollto" ).startsWith( "gdfrom-" ) ) { - // There is no active article saved in history, but we have it as a parameter. - // setCurrentArticle will save it and scroll there. - setCurrentArticle( Qt4x5::Url::queryItemValue( url, "scrollto" ), true ); + QString const scrollTo = Qt4x5::Url::queryItemValue( url, "scrollto" ); + if( isScrollTo( scrollTo ) ) + { + // There is no active article saved in history, but we have it as a parameter. + // setCurrentArticle will save it and scroll there. + setCurrentArticle( scrollTo, true ); + } } @@ -675,10 +702,10 @@ QStringList ArticleView::getArticlesList() QString ArticleView::getActiveArticleId() { QString currentArticle = getCurrentArticle(); - if ( !currentArticle.startsWith( "gdfrom-" ) ) + if ( !isScrollTo( currentArticle ) ) return QString(); // Incorrect id - return currentArticle.mid( 7 ); + return dictionaryIdFromScrollTo( currentArticle ); } QString ArticleView::getCurrentArticle() @@ -693,7 +720,7 @@ QString ArticleView::getCurrentArticle() void ArticleView::jumpToDictionary( QString const & id, bool force ) { - QString targetArticle = "gdfrom-" + id; + QString targetArticle = scrollToFromDictionaryId( id ); // jump only if neceessary, or when forced if ( force || targetArticle != getCurrentArticle() ) @@ -704,13 +731,14 @@ void ArticleView::jumpToDictionary( QString const & id, bool force ) void ArticleView::setCurrentArticle( QString const & id, bool moveToIt ) { - if ( !id.startsWith( "gdfrom-" ) ) + if ( !isScrollTo( id ) ) return; // Incorrect id if ( !ui.definition->isVisible() ) return; // No action on background page, scrollIntoView there don't work - if ( getArticlesList().contains( id.mid( 7 ) ) ) + QString const dictionaryId = dictionaryIdFromScrollTo( id ); + if ( getArticlesList().contains( dictionaryId ) ) { if ( moveToIt ) ui.definition->page()->mainFrame()->evaluateJavaScript( QString( "document.getElementById('%1').scrollIntoView(true);" ).arg( id ) ); @@ -721,7 +749,7 @@ void ArticleView::setCurrentArticle( QString const & id, bool moveToIt ) ui.definition->history()->currentItem().setUserData( userData ); ui.definition->page()->mainFrame()->evaluateJavaScript( - QString( "gdMakeArticleActive( '%1' );" ).arg( id.mid( 7 ) ) ); + QString( "gdMakeArticleActive( '%1' );" ).arg( dictionaryId ) ); } } @@ -737,7 +765,8 @@ bool ArticleView::isFramedArticle( QString const & ca ) return false; return ui.definition->page()->mainFrame()-> - evaluateJavaScript( QString( "!!document.getElementById('gdexpandframe-%1');" ).arg( ca.mid( 7 ) ) ).toBool(); + evaluateJavaScript( QString( "!!document.getElementById('gdexpandframe-%1');" ) + .arg( dictionaryIdFromScrollTo( ca ) ) ).toBool(); } bool ArticleView::isExternalLink( QUrl const & url ) @@ -765,8 +794,7 @@ void ArticleView::tryMangleWebsiteClickedUrl( QUrl & url, Contexts & contexts ) if ( result.type() == QVariant::String ) { // Looks this way - - contexts[ ca.mid( 7 ) ] = QString::fromLatin1( url.toEncoded() ); + contexts[ dictionaryIdFromScrollTo( ca ) ] = QString::fromLatin1( url.toEncoded() ); QUrl target; @@ -798,7 +826,7 @@ void ArticleView::updateCurrentArticleFromCurrentFrame( QWebFrame * frame ) if ( frameName.startsWith( "gdexpandframe-" ) ) { - QString newCurrent = "gdfrom-" + frameName.mid( 14 ); + QString newCurrent = scrollToFromDictionaryId( frameName.mid( 14 ) ); if ( getCurrentArticle() != newCurrent ) setCurrentArticle( newCurrent, false ); @@ -1190,7 +1218,7 @@ void ArticleView::openLink( QUrl const & url, QUrl const & ref, { if( dictName.compare( QString::fromUtf8( allDictionaries[ i ]->getName().c_str() ) ) == 0 ) { - newScrollTo = QString( "gdfrom-" ) + QString::fromUtf8( allDictionaries[ i ]->getId().c_str() ); + newScrollTo = scrollToFromDictionaryId( QString::fromUtf8( allDictionaries[ i ]->getId().c_str() ) ); break; } } @@ -2004,7 +2032,7 @@ void ArticleView::contextMenuRequested( QPoint const & pos ) QString id = tableOfContents[ result ]; if ( id.size() ) - setCurrentArticle( "gdfrom-" + id, true ); + setCurrentArticle( scrollToFromDictionaryId( id ), true ); } } #if 0 @@ -2124,7 +2152,7 @@ void ArticleView::moveOneArticleUp() { QStringList lst = getArticlesList(); - int idx = lst.indexOf( current.mid( 7 ) ); + int idx = lst.indexOf( dictionaryIdFromScrollTo( current ) ); if ( idx != -1 ) { @@ -2133,7 +2161,7 @@ void ArticleView::moveOneArticleUp() if ( idx < 0 ) idx = lst.size() - 1; - setCurrentArticle( "gdfrom-" + lst[ idx ], true ); + setCurrentArticle( scrollToFromDictionaryId( lst[ idx ] ), true ); } } } @@ -2146,13 +2174,13 @@ void ArticleView::moveOneArticleDown() { QStringList lst = getArticlesList(); - int idx = lst.indexOf( current.mid( 7 ) ); + int idx = lst.indexOf( dictionaryIdFromScrollTo( current ) ); if ( idx != -1 ) { idx = ( idx + 1 ) % lst.size(); - setCurrentArticle( "gdfrom-" + lst[ idx ], true ); + setCurrentArticle( scrollToFromDictionaryId( lst[ idx ] ), true ); } } } @@ -2230,10 +2258,10 @@ void ArticleView::on_highlightAllButton_clicked() void ArticleView::onJsActiveArticleChanged(QString const & id) { - if ( !id.startsWith( "gdfrom-" ) ) + if ( !isScrollTo( id ) ) return; // Incorrect id - emit activeArticleChanged( this, id.mid( 7 ) ); + emit activeArticleChanged( this, dictionaryIdFromScrollTo( id ) ); } void ArticleView::doubleClicked( QPoint pos ) diff --git a/articleview.hh b/articleview.hh index d9d7d9f0..2c274e7e 100644 --- a/articleview.hh +++ b/articleview.hh @@ -90,6 +90,9 @@ public: typedef QMap< QString, QString > Contexts; + /// Returns "gdfrom-" + dictionaryId. + static QString scrollToFromDictionaryId( QString const & dictionaryId ); + /// Shows the definition of the given word with the given group. /// scrollTo can be optionally set to a "gdfrom-xxxx" identifier to position /// the page to that article on load. diff --git a/mainwindow.cc b/mainwindow.cc index 5b83b391..fb4ed026 100644 --- a/mainwindow.cc +++ b/mainwindow.cc @@ -2330,7 +2330,7 @@ void MainWindow::translateInputFinished( bool checkModifiers ) } void MainWindow::respondToTranslationRequest( Config::InputPhrase const & phrase, - bool checkModifiers, QString const & dictID ) + bool checkModifiers, QString const & scrollTo ) { if ( phrase.isValid() ) { @@ -2338,7 +2338,7 @@ void MainWindow::respondToTranslationRequest( Config::InputPhrase const & phrase if ( checkModifiers && ( mods & (Qt::ControlModifier | Qt::ShiftModifier) ) ) addNewTab(); - showTranslationFor( phrase, 0, dictID ); + showTranslationFor( phrase, 0, scrollTo ); if ( cfg.preferences.searchInDock ) { @@ -2779,7 +2779,7 @@ void MainWindow::showHistoryItem( QString const & word ) void MainWindow::showTranslationFor( Config::InputPhrase const & phrase, unsigned inGroup, - QString const & dictID ) + QString const & scrollTo ) { ArticleView *view = getCurrentArticleView(); @@ -2789,7 +2789,7 @@ void MainWindow::showTranslationFor( Config::InputPhrase const & phrase, ( groupInstances.empty() ? 0 : groupInstances[ groupList->currentIndex() ].id ); - view->showDefinition( phrase, group, dictID ); + view->showDefinition( phrase, group, scrollTo ); updatePronounceAvailability(); updateFoundInDictsList(); @@ -3906,7 +3906,7 @@ void MainWindow::headwordReceived( const QString & word, const QString & ID ) toggleMainWindow( true ); setTranslateBoxTextAndClearSuffix( Folding::escapeWildcardSymbols( word ), NoPopupChange ); respondToTranslationRequest( Config::InputPhrase::fromPhrase( word ), - false, "gdfrom-" + ID ); + false, ArticleView::scrollToFromDictionaryId( ID ) ); } void MainWindow::updateFavoritesMenu() diff --git a/mainwindow.hh b/mainwindow.hh index b39ce125..2b2806c4 100644 --- a/mainwindow.hh +++ b/mainwindow.hh @@ -260,7 +260,7 @@ private: QString unescapeTabHeader( QString const & header ); void respondToTranslationRequest( Config::InputPhrase const & phrase, - bool checkModifiers, QString const & dictID = QString() ); + bool checkModifiers, QString const & scrollTo = QString() ); void updateSuggestionList(); void updateSuggestionList( QString const & text ); @@ -406,7 +406,7 @@ private slots: void mutedDictionariesChanged(); void showTranslationFor( Config::InputPhrase const &, unsigned inGroup = 0, - QString const & dictID = QString() ); + QString const & scrollTo = QString() ); void showTranslationFor( QString const & ); void showTranslationFor( QString const &, QStringList const & dictIDs, diff --git a/preferences.cc b/preferences.cc index 59eddea2..1e83f78f 100644 --- a/preferences.cc +++ b/preferences.cc @@ -109,7 +109,7 @@ Preferences::Preferences( QWidget * parent, Config::Class & cfg_ ): for( QStringList::iterator i = availHelps.begin(); i != availHelps.end(); ++i ) { - QString loc = i->mid( 7, i->length() - 11 ); + QString loc = i->mid( 7, i->length() - 11 ); // e.g. *i == "gdhelp_en.qch" => loc == "en" QString lang = loc.mid( 0, 2 ); QString reg; if(loc.length() >= 5 )