Add colon_delimiter option and change default behavior

[ShuiRuTian]

133

Although we did add colon(':') as key value pair separator: https://github.com/zonyitoo/rust-ini/pull/29

It's more common for user to only treat '=' as the only separator.

And add colon make some edge condition broken, .e.g, some config in .npmrc(it's ini in fact), which has ":" as part of the key.

I add an option in this PR, and change the default behavior to not treat ':' as separator.

I agree it's arguable to change default behavior, what's your opinion?

[zonyitoo]

Why not just add an option called key_value_delimiter to let users to specify which delimiter they want to use.

[ShuiRuTian]

Good suggestion, but it seems not that easy. We might need to talk about some design.

1. What should be the type of key_value_delimiter? For user, it's more reasonable to be Vec<char>, but internally, it's used as &[Option<char>]. Maybe we need to create a new type like ParseOptionInner, when creating parser, we convert the user interface ParseOption to inner data strucutre ParseOptionInner

2. What should we do if user pass an empty vector? It seems to be an UB.

3. An error is thrown, what should we do? Feels we need further refactor. Otherwise we need clone, and need to collect memory each time, which is not very good.

error[E0502]: cannot borrow *self as mutable because it is also borrowed as immutable --> src\lib.rs:1537:9	1537	self.parse_str_until(&self.opt.key_value_delimiter, false)	^^^^^---------------^-----------------------------^^^^^^^^
		immutable borrow occurs here
	immutable borrow later used by call
mutable borrow occurs here

4. To resolve 3, const generic is a resolution, but it causes most of public signature changed. Is this acceptable?

[ShuiRuTian]

Just pushed the changes which uses generic const, what's your thoughts?

Generic const makes default behavior more subtle...

[zonyitoo]

That would become a breaking change, because ParseOption is now has a generic type parameter.

Why not we just keep key_value_delimiter to has type Vec<Option<char>> and then add a member function fn set_key_value_delimiter(list: &[char]), that would make everything much easier.

key_value_delimiter must not be an empty list, and it should't has None inside of it. It could be checked in Ini::load_from_*_opt functions.

[zonyitoo]

On the other hand, if delimiter is now configurable, whether there are any usecases that wants to have a delimiter with multiple chars, for example, "==". That would require lots of refactors, I would prefer not to do this right now.

[ShuiRuTian]

Why not we just keep key_value_delimiter to has type Vec<Option> and then add a member function fn set_key_value_delimiter(list: &[char]), that would make everything much easier.

The main concern from my side was "self.parse_str_until(&self.opt.key_value_delimiter.clone(), false)", about the perf of clone.

As we could see, it needs clone to satisify the rust borrow constraint, for Vec, clone needs heap allocation. Although I never profile such stuff, but heap allocation is usually concerned kind of slow. And it's why I used const generic, it allows us to use stack rather than heap.

Have no idea, maybe it's fine, anyway, how slow it could be?