Re: [PATCH 08/17] revert: Separate cmdline parsing from functional code

[Jonathan Nieder <jrnieder@gmail.com>]

Hi,

Ramkumar Ramachandra wrote:

> Currently, revert_or_cherry_pick does too many things including
> argument parsing and setting up to pick the commits; this doesn't make
> a good API.

On the contrary, generally speaking single-call APIs are often very
pleasant to work with.  Consider the difference between

	struct compressor c;
	memset(&c, 0, sizeof(c));
	init_compressor(&c);
	set_compressor_opts(c, COMPRESSOR_OPT_FAST);
	set_compressor_input("foo.c");
	set_compressor_output("foo.c.compressed");
	while (compressor_pump() != COMPRESSOR_DONE)
		; /* just looping */
	fini_compressor(&c);

and

	compress_file("foo.c.compressed", "foo.c", COMPRESSOR_OPT_FAST);

The latter is more pleasant, right?

So this is probably not about the function doing too many things but
something else.

By the way, the API still has the problem described in [1]: it could
not be used from another translation unit, since one of the parameters
(the "me" variable) is passed through a global instead of on the stack
or in registers.

If the goal is a pleasant API, a good way to explain it is sometimes

	Currently you have to do:

		...

	After this change, you can write:

		...

	Isn't that nice?

[1] http://thread.gmane.org/gmane.comp.version-control.git/176647/focus=176668