On Sun, Oct 06, 2024 at 02:41:22PM -0400, Eric Sunshine wrote:

[snip]

quoted

Why not simply do the following things here (I don't think
"git_contents" is a good name, however I am not familiar with the
worktree, I cannot give some advice here).

I found the name "git_contents" clear enough and understood its
purpose at-a-glance, so I think it's a reasonably good choice. A
slightly better name might be "gitfile_contents" or perhaps
"dotgit_contents" for consistency with other similarly-named variables
in this function.

Thanks, I don't know the context here.

quoted

If "git_contents" is not
NULL, there is no need for us to check the "err" variable.

I'm not sure we would want to change this; the existing logic seems
clear enough.

The reason why I don't think we need to check the "err" variable is that
the "git_contents" and "err" is relevant. If "git_contents" is not NULL,
the "err" must be zero unless there are bugs in "read_gitfile_gently".
So, if we already check "git_contents", why do we need to check again
for "err"?

But, I agree with you that the existing logic is clear enough. I just
explain further what I mean here.

Thanks,
Jialuo