Re: [Newcomer][PATCH] graph.c: change graph_line->width type to int to remove sign-compare warning

From: O. C. <ocarneiro1@gmail.com>
Date: May 23, 2025, 03:25 UTC
Message-ID: <CADHqMhY85ZUd=NVq9dfUrBZEzEhfqwc-iYh=gcQ9J1hg=EkAsg@mail.gmail.com>
In-Reply-To: <xmqqtt5d7e9z.fsf@gitster.g>

On Wed, May 21, 2025 at 8:08 PM Junio C Hamano <gitster@pobox.com> wrote:

> An obvious question after reading the above explanation is if it
> also would be a valid solution to change the type of git_graph.width
> to match the type of graph_line.width instead.  And if so, what was
> the reason why you chose to match their types in this direction?

While writing the patch, I saw how the git_graph and graph_line structs related to each other, and understood git_graph as being the "more important" struct in this context. So, it seemed more reasonable to have graph_line.width comply with git_graph.width type.

I do see the opposite point, taking notice of graph_line.width interactions with "size_t". With those in mind, having the type as "size_t" does also make a lot of sense.

However, considering that:

> "git log --graph" output is certainly something that should not exceed
> thousands for sane people

It feels intuitive to " use platform natural "int" " as you put it.

> By the way, these lines are overly long.  We aim to limit our (physical) line
> length to around 70 chars or so by line wrapping.

Sorry about that, I'll keep them shorter.

Lastly, thanks for the quick and careful response to the patch. As a first timer I appreciate the discussion.

- Octavio