Re: [GSoC PATCH v2 2/4] rev-parse: use format_path for path formatting
From: Justin Tobler <hidden>
Date: 2026-06-08 17:54:22
On 26/06/05 10:00PM, K Jayatheerth wrote:
Now that the core path-formatting logic has been abstracted into format_path() inside path.c, remove the localized duplicate formatting mechanics from builtin/rev-parse.c. Drop the usage of the old local format_type and default_type enums, and update print_path() to act as a light wrapper around the new shared engine. Resolve user-provided formatting flags directly within rev-parse to pass the final determined path_format to format_path().
So if the format isn't explicitly set by the user via the `--path-format` option, the default formatting strategy used depends on the path being printed. IOW, there is no consistent default path format here.
quoted hunk ↗ jump to hunk
Signed-off-by: K Jayatheerth <redacted> Mentored-by: Justin Tobler [off-list ref] Mentored-by: Lucas Seiki Oshiro [off-list ref] --- builtin/rev-parse.c | 103 ++++++++++---------------------------------- 1 file changed, 23 insertions(+), 80 deletions(-)diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c index 218b5f34d6..c78bdc04c1 100644 --- a/builtin/rev-parse.c +++ b/builtin/rev-parse.c@@ -632,73 +632,16 @@ static void handle_ref_opt(const char *pattern, const char *prefix) clear_ref_exclusions(&ref_excludes); } -enum format_type { - /* We would like a relative path. */ - FORMAT_RELATIVE, - /* We would like a canonical absolute path. */ - FORMAT_CANONICAL, - /* We would like the default behavior. */ - FORMAT_DEFAULT, -}; - -enum default_type { - /* Our default is a relative path. */ - DEFAULT_RELATIVE, - /* Our default is a relative path if there's a shared root. */ - DEFAULT_RELATIVE_IF_SHARED, - /* Our default is a canonical absolute path. */ - DEFAULT_CANONICAL, - /* Our default is not to modify the item. */ - DEFAULT_UNMODIFIED, -}; - -static void print_path(const char *path, const char *prefix, enum format_type format, enum default_type def) +static void print_path(const char *path, const char *prefix, + int arg_path_format, enum path_format def_format) {
[snip]
+ struct strbuf sb = STRBUF_INIT; + enum path_format fmt = (arg_path_format != -1) ? arg_path_format : def_format;
hmmm, so `arg_path_format` specifies what the user-provided format and acts as a sentinel to signal there is no value provided and the fallback format needs to be used. This feels a tad bit awkward to me. I wonder if we should introduce a PATH_FORMAT_DEFAULT to the `path_format` enum that maps to one of the existing enum values in `path.c:format_path()`. Here in `print_path()`, we could then intercept a PATH_FORMAT_DEFAULT value and override it to the specified `def_format`. I'm not sure if this is ultimately that much better though. -Justin