-
Notifications
You must be signed in to change notification settings - Fork 12
Rewrite parser more readable, little faster #8
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
base: master
Are you sure you want to change the base?
Conversation
@benoitc thumbs up/down - making things better, or uninteresting? Please let me know when you get a chance. |
@unix1 sorry I missed this change.... will have a look later in the day. |
@benoitc, any luck? Feedback? I'm all ears. |
@benoitc ping? |
I completely forgot that change... Sorry for that ... I will look at it now. |
more than one year later... Sorry for that... Just if you remember, can you tell me what you meant by |
@unix1 i guess i see the point. I tested your implementation and it mostly worked. See comments. I know it's a little late to ask, but in case of what was your idea to add support for multilines values? |
@@ -535,65 +534,36 @@ parse_ini_file(ConfName, IniFile) -> | |||
Msg = list_to_binary(io_lib:format(Fmt, [IniFilename])), | |||
throw({startup_error, Msg}) | |||
end, | |||
{match, Sections} = parse_ini_bin_to_sections(IniBin), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the case where there are no sections in the file should be handled here. It's useful when you need to pass a local file that will contains the updates.
@benoitc thanks for looking and providing feedback. I'll probably have time to look at this this weekend. As for multiline values, I remember I couldn't find a standard way of doing it (people do it their own ways when they think they need it, and current master didn't support it) - that's why I didn't put it in. I don't have a use case; unless you have one, I don't think it should be there. In the hypothetical future where it's needed, we could say: any config value that ends with a backslash continues on next line: [letter]
message = Hello,\
This is a message\
Bye
from = Foo or any non-config non-section non-comment non-empty line: [letter]
message = Hello,
This is a message
Bye
from = Foo But it gets ugly fast; and again, unless you have a use case, I'd keep it out for now. |
@benoitc let me know if you are open to this kind of parser rewrite. Some notes:
Please let me know your thoughts.