Thread (234 messages) 234 messages, 5 authors, 2017-04-07

Re: [PATCH v5 07/24] files-backend: add and use files_refname_path()

flat view

From: Michael Haggerty <hidden>
Date: 2017-02-28 17:42:17

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/
quoted hunk ↗ jump to hunk
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 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()`
quoted hunk ↗ jump to hunk
 /*
  * 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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help