Re: [GSoC PATCH v2 1/4] path: introduce format_path() for centralized path formatting
From: K Jayatheerth <hidden>
Date: 2026-06-09 02:47:24
Nitpick: the documentation is clear to me, but maybe the function name "format" and the parameter name "buf" can mislead the user to think that it only formats the path without appending to the existing string in `buf`. My suggestion is to rename them to something like `append_formatted_path` and `dest`, respectively.
Ok, that's a good point! I will add this in the next series!
quoted
+test_repo_info_path () { + field_name=$1 + expect_absolute_eval=$2 + expect_relative=$3 + env_prefix=$4This helper function needs a documentation.
Alright, I will add that.
quoted
+ test_expect_success "query individual key: path.$field_name.absolute${env_prefix:+ ($env_prefix)}" 'This makes the output polluted. What about changing it by something like: test_expect_success "absolute: $label' '...' test_expect_success "relative: $label' '...' with a custom label?
Ahh, interesting. I agree, I will look into this!
quoted
+ +test_expect_success 'setup test repository layout for path fields' ' + git init test-repo && + mkdir -p test-repo/sub +'The helper function `test_repo_info_path` is relying too much on the existence of the `test-repo`. I think it would be better to add a new parameter `repo_name` (or similar) because 1. You could move this creation to the helper function and you won't need to place the test after that creation 2. You could use different for each (test_repo_info_path call, path format) pair. Currently, if more than one test fails, its result is overwritten and the `expect` and `actual` files from the trash directory will be the last of the broken tests. 3. You won't need to use the hacky 'echo "$(cd .. && pwd)' This applies my suggestions (feel free to use, adapt or discard it):
Thanks! That is helpful. Regards, - K Jayatheerth