Michael Rappazzo <rappa...@gmail.com> writes:

> +static int parse_ref(char *path_to_ref, struct strbuf *ref, int *is_detached)
> +{
> +     if (is_detached)
> +             *is_detached = 0;
> +     if (!strbuf_readlink(ref, path_to_ref, 0))
> +             if (!starts_with(ref->buf, "refs/") ||
> +                             check_refname_format(ref->buf, 0))
> +                     return -1;
> +     else if (strbuf_read_file(ref, path_to_ref, 0) >= 0) {

Which "if" does this "else if" pair with?  I think you would want to
clarify not with indentation but with a brace here.

        if (!strbuf_readlink(...)) {
                /* HEAD is a symlink */
                if (!starts_with(...) || check_refname_format(...))
                        return -1; /* malformed */
        } else if (strbuf_read_file(...) >= 0) {
                /* textual symref */
                ...

Alternatively, you could do it this way.

        if (!strbuf_readlink(...) &&
            (!starts_with(...) || check_refname_format(...)))
                return -1;
        else if (strbuf_read_file(...) >= 0) {
                ...

I do not know which one is more readable, though.  Probably former,
even though it is a bit more verbose, is easier to follow.

> +             if (!starts_with(ref->buf, "ref:"))
> +                     if (is_detached)
> +                             *is_detached = 1;
> +             else {

Same here.


 worktree.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/worktree.c b/worktree.c
index c049947..f16c082 100644
--- a/worktree.c
+++ b/worktree.c
@@ -17,22 +17,25 @@ static int parse_ref(char *path_to_ref, struct strbuf *ref, 
int *is_detached)
 {
        if (is_detached)
                *is_detached = 0;
-       if (!strbuf_readlink(ref, path_to_ref, 0))
+       if (!strbuf_readlink(ref, path_to_ref, 0)) {
+               /* HEAD is symbolic link */
                if (!starts_with(ref->buf, "refs/") ||
                                check_refname_format(ref->buf, 0))
                        return -1;
-       else if (strbuf_read_file(ref, path_to_ref, 0) >= 0) {
-               if (!starts_with(ref->buf, "ref:"))
+       } else if (strbuf_read_file(ref, path_to_ref, 0) >= 0) {
+               /* textual symref or detached */
+               if (!starts_with(ref->buf, "ref:")) {
                        if (is_detached)
                                *is_detached = 1;
-               else {
+               } else {
                        strbuf_remove(ref, 0, strlen("ref:"));
                        strbuf_trim(ref);
                        if (check_refname_format(ref->buf, 0))
                                return -1;
                }
-       } else
+       } else {
                return -1;
+       }
        return 0;
 }
 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to