From fae1fdae1c8688118d9e02e5c1154fb075ca7c08 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 26 Nov 2019 22:29:06 +0100 Subject: [PATCH 1/3] CI: allow meaningful error messages - Move set -e down in the script so that the error-handling at the top (which prints meaningful error messages) isn't short-circuited by the -e. --- ci/calamaresstyle | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ci/calamaresstyle b/ci/calamaresstyle index 46acbdc3d..8b13b6f31 100755 --- a/ci/calamaresstyle +++ b/ci/calamaresstyle @@ -6,8 +6,6 @@ # You can pass in directory names, in which case the files # in that directory (NOT below it) are processed. # -set -e - AS=$( which astyle ) for _cf in clang-format-7 clang-format-8 clang-format70 clang-format80 @@ -22,6 +20,8 @@ test -n "$CF" || { echo "! No clang-format-7 found in PATH"; exit 1 ; } test -x "$AS" || { echo "! $AS is not executable."; exit 1 ; } test -x "$CF" || { echo "! $CF is not executable."; exit 1 ; } +set -e + any_dirs=no for d in "$@" do From ec605adf3f29d29bdea65b3842e44d3b581725ec Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 26 Nov 2019 22:37:25 +0100 Subject: [PATCH 2/3] [license] Tidy code - Move retranslation to a separate slot to allow it to be formatted nicely. - Use calculated m_allLicensesOptional in retranslation. - Untangle determining if all licenses are optional; std::none_of returns true on an empty list. --- src/modules/license/LicensePage.cpp | 51 +++++++++++++++-------------- src/modules/license/LicensePage.h | 5 ++- 2 files changed, 31 insertions(+), 25 deletions(-) diff --git a/src/modules/license/LicensePage.cpp b/src/modules/license/LicensePage.cpp index 2cc025f83..69dd0a9c6 100644 --- a/src/modules/license/LicensePage.cpp +++ b/src/modules/license/LicensePage.cpp @@ -126,7 +126,6 @@ LicensePage::LicensePage( QWidget* parent ) CALAMARES_RETRANSLATE( ui->acceptCheckBox->setText( tr( "I accept the terms and conditions above." ) ); ) } - void LicensePage::setEntries( const QList< LicenseEntry >& entriesList ) { @@ -134,27 +133,36 @@ LicensePage::setEntries( const QList< LicenseEntry >& entriesList ) m_entries.clear(); m_entries.reserve( entriesList.count() ); - const bool required - = std::any_of( entriesList.cbegin(), entriesList.cend(), []( const LicenseEntry& e ) { return e.m_required; } ); - if ( entriesList.isEmpty() ) - { - m_allLicensesOptional = true; - } - else - { - m_allLicensesOptional = !required; - } + auto isRequired = []( const LicenseEntry& e ) { return e.m_required; }; + m_allLicensesOptional = std::none_of( entriesList.cbegin(), entriesList.cend(), isRequired ); checkAcceptance( false ); - CALAMARES_RETRANSLATE( if ( required ) { + for ( const LicenseEntry& entry : entriesList ) + { + LicenseWidget* w = new LicenseWidget( entry ); + ui->licenseEntriesLayout->addWidget( w ); + m_entries.append( w ); + } + ui->licenseEntriesLayout->addStretch(); + + CALAMARES_RETRANSLATE_SLOT( &LicensePage::retranslate ) +} + +void +LicensePage::retranslate() +{ + if ( !m_allLicensesOptional ) + { ui->mainText->setText( tr( "

License Agreement

" "This setup procedure will install proprietary " "software that is subject to licensing terms." ) ); ui->additionalText->setText( tr( "Please review the End User License " "Agreements (EULAs) above.
" "If you do not agree with the terms, the setup procedure cannot continue." ) ); - } else { + } + else + { ui->mainText->setText( tr( "

License Agreement

" "This setup procedure can install proprietary " "software that is subject to licensing terms " @@ -164,18 +172,13 @@ LicensePage::setEntries( const QList< LicenseEntry >& entriesList ) "Agreements (EULAs) above.
" "If you do not agree with the terms, proprietary software will not " "be installed, and open source alternatives will be used instead." ) ); - } ui->retranslateUi( this ); - - for ( const auto& w - : m_entries ) w->retranslateUi(); ) - - for ( const LicenseEntry& entry : entriesList ) - { - LicenseWidget* w = new LicenseWidget( entry ); - ui->licenseEntriesLayout->addWidget( w ); - m_entries.append( w ); } - ui->licenseEntriesLayout->addStretch(); + ui->retranslateUi( this ); + + for ( const auto& w : m_entries ) + { + w->retranslateUi(); + } } diff --git a/src/modules/license/LicensePage.h b/src/modules/license/LicensePage.h index bd9543937..65f26b543 100644 --- a/src/modules/license/LicensePage.h +++ b/src/modules/license/LicensePage.h @@ -83,10 +83,13 @@ public slots: * - the user has ticked the "OK" box. * This function calls updateGlobalStorage() as needed, and updates * the appearance of the page as needed. @p checked indicates whether - * the checkbox has been ticked or not. + * the checkbox has been ticked or not. (e.g. when @p checked is true, + * you can continue regardless) */ void checkAcceptance( bool checked ); + void retranslate(); + signals: void nextStatusChanged( bool status ); From 2a45765b93e9c5aafa32dd59751c20452fce6aa7 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Wed, 27 Nov 2019 12:17:33 +0100 Subject: [PATCH 3/3] [license] Next depends not just on the checked box - Toggling the checkbox could disable the next button because only the checked-state was used, instead of the next-is-enabled-if-everything-is-optional member variable. FIXES #1271 --- src/modules/license/LicensePage.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/license/LicensePage.cpp b/src/modules/license/LicensePage.cpp index 69dd0a9c6..c5ba38133 100644 --- a/src/modules/license/LicensePage.cpp +++ b/src/modules/license/LicensePage.cpp @@ -211,5 +211,5 @@ LicensePage::checkAcceptance( bool checked ) { ui->acceptFrame->setStyleSheet( "#acceptFrame { padding: 3px }" ); } - emit nextStatusChanged( checked ); + emit nextStatusChanged( m_isNextEnabled ); }