On Sat, 10 Jun 2017 17:43:29 -0700 Brandon Williams <bmw...@google.com> wrote:
> I disagree with a few points of what jonathan said (mostly about > removing the config from the repo object, as I like the idea of nothing > knowing about a 'config_set' object) and I think this problem could be > solved in a couple ways. Ah...is the plan to eventually delete the git_configset_.* functions and only have repo_config_get_.* functions? If yes, I would prefer the in-between state to be "git_config_set_get_int(repo->configset, ...)" instead of "repo_config_get_int(repo, ...)" to avoid the parallel set of functions (which will make it more troublesome, for example, to add support for a new data type) but I can see the benefits of having the repo_config_get_.* functions too (conciseness and not having to make one final find-and-replace when we finally complete the migration, at least) so I don't feel too strongly about this. > I don't think that the in-memory global variable 'quotepath' (or > whatever its called) should live in the repository object (I mean it's > already contained in the config) but rather 'quotepath' is specific to > how ls-files handles its output. So really what should happen is you > pass a pair of objects to the ls-files machinery (or any other command's > machinery) (1) the repository object being operated on and (2) an > options struct which can be configured based on the repository. So when > recursing you can do something like the following: > > repo_init(submodule, path_to_submodule); > configure_opts(sub_opts, submodule, super_opts) > ls_files(submodule, sub_opts); > > This eliminates bloating 'struct repository' and would allow you to have > things configured differently in submodules (which is crazy if you ask > me, but people do crazy things). This does sound good to me.
