update stumpless_param_to_string format

[goatshriek]

The stumpless_param_to_string function was originally implemented with a relatively arbitrary string format of <name>:<value>. However, as the CLI logger is developed, and the stumpless_new_param_from_string function is implemented in #360, there is a strong case for this format to instead be name="value". This change adjusts the function, it's documentation, and tests to this new format.

General Approach

There are a few details left out of the following approach, for you to fill in as you encounter them. If you find you need help, please ask here or on the project gitter and someone can help you get past the stumbling block.

There are three main things that need to change for this adjustment: the implementation itself, the documentation, and the test suite. The development documentation has everything that you need to get started and more. This change is relatively simple, so you won't need to do much more than build the library and run the test suite.

Starting with the implementation will be most straightforward approach. The function is implemented in src/param.c. The adjustments to match the new format should be relatively minor.

After changing the implementation, the tests will be failing. You can find them in test/function/param.cpp. Run the test suite (using the check target is the easiest way to do this). Fix anything that has broken after your update. You will likely also need to adjust some of the tests for stumpless_element_to_string as well in test/function/element.cpp - simply update the tests to match the new format.

Finally, update the documentation in both include/stumpless/param.h for stumpless_param_to_string and include/stumpless/element.h for stumpless_element_to_string.

[rmknan]

I would like to help with this!!! Can you give some more details to help me get started?

[goatshriek]

Hello @rmknan, please feel free to work on it. What details do you need beyond the description above?

[rmknan]

I think it's pretty clear how to start and how to go about it. I will start and ask further if any issues come up. Can you assign this issue to me?

[rmknan]

Based on my understanding to change the current format to the specified one, I need to make changes to the code below.

To change it to desired format of name="value". Would it be

/* name="value"  */
    format = alloc_mem( value_len + name_len + 5 );
    if( !format ) {
      goto fail;
    }

    // memcpy(format + 1, name, name_len);
    // memcpy(format + name_len + 4, value, value_len);

    memcpy(format + 1, name, name_len);
    memcpy(format + name_len + 3, value, value_len);

    unlock_param( param );

    // format[0] = '<';
    // format[name_len + 1] = '>';
    // format[name_len + 2] = ':';
    // format[name_len + 3] = '<';
    // format[name_len + value_len + 4] = '>';
    // format[name_len + value_len + 5] = '\0';
    format[name_len + 1] = '=';
    format[name_len + 2] = '\"';
    format[name_len + value_len + 3] = '"';
    format[name_len + value_len + 4] = '\0';

Please let me know if this is right? Thank you.

[goatshriek]

This looks close, but may have some small problems. A good next step would be to make this change yourself, and see what the result of the tests is afterwards.

[rmknan]

I go to the Latest test log to check the errors. It lists out errors in param.cpp and element.cpp. Do I change the code to whatever the log says?

format
    Which is: "\xDB" "basic-name=\"basic-value\""
  "basic-name=\"basic-value\""

format
    Which is: "<element-with-params>:[Nparam-1=\"value-1\",\xFEparam-2=\"value-2\"]"
  "element-with-params=[param-1=\"value-1\",param-2=\"value-2\"]"

Edit : I tried changing it to whatever it said before but now it suggest something else

This looks close, but may have some small problems

How do identify those problems? Thank you.

[goatshriek]

It isn't clear to me exactly what the larger context of those snippets is, but I believe they are from the GetElementToStringWithParams test. You appear to have modified the expected string to be correct.

Note that the first character of the params is invalid (N and \xFE). This is a symptom of an off-by-one error in your param to string logic where you are starting to write the param name at offset 1 where it was before the < character was removed, instead of 0. Double check the position of the name, value, and quote/equal characters in your implementation to hopefully resolve this.

[rmknan]

Could you elaborate further,

format
    Which is: "\xDB" "basic-name=\"basic-value\""
  "basic-name=\"basic-value\""

This one is from param.cpp (GetParamToString). Since there is no < character in the new format, "basic-name=\"basic-value\"" this should hold true.

As for the other snippet, its from GetElementToStringWithParams. Could you tell me what does this line mean? and what its doing? mainly the <param-1>:<value-1>

EXPECT_STREQ( format, "<element-with-params>:[<param-1>:<value-1>,<param-2>:<value-2>]" );

[goatshriek]

In your new implementation, the line memcpy(format + 1, name, name_len); is incorrect - remove the +1. Because you aren't writing the first character you're seeing uninitialized characters as the first character in the string of the param name. You'll also need to adjust other offsets and sizes as a trickle down of this change.

Tests that use the <>:<> format will need to be updated, as the STREQ line you included above. But first, concentrate on removing the incorrect characters from the tests you first saw as failing.

[rmknan]

So my param.cpp test passes but element.cpp test fails. The log file shows

format
    Which is: "<element-with-params>:[param-1=\"value-1\",param-2=\"value-2\"]"
  "element-with-params=[param-1=\"value-1\",param-2=\"value-2\"]"

The [param-1=\"value-1\",param-2=\"value-2\"]" comes out correct but the former part is not.

I tried changing things on the element.cpp file but have not found anything to solve this yet. I tried checking the src/element.c and there it specifies the format for this, So would i have to change it there for it effect it here?

[rmknan]

I tried changing it just to to test it out, comes close to matching with just the hT.

format
Which is: "element-with-params=[hTparam-1=\"value-1\",param-2=\"value-2\"]"
  "element-with-params=[param-1=\"value-1\",param-2=\"value-2\"]"

[goatshriek]

I believe <element-with-params>:[param-1=\"value-1\",param-2=\"value-2\"] is the expected output for the element tests after this change.

I'm not sure what you changed to achieve the above output of element-with-params=[hTparam-1=\"value-1\",param-2=\"value-2\"]? Do not make any changes to the element to string routine as part of this change, that will be a separate change.