[PATCH 51 of 55 RFC c-hglib:level1] hg_tip: creating a high level function for mercurial tip command

[Idan Kamara]

On Sat, Sep 14, 2013 at 3:34 PM, Iulian Stana <julian.stana at gmail.com>wrote:

>
>  + *
>>> + * \param handle The handle of the connection, wherewith I want to
>>> communicate
>>> + * \param callback A function that will handle error data.
>>> + *                 A NULL pointer will ignore error data.
>>>
>>
>> What meaningful thing can a user do in this callback?
>>
>>
>
> What ever he wants to do with error data, print it do the stderr, store in
> some place
> or I don't know parse this data in his own way is his decision.
>

Parsing error data is a bit of a problem in Mercurial, since its output
isn't guaranteed to be stable between releases.
But more importantly, I don't think there's anything meaningful to do when
'hg tip' fails..

You already have errno to check for command errors, does it also expose a
generic 'command failed' error code?
If so, you could use that and store the error string somewhere, and have
strerror (or similar), return it.



>
>
>
>>  + * \param argument The option list. Will contain all option that you
>>> wish.
>>>
>>
>> If you let the user pass in more arguments, it has the potential of:
>>
>> 1) breaking your changeset parser (user passed --template)
>> 2) leaving unparsed data after you successfully parsed the changeset (not
>> sure how you deal with that)
>>
>>
> The last --template will overwrite all preview templates probably I must
> tell in
> documentation that is not allowed to use other template...
>

> hg tip --template '{rev}' --template '{node}' --template 'Template - {rev}
> - {node}\n'
>
> I'm not sure you really want that.
>>
>
> I really don't want to leave unparse data.
>

Right, so I'm not sure if you agree there's an issue here..

My comment was directed at making you question whether you really want to
allow
the user to pass arbitrary arguments to this function (and every other 'top
level' function
such as this).


>
>
>>
>>
>>> + * \retval cset The command return succesful, with a parse data.
>>>
>>
>> hg tip always returns a single changeset, wouldn't it be nicer if you
>> received a pointer
>> to hg_cset_entry and wrote the result there, rather than allocating one
>> inside?
>>
>
> Probably, I don't know:) I thought that it would be nice to return a
> cset_entry and
> to give user less work to do :) But this thing can be easy changed.
>

The current approach strikes me as more work for the user, because then
he's forced
to free() the malloc'ed cset. Where as if you let him pass a pointer, he
could
either pass a malloc'ed cset, or one off the stack, but the choice is his.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20130915/9a4298b6/attachment.html>