Johannes Schindelin <[email protected]> writes:
>> + if (one_of(term, "help", "start", "skip", "next", "reset", "visualize",
>> + "replay", "log", "run", NULL))
>
> If I understood Junio correctly, he meant to line up the second line with
> the corresponding level. In this case, as "replay" is a parameter of the
> one_of() function, the indentation would look like this instead:
>
> if (one_of(term, "help", "start", "skip", "next", "reset", "visualize",
> "replay", "log", "run", NULL))
Thanks for clarification. It may also make sense to wrap the first
line one word earlier.
>> + die("can't use the builtin command '%s' as a term", term);
>> +
>> + /* In theory, nothing prevents swapping
>> + * completely good and bad, but this situation
>> + * could be confusing and hasn't been tested
>> + * enough. Forbid it for now.
>> + */
>
> I see quite a few comments that put the closing "*/" on its own line, but
> do not award the same pleasure to the opening "/*". Personally, I much
> prefer the opening "/*" to have an entire line to itself, too, but I guess
> that there is enough precendence in Git's source code to accept both
> forms.
We do want to see "/*" and "*/" on their own lines, and new code
should definitely do so.
>> + 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);
>
> I am still convinced that
>
> if ((one_of(term, "bad", "new", NULL) && strcmp(orig_term, "bad")) ||
> (one_of(term, "good", "old", NULL) && strcmp(orig_term, "good")))
> die("can't change the meaning of term '%s'", term);
>
> looks so much nicer.
... and more importantly, easier to understand what is going on.
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
