[vlmc-devel] [PATCH 2/2] Implement SettingValue::Type::List

[yikei lu]

I don't aim to solve the mismatch problem of this.

    EffectsEngine::EffectList::const_iterator   ite = m_filters.end();
    while ( it != ite )
    {
        project.writeStartElement( "effect" );
        project.writeAttribute( "name",
(*it)->effectInstance()->effect()->name() );
...

Like this. As I mentioned before, I aimed to solve many QList used in
many classes. We can use attributes instead of creating child elements
in it though.

+            if ( val->get().canConvert<QString>() == false )
+                vlmcWarning() << "Can't serialize" << val->key();

If we can't convert values to QString, it will yield an error, which
is the same as the current version. We don't save types of values in
files. like
<setting key="vlmc/VLMCLang" value="default"/>
<setting key="vlmc/AutomaticBackupInterval" value="5"/>

2016-03-11 22:29 GMT+09:00 Hugo Beauzée-Luyssen <hugo at beauzee.fr>:
> On 03/11/2016 07:09 AM, Yikai Lu wrote:
>>
>> In order to remove save/load methods in many classes, we need to be able
>> to save QList.
>>
>> ---
>>   src/Settings/SettingValue.h |  1 +
>>   src/Settings/Settings.cpp   | 52
>> ++++++++++++++++++++++++++++++++++++++-------
>>   2 files changed, 45 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/Settings/SettingValue.h b/src/Settings/SettingValue.h
>> index ebc225e..cf9932f 100644
>> --- a/src/Settings/SettingValue.h
>> +++ b/src/Settings/SettingValue.h
>> @@ -46,6 +46,7 @@ class   SettingValue : public QObject
>>               Language,
>>               KeyboardShortcut,
>>               Path,
>> +            List,
>>               ByteArray, // For now this is only for private variables,
>> and is not expected to be used at any time
>>               //For effect engine settings:
>>               Color,
>> diff --git a/src/Settings/Settings.cpp b/src/Settings/Settings.cpp
>> index ab48a23..2b5dc26 100644
>> --- a/src/Settings/Settings.cpp
>> +++ b/src/Settings/Settings.cpp
>> @@ -77,10 +77,25 @@ Settings::save( QXmlStreamWriter& project )
>>           if ( ( val->flags() & SettingValue::Runtime ) != 0 )
>>               continue ;
>>           project.writeStartElement( "setting" );
>> -        project.writeAttribute( "key", val->key() );
>> -        if ( val->get().canConvert<QString>() == false )
>> -            vlmcWarning() << "Can't serialize" << val->key();
>> -        project.writeAttribute( "value", val->get().toString() );
>> +        if ( val->type() == SettingValue::List )
>> +        {
>> +            project.writeAttribute( "key", val->key() + "/List" );
>> +            for ( const QVariant& var: val->get().toList() ) {
>> +                project.writeStartElement( "setting" );
>
>
> This seems to imply that all lists will be stored in a <setting> tag, which
> feels wrong.
> The way we differentiate between project & global settings in the Setting
> class is quite wrong too though.
> I also don't really understand the "/List" suffix. Theoretically, you don't
> care about the type of the value you're saving, QVariant will handle it for
> you.
>
>
>> +                project.writeAttribute( "key", val->key() );
>> +                if ( var.canConvert<QString>() == false )
>> +                    vlmcWarning() << "Can't serialize" << val->key();
>> +                project.writeAttribute( "value", var.toString() );
>> +                project.writeEndElement();
>> +            }
>> +        }
>> +        else
>> +        {
>> +            project.writeAttribute( "key", val->key() );
>> +            if ( val->get().canConvert<QString>() == false )
>> +                vlmcWarning() << "Can't serialize" << val->key();
>> +            project.writeAttribute( "value", val->get().toString() );
>> +        }
>>           project.writeEndElement();
>>       }
>>       project.writeEndElement();
>> @@ -100,15 +115,36 @@ Settings::load( const QDomDocument& document )
>>       while ( s.isNull() == false )
>>       {
>>           QString     key = s.attribute( "key" );
>> -        QString     value = s.attribute( "value" );
>> +        QString     value;
>>
>>           if ( key.isEmpty() == true )
>>               vlmcWarning() << "Invalid setting node.";
>>           else
>>           {
>> -            vlmcDebug() << "Loading" << key << "=>" << value;
>> -            if ( setValue( key, value ) == false )
>> -                vlmcWarning() << "Loaded invalid project setting:" <<
>> key;
>> +            if ( key.endsWith("/List") )
>> +            {
>> +                key.chop(5);
>> +                s = s.firstChildElement( "setting" );
>> +                QList<QVariant> list;
>> +                while ( s.isNull() == false )
>> +                {
>> +                    if ( key.isEmpty() == true )
>> +                        vlmcWarning() << "Invalid setting node.";
>> +                    value = s.attribute( "value" );
>> +                    vlmcDebug() << "Loading" << key << "=>" << value;
>> +                    list << QVariant::fromValue( value );
>> +                    s = s.nextSiblingElement();
>> +                }
>> +                if ( setValue( key, list ) == false )
>> +                    vlmcWarning() << "Loaded invalid project setting:" <<
>> key;
>> +            }
>> +            else
>> +            {
>> +                value = s.attribute( "value" );
>> +                vlmcDebug() << "Loading" << key << "=>" << value;
>> +                if ( setValue( key, value ) == false )
>> +                    vlmcWarning() << "Loaded invalid project setting:" <<
>> key;
>> +            }
>>           }
>>           s = s.nextSiblingElement();
>>       }
>>
>
> While we could indeed handle a List type, the question would be "a list of
> what". So far, all SettingValue types are "plain" type, whereas QList is a
> template type. We could always handle a QList of QVariant, but this sounds
> not as straight forward as using an integer, a QString, ...
> It would also be close to impossible to expose in the GUI, though we could
> keep this type as a private setting.
>
> Anyway, it sounds to me like you're trying to solve the mismatch between
> <project><settings> settings list </settings></project> for a project file,
> and the <vlmc><settings> settings list </settings></vlmc> we talked about on
> IRC. That isn't the right way to do it as far as I'm concerned.
>
>
> Regards,
> _______________________________________________
> Vlmc-devel mailing list
> Vlmc-devel at videolan.org
> https://mailman.videolan.org/listinfo/vlmc-devel