On 22.07.2018 17:48, Tanu Kaskinen wrote:
> On Sun, 2018-07-22 at 16:02 +0200, Georg Chini wrote:
>> On 21.07.2018 20:17, Tanu Kaskinen wrote:
>>> On Mon, 2018-04-09 at 19:35 +0200, Georg Chini wrote:
>>>> The patch adds the possibility to escape curly braces within parameter strings
>>>> and introduces several new functions that can be used for writing parameters.
>>>>
>>>> For writing, the structure pa_message_param, which is a wrapper for pa_strbuf
>>>> has been created. Following new write functions are available:
>>>>
>>>> pa_message_param_new() - creates a new pa_message_param structure
>>>> pa_message_param_to_string() - converts a pa_message_param to string and frees
>>>> the structure
>>>
>>>> The function pa_message_param_write_string()
>>>> has a parameter do_escape.
>>> Why not do escaping always?
>> Because what you are writing as a string may be a list that you have
>> prepared
>> previously. Then you will not want escaping. You may for example create
>> a list
>> from an array and then insert this list as one string into the final
>> parameter list
>> or have a function that converts a certain structure to a parameter
>> string and
>> then write the result of this function as one element of the final list.
> My mental model is that parameters have types, list type is different
> than string type, and write_string() is only meant for writing values
> of the string type.
>
> Can you add a write_raw() function?
Yes, this is done in patch 7. But the raw write function differs from what
write_string() is doing. write_string() writes one element of a list,
that is
it encloses the string in braces. The raw write function is intended for
situations where you can't write a complete element with one write, so
it does not add any braces. I am still of the opinion, that a structure
or array converted to a parameter string is a string, so writing something
like this should be done with write_string().
Also writing unescaped strings in situations where escaping is not necessary
saves the overhead of looping over all the characters.
>>> core-util already contains pa_unescape() that does the same thing more
>>> efficiently (if you drop the single quote thing).
>> pa_unescape() currently does not do the same thing. It removes all
>> escape characters, while I only want to remove the characters
>> I actually introduced (those before { or }).
>> I can however modify pa_unescape() to take the same arguments
>> as pa_escape().
> I don't see the need for being selective when unescaping. Nothing
> breaks if all (except escaped) backslashes are stripped.
You are right, if previously all backslashes in the original string
have been escaped, nothing will break. I was still thinking of the
old solution where I did not escape backslashes.
>
>>>> +
>>>> +/* Read functions */
>>>> +
>>>> /* Split the specified string into elements. An element is defined as
>>>> * a sub-string between curly braces. The function is needed to parse
>>>> * the parameters of messages. Each time it is called returns the position
>>>> * of the current element in result and the state pointer is advanced to
>>>> - * the next list element.
>>>> + * the next list element. On return, the parameter *is_unpacked indicates
>>>> + * if the string is plain text or contains a sub-list. is_unpacked may
>>>> + * be NULL.
>>> is_unpacked looks like unnecessary complexity.
>>> pa_message_params_read_string() should always unescape the value.
>> It may be possible, that the string you read is a list. Consider the
>> following
>> parameter list: {string1}{some nested structure}{string2}. You can now
>> read this list as three strings and then continue to read the elements of
>> the nested structure from the second string. You might even create a
>> function
>> that takes a string and outputs a structure. So you are not forced to go
>> to the full depth of nesting on the first pass. This makes it much easier
>> to handle deeply nested parameter lists. For me this behavior is an
>> important
>> feature and I do not want to drop it. See also my comment on why I do
>> not always want escaping.
> Doesn't split_list() already allow this, why do you want to use
> read_string() to do the same thing as split_list()?
read_string() and split_list() are very similar and we could live
without read_string(). It is provided as a counterpart to write_string()
and for convenience additionally does the unescaping if necessary
like write_string does the escaping.
I don't see why this is a problem. It depends on the context which
is the better function to use.
