[PATCH 10/20] doc: git-foo was obsoleted several years ago

[PATCH 10/20] doc: git-foo was obsoleted several years ago

[Felipe Contreras]

So replace 'git-foo' with 'git foo'.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/git-checkout.txt | 4 ++--
 Documentation/git-commit.txt   | 4 ++--
 Documentation/git-rebase.txt   | 4 ++--
 Documentation/git-status.txt   | 4 ++--
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index 91294f8..7560113 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -1,9 +1,9 @@
-git-checkout(1)
+git checkout(1)
 ===============
 
 NAME
 ----
-git-checkout - Checkout a branch or paths to the working tree
+git checkout - Checkout a branch or paths to the working tree
 
 SYNOPSIS
 --------
diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 1a7616c..51f6b81 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -1,9 +1,9 @@
-git-commit(1)
+git commit(1)
 =============
 
 NAME
 ----
-git-commit - Record changes to the repository
+git commit - Record changes to the repository
 
 SYNOPSIS
 --------
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 94e07fd..9a8d158 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -1,9 +1,9 @@
-git-rebase(1)
+git rebase(1)
 =============
 
 NAME
 ----
-git-rebase - Forward-port local commits to the updated upstream head
+git rebase - Forward-port local commits to the updated upstream head
 
 SYNOPSIS
 --------
diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index a4acaa0..6fb7561 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -1,9 +1,9 @@
-git-status(1)
+git status(1)
 =============
 
 NAME
 ----
-git-status - Show the working tree status
+git status - Show the working tree status
 
 
 SYNOPSIS
-- 
1.8.4-fc

[PATCH 00/20] Trivial paches

[Felipe Contreras]

Felipe Contreras (20):
  merge: simplify ff-only option
  t: replace pulls with merges
  pull: cleanup documentation
  fetch: add missing documentation
  remote: fix trivial memory leak
  revision: add missing include
  shortlog: add missing declaration
  branch: trivial style fix
  sha1-name: trivial style cleanup
  doc: git-foo was obsoleted several years ago
  symbolic-ref: trivial style fix
  alias: trivial style fix
  transport-helper: trivial style fix
  describe: trivial style fixes
  pretty: trivial style fix
  revision: trivial style fixes
  diff: trivial style fix
  run-command: trivial style fixes
  setup: trivial style fixes
  add: avoid yoda conditions

 Documentation/git-checkout.txt         |  4 ++--
 Documentation/git-commit.txt           |  4 ++--
 Documentation/git-fetch.txt            |  3 +++
 Documentation/git-pull.txt             |  4 ++--
 Documentation/git-rebase.txt           |  4 ++--
 Documentation/git-status.txt           |  4 ++--
 alias.c                                | 13 +++++++------
 builtin/add.c                          |  2 +-
 builtin/branch.c                       |  3 +--
 builtin/describe.c                     |  7 +++----
 builtin/diff.c                         |  2 +-
 builtin/merge.c                        | 11 ++---------
 builtin/symbolic-ref.c                 |  2 +-
 pretty.c                               |  2 +-
 remote.c                               |  3 ++-
 revision.c                             | 14 ++++++--------
 revision.h                             |  1 +
 run-command.c                          | 13 +++++--------
 setup.c                                |  4 ++--
 sha1_name.c                            |  1 -
 shortlog.h                             |  2 ++
 t/annotate-tests.sh                    |  2 +-
 t/t4200-rerere.sh                      |  2 +-
 t/t9114-git-svn-dcommit-merge.sh       |  2 +-
 t/t9500-gitweb-standalone-no-errors.sh |  2 +-
 transport-helper.c                     |  1 +
 26 files changed, 53 insertions(+), 59 deletions(-)

-- 
1.8.4-fc

[PATCH 03/20] pull: cleanup documentation

[Felipe Contreras]

'origin/master' is very clear, no need to specify the 'remotes/' prefix,
or babysit the user.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/git-pull.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index beea10b..03a39bc 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -39,7 +39,7 @@ Assume the following history exists and the current branch is
 "`master`":
 
 ------------
-	  A---B---C master on origin
+	  A---B---C origin/master
 	 /
     D---E---F---G master
 ------------
@@ -51,7 +51,7 @@ result in a new commit along with the names of the two parent commits
 and a log message from the user describing the changes.
 
 ------------
-	  A---B---C remotes/origin/master
+	  A---B---C origin/master
 	 /         \
     D---E---F---G---H master
 ------------
-- 
1.8.4-fc

[PATCH 20/20] add: avoid yoda conditions

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 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.4-fc

[PATCH 08/20] branch: trivial style fix

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 builtin/branch.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index ad0f86d..5696cf0 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -975,9 +975,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 			die(_("no such branch '%s'"), argv[0]);
 		}
 
-		if (!branch_has_merge_config(branch)) {
+		if (!branch_has_merge_config(branch))
 			die(_("Branch '%s' has no upstream information"), branch->name);
-		}
 
 		strbuf_addf(&buf, "branch.%s.remote", branch->name);
 		git_config_set_multivar(buf.buf, NULL, NULL, 1);
-- 
1.8.4-fc

[PATCH 15/20] pretty: trivial style fix

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 pretty.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pretty.c b/pretty.c
index b4e32b7..962e82b 100644
--- a/pretty.c
+++ b/pretty.c
@@ -497,7 +497,7 @@ void pp_user_info(struct pretty_print_context *pp,
 static int is_empty_line(const char *line, int *len_p)
 {
 	int len = *len_p;
-	while (len && isspace(line[len-1]))
+	while (len && isspace(line[len - 1]))
 		len--;
 	*len_p = len;
 	return !len;
-- 
1.8.4-fc

[PATCH 19/20] setup: trivial style fixes

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 setup.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/setup.c b/setup.c
index f08dd64..c717db4 100644
--- a/setup.c
+++ b/setup.c
@@ -566,7 +566,7 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)
 {
 	const char *env_ceiling_dirs = getenv(CEILING_DIRECTORIES_ENVIRONMENT);
 	struct string_list ceiling_dirs = STRING_LIST_INIT_DUP;
-	static char cwd[PATH_MAX+1];
+	static char cwd[PATH_MAX + 1];
 	const char *gitdirenv, *ret;
 	char *gitfile;
 	int len, offset, offset_parent, ceil_offset = -1;
@@ -581,7 +581,7 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)
 	if (nongit_ok)
 		*nongit_ok = 0;
 
-	if (!getcwd(cwd, sizeof(cwd)-1))
+	if (!getcwd(cwd, sizeof(cwd) - 1))
 		die_errno("Unable to read current working directory");
 	offset = len = strlen(cwd);
 
-- 
1.8.4-fc

[PATCH 13/20] transport-helper: trivial style fix

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 transport-helper.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/transport-helper.c b/transport-helper.c
index b32e2d6..673b7c2 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -269,6 +269,7 @@ static const char *unsupported_options[] = {
 	TRANS_OPT_THIN,
 	TRANS_OPT_KEEP
 	};
+
 static const char *boolean_options[] = {
 	TRANS_OPT_THIN,
 	TRANS_OPT_KEEP,
-- 
1.8.4-fc

[PATCH 01/20] merge: simplify ff-only option

[Felipe Contreras]

No functional changes.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 builtin/merge.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index 02a69c1..41fb66d 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -186,13 +186,6 @@ static int option_parse_n(const struct option *opt,
 	return 0;
 }
 
-static int option_parse_ff_only(const struct option *opt,
-			  const char *arg, int unset)
-{
-	fast_forward = FF_ONLY;
-	return 0;
-}
-
 static struct option builtin_merge_options[] = {
 	{ OPTION_CALLBACK, 'n', NULL, NULL, NULL,
 		N_("do not show a diffstat at the end of the merge"),
@@ -210,9 +203,9 @@ static struct option builtin_merge_options[] = {
 	OPT_BOOL('e', "edit", &option_edit,
 		N_("edit message before committing")),
 	OPT_SET_INT(0, "ff", &fast_forward, N_("allow fast-forward (default)"), FF_ALLOW),
-	{ OPTION_CALLBACK, 0, "ff-only", NULL, NULL,
+	{ OPTION_SET_INT, 0, "ff-only", &fast_forward, NULL,
 		N_("abort if fast-forward is not possible"),
-		PARSE_OPT_NOARG | PARSE_OPT_NONEG, option_parse_ff_only },
+		PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, FF_ONLY },
 	OPT_RERERE_AUTOUPDATE(&allow_rerere_auto),
 	OPT_BOOL(0, "verify-signatures", &verify_signatures,
 		N_("Verify that the named commit has a valid GPG signature")),
-- 
1.8.4-fc

[PATCH 16/20] revision: trivial style fixes

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 revision.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/revision.c b/revision.c
index 0173e01..4049867 100644
--- a/revision.c
+++ b/revision.c
@@ -1503,7 +1503,7 @@ struct cmdline_pathspec {
 static void append_prune_data(struct cmdline_pathspec *prune, const char **av)
 {
 	while (*av) {
-		ALLOC_GROW(prune->path, prune->nr+1, prune->alloc);
+		ALLOC_GROW(prune->path, prune->nr + 1, prune->alloc);
 		prune->path[prune->nr++] = *(av++);
 	}
 }
@@ -1515,7 +1515,7 @@ static void read_pathspec_from_stdin(struct rev_info *revs, struct strbuf *sb,
 		int len = sb->len;
 		if (len && sb->buf[len - 1] == '\n')
 			sb->buf[--len] = '\0';
-		ALLOC_GROW(prune->path, prune->nr+1, prune->alloc);
+		ALLOC_GROW(prune->path, prune->nr + 1, prune->alloc);
 		prune->path[prune->nr++] = xstrdup(sb->buf);
 	}
 }
@@ -2118,7 +2118,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 		 *	call init_pathspec() to set revs->prune_data here.
 		 * }
 		 */
-		ALLOC_GROW(prune_data.path, prune_data.nr+1, prune_data.alloc);
+		ALLOC_GROW(prune_data.path, prune_data.nr + 1, prune_data.alloc);
 		prune_data.path[prune_data.nr++] = NULL;
 		parse_pathspec(&revs->prune_data, 0, 0,
 			       revs->prefix, prune_data.path);
@@ -2971,7 +2971,7 @@ static struct commit *get_revision_internal(struct rev_info *revs)
 	if (revs->max_count) {
 		c = get_revision_1(revs);
 		if (c) {
-			while (0 < revs->skip_count) {
+			while (revs->skip_count > 0) {
 				revs->skip_count--;
 				c = get_revision_1(revs);
 				if (!c)
@@ -2986,9 +2986,8 @@ static struct commit *get_revision_internal(struct rev_info *revs)
 	if (c)
 		c->object.flags |= SHOWN;
 
-	if (!revs->boundary) {
+	if (!revs->boundary)
 		return c;
-	}
 
 	if (!c) {
 		/*
@@ -3034,9 +3033,8 @@ struct commit *get_revision(struct rev_info *revs)
 
 	if (revs->reverse) {
 		reversed = NULL;
-		while ((c = get_revision_internal(revs))) {
+		while ((c = get_revision_internal(revs)))
 			commit_list_insert(c, &reversed);
-		}
 		revs->commits = reversed;
 		revs->reverse = 0;
 		revs->reverse_output_stage = 1;
-- 
1.8.4-fc

Re: [PATCH 16/20] revision: trivial style fixes

[Stefan Beller]

On 10/12/2013 09:07 AM, Felipe Contreras wrote:
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  revision.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/revision.c b/revision.c
> index 0173e01..4049867 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -1503,7 +1503,7 @@ struct cmdline_pathspec {
>  static void append_prune_data(struct cmdline_pathspec *prune, const char **av)
>  {
>  	while (*av) {
> -		ALLOC_GROW(prune->path, prune->nr+1, prune->alloc);
> +		ALLOC_GROW(prune->path, prune->nr + 1, prune->alloc);
>  		prune->path[prune->nr++] = *(av++);
>  	}
>  }
> @@ -1515,7 +1515,7 @@ static void read_pathspec_from_stdin(struct rev_info *revs, struct strbuf *sb,
>  		int len = sb->len;
>  		if (len && sb->buf[len - 1] == '\n')
>  			sb->buf[--len] = '\0';
> -		ALLOC_GROW(prune->path, prune->nr+1, prune->alloc);
> +		ALLOC_GROW(prune->path, prune->nr + 1, prune->alloc);
>  		prune->path[prune->nr++] = xstrdup(sb->buf);
>  	}
>  }
> @@ -2118,7 +2118,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
>  		 *	call init_pathspec() to set revs->prune_data here.
>  		 * }
>  		 */
> -		ALLOC_GROW(prune_data.path, prune_data.nr+1, prune_data.alloc);
> +		ALLOC_GROW(prune_data.path, prune_data.nr + 1, prune_data.alloc);
>  		prune_data.path[prune_data.nr++] = NULL;
>  		parse_pathspec(&revs->prune_data, 0, 0,
>  			       revs->prefix, prune_data.path);
> @@ -2971,7 +2971,7 @@ static struct commit *get_revision_internal(struct rev_info *revs)
>  	if (revs->max_count) {
>  		c = get_revision_1(revs);
>  		if (c) {
> -			while (0 < revs->skip_count) {
> +			while (revs->skip_count > 0) {
>  				revs->skip_count--;
>  				c = get_revision_1(revs);
>  				if (!c)
> @@ -2986,9 +2986,8 @@ static struct commit *get_revision_internal(struct rev_info *revs)
>  	if (c)
>  		c->object.flags |= SHOWN;
>  
> -	if (!revs->boundary) {
> +	if (!revs->boundary)
>  		return c;
> -	}
>  
>  	if (!c) {
>  		/*
> @@ -3034,9 +3033,8 @@ struct commit *get_revision(struct rev_info *revs)
>  
>  	if (revs->reverse) {
>  		reversed = NULL;
> -		while ((c = get_revision_internal(revs))) {
> +		while ((c = get_revision_internal(revs)))
>  			commit_list_insert(c, &reversed);
> -		}

While talking about trivia, what is the projects stance on
such constructs here?
	while (foo)
		bar();
	foobar();

I've seen an empty line between the bar(); and the foobar(); often, which suits readability.
What's the stance on such an empty line here?

>  		revs->commits = reversed;
>  		revs->reverse = 0;
>  		revs->reverse_output_stage = 1;
> 

Thanks,
Stefan

Re: [PATCH 16/20] revision: trivial style fixes

[Felipe Contreras]

Stefan Beller wrote:
> While talking about trivia, what is the projects stance on
> such constructs here?
> 	while (foo)
> 		bar();
> 	foobar();
> 
> I've seen an empty line between the bar(); and the foobar(); often, which suits readability.
> What's the stance on such an empty line here?

I don't think there's any guideline on that. Personally I choose depending on
the context.

----
void test(void)
{
 	while (foo)
 		bar();
 	foobar();
}
----

----
	while (meh) {
		/*
		 * bunch of stuff
		 */
	}
	
 	while (foo)
 		bar();

 	foobar();
----

Cheers.

-- 
Felipe Contreras

[PATCH 17/20] diff: trivial style fix

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 builtin/diff.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/diff.c b/builtin/diff.c
index 2fb8c5d..adb93a9 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -169,7 +169,7 @@ static int builtin_diff_tree(struct rev_info *revs,
 	if (ent1->item->flags & UNINTERESTING)
 		swap = 1;
 	sha1[swap] = ent0->item->sha1;
-	sha1[1-swap] = ent1->item->sha1;
+	sha1[1 - swap] = ent1->item->sha1;
 	diff_tree_sha1(sha1[0], sha1[1], "", &revs->diffopt);
 	log_tree_diff_flush(revs);
 	return 0;
-- 
1.8.4-fc

[PATCH 05/20] remote: fix trivial memory leak

[Felipe Contreras]

There's no need to set the default remote name beforehand, only to be
overridden later on, and causing a memory leak, we can do it after the
configuration has been handled.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 remote.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/remote.c b/remote.c
index e9fedfa..ce9d82a 100644
--- a/remote.c
+++ b/remote.c
@@ -483,7 +483,6 @@ static void read_config(void)
 	int flag;
 	if (default_remote_name) /* did this already */
 		return;
-	default_remote_name = xstrdup("origin");
 	current_branch = NULL;
 	head_ref = resolve_ref_unsafe("HEAD", sha1, 0, &flag);
 	if (head_ref && (flag & REF_ISSYMREF) &&
@@ -492,6 +491,8 @@ static void read_config(void)
 			make_branch(head_ref + strlen("refs/heads/"), 0);
 	}
 	git_config(handle_config, NULL);
+	if (!default_remote_name)
+		default_remote_name = xstrdup("origin");
 	alias_all_urls();
 }
 
-- 
1.8.4-fc

[PATCH 07/20] shortlog: add missing declaration

[Felipe Contreras]

Otherwise we would have to include commit.h.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 shortlog.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/shortlog.h b/shortlog.h
index de4f86f..54bc07c 100644
--- a/shortlog.h
+++ b/shortlog.h
@@ -19,6 +19,8 @@ struct shortlog {
 	struct string_list mailmap;
 };
 
+struct commit;
+
 void shortlog_init(struct shortlog *log);
 
 void shortlog_add_commit(struct shortlog *log, struct commit *commit);
-- 
1.8.4-fc

[PATCH 11/20] symbolic-ref: trivial style fix

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 builtin/symbolic-ref.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/symbolic-ref.c b/builtin/symbolic-ref.c
index f481959..71286b4 100644
--- a/builtin/symbolic-ref.c
+++ b/builtin/symbolic-ref.c
@@ -47,7 +47,7 @@ int cmd_symbolic_ref(int argc, const char **argv, const char *prefix)
 	git_config(git_default_config, NULL);
 	argc = parse_options(argc, argv, prefix, options,
 			     git_symbolic_ref_usage, 0);
-	if (msg &&!*msg)
+	if (msg && !*msg)
 		die("Refusing to perform update with empty message");
 
 	if (delete) {
-- 
1.8.4-fc

[PATCH 06/20] revision: add missing include

[Felipe Contreras]

Otherwise we might not have 'struct diff_options'.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 revision.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/revision.h b/revision.h
index e7f1d21..89132df 100644
--- a/revision.h
+++ b/revision.h
@@ -5,6 +5,7 @@
 #include "grep.h"
 #include "notes.h"
 #include "commit.h"
+#include "diff.h"
 
 #define SEEN		(1u<<0)
 #define UNINTERESTING   (1u<<1)
-- 
1.8.4-fc

[PATCH 18/20] run-command: trivial style fixes

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 run-command.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/run-command.c b/run-command.c
index 1b7f88e..3914d9c 100644
--- a/run-command.c
+++ b/run-command.c
@@ -406,13 +406,12 @@ fail_pipe:
 					unsetenv(*cmd->env);
 			}
 		}
-		if (cmd->git_cmd) {
+		if (cmd->git_cmd)
 			execv_git_cmd(cmd->argv);
-		} else if (cmd->use_shell) {
+		else if (cmd->use_shell)
 			execv_shell_cmd(cmd->argv);
-		} else {
+		else
 			sane_execvp(cmd->argv[0], (char *const*) cmd->argv);
-		}
 		if (errno == ENOENT) {
 			if (!cmd->silent_exec_failure)
 				error("cannot run %s: %s", cmd->argv[0],
@@ -446,7 +445,6 @@ fail_pipe:
 		cmd->pid = -1;
 	}
 	close(notify_pipe[0]);
-
 }
 #else
 {
@@ -480,11 +478,10 @@ fail_pipe:
 	if (cmd->env)
 		env = make_augmented_environ(cmd->env);
 
-	if (cmd->git_cmd) {
+	if (cmd->git_cmd)
 		cmd->argv = prepare_git_cmd(cmd->argv);
-	} else if (cmd->use_shell) {
+	else if (cmd->use_shell)
 		cmd->argv = prepare_shell_cmd(cmd->argv);
-	}
 
 	cmd->pid = mingw_spawnvpe(cmd->argv[0], cmd->argv, env, cmd->dir,
 				  fhin, fhout, fherr);
-- 
1.8.4-fc

[PATCH 02/20] t: replace pulls with merges

[Felipe Contreras]

This is what the code intended.

No functional changes.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 t/annotate-tests.sh                    | 2 +-
 t/t4200-rerere.sh                      | 2 +-
 t/t9114-git-svn-dcommit-merge.sh       | 2 +-
 t/t9500-gitweb-standalone-no-errors.sh | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh
index 99caa42..c9d105d 100644
--- a/t/annotate-tests.sh
+++ b/t/annotate-tests.sh
@@ -92,7 +92,7 @@ test_expect_success 'blame 2 authors + 1 branch2 author' '
 '
 
 test_expect_success 'merge branch1 & branch2' '
-	git pull . branch1
+	git merge branch1
 '
 
 test_expect_success 'blame 2 authors + 2 merged-in authors' '
diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh
index 7f6666f..cf19eb7 100755
--- a/t/t4200-rerere.sh
+++ b/t/t4200-rerere.sh
@@ -172,7 +172,7 @@ test_expect_success 'first postimage wins' '
 	git show second^:a1 | sed "s/To die: t/To die! T/" >a1 &&
 	git commit -q -a -m third &&
 
-	test_must_fail git pull . first &&
+	test_must_fail git merge first &&
 	# rerere kicked in
 	! grep "^=======\$" a1 &&
 	test_cmp expect a1
diff --git a/t/t9114-git-svn-dcommit-merge.sh b/t/t9114-git-svn-dcommit-merge.sh
index f524d2f..d33d714 100755
--- a/t/t9114-git-svn-dcommit-merge.sh
+++ b/t/t9114-git-svn-dcommit-merge.sh
@@ -62,7 +62,7 @@ test_expect_success 'setup git mirror and merge' '
 	echo friend > README &&
 	cat tmp >> README &&
 	git commit -a -m "friend" &&
-	git pull . merge
+	git merge merge
 	'
 
 test_debug 'gitk --all & sleep 1'
diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
index 6fca193..3864388 100755
--- a/t/t9500-gitweb-standalone-no-errors.sh
+++ b/t/t9500-gitweb-standalone-no-errors.sh
@@ -328,7 +328,7 @@ test_expect_success \
 	 git add b &&
 	 git commit -a -m "On branch" &&
 	 git checkout master &&
-	 git pull . b &&
+	 git merge b &&
 	 git tag merge_commit'
 
 test_expect_success \
-- 
1.8.4-fc

[PATCH 12/20] alias: trivial style fix

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 alias.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/alias.c b/alias.c
index eb9f08b..9938f03 100644
--- a/alias.c
+++ b/alias.c
@@ -5,7 +5,7 @@ static char *alias_val;
 
 static int alias_lookup_cb(const char *k, const char *v, void *cb)
 {
-	if (!prefixcmp(k, "alias.") && !strcmp(k+6, alias_key)) {
+	if (!prefixcmp(k, "alias.") && !strcmp(k + 6, alias_key)) {
 		if (!v)
 			return config_error_nonbool(k);
 		alias_val = xstrdup(v);
@@ -34,7 +34,7 @@ int split_cmdline(char *cmdline, const char ***argv)
 	int src, dst, count = 0, size = 16;
 	char quoted = 0;
 
-	*argv = xmalloc(sizeof(char *) * size);
+	*argv = xmalloc(sizeof(**argv) * size);
 
 	/* split alias_string */
 	(*argv)[count++] = cmdline;
@@ -45,7 +45,7 @@ int split_cmdline(char *cmdline, const char ***argv)
 			while (cmdline[++src]
 					&& isspace(cmdline[src]))
 				; /* skip */
-			ALLOC_GROW(*argv, count+1, size);
+			ALLOC_GROW(*argv, count + 1, size);
 			(*argv)[count++] = cmdline + dst;
 		} else if (!quoted && (c == '\'' || c == '"')) {
 			quoted = c;
@@ -76,12 +76,13 @@ int split_cmdline(char *cmdline, const char ***argv)
 		return -SPLIT_CMDLINE_UNCLOSED_QUOTE;
 	}
 
-	ALLOC_GROW(*argv, count+1, size);
+	ALLOC_GROW(*argv, count + 1, size);
 	(*argv)[count] = NULL;
 
 	return count;
 }
 
-const char *split_cmdline_strerror(int split_cmdline_errno) {
-	return split_cmdline_errors[-split_cmdline_errno-1];
+const char *split_cmdline_strerror(int split_cmdline_errno)
+{
+	return split_cmdline_errors[-split_cmdline_errno - 1];
 }
-- 
1.8.4-fc

Re: [PATCH 12/20] alias: trivial style fix

[Stefan Beller]

On 10/12/2013 09:07 AM, Felipe Contreras wrote:
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Not sure if it's worth by a newcomer. ;)

Reviewed-by:  Stefan Beller <stefanbeller@googlemail.com>

> ---
>  alias.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/alias.c b/alias.c
> index eb9f08b..9938f03 100644
> --- a/alias.c
> +++ b/alias.c
> @@ -5,7 +5,7 @@ static char *alias_val;
>  
>  static int alias_lookup_cb(const char *k, const char *v, void *cb)
>  {
> -	if (!prefixcmp(k, "alias.") && !strcmp(k+6, alias_key)) {
> +	if (!prefixcmp(k, "alias.") && !strcmp(k + 6, alias_key)) {
>  		if (!v)
>  			return config_error_nonbool(k);
>  		alias_val = xstrdup(v);
> @@ -34,7 +34,7 @@ int split_cmdline(char *cmdline, const char ***argv)
>  	int src, dst, count = 0, size = 16;
>  	char quoted = 0;
>  
> -	*argv = xmalloc(sizeof(char *) * size);
> +	*argv = xmalloc(sizeof(**argv) * size);
>  
>  	/* split alias_string */
>  	(*argv)[count++] = cmdline;
> @@ -45,7 +45,7 @@ int split_cmdline(char *cmdline, const char ***argv)
>  			while (cmdline[++src]
>  					&& isspace(cmdline[src]))
>  				; /* skip */
> -			ALLOC_GROW(*argv, count+1, size);
> +			ALLOC_GROW(*argv, count + 1, size);
>  			(*argv)[count++] = cmdline + dst;
>  		} else if (!quoted && (c == '\'' || c == '"')) {
>  			quoted = c;
> @@ -76,12 +76,13 @@ int split_cmdline(char *cmdline, const char ***argv)
>  		return -SPLIT_CMDLINE_UNCLOSED_QUOTE;
>  	}
>  
> -	ALLOC_GROW(*argv, count+1, size);
> +	ALLOC_GROW(*argv, count + 1, size);
>  	(*argv)[count] = NULL;
>  
>  	return count;
>  }
>  
> -const char *split_cmdline_strerror(int split_cmdline_errno) {
> -	return split_cmdline_errors[-split_cmdline_errno-1];
> +const char *split_cmdline_strerror(int split_cmdline_errno)
> +{
> +	return split_cmdline_errors[-split_cmdline_errno - 1];
>  }
> 

Re: [PATCH 12/20] alias: trivial style fix

[Jonathan Nieder]

Stefan Beller wrote:
> On 10/12/2013 09:07 AM, Felipe Contreras wrote:

>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>
> Not sure if it's worth by a newcomer. ;)

A second set of eyes is always welcome.

My thoughts: I have to admit I don't see much value in mechanical
replacements like this one when done piecemeal and without other more
significant changes on top.  If this was fixing some particular
eyesore, I would understand, and a little preparatory cleanup to make
another patch on top more readable is always welcome, but this kind of
thing:

>> --- a/alias.c
>> +++ b/alias.c
>> @@ -5,7 +5,7 @@ static char *alias_val;
>>  
>>  static int alias_lookup_cb(const char *k, const char *v, void *cb)
>>  {
>> -	if (!prefixcmp(k, "alias.") && !strcmp(k+6, alias_key)) {
>> +	if (!prefixcmp(k, "alias.") && !strcmp(k + 6, alias_key)) {

does not look worth the churn and mailing list noise to me.

A patch that globally took care of these var+constant constructs
without surrounding space and did nothing else, once and for all to
avoid later noise, may or may not be useful.  I suspect even that
wouldn't be worth it, since "k+6" already seems perfectly readable.

So, not too excited by this change.

Hope that helps,
Jonathan

Re: [PATCH 12/20] alias: trivial style fix

[Felipe Contreras]

Jonathan Nieder wrote:
> Stefan Beller wrote:
> > On 10/12/2013 09:07 AM, Felipe Contreras wrote:
> 
> >> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> >
> > Not sure if it's worth by a newcomer. ;)
> 
> A second set of eyes is always welcome.
> 
> My thoughts: I have to admit I don't see much value in mechanical
> replacements like this one when done piecemeal and without other more
> significant changes on top.

A code-style fix is a code-style fix. If you don't send and apply code-style
fixes, your code would remain forever inconsistent, and not following the
style.

It's interesting how the most successfull project in history has a different
take on this:

http://article.gmane.org/gmane.linux.acpi.devel/42407

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=915ea7e41439efa7793814cdf4338cb6b003538a
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=21fcb34e28e99291e91d83422f2824f11b3c9ce9
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=28ee793e7ad4a00e41c6267075501694c94451fb

> >> --- a/alias.c
> >> +++ b/alias.c
> >> @@ -5,7 +5,7 @@ static char *alias_val;
> >>  
> >>  static int alias_lookup_cb(const char *k, const char *v, void *cb)
> >>  {
> >> -	if (!prefixcmp(k, "alias.") && !strcmp(k+6, alias_key)) {
> >> +	if (!prefixcmp(k, "alias.") && !strcmp(k + 6, alias_key)) {
> 
> does not look worth the churn and mailing list noise to me.
> 
> A patch that globally took care of these var+constant constructs
> without surrounding space and did nothing else, once and for all to
> avoid later noise, may or may not be useful.  I suspect even that
> wouldn't be worth it, since "k+6" already seems perfectly readable.

This comes from the Linux kernel code-style:

----
Use one space around (on each side of) most binary and ternary operators,
such as any of these:

	=  +  -  <  >  *  /  %  |  &  ^  <=  >=  ==  !=  ?  :
----

So no, to me and a huge lot of developers it's not "perfectly readable".

-- 
Felipe Contreras

[PATCH 04/20] fetch: add missing documentation

[Felipe Contreras]

There's no mention of the 'origin' default, or the fact that the
upstream tracking branch remote is used.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/git-fetch.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt
index e08a028..7e75dc4 100644
--- a/Documentation/git-fetch.txt
+++ b/Documentation/git-fetch.txt
@@ -37,6 +37,9 @@ or from several repositories at once if <group> is given and
 there is a remotes.<group> entry in the configuration file.
 (See linkgit:git-config[1]).
 
+When no remote is specified, by the 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.4-fc

[PATCH 09/20] sha1-name: trivial style cleanup

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 sha1_name.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/sha1_name.c b/sha1_name.c
index 0e5fe7f..e9c2999 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -343,7 +343,6 @@ static int get_short_sha1(const char *name, int len, unsigned char *sha1,
 	return status;
 }
 
-
 int for_each_abbrev(const char *prefix, each_abbrev_fn fn, void *cb_data)
 {
 	char hex_pfx[40];
-- 
1.8.4-fc

[PATCH 14/20] describe: trivial style fixes

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 builtin/describe.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/builtin/describe.c b/builtin/describe.c
index b9d3603..6f62109 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -9,7 +9,7 @@
 #include "hash.h"
 #include "argv-array.h"
 
-#define SEEN		(1u<<0)
+#define SEEN		(1u << 0)
 #define MAX_TAGS	(FLAG_BITS - 1)
 
 static const char * const describe_usage[] = {
@@ -36,7 +36,6 @@ static const char *diff_index_args[] = {
 	"diff-index", "--quiet", "HEAD", "--", NULL
 };
 
-
 struct commit_name {
 	struct commit_name *next;
 	unsigned char peeled[20];
@@ -46,6 +45,7 @@ struct commit_name {
 	unsigned char sha1[20];
 	char *path;
 };
+
 static const char *prio_names[] = {
 	"head", "lightweight", "annotated",
 };
@@ -488,9 +488,8 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
 	} else if (dirty) {
 		die(_("--dirty is incompatible with commit-ishes"));
 	} else {
-		while (argc-- > 0) {
+		while (argc-- > 0)
 			describe(*argv++, argc == 0);
-		}
 	}
 	return 0;
 }
-- 
1.8.4-fc