Re: [PATCH 2/2] read-tree: migrate to parse-options

Junio C Hamano wrote:

I think there are advantages to parse-optification, for plumbing and porcelains. Two reasons I see are:

1. Providing a unified way of handling options
2. Providing a consistent usage message format

Obviously, 1 reduces the bugs associated with parsing options (strcmp vs. prefixcmp, incorrect argv+offset). For number 2, I think it helps users when they see the same style of usage messages with each command. It's also nice to get a quick help message without opening the man pages or using git help <command>.

Admittedly this patch ends up adding 20 or so lines. Do the above points justify this? I think so. I think the added lines can be attributed to the rougher edges of parse-opts exposed by this patch. You can't take the address of bit fields, so 6 lines are dealing with this problem. Where are the other 15 lines coming from?

Looking over it again, I think I may be able to cut the overhead down by refactoring three of the callbacks into boolean options. There's a lot of duplication there, which can be simplified. I was trying to make this a straight port which is probably not so good for convincing you that it's worthwhile.

For now, I don't mind this patch being dropped (there's an ambiguity in the callbacks returning non-zero I'd like to fix too). I'll try and get a new patch (or maybe this patch with the oneline fix and a refactoring patch) out later tonight that actually reduces the amount of lines, instead of grossly adding to them.