On Tue, Apr 03, 2018 at 06:28:42PM +0200, Johannes Schindelin wrote: > While a neat theoretical construct, state machines are hard to read. In > this instance, it does not even make a whole lot of sense because we are > more interested in flags, anyway: has the section been seen? Has the key > been seen? Does the current section match the key we are looking for? > > Besides, the state `SECTION_SEEN` was named in a misleading way: it did > not indicate that we saw the section matching the key we are looking > for, but it instead indicated that we are *currently* in that section. > > Let's just replace the state machine logic by clear and obvious flags. > > This will also make it easier to review the upcoming patches to use the > newly-introduced `event_fn` callback of the config parser.
I think this is probably a good direction. But one thing state machines can help with is keeping the state to a manageable size. With 3 bits of flags, we now have 8 possible states, up from the previous 4. Clearly some of those are nonsensical (can you be in key_seen without section_seen? I'd think not), but it's up to the code to interpret and reset those manually. I'll defer to your judgement, though, on this making things for the future patches more readable. You spend a lot more time poking at it than I have. -Peff
