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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano wrote:
> Sorry, but I have to ask: Why?

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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]