Re: [PATCH 0/4] repo: add support for path-related fields

[Tian Yuchen <a3205153416@gmail.com>]

Hi JAYATHEERTH,

Thanks for the reply.

> To clarify my earlier comment: I wasn't arguing against ref-filter.
> In fact, I’m more inclined toward using the best tool for the job.
> My earlier point was mainly about behavioral similarity and how both
> belong to the same camp even though they might seem different.

 > I just meant both the ideas are in the same camp just unrealized.

That's exactly right. I was just reminding that there's a ready-made 
solution available that seems to work very well. Wouldn't reinventing 
something that already exists, with only minor differences, cause 
confusion? (´;ω;`)

> something like this:
> 
> static const struct repo_info_field repo_info_field[] = {
>      { "layout.bare", get_layout_bare },
>      { "layout.shallow", get_layout_shallow },
>      { "object.format", get_object_format },
>      { "path.toplevel", get_path_toplevel },
> };
> 
> This array contains all the keys
> You do not need to hardcode path.absolute.toplevel,
> path.relative.toplevel, etc., in the array...
> 
> Instead,
> 
> If the user asks for path.absolute.toplevel:
> You detect the absolute. middle part. strip it out to find the base
> key path.toplevel.
> You find path.toplevel in the aray, the array works with default
> values when entered --all
> 
> /*
>   * Helper to parse the key variant.
>   * Takes "path.absolute.git-dir" -> returns "path.git-dir" and sets
> opts->format.
>   */
> static char *normalize_key(const char *raw_key, struct repo_info_opts *opts)
> {
>      const char *suffix;
> 
>      /* Check for "path.absolute." prefix */
>      if (skip_prefix(raw_key, "path.absolute.", &suffix)) {
>          opts->path_format = PATH_FORMAT_ABSOLUTE;
>          return xstrfmt("path.%s", suffix);
>      }
> 
>      /* Check for "path.relative." prefix */
>      if (skip_prefix(raw_key, "path.relative.", &suffix)) {
>          opts->path_format = PATH_FORMAT_RELATIVE;
>          return xstrfmt("path.%s", suffix);
>      }
> 
>      /* No variant found, return raw key as-is */
>      return xstrdup(raw_key);
> }

I see. What you've written matches what you described — it essentially 
replicates the functionality of ref-filter.c. While I understand this is 
just a simple code implementation demo:

 >          opts->path_format = PATH_FORMAT_ABSOLUTE;

This implementation appears unable to support input like 'git repo-info 
--keys=path.absolute.toplevel,path.relative.gitdir', meaning it cannot 
handle multiple paths output from a single call as previously mentioned 
by Brain. The 'opts' here should be a global shared state, right?

I think it's better for the parser to allocate a separate memory for 
each arg it encounters. But then we'd be back to implementing something 
like struct used_atom, hahaha (ゝ∀･)

Thank you again for your email.

Yuchen

(I feel like we've been on this topic for too long. If you don't want to 
reply, you don't have to :-)