<div dir="auto"><div dir="auto">Thanks for your reply. <br></div><div dir="auto">Just to be sure that I will not jeopardize (again) the mailing list, could you confirm I understood correctly? I should now :</div><div dir="auto">- squash the main commit and the change I have made after you reviewed it</div><div dir="auto">- send a fresh (that does not reply to anything) patchset to the mailing list</div><div dir="auto">- append a "(v2)" to the title of the cover letter of this fresh patchset</div><div dir="auto"><br></div><div dir="auto">Is that correct?</div><div dir="auto">Thanks
 for the help (maybe this workflow could be described on your wiki, for 
people like me who are contributing for the first time via a mailing 
list? Or maybe it is written already, but I failed to find it?)</div><div dir="auto"><br></div><div dir="auto">Jérôme</div><div dir="auto"><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, 15 Oct 2019, 14:15 Alexandre Janniaux, <<a href="mailto:ajanni@videolabs.io">ajanni@videolabs.io</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi,<br>
<br>
This looks correct and a good idea, but you will need to<br>
send clean patches.<br>
<br>
> I am new to using mailing lists, so I hope I replied correctly and did not mess with the --in-reply-to option.<br>
> 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?<br>
<br>
We usually prefer proper commit and use git range-diff with<br>
the previous version to compare, so the patch can be merged<br>
without resend.<br>
<br>
Also, you have to put the GPLv2 license for Qt files, as<br>
the module is itself in GPLv2. You can grab it from any<br>
file from the Qt module.<br>
<br>
Regards,<br>
--<br>
Alexandre Janniaux<br>
Videolabs<br>
<br>
On Mon, Oct 14, 2019 at 10:59:06AM +0200, Jérôme Froissart wrote:<br>
> ---<br>
> You are right, I forgot to call config_StringEscape.<br>
> 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.<br>
><br>
> I am new to using mailing lists, so I hope I replied correctly and did not mess with the --in-reply-to option.<br>
> 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?<br>
><br>
> Thanks<br>
><br>
><br>
>  modules/gui/qt/util/soutmrl.cpp | 7 ++++++-<br>
>  modules/gui/qt/util/soutmrl.hpp | 2 +-<br>
>  2 files changed, 7 insertions(+), 2 deletions(-)<br>
><br>
> diff --git a/modules/gui/qt/util/soutmrl.cpp b/modules/gui/qt/util/soutmrl.cpp<br>
> index 90615c6c86..6dc316ec42 100644<br>
> --- a/modules/gui/qt/util/soutmrl.cpp<br>
> +++ b/modules/gui/qt/util/soutmrl.cpp<br>
> @@ -14,7 +14,12 @@ QString MrlModule::to_string() const<br>
>          s += it->first;<br>
>          if( it->second.to_string().compare("") != 0 )<br>
>          {<br>
> -            s += "=" + it->second.to_string();<br>
> +            char *psz = config_StringEscape( qtu(it->second.to_string()) );<br>
> +            if( psz )<br>
> +            {<br>
> +                s += "=" + qfu( psz );<br>
> +                free( psz );<br>
> +            }<br>
>          }<br>
>          ++it;<br>
>          if( it != options.end() )<br>
> diff --git a/modules/gui/qt/util/soutmrl.hpp b/modules/gui/qt/util/soutmrl.hpp<br>
> index 2e92e9064e..b18016fef3 100644<br>
> --- a/modules/gui/qt/util/soutmrl.hpp<br>
> +++ b/modules/gui/qt/util/soutmrl.hpp<br>
> @@ -90,7 +90,7 @@ private:<br>
>  ///     - any number of key(=value) pairs<br>
>  ///       values can be nested modules<br>
>  ///<br>
> -/// Example of MRL: HEADERmodule1{a,b=val}:module2:module3{opt,arg=1,stuff=nestedModule{subkey=subvalue}}<br>
> +/// Example of MRL: HEADERmodule1{a,b=val}:module2:module3{opt,arg=\"value with automatically escaped quotes\",stuff=nestedModule{subkey=subvalue}}<br>
>  class SoutMrl<br>
>  {<br>
>  public:<br>
> --<br>
> 2.20.1<br>
><br>
> _______________________________________________<br>
> vlc-devel mailing list<br>
> To unsubscribe or modify your subscription options:<br>
> <a href="https://mailman.videolan.org/listinfo/vlc-devel" rel="noreferrer noreferrer" target="_blank">https://mailman.videolan.org/listinfo/vlc-devel</a><br>
_______________________________________________<br>
vlc-devel mailing list<br>
To unsubscribe or modify your subscription options:<br>
<a href="https://mailman.videolan.org/listinfo/vlc-devel" rel="noreferrer noreferrer" target="_blank">https://mailman.videolan.org/listinfo/vlc-devel</a></blockquote></div>