On 02/22/2017 03:04 PM, Nguyễn Thái Ngọc Duy wrote: > Keep repo-related path handling in one place. This will make it easier > to add submodule/multiworktree support later. > > This automatically adds the "if submodule then use the submodule version > of git_path" to other call sites too. But it does not mean those > operations are sumodule-ready. Not yet.
s/sumodule/submodule/ > Signed-off-by: Nguyễn Thái Ngọc Duy <pclo...@gmail.com> > --- > refs/files-backend.c | 45 +++++++++++++++++++++++++-------------------- > 1 file changed, 25 insertions(+), 20 deletions(-) > > diff --git a/refs/files-backend.c b/refs/files-backend.c > index 7b4ea4c56..72f4e1746 100644 > --- a/refs/files-backend.c > +++ b/refs/files-backend.c > @@ -1180,6 +1180,18 @@ static void files_reflog_path(struct files_ref_store > *refs, > strbuf_git_path(sb, "logs/%s", refname); > } > > +static void files_refname_path(struct files_ref_store *refs, > + struct strbuf *sb, > + const char *refname) > +{ > + if (refs->submodule) { > + strbuf_git_path_submodule(sb, refs->submodule, "%s", refname); > + return; > + } > + > + strbuf_git_path(sb, "%s", refname); > +} Maybe it's just me, but I find it odd to exit early here when the first exit isn't due to an error. For me, structuring this like `if () call1(); else call2();` would make it clearer that the two code paths are equally-valid alternatives, and either one or the other will be executed. I had the same feeling when I read `files_reflog_path()` > /* > * Get the packed_ref_cache for the specified files_ref_store, > * creating it if necessary. > @@ -1251,10 +1263,7 @@ static void read_loose_refs(const char *dirname, > struct ref_dir *dir) > size_t path_baselen; > int err = 0; > > - if (refs->submodule) > - err = strbuf_git_path_submodule(&path, refs->submodule, "%s", > dirname); > - else > - strbuf_git_path(&path, "%s", dirname); > + files_refname_path(refs, &path, dirname); It's so nice to see these ugly `if (refs->submodule)` code blocks disappearing! > [...] Michael