Ah, thanks for spotting this bug! It is a minor one, but this now makes me think that we should definitely do this refactoring of a struct containing all the relevant config state and passing it to functions as much as possible (as opposed to merely leaning towards the idea). "Glen Choo via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes: > Fix this by refactoring the current_config_* functions into variants > that don't BUG() when we aren't reading config, and using the resulting > functions in die_bad_number(). Refactoring is needed because "git config > --get[-regexp] --type=int" parses the int value _after_ parsing the > config file, which will run into the BUG(). You say "fix this", but are there actually 2 bugs (so, "fix these")? Firstly, that BUG() is run into when invoking "git config" the way you describe, and secondly, die_bad_number() only reading cf and not checking kvi to see if anything's there. (I'm not sure how to reproduce the latter, though.) > Also, plumb "struct config_reader" into the new functions. This isn't > necessary per se, but this generalizes better, so it might help us avoid > yet another refactor. Hmm...I thought this would be desired because we don't want the_reader to be used from non-public functions anyway, so we can just state that that is the reason (and not worry about using future refactors as a justification). The code itself looks good.
