John Szakmeister <john@xxxxxxxxxxxxxxx> writes:
> On Thu, May 9, 2013 at 4:53 AM, Felipe Contreras
> <felipe.contreras@xxxxxxxxx> wrote:
> [snip]
>>>> @@ -562,9 +561,7 @@ static void handle_tags_and_duplicates(struct string_list *extra_refs)
>>>> break;
>>>> case OBJ_COMMIT:
>>>> /* create refs pointing to already seen commits */
>>>> - commit = (struct commit *)object;
>>>> - printf("reset %s\nfrom :%d\n\n", name,
>>>> - get_object_mark(&commit->object));
>>>> + printf("reset %s\nfrom :%d\n\n", name, get_object_mark(object));
>>>
>>> FWIW, this line is now too long (exceeds 80 columns). Good catch on
>>> the casting though.
>> ...
>> The key word being *try*.
>
> I saw that, but you actively joined the lines, and there was no need
> to. It didn't even require trying to keep it within 80 columns. :-)
That is somewhat a flawed argument.
Sometimes it makes it easier to have a single line that is slightly
longer than 80-col than split it across two lines. "Now one of the
arguments to this function invocation is much shorter. The only
reason the invocation originally was split into two lines was
because its argument list was too long, so joining them into a
single line _might_ make it easier to read. Let's try to see how it
reads." is a reasonable line of thought, and at that point, it
starts to require trying to keep it under 80-col.
In this particular case, however, I do not think that the single
line is particularly easier to read, but I do not think the original
is better, either. The easiest to read would be more like this:
printf("reset %s\nfrom :%d\n\n",
name, get_object_mark(object));
to show which argument corresponds to which %conversion [*1*]
[Footnote]
*1* When the format string has too many embedded LFs, showing it
like this:
printf("reset %s\n"
"from :%d\n\n",
name, get_object_mark(object));
is even better, but I do not think it applies to this particular
case.
--
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