napcok/calamares
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[libcalamaresui] Fix slideshowAPI loading

Browse Source 
In 022045ae05 a regression was introduced: if no *slideshowAPI*
is specified in the branding file, Calamares refuses to start, with
a YAML failure.

Before the refactoring, we had `YAML::Node doc` and looked up
the *slideshowAPI* in it with `doc["slideshowAPI"]`. After the
refactoring, we had `const YAML::Node& doc`. The `const` makes
all the difference:
 - subscripting a non-existent key in a mutable Node silently
   returns a Null node (and possibly inserts the key);
 - subscripting a non-existent key in a const Node returns an
   invalid or undefined node.

Calling IsNull() or IsScalar() on a Null node works: the functions
return a bool. Calling them on an invalid node throws an exception.

So in the **const** case, this code can throws an exception that it
doesn't in the non-const case:
    `doc[ "slideshowAPI" ].IsScalar()`

- Massage the code to check for validity before checking for scalar
- Add a `get()` that produces more useful exception types when
  looking up an invalid key
- Use `get()` to lookup the slideshow node just once.
This commit is contained in:
Adriaan de Groot 2020-07-09 11:28:09 +02:00
parent a58d59d86c
commit e1f4224bed
1 changed files with 22 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
src/libcalamaresui/Branding.cpp
View File

@ -367,7 +367,7 @@ Branding::WindowDimension::isValid() const
} }
/// @brief Guard against cases where the @p key doesn't exist in @p doc /// @brief Get a string (empty is @p key doesn't exist) from @p key in @p doc
static inline QString static inline QString
getString( const YAML::Node& doc, const char* key ) getString( const YAML::Node& doc, const char* key )
{ {
@ -378,6 +378,18 @@ getString( const YAML::Node& doc, const char* key )
return QString(); return QString();
} }
/// @brief Get a node (throws if @p key doesn't exist) from @p key in @p doc
static inline YAML::Node
get( const YAML::Node& doc, const char* key )
{
auto r = doc[ key ];
if ( !r.IsDefined() )
{
throw YAML::KeyNotFound( YAML::Mark::null_mark(), std::string( key ) );
}
return r;
}
static inline void static inline void
flavorAndSide( const YAML::Node& doc, const char* key, Branding::PanelFlavor& flavor, Branding::PanelSide& side ) flavorAndSide( const YAML::Node& doc, const char* key, Branding::PanelFlavor& flavor, Branding::PanelSide& side )
{ {
@ -514,10 +526,11 @@ Branding::initSlideshowSettings( const YAML::Node& doc )
{ {
QDir componentDir( componentDirectory() ); QDir componentDir( componentDirectory() );
if ( doc[ "slideshow" ].IsSequence() ) auto slideshow = get( doc, "slideshow" );
if ( slideshow.IsSequence() )
{ {
QStringList slideShowPictures; QStringList slideShowPictures;
doc[ "slideshow" ] >> slideShowPictures; slideshow >> slideShowPictures;
for ( int i = 0; i < slideShowPictures.count(); ++i ) for ( int i = 0; i < slideShowPictures.count(); ++i )
{ {
QString pathString = slideShowPictures[ i ]; QString pathString = slideShowPictures[ i ];
@ -535,9 +548,9 @@ Branding::initSlideshowSettings( const YAML::Node& doc )
m_slideshowAPI = -1; m_slideshowAPI = -1;
} }
#ifdef WITH_QML #ifdef WITH_QML
else if ( doc[ "slideshow" ].IsScalar() ) else if ( slideshow.IsScalar() )
{ {
QString slideshowPath = QString::fromStdString( doc[ "slideshow" ].as< std::string >() ); QString slideshowPath = QString::fromStdString( slideshow.as< std::string >() );
QFileInfo slideshowFi( componentDir.absoluteFilePath( slideshowPath ) ); QFileInfo slideshowFi( componentDir.absoluteFilePath( slideshowPath ) );
if ( !slideshowFi.exists() || !slideshowFi.fileName().toLower().endsWith( ".qml" ) ) if ( !slideshowFi.exists() || !slideshowFi.fileName().toLower().endsWith( ".qml" ) )
bail( m_descriptorPath, bail( m_descriptorPath,
@ -546,7 +559,9 @@ Branding::initSlideshowSettings( const YAML::Node& doc )
m_slideshowPath = slideshowFi.absoluteFilePath(); m_slideshowPath = slideshowFi.absoluteFilePath();
// API choice is relevant for QML slideshow // API choice is relevant for QML slideshow
int api = doc[ "slideshowAPI" ].IsScalar() ? doc[ "slideshowAPI" ].as< int >() : -1; // TODO:3.3: use get(), make slideshowAPI required
int api
= ( doc[ "slideshowAPI" ] && doc[ "slideshowAPI" ].IsScalar() ) ? doc[ "slideshowAPI" ].as< int >() : -1;
if ( ( api < 1 ) || ( api > 2 ) ) if ( ( api < 1 ) || ( api > 2 ) )
{ {
cWarning() << "Invalid or missing *slideshowAPI* in branding file."; cWarning() << "Invalid or missing *slideshowAPI* in branding file.";
@ -555,7 +570,7 @@ Branding::initSlideshowSettings( const YAML::Node& doc )
m_slideshowAPI = api; m_slideshowAPI = api;
} }
#else #else
else if ( doc[ "slideshow" ].IsScalar() ) else if ( slideshow.IsScalar() )
{ {
cWarning() << "Invalid *slideshow* setting, must be list of images."; cWarning() << "Invalid *slideshow* setting, must be list of images.";
} }