[vlc-devel] Essai envoi

[Alexandre Janniaux]

Hi,

This looks correct and a good idea, but you will need to
send clean patches.

> I am new to using mailing lists, so I hope I replied correctly and did not mess with the --in-reply-to option.
> Also, I am sending a single "code review" commit, is that all right for you? Or do you rather prefer one commit that squashes this correction with the previous proposed patch?

We usually prefer proper commit and use git range-diff with
the previous version to compare, so the patch can be merged
without resend.

Also, you have to put the GPLv2 license for Qt files, as
the module is itself in GPLv2. You can grab it from any
file from the Qt module.

Regards,
--
Alexandre Janniaux
Videolabs

On Mon, Oct 14, 2019 at 10:59:06AM +0200, Jérôme Froissart wrote:
> ---
> You are right, I forgot to call config_StringEscape.
> Here is a correction patch. Note that it escapes quotes and slashes, it does not percent-encode special chars. I guess it is the correct behaviour (since it was used in the implementation I removed), but please tell me if some percent-escaping should take place as well.
>
> I am new to using mailing lists, so I hope I replied correctly and did not mess with the --in-reply-to option.
> Also, I am sending a single "code review" commit, is that all right for you? Or do you rather prefer one commit that squashes this correction with the previous proposed patch?
>
> Thanks
>
>
>  modules/gui/qt/util/soutmrl.cpp | 7 ++++++-
>  modules/gui/qt/util/soutmrl.hpp | 2 +-
>  2 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/modules/gui/qt/util/soutmrl.cpp b/modules/gui/qt/util/soutmrl.cpp
> index 90615c6c86..6dc316ec42 100644
> --- a/modules/gui/qt/util/soutmrl.cpp
> +++ b/modules/gui/qt/util/soutmrl.cpp
> @@ -14,7 +14,12 @@ QString MrlModule::to_string() const
>          s += it->first;
>          if( it->second.to_string().compare("") != 0 )
>          {
> -            s += "=" + it->second.to_string();
> +            char *psz = config_StringEscape( qtu(it->second.to_string()) );
> +            if( psz )
> +            {
> +                s += "=" + qfu( psz );
> +                free( psz );
> +            }
>          }
>          ++it;
>          if( it != options.end() )
> diff --git a/modules/gui/qt/util/soutmrl.hpp b/modules/gui/qt/util/soutmrl.hpp
> index 2e92e9064e..b18016fef3 100644
> --- a/modules/gui/qt/util/soutmrl.hpp
> +++ b/modules/gui/qt/util/soutmrl.hpp
> @@ -90,7 +90,7 @@ private:
>  ///     - any number of key(=value) pairs
>  ///       values can be nested modules
>  ///
> -/// Example of MRL: HEADERmodule1{a,b=val}:module2:module3{opt,arg=1,stuff=nestedModule{subkey=subvalue}}
> +/// Example of MRL: HEADERmodule1{a,b=val}:module2:module3{opt,arg=\"value with automatically escaped quotes\",stuff=nestedModule{subkey=subvalue}}
>  class SoutMrl
>  {
>  public:
> --
> 2.20.1
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel