Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove quotes from parsed values #143

Closed
wants to merge 1 commit into from
Closed

Conversation

angeloc
Copy link

@angeloc angeloc commented Aug 8, 2022

Removing quotes while parsin if the INI_NO_STRING_QUOTE is enabled.

Signed-off-by: Angelo Compagnucci [email protected]

Removing quotes while parsin if the INI_NO_STRING_QUOTE is enabled.

Signed-off-by: Angelo Compagnucci <[email protected]>
@benhoyt
Copy link
Owner

benhoyt commented Aug 8, 2022

Hi @angeloc, thanks for the submission. However, I'd rather not include this change. I want to keep inih fairly minimalist, and it's easy to do this quote removal in the handler if needed.

Also, in my opinion this feature isn't particularly useful without unescaping, for example sentence = "The \"quick\" brown fox.". But once again, it would be easy to do with quote removal and unescaping with an unescape() function called on the value in the handler.

See a similar feature proposed (INI_REMOVE_LEADING_QUOTE) in #106.

@benhoyt benhoyt closed this Aug 8, 2022
@angeloc
Copy link
Author

angeloc commented Aug 9, 2022

@benhoyt Not much happy with the decision, honestly, but it's your code after all!
Anyway, just for the sake of discussion, I don't think this proposal has anything to do with escaping.
My problem is that I have a QT software which produces quoted values in INI file and I don't want quotes in the C software consuming it. Quoted values are permitted from the standard: I can understand python confgparser doesn't implement it, but removing quotes from a string in python is ridiculously easy.
Removing quotes is easy in the library and incredibly useful for the user of the library. Implementing another external function only to have quotes removed is overkill and not that easy. You probably end up needing to import another string c library to do some serious (and secure) manipulation.
Anyway, I'll stick to my fork for my project.
Thanks anyway!

@benhoyt
Copy link
Owner

benhoyt commented Aug 9, 2022

I don't really understand why this needs to be in inih itself, though. To reproduce the functionality in your PR, it seems to me just as easy (and a bit more explicit) to add a call to a simple strip_quotes() function, something like so (taken from the example on in the inih README). This is untested, but should be similar to what you did in the PR:

static int handler(void* user, const char* section, const char* name,
                   const char* value)
{
    char* stripped_value = strip_quotes((char*)value); // add this line to your handler function
    // handle section, name, stripped_value
    return 1;
}

// new function: strip quotes from left and right
char* strip_quotes(char* s) {
    char* p = s + strlen(s);
    while (p > s && unsigned char)(*p) == '"') // trim right
        *p = '\0';
    while (*s && (unsigned char)(*s) == '"') // trim left
        s++;
    return s
}

int main(int argc, char* argv[])
{
    configuration config;

    if (ini_parse("test.ini", handler, &config) < 0) {
        printf("Can't load 'test.ini'\n");
        return 1;
    }
    printf("Config loaded from 'test.ini': version=%d, name=%s, email=%s\n",
        config.version, config.name, config.email);
    return 0;
}

Using a fork is fine though if you prefer!

@angeloc
Copy link
Author

angeloc commented Aug 9, 2022

Indeed. This implies rescanning all the string not even one but two times, when the library has already done the very same thing ...
The goal of the library should be obtaining the values from the INI, thus removing the quotes is a tiny addiction toward the library easy friendliness imho.

Anyway, after your example, I'm more convinced of my solution ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants