Skip to content

Add INIReader Sections and Keys methods #186

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

Merged
merged 13 commits into from
Mar 1, 2025
Merged

Conversation

Ishan09811
Copy link
Contributor

This PR adds a SetValue function to the INIReader class, allowing users to modify INI values.

Copy link
Owner

@benhoyt benhoyt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per comments below, I won't be adding SetValue, but I'm open to adding Sections and Keys -- thanks.

Could you also please add an example/test of each of these methods in https://github.com/benhoyt/inih/blob/master/examples/INIReaderExample.cpp (and update the cpptest.txt output so CI passes). Something like:

Sections and keys:
- protocol: version
- user: active, email, name, pi, trillion

@@ -109,6 +109,32 @@ bool INIReader::GetBoolean(const string& section, const string& name, bool defau
return default_value;
}

void INIReader::SetValue(const std::string& section, const std::string& name, const std::string& value) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi there, thanks for the contribution! I'm not going to add setting/writing functionality to inih or INIReader, so I'm afraid I'm not going to be adding this method. However, I'm willing to consider Sections and Keys -- I'll discuss those below.

@benhoyt
Copy link
Owner

benhoyt commented Mar 1, 2025

@Ishan09811
Copy link
Contributor Author

i actually needed the SetValue function and will be very useful for other users also so I will still request you to keep SetValue function but if you don't then no worry i will make a subclass for my usecase and I am happy to contribute

@benhoyt
Copy link
Owner

benhoyt commented Mar 1, 2025

i actually needed the SetValue function and will be very useful for other users also so I will still request you to keep SetValue function but if you don't then no worry i will make a subclass for my usecase and I am happy to contribute

Yep, you'll need to subclass for that, sorry.

@Ishan09811
Copy link
Contributor Author

Ishan09811 commented Mar 1, 2025

Ok thanks for your quick review and now the test hopefully will work fine and I think you can remove the SetValue function as you said you will not keep it so i just want to inform that editing by maintainers is enabled for this pr so you can remove the SetValue function easily.

@Ishan09811
Copy link
Contributor Author

do you know why is it failing because it is not outputing any error message so I can't find what actually causing the error

@Ishan09811
Copy link
Contributor Author

finally fixed the issue actually before I thought that it is failed when compiling but when actually I saw it carefully then i understand that it is trying find any difference between the printed message and cpptest.txt so now i fixed the issue lol

@Ishan09811 Ishan09811 requested a review from benhoyt March 1, 2025 17:50
@benhoyt benhoyt changed the title Add SetValue function to INIReader for modifying INI values Add INIReader Sections and Keys methods Mar 1, 2025
@benhoyt benhoyt merged commit 93f392b into benhoyt:master Mar 1, 2025
2 checks passed
@benhoyt
Copy link
Owner

benhoyt commented Mar 1, 2025

Thanks for the contribution!

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