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

Iulian Stana julian.stana at gmail.com
Tue Sep 17 04:25:50 CDT 2013


2013/9/17 Idan Kamara <idankk86 at gmail.com>

> On Sun, Sep 15, 2013 at 11:45 AM, Giovanni Gherdovich <
> g.gherdovich at gmail.com> wrote:
>
>> Hello Idan and Iulian,
>>
>> 2013/9/15 Idan Kamara <idankk86 at gmail.com>:
>> > 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.
>>
>> They idea behind a callback to handle error messages is to let the user
>> decide if (1) print the err msg somewhere, like dialogue box or stderr or
>> (2) ignore.
>> Below I paste the IRC transcripts of the discussion we had with Matt
>> at the time.
>>
>>
>> >>>> + * \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..
>>
>> I agree that there is an issue here.
>> We don't just proxy commands trasparently to commandserver,
>> we inject a template that must be respected.
>>
>> What does python-hglib and JavaHG do about that?
>
>
> The high level interface doesn't let you pass arbitrary flags, it is
> 'typed', so you
> can only pass what each command expects (excluding --template/style or
> other
> flags that might alter the output in an unexpected way).
>

Hi, Idan,

My high level commands signature can be found in the following forms:

1) hg_cset_entry *hg_tip(hg_handle *handle, int(*callback)(const char
*msg, size_t
len), char *argument[]);

2) int hg_pull(hg_handle *handle, int(*callback)(const char *msg, size_t
len), char *argument[]);

3) hg_csetstream_buffer *hg_log(hg_handle *handle, int(*callback)(const
char *msg, size_t len), char *argument[]);

For the first case I think that it will be better to pass the hg_cset_entry
structure
by a function argument, like you proposed, this means that those function
will be
more like second form, will return the exitcode received from cmd-server.

In the third form the function will return a yield-like mechanism, in this
case I am
allocate the space, but I also free this space if you use the the
hg_fetch_entry()
functions that I made available for users.

Probably you already see the pattern of those functions:

The first argument is the handle of the connection.

The second argument is a callback function that will handle the error data
received from cmd-server. My first thought was to store this data in a
pointer
and to let user to decide when he needs this data. Matt suggest that it
would
be better to use this callback function, in this way I don't need anymore to
store the error data. I just pass the received error data to this function.

The third argument is a list of arguments (a null terminate list that will
store
all options and all flags that users need). Why I choose this option? That
time when I started to think about possibilities of passing the flags I
found
three options:
1) to lay the options in the signature, like you did in the python-hglib,
but in C you cannot call a function just with the needed argument,
so you must pass a value for each agument,
2) to pass a list with wished options, like I already did
3) to create a flag structure for each command like Martin suggest
in his mail....

After some discussion we decided to use the second option, because
it's easier to implement, and will make the code more clear, I agree
with you that its not safe to let user pass what option he want, but in
this way, some features flags can be used more easier...

I think that Martin option will be the best in my case, if you have other
suggestion for how to implement this issue please tell me, in the meantime
I will start to prepare the ground for this change.

-- 
Iulian
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20130917/9f58a830/attachment.html>


More information about the Mercurial-devel mailing list