Re: [PATCH v2 1/5] worktree: add CLI/config options for relative path linking
From: Caleb White <hidden>
Date: 2024-10-30 20:21:37
On Wed Oct 30, 2024 at 3:16 PM CDT, Taylor Blau wrote:
On Wed, Oct 30, 2024 at 05:27:33AM +0000, Caleb White wrote:quoted
quoted
quoted
diff --git a/builtin/worktree.c b/builtin/worktree.c index dae63dedf4cac2621f51f95a39aa456b33acd894..c1130be5890c905c0b648782a834eb8dfcd79ba5 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c@@ -134,6 +134,9 @@ static int git_worktree_config(const char *var, const char *value, if (!strcmp(var, "worktree.guessremote")) { guess_remote = git_config_bool(var, value); return 0; + } else if (!strcmp(var, "worktree.userelativepaths")) { + use_relative_paths = git_config_bool(var, value);As we're trying to remove global variables from libgit.a as part of the libification effort I'd be much happier if "use_relative_paths" was declared as a "static int" in this file and then passed down to the functions that need it rather than declaring it as a global in "worktree.c".I can create a getter/setter in the worktree API to handle this, but I'd rather not pass it as an argument to every function that needs it as that would be a lot of changes. All of these functions would need their signatures updated to include the new parameter: - `add_worktree()` - `update_worktree_location()` - `repair_worktree_at_path()` - `repair_worktrees()` - `repair_worktree()` - `write_worktree_linking_files()`There is no reason to have a "getter" and "setter" for a extern'd variable. I agree that it would be preferable to have use_relative_paths be a static int within this compilation unit and to pass it to the above functions.
If I created a getter/setter then the variable would no longer be extern'd. To be clear, you're advocating that I change the function signature for all of the functions listed above to include the new parameter? That seems like a lot of parameter bloat when I could just set the variable in this compilation unit and access it directly in the `write_worktree_linking_files()` function. Best, Caleb