Re: [PATCH 4/4] repo: add the field path.toplevel

From: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
Date: Mar 1, 2026, 20:21 UTC
Message-ID: <9789E676-4DE0-4C4C-BCAC-5BD880A51CE1@gmail.com>
In-Reply-To: <71e42a01-6077-48fc-876e-555431d1288f@gmail.com>

> Hi Lucas,

Hi, Tian!

Show 6 quoted lines

> > +void strbuf_add_path(struct strbuf *sb, const char *path, const char > *prefix, enum path_format_type format, enum path_default_type def)
> 
> Isn't it a bit inappropriate for a generic character concatenation
> function to know about format and def? I don't think this should be
> the responsibility of a low-level function, at least not
> str_buf_add_path().

I don't think it can be considered a low-level function, but I agree that its name can be misleading.

> > + prefix = cwd = xgetcwd()
> 
> Will there be a performance regression? Since xgetcwd() here is a
> system call, right?

In this case, no, it is defined in wrapper.h.

Show 7 quoted lines

> I don't think we should add the two new parameters to all get_
> functions here. As changed in your patch, functions like
> get_object_format don't really, need to know about prefix or format,
> so the corresponding parameters are marked as UNUSED. Imagine if
> more and more data needs to be retrieved by these get_ series
> functions in the future — is it really advisable to add unnecessary
> parameters to all remaining functions just for the sake of a few?

In this case, we need to add them to match the signature of get_value_fn. Those values will be useful for all the path.*, but if we start to add more than that I agree that we'll need to think in a better solution.

> I'm not entirely sure about the above content either; I'm just
> throwing out ideas to spark discussion. (´～`)

Thanks, it's also good to see more points of view. I'm also not sure about it :-)