# [PATCH 0/7] Trivial patches 11 messages from 2013-12-08 to 2013-12-09. Participants: Felipe Contreras, Philip Oakley, Junio C Hamano. Thread: https://gitlist.dev/t/35499 ## Felipe Contreras, 2013-12-08 05:56 Subject: [PATCH 0/7] Trivial patches Message-ID: <1386482220-18558-1-git-send-email-felipe.contreras@gmail.com> URL: https://gitlist.dev/e/1386482220-18558-1-git-send-email-felipe.contreras%40gmail.com ``` Felipe Contreras (7): config: avoid yoda conditions add: avoid yoda conditions abspath: trivial style fix t: trivial whitespace cleanups fetch: add missing documentation sha1_name: cleanup interpret_branch_name() sha1_name: simplify track finding Documentation/git-fetch.txt | 3 +++ abspath.c | 2 +- builtin/add.c | 2 +- config.c | 4 ++-- sha1_name.c | 33 +++++++++++++++++---------------- t/t0002-gitfile.sh | 3 +-- t/t0003-attributes.sh | 1 - 7 files changed, 25 insertions(+), 23 deletions(-) -- 1.8.5.1+fc1.2.gebd1fb1 ``` ## Felipe Contreras, 2013-12-08 05:56 Subject: [PATCH 1/7] config: avoid yoda conditions Message-ID: <1386482220-18558-2-git-send-email-felipe.contreras@gmail.com> URL: https://gitlist.dev/e/1386482220-18558-2-git-send-email-felipe.contreras%40gmail.com In-Reply-To: <1386482220-18558-1-git-send-email-felipe.contreras@gmail.com> ``` Signed-off-by: Felipe Contreras --- config.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/config.c b/config.c index e1d66a1..7a44414 100644 --- a/config.c +++ b/config.c @@ -610,7 +610,7 @@ static int git_config_maybe_bool_text(const char *name, const char *value) int git_config_maybe_bool(const char *name, const char *value) { int v = git_config_maybe_bool_text(name, value); - if (0 <= v) + if (v >= 0) return v; if (git_parse_int(value, &v)) return !!v; @@ -620,7 +620,7 @@ int git_config_maybe_bool(const char *name, const char *value) int git_config_bool_or_int(const char *name, const char *value, int *is_bool) { int v = git_config_maybe_bool_text(name, value); - if (0 <= v) { + if (v >= 0) { *is_bool = 1; return v; } -- 1.8.5.1+fc1.2.gebd1fb1 ``` ## Felipe Contreras, 2013-12-08 05:56 Subject: [PATCH 2/7] add: avoid yoda conditions Message-ID: <1386482220-18558-3-git-send-email-felipe.contreras@gmail.com> URL: https://gitlist.dev/e/1386482220-18558-3-git-send-email-felipe.contreras%40gmail.com In-Reply-To: <1386482220-18558-1-git-send-email-felipe.contreras@gmail.com> ``` Signed-off-by: Felipe Contreras --- builtin/add.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/add.c b/builtin/add.c index 226f758..9b30356 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -429,7 +429,7 @@ int cmd_add(int argc, const char **argv, const char *prefix) argc--; argv++; - if (0 <= addremove_explicit) + if (addremove_explicit >= 0) addremove = addremove_explicit; else if (take_worktree_changes && ADDREMOVE_DEFAULT) addremove = 0; /* "-u" was given but not "-A" */ -- 1.8.5.1+fc1.2.gebd1fb1 ``` ## Felipe Contreras, 2013-12-08 05:56 Subject: [PATCH 3/7] abspath: trivial style fix Message-ID: <1386482220-18558-4-git-send-email-felipe.contreras@gmail.com> URL: https://gitlist.dev/e/1386482220-18558-4-git-send-email-felipe.contreras%40gmail.com In-Reply-To: <1386482220-18558-1-git-send-email-felipe.contreras@gmail.com> ``` Signed-off-by: Felipe Contreras --- abspath.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/abspath.c b/abspath.c index e390994..8b3385a 100644 --- a/abspath.c +++ b/abspath.c @@ -143,7 +143,7 @@ static const char *real_path_internal(const char *path, int die_on_error) error_out: free(last_elem); if (*cwd && chdir(cwd)) - die_errno ("Could not change back to '%s'", cwd); + die_errno("Could not change back to '%s'", cwd); return retval; } -- 1.8.5.1+fc1.2.gebd1fb1 ``` ## Felipe Contreras, 2013-12-08 05:56 Subject: [PATCH 4/7] t: trivial whitespace cleanups Message-ID: <1386482220-18558-5-git-send-email-felipe.contreras@gmail.com> URL: https://gitlist.dev/e/1386482220-18558-5-git-send-email-felipe.contreras%40gmail.com In-Reply-To: <1386482220-18558-1-git-send-email-felipe.contreras@gmail.com> ``` Signed-off-by: Felipe Contreras --- t/t0002-gitfile.sh | 3 +-- t/t0003-attributes.sh | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/t/t0002-gitfile.sh b/t/t0002-gitfile.sh index cb14425..37e9396 100755 --- a/t/t0002-gitfile.sh +++ b/t/t0002-gitfile.sh @@ -7,7 +7,7 @@ Verify that plumbing commands work when .git is a file . ./test-lib.sh objpath() { - echo "$1" | sed -e 's|\(..\)|\1/|' + echo "$1" | sed -e 's|\(..\)|\1/|' } objck() { @@ -19,7 +19,6 @@ objck() { fi } - test_expect_success 'initial setup' ' REAL="$(pwd)/.real" && mv .git "$REAL" diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh index 0b98b6f..b9d7947 100755 --- a/t/t0003-attributes.sh +++ b/t/t0003-attributes.sh @@ -13,7 +13,6 @@ attr_check () { test_line_count = 0 err } - test_expect_success 'setup' ' mkdir -p a/b/d a/c b && ( -- 1.8.5.1+fc1.2.gebd1fb1 ``` ## Felipe Contreras, 2013-12-08 05:56 Subject: [PATCH 5/7] fetch: add missing documentation Message-ID: <1386482220-18558-6-git-send-email-felipe.contreras@gmail.com> URL: https://gitlist.dev/e/1386482220-18558-6-git-send-email-felipe.contreras%40gmail.com In-Reply-To: <1386482220-18558-1-git-send-email-felipe.contreras@gmail.com> ``` There's no mention of the 'origin' default, or the fact that the upstream tracking branch remote is used. Signed-off-by: Felipe Contreras --- Documentation/git-fetch.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt index e08a028..a7b245d 100644 --- a/Documentation/git-fetch.txt +++ b/Documentation/git-fetch.txt @@ -37,6 +37,9 @@ or from several repositories at once if is given and there is a remotes. entry in the configuration file. (See linkgit:git-config[1]). +When no remote is specified, by default the `origin` remote will be used, +unless there's an upstream branch configured for the current branch. + OPTIONS ------- include::fetch-options.txt[] -- 1.8.5.1+fc1.2.gebd1fb1 ``` ## Philip Oakley, 2013-12-08 05:56 Subject: Re: [PATCH 6/7] sha1_name: cleanup interpret_branch_name() Message-ID: <67130C4F312A414DAB7EA610C78C9103@PhilipOakley> URL: https://gitlist.dev/e/67130C4F312A414DAB7EA610C78C9103%40PhilipOakley In-Reply-To: <1386482220-18558-7-git-send-email-felipe.contreras@gmail.com> ``` From: "Felipe Contreras" > The 'upstream' variable doesn't hold an "upstream", but a branch, so > make it clearer. > > Signed-off-by: Felipe Contreras > --- > sha1_name.c | 18 +++++++++--------- > 1 file changed, 9 insertions(+), 9 deletions(-) > > diff --git a/sha1_name.c b/sha1_name.c > index e9c2999..e27db88 100644 > --- a/sha1_name.c > +++ b/sha1_name.c > @@ -1072,7 +1072,7 @@ static int reinterpret(const char *name, int > namelen, int len, struct strbuf *bu > int interpret_branch_name(const char *name, int namelen, struct strbuf > *buf) > { > char *cp; > - struct branch *upstream; > + struct branch *branch; > int len = interpret_nth_prior_checkout(name, buf); > int tmp_len; > > @@ -1102,26 +1102,26 @@ int interpret_branch_name(const char *name, > int namelen, struct strbuf *buf) > > len = cp + tmp_len - name; > cp = xstrndup(name, cp - name); > - upstream = branch_get(*cp ? cp : NULL); > + branch = branch_get(*cp ? cp : NULL); > /* > * Upstream can be NULL only if cp refers to HEAD and HEAD > * points to something different than a branch. Doesn't this comment also need updating, or at least clarifying any contradictions between variable names and the concepts described. That is does "Upstream" refer to the original variable 'upstream', and now the final "branch" can be confused with the new variable name. I'm not arguing against the change, rather just noting an additional consequence. > */ > - if (!upstream) > + if (!branch) > die(_("HEAD does not point to a branch")); > - if (!upstream->merge || !upstream->merge[0]->dst) { > - if (!ref_exists(upstream->refname)) > + if (!branch->merge || !branch->merge[0]->dst) { > + if (!ref_exists(branch->refname)) > die(_("No such branch: '%s'"), cp); > - if (!upstream->merge) { > + if (!branch->merge) { > die(_("No upstream configured for branch '%s'"), > - upstream->name); > + branch->name); > } > die( > _("Upstream branch '%s' not stored as a remote-tracking branch"), > - upstream->merge[0]->src); > + branch->merge[0]->src); > } > free(cp); > - cp = shorten_unambiguous_ref(upstream->merge[0]->dst, 0); > + cp = shorten_unambiguous_ref(branch->merge[0]->dst, 0); > strbuf_reset(buf); > strbuf_addstr(buf, cp); > free(cp); > -- > 1.8.5.1+fc1.2.gebd1fb1 > > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ``` ## Felipe Contreras, 2013-12-08 05:56 Subject: [PATCH 6/7] sha1_name: cleanup interpret_branch_name() Message-ID: <1386482220-18558-7-git-send-email-felipe.contreras@gmail.com> URL: https://gitlist.dev/e/1386482220-18558-7-git-send-email-felipe.contreras%40gmail.com In-Reply-To: <1386482220-18558-1-git-send-email-felipe.contreras@gmail.com> ``` The 'upstream' variable doesn't hold an "upstream", but a branch, so make it clearer. Signed-off-by: Felipe Contreras --- sha1_name.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/sha1_name.c b/sha1_name.c index e9c2999..e27db88 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -1072,7 +1072,7 @@ static int reinterpret(const char *name, int namelen, int len, struct strbuf *bu int interpret_branch_name(const char *name, int namelen, struct strbuf *buf) { char *cp; - struct branch *upstream; + struct branch *branch; int len = interpret_nth_prior_checkout(name, buf); int tmp_len; @@ -1102,26 +1102,26 @@ int interpret_branch_name(const char *name, int namelen, struct strbuf *buf) len = cp + tmp_len - name; cp = xstrndup(name, cp - name); - upstream = branch_get(*cp ? cp : NULL); + branch = branch_get(*cp ? cp : NULL); /* * Upstream can be NULL only if cp refers to HEAD and HEAD * points to something different than a branch. */ - if (!upstream) + if (!branch) die(_("HEAD does not point to a branch")); - if (!upstream->merge || !upstream->merge[0]->dst) { - if (!ref_exists(upstream->refname)) + if (!branch->merge || !branch->merge[0]->dst) { + if (!ref_exists(branch->refname)) die(_("No such branch: '%s'"), cp); - if (!upstream->merge) { + if (!branch->merge) { die(_("No upstream configured for branch '%s'"), - upstream->name); + branch->name); } die( _("Upstream branch '%s' not stored as a remote-tracking branch"), - upstream->merge[0]->src); + branch->merge[0]->src); } free(cp); - cp = shorten_unambiguous_ref(upstream->merge[0]->dst, 0); + cp = shorten_unambiguous_ref(branch->merge[0]->dst, 0); strbuf_reset(buf); strbuf_addstr(buf, cp); free(cp); -- 1.8.5.1+fc1.2.gebd1fb1 ``` ## Felipe Contreras, 2013-12-08 05:57 Subject: [PATCH 7/7] sha1_name: simplify track finding Message-ID: <1386482220-18558-8-git-send-email-felipe.contreras@gmail.com> URL: https://gitlist.dev/e/1386482220-18558-8-git-send-email-felipe.contreras%40gmail.com In-Reply-To: <1386482220-18558-1-git-send-email-felipe.contreras@gmail.com> ``` It's more efficient to check for the braces first, and we already have the length of the whole thing. Signed-off-by: Felipe Contreras --- sha1_name.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/sha1_name.c b/sha1_name.c index e27db88..1a68103 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -417,7 +417,7 @@ static int ambiguous_path(const char *path, int len) static inline int upstream_mark(const char *string, int len) { - const char *suffix[] = { "@{upstream}", "@{u}" }; + const char *suffix[] = { "upstream", "u" }; int i; for (i = 0; i < ARRAY_SIZE(suffix); i++) { @@ -475,7 +475,7 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1) nth_prior = 1; continue; } - if (!upstream_mark(str + at, len - at)) { + if (!upstream_mark(str + at + 2, len - at - 3)) { reflog_len = (len-1) - (at+2); len = at; } @@ -1074,7 +1074,6 @@ int interpret_branch_name(const char *name, int namelen, struct strbuf *buf) char *cp; struct branch *branch; int len = interpret_nth_prior_checkout(name, buf); - int tmp_len; if (!namelen) namelen = strlen(name); @@ -1096,11 +1095,13 @@ int interpret_branch_name(const char *name, int namelen, struct strbuf *buf) if (len > 0) return reinterpret(name, namelen, len, buf); - tmp_len = upstream_mark(cp, namelen - (cp - name)); - if (!tmp_len) + len = namelen - (cp - name); + if (cp[1] != '{' && cp[len - 1] != '}') + return -1; + + if (!upstream_mark(cp + 2, len - 3)) return -1; - len = cp + tmp_len - name; cp = xstrndup(name, cp - name); branch = branch_get(*cp ? cp : NULL); /* @@ -1125,7 +1126,7 @@ int interpret_branch_name(const char *name, int namelen, struct strbuf *buf) strbuf_reset(buf); strbuf_addstr(buf, cp); free(cp); - return len; + return namelen; } int strbuf_branchname(struct strbuf *sb, const char *name) -- 1.8.5.1+fc1.2.gebd1fb1 ``` ## Junio C Hamano, 2013-12-09 21:24 Subject: Re: [PATCH 3/7] abspath: trivial style fix Message-ID: URL: https://gitlist.dev/e/xmqqeh5lwwej.fsf%40gitster.dls.corp.google.com In-Reply-To: <1386482220-18558-4-git-send-email-felipe.contreras@gmail.com> ``` Felipe Contreras writes: > Signed-off-by: Felipe Contreras > --- > abspath.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/abspath.c b/abspath.c > index e390994..8b3385a 100644 > --- a/abspath.c > +++ b/abspath.c > @@ -143,7 +143,7 @@ static const char *real_path_internal(const char *path, int die_on_error) > error_out: > free(last_elem); > if (*cwd && chdir(cwd)) > - die_errno ("Could not change back to '%s'", cwd); > + die_errno("Could not change back to '%s'", cwd); Will queue; thanks. By the way, there are quite a few hits from git grep -e '\(die\|error\|warning\) (' -- \*.c even if we ignore borrowed code that we do not want to touch up with this kind of change. Some may have overlapping changes in flight that makes it better to leave them as they are for now, but some are in reasonably quiescent areas. ``` ## Junio C Hamano, 2013-12-09 21:26 Subject: Re: [PATCH 5/7] fetch: add missing documentation Message-ID: URL: https://gitlist.dev/e/xmqqa9g9wwa7.fsf%40gitster.dls.corp.google.com In-Reply-To: <1386482220-18558-6-git-send-email-felipe.contreras@gmail.com> ``` Felipe Contreras writes: > There's no mention of the 'origin' default, or the fact that the > upstream tracking branch remote is used. > > Signed-off-by: Felipe Contreras > --- > Documentation/git-fetch.txt | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt > index e08a028..a7b245d 100644 > --- a/Documentation/git-fetch.txt > +++ b/Documentation/git-fetch.txt > @@ -37,6 +37,9 @@ or from several repositories at once if is given and > there is a remotes. entry in the configuration file. > (See linkgit:git-config[1]). > > +When no remote is specified, by default the `origin` remote will be used, > +unless there's an upstream branch configured for the current branch. > + It is better than saying nothing, but the reader will be left wondering what happens when "upstream branch" is configured with the above text. When no remote is specified, by default the `origin` remote will be used, unless there's an upstream branch configured for the current branch, in which case the `fetch` goes to the remote the specified upstream remote-tracking branch is taken from. or something, perhaps? > OPTIONS > ------- > include::fetch-options.txt[] ```