D28855: put minimumkeepsize actually in the netpref KCM

[Harald Sitter]

sitter abandoned this revision.
sitter added a comment.


  moved to https://invent.kde.org/frameworks/kio/-/merge_requests/123

REPOSITORY
  R241 KIO

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

To: sitter, ngraham, dfaure
Cc: ahmadsamir, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns

D28855: put minimumkeepsize actually in the netpref KCM

[Harald Sitter]

sitter added a comment.


  Coding style is actually wildly inconsistent in that file, so I've stuck to 
what similar other lines have.

REPOSITORY
  R241 KIO

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

To: sitter, ngraham, dfaure
Cc: ahmadsamir, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns

D28855: put minimumkeepsize actually in the netpref KCM

[Ahmad Samir]

ahmadsamir added inline comments.

INLINE COMMENTS

> netpref.cpp:87
> +connect(cb_globalMarkPartial, &QAbstractButton::toggled,
> +partialWidget, [partialWidget](bool checked) { 
> partialWidget->setEnabled(checked); });
> +globalLayout->addWidget(partialWidget);

Why not "&QWidget::setEnabled" instead of the lambda?

> netpref.cpp:89
> +globalLayout->addWidget(partialWidget);
> +auto partialLayout = new QFormLayout(gb_Global);
> +partialLayout->setContentsMargins(20, 0, 0, 0); // indent below mark 
> partial

This triggers this warning:

  QLayout: Attempting to add QLayout "" to QGroupBox "", which already has a 
layout

I've just found out that QWidget::setLayout(QLayout *layout) will take 
ownership of "layout":
https://doc.qt.io/qt-5/qwidget.html#setLayout
so I guess there is no need to set a parent for partialLayout when 
instantiating it.

> netpref.cpp:92
> +partialWidget->setLayout(partialLayout);
> +sb_globalMinimumKeepSize = new KPluralHandlingSpinBox( this );
> +sb_globalMinimumKeepSize->setSuffix( ki18np( " byte", " bytes" ) );

Coding style, no space after ( or before ), IIUC.

> netpref.cpp:93
> +sb_globalMinimumKeepSize = new KPluralHandlingSpinBox( this );
> +sb_globalMinimumKeepSize->setSuffix( ki18np( " byte", " bytes" ) );
> +connect(sb_globalMinimumKeepSize, 
> QOverload::of(&QSpinBox::valueChanged),

Same.

> netpref.cpp:96
> +this, &KIOPreferences::configChanged);
> +partialLayout->addRow(i18nc( "@label:spinbox", "Minimum size required to 
> keep on cancel:" ),
> +  sb_globalMinimumKeepSize);

"If cancelled, automatically delete partially uploaded files smaller than"

And same coding style as before.

REPOSITORY
  R241 KIO

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

To: sitter, ngraham, dfaure
Cc: ahmadsamir, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns

D28855: put minimumkeepsize actually in the netpref KCM

[Harald Sitter]

sitter added a comment.


  And didn't we have a better spinny box for byte units somewhere? Where the 
user can put `1 byte` or `1 kib` or `1 gib` and the box knows what to do?

REPOSITORY
  R241 KIO

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

To: sitter, ngraham, dfaure
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D28855: put minimumkeepsize actually in the netpref KCM

[Harald Sitter]

sitter added a comment.


  F8240046: Screenshot_20200415_164836.png 

  
  I'm very open for better labels, it's a horrible concept to explain in a few 
words.

REPOSITORY
  R241 KIO

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

To: sitter, ngraham, dfaure
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D28855: put minimumkeepsize actually in the netpref KCM

[Harald Sitter]

sitter updated this revision to Diff 80204.
sitter added a comment.


  improve label a tad

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28855?vs=80203&id=80204

BRANCH
  master

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

AFFECTED FILES
  src/kcms/kio/netpref.cpp
  src/kcms/kio/netpref.h

To: sitter, ngraham, dfaure
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D28855: put minimumkeepsize actually in the netpref KCM

[Harald Sitter]

sitter created this revision.
sitter added reviewers: ngraham, dfaure.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
sitter requested review of this revision.

REVISION SUMMARY
  I didn't even know it was a thing! by the looks of it the setting only
  applies to retention of .part files on aborting transfers (that is, files
  must be at least this size so they get retained on aborting transfers).
  so the setting is in a form inside a widget and that widget's enabled
  state is contient on partial marking being enabled
  
  CCBUG: 419987

TEST PLAN
  look at the kcm and toggle the button

REPOSITORY
  R241 KIO

BRANCH
  master

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

AFFECTED FILES
  src/kcms/kio/netpref.cpp
  src/kcms/kio/netpref.h

To: sitter, ngraham, dfaure
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns