GUI: optionsdialog cleanup

Diapolo commented at 9:38 am on July 9, 2012: none

This pull is intended to prepare the optionsdialog for further changes to the proxy options and includes a few changes that would bloat a proxy options pull.

remove duplicate includes, that are already present in ui_optionsdialog.h
change QIntValidator to not allow 0 as port-number
re-order some function calls to match the Ui element order, for better readbility and to prepare for the addition of further IPv6 and Tor proxy options
restat warning for the language selection is only shown, when the language was changed (not on simply activating the Ui element)
modify proxy warning message “This setting…” into “Proxy settings…”
split check for object == ui->proxyIp into seperate if-clause
unify used format for comments in the code
introduce handleProxyIpValid() function, which handles UI elements and the save button states for valid/invalid proxy IPs

This is what I intend to do with the proxy options (not included here): planned proxy options

in src/qt/optionsdialog.cpp: in 99d5583943 outdated

199     }
200 }
201 
202 bool OptionsDialog::eventFilter(QObject *object, QEvent *event)
203 {
204-    if(object == ui->proxyIp && event->type() == QEvent::FocusOut)

laanwj commented at 10:34 am on July 10, 2012:

What did you change here?

Diapolo commented at 11:08 am on July 10, 2012:

I simply split the if(case 1 && case 2) into 2 if(case 1) { if(case 2) } as this is likely needed, when adding more proxy options. I remember you learned me to split a pull like this off from a new feature pull :).

laanwj commented at 11:38 am on July 10, 2012:

Ok, yes, that makes sense.

in src/qt/optionsdialog.cpp: in 99d5583943 outdated

233+            else
234+            {
235+                fProxyIpValid = true;
236+                ui->statusLabel->clear();
237+                emit proxyIpValid(true);
238+            }

laanwj commented at 10:34 am on July 10, 2012:

emit proxyIpValid(fProxyIpValid)?

Diapolo commented at 11:06 am on July 10, 2012:

Yeah, that’s good.

Edit: Updated with the last rebase!

in src/qt/optionsdialog.cpp: in 722e305573 outdated

227+            {
228+                ui->proxyIp->setValid(false);
229+                fProxyIpValid = false;
230+                ui->statusLabel->setStyleSheet("QLabel { color: red; }");
231+                ui->statusLabel->setText(tr("The supplied proxy address is invalid."));
232+                emit proxyIpValid(fProxyIpValid);

laanwj commented at 11:37 am on July 10, 2012:

You can factor it out of the IF/ELSE statement too :)

Diapolo commented at 11:50 am on July 10, 2012:

I had that in mind too, don’t ask me why I didn’t do it. Perhaps I like rebasing so much :D.

Edit: Take a look at the code, I guess this is now even better!

in src/qt/optionsdialog.cpp: in 6580e8625e outdated

222-            ui->statusLabel->clear();
223-            emit proxyIpValid(true);
224+            /* Check proxyIp for a valid IPv4/IPv6 address */
225+            CService addr;
226+            fProxyIpValid = LookupNumeric(ui->proxyIp->text().toStdString().c_str(), addr);
227+            if(!fProxyIpValid)

laanwj commented at 6:30 am on July 17, 2012:

Lol I guess you could move this to a signal handler for proxyIpValid now, instead of here (though I think this is fine, it’s your choice).

Diapolo commented at 8:22 am on July 17, 2012:

I will create an additional commit for this, take a look and then we decide if we leave this as is or use the version after the commit.

Diapolo commented at 9:05 am on July 17, 2012: none

The last commits ensures we can easily handle an extended networking page (with additional QValidatedLineEdit fields) in eventFilter() without duplicating much UI element code.

Diapolo commented at 2:00 pm on July 21, 2012: none

@laanwj I would like to merge that last commit after you took a look.

optionsdialog cleanup

- remove duplicate includes, that are already present in ui_optionsdialog.h
- change QIntValidator to not allow 0 as port-number
- re-order some function calls to match the Ui element order, for better readbility and to prepare for the addition of further IPv6 and Tor proxy options
- restat warning for the language selection is only shown, when the language was changed (not on simply activating the Ui element)
- split check for object == ui->proxyIp into seperate if-clause
- micro-optimize the code in the above mentioned if-clause
- unify used format for comments in the code
- introduce handleProxyIpValid() function, which handles UI elements and the
save button states for valid/invalid proxy IPs

1376a542b4

in src/qt/optionsdialog.cpp: in 20239bff77 outdated

185@@ -193,7 +186,7 @@ void OptionsDialog::showRestartWarning_Proxy()
186 {
187     if(!fRestartWarningDisplayed_Proxy)
188     {
189-        QMessageBox::warning(this, tr("Warning"), tr("This setting will take effect after restarting Bitcoin."), QMessageBox::Ok);

laanwj commented at 3:22 pm on July 21, 2012:

Customizing this message causes extra work for the translators. I think it’s better to keep this message generic, so that we can re-use it for all settings that need restarting.

laanwj referenced this in commit 2e53709f17 on Jul 27, 2012

laanwj merged this on Jul 27, 2012

laanwj closed this on Jul 27, 2012

suprnurd referenced this in commit 4a9fbca080 on Dec 5, 2017

DrahtBot locked this on Sep 8, 2021

GUI: optionsdialog cleanup #1571