On Tue, Mar 22, 2016 at 5:58 AM, Stefan Beller <sbeller@xxxxxxxxxx> wrote:
> On Mon, Mar 21, 2016 at 12:00 PM, Pranit Bauva <pranit.bauva@xxxxxxxxx> wrote:
>> Convert the code literally without changing its design even though it
>> seems that its obscure as to the use of comparing revision to different bisect
>> arguments which seems like a problem in shell because of the way
>> function arguments are handled.
>
> How would I use the C version instead of the shell version now?
> I'd imagine you'd want to change calls in git-bisect.sh from
> check_term_format <term> <bad/new>
> to be:
> git bisect--helper check_term_format <term> <bad/new>
> and "git bisect--helper" would then call the new static method?
> Once you have the C version working (do we need additional tests
> or can we rely on the test suite being enough for now?),
> you can also delete the shell version.
I have to yet think about this. Currently, I just called this function
from cmd_bisect__helper().
> Thanks,
> Stefan
>
>>
>> Signed-off-by: Pranit Bauva <pranit.bauva@xxxxxxxxx>
>> ---
>> builtin/bisect--helper.c | 23 +++++++++++++++++++++++
>> 1 file changed, 23 insertions(+)
>>
>> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
>> index 3324229..61abe68 100644
>> --- a/builtin/bisect--helper.c
>> +++ b/builtin/bisect--helper.c
>> @@ -2,12 +2,35 @@
>> #include "cache.h"
>> #include "parse-options.h"
>> #include "bisect.h"
>> +#include "refs.h"
>>
>> static const char * const git_bisect_helper_usage[] = {
>> N_("git bisect--helper --next-all [--no-checkout]"),
>> NULL
>> };
>>
>> +static int check_term_format(const char *term, const char *revision, int flag) {
>> + if (check_refname_format(term, flag))
>> + die("'%s' is not a valid term", term);
>> +
>> + if (!strcmp(term, "help") || !strcmp(term, "start") ||
>> + !strcmp(term, "skip") || !strcmp(term, "next") ||
>> + !strcmp(term, "reset") || !strcmp(term, "visualize") ||
>> + !strcmp(term, "replay") || !strcmp(term, "log") ||
>> + !strcmp(term, "run"))
>> + die("can't use the builtin command '%s' as a term", term);
>
> "terms" is missing?
>
> eval_gettext would translate into C as
> die (_("translatable message"));
>
>
>> +
>> + if (!strcmp(term, "bad") || !strcmp(term, "new"))
>> + if(strcmp(revision, "bad"))
>> + die("can't change the meaning of term '%s'", term);
>> +
>> + if (!strcmp(term, "good") || !strcmp(term, "old"))
>> + if (strcmp(revision, "good"))
>> + die("can't change the meaning of term '%s'", term);
>> +
>> + return 1;
>
> Why 1? Usually we use 0 for success in C. die(...) returns with non zero,
> so having 0 here would help us in the shell code to see if the C version
> died (or not).
Sorry I forgot to change the value. I had initially kept it to 0 but
then to do some tweaks, I changed it to 1. I had to do manual tweaks
as I am still scared to use gdb for big projects. I am trying to get
more comfortable with it. Also reading the patch again, I seem to have
forgotten the function declaration statement also.
[1] : http://thread.gmane.org/gmane.comp.version-control.git/289299/focus=289364
--
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