D20052: Make tabs rename-able

[Elvis Angelaccio]

elvisangelaccio requested changes to this revision.
elvisangelaccio added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> dolphintabbar.cpp:23
>  #include <KLocalizedString>
> +#include <KNameAndUrlInputDialog>
>  

Unused, please remove it.

> dolphintabbar.cpp:166
> +        } else if (selectedAction == renameTabAction) {
> +            bool result = false;
> +            QString newTabName = QInputDialog::getText(this, 

We could use a more descriptive variable name here, for example `renamed` or similar.

> dolphintabbar.cpp:167
> +            bool result = false;
> +            QString newTabName = QInputDialog::getText(this, 
> +                i18n("Rename Tab"), i18n("New tab name"), QLineEdit::Normal, tabText(index), &result

Please add `const`

> dolphintabbar.cpp:168
> +            QString newTabName = QInputDialog::getText(this, 
> +                i18n("Rename Tab"), i18n("New tab name"), QLineEdit::Normal, tabText(index), &result
> +            );

Maybe we can add a colon at the end of "New tab name" ?

> dolphintabpage.h:143
> +     */
> +    QUuid getUuid() const;
>  

Coding style: please call it `uuid()`, we don't usually use the getXXX naming style for getters.

> dolphintabwidget.cpp:38
> +    m_lastViewedTab(0),
> +    m_tabNames()
>  {

Not needed, the constructor is implicitly called for QMap objects.

> dolphintabwidget.cpp:280
>  
> +void DolphinTabWidget::renameTab(int index, QString& name) {
> +    auto uuid = tabPageAt(index)->getUuid();

Coding style: brace should go to next line for functions.

> dolphintabwidget.cpp:286
> +    }
> +    else {
> +        tabBar()->setTabText(index, name);

Coding style: else starts at the end of previous line.

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D20052

To: krutovmikhail, #dolphin, ngraham, elvisangelaccio
Cc: elvisangelaccio, ngraham, kfm-devel, alexde, feverfew, meven, spoorun, navarromorales, firef, andrebarros, emmanuelp, mikesomov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20190326/9b392f35/attachment.htm>