D14610: Use KLineEdit for folder name if folder has write access, else use QLabel

[Henrik Fehlauer]

rkflx added a comment.


  In D14610#303537 <https://phabricator.kde.org/D14610#303537>, @rkflx wrote:
  
  > `KPropertiesDialog::setFileNameReadOnly`
  > `m_bFromTemplate`
  
  
  @shubham Any comments on that (see my questions to @dfaure above)?

INLINE COMMENTS

> kpropertiesdialog.cpp:988
> +    KFileItemListProperties itemList(KFileItemList() << item);
> +    if (d->bMultiple || isTrash || hasRoot || !itemList.supportsMoving() || (itemList.isDirectory() & !itemList.supportsWriting())) {
>          QLabel *lab = new QLabel(d->m_frame);

Without your patch, we only tested for `!itemList.supportsMoving()`. Now you also test for `(itemList.isDirectory() & !itemList.supportsWriting()`. Could you explain in what cases we need that new test? As far as I can see this blocks renaming the folder you removed the write permission from (while only its children should be prevented from being renamed).

(`&` instead of `&&` also caught my eye.)

> kpropertiesdialog.cpp:999-1019
> +            d->m_lined = new KLineEdit(d->m_frame);
> +            d->m_lined->setObjectName("KFilePropsPlugin::nameLineEdit");
> +            d->m_lined->setText(filename);
> +            d->nameArea = d->m_lined;
> +            d->m_lined->setFocus();
> +            grid->addWidget(d->nameArea, curRow++, 2);
> +        

Please don't add additional indentation here.

> kpropertiesdialog.cpp:1021
>  
> -    grid->addWidget(d->nameArea, curRow++, 2);
> -

You are moving that line to both the `if` and the `else` branch. Why not keep it here?

REPOSITORY
  R241 KIO

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

To: shubham, rkflx, dfaure, ngraham
Cc: kde-frameworks-devel, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20180805/ac543475/attachment.html>