{"thread":{"id":"35252","subject":"[PATCH 00/16] Trivial patches","startedAt":"2013-10-31T09:25:31Z","lastAt":"2013-11-01T12:20:24Z","messageCount":48,"participants":["Felipe Contreras","Max Horn","Junio C Hamano","Martin von Zweigbergk","David Aguilar","SZEDER Gábor"],"isPatch":true,"patchVersion":1,"patchTotal":16},"messages":[{"id":"229878","messageId":"1383211547-9145-1-git-send-email-felipe.contreras@gmail.com","threadId":"35252","inReplyTo":null,"subject":"[PATCH 00/16] Trivial patches","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-31T09:25:31Z","receivedAt":"2013-10-31T09:25:31Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Most of these have been sent before, but were not applied for one reason or\nanother.\n\nFelipe Contreras (16):\n  merge: simplify ff-only option\n  t: replace pulls with merges\n  pull: cleanup documentation\n  fetch: add missing documentation\n  revision: add missing include\n  shortlog: add missing declaration\n  branch: trivial style fix\n  sha1-name: trivial style cleanup\n  transport-helper: trivial style fix\n  describe: trivial style fixes\n  pretty: trivial style fix\n  revision: trivial style fixes\n  diff: trivial style fix\n  run-command: trivial style fixes\n  setup: trivial style fixes\n  add: avoid yoda conditions\n\n Documentation/git-fetch.txt            |  3 +++\n Documentation/git-pull.txt             |  4 ++--\n builtin/add.c                          |  2 +-\n builtin/branch.c                       |  3 +--\n builtin/describe.c                     |  7 +++----\n builtin/diff.c                         |  2 +-\n builtin/merge.c                        | 11 ++---------\n pretty.c                               |  2 +-\n revision.c                             | 14 ++++++--------\n revision.h                             |  1 +\n run-command.c                          | 13 +++++--------\n setup.c                                |  4 ++--\n sha1_name.c                            |  1 -\n shortlog.h                             |  2 ++\n t/annotate-tests.sh                    |  2 +-\n t/t4200-rerere.sh                      |  2 +-\n t/t9114-git-svn-dcommit-merge.sh       |  2 +-\n t/t9500-gitweb-standalone-no-errors.sh |  2 +-\n transport-helper.c                     |  1 +\n 19 files changed, 35 insertions(+), 43 deletions(-)\n\n-- \n1.8.4.2+fc1\n"},{"id":"229880","messageId":"1383211547-9145-2-git-send-email-felipe.contreras@gmail.com","threadId":"35252","inReplyTo":"1383211547-9145-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 01/16] merge: simplify ff-only option","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-31T09:25:32Z","receivedAt":"2013-10-31T09:25:32Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"No functional changes.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n builtin/merge.c | 11 ++---------\n 1 file changed, 2 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 02a69c1..41fb66d 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -186,13 +186,6 @@ static int option_parse_n(const struct option *opt,\n \treturn 0;\n }\n \n-static int option_parse_ff_only(const struct option *opt,\n-\t\t\t  const char *arg, int unset)\n-{\n-\tfast_forward = FF_ONLY;\n-\treturn 0;\n-}\n-\n static struct option builtin_merge_options[] = {\n \t{ OPTION_CALLBACK, 'n', NULL, NULL, NULL,\n \t\tN_(\"do not show a diffstat at the end of the merge\"),\n@@ -210,9 +203,9 @@ static struct option builtin_merge_options[] = {\n \tOPT_BOOL('e', \"edit\", &option_edit,\n \t\tN_(\"edit message before committing\")),\n \tOPT_SET_INT(0, \"ff\", &fast_forward, N_(\"allow fast-forward (default)\"), FF_ALLOW),\n-\t{ OPTION_CALLBACK, 0, \"ff-only\", NULL, NULL,\n+\t{ OPTION_SET_INT, 0, \"ff-only\", &fast_forward, NULL,\n \t\tN_(\"abort if fast-forward is not possible\"),\n-\t\tPARSE_OPT_NOARG | PARSE_OPT_NONEG, option_parse_ff_only },\n+\t\tPARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, FF_ONLY },\n \tOPT_RERERE_AUTOUPDATE(&allow_rerere_auto),\n \tOPT_BOOL(0, \"verify-signatures\", &verify_signatures,\n \t\tN_(\"Verify that the named commit has a valid GPG signature\")),\n-- \n1.8.4.2+fc1\n"},{"id":"229879","messageId":"1383211547-9145-3-git-send-email-felipe.contreras@gmail.com","threadId":"35252","inReplyTo":"1383211547-9145-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 02/16] t: replace pulls with merges","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-31T09:25:33Z","receivedAt":"2013-10-31T09:25:33Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"This is what the code intended.\n\nNo functional changes.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n t/annotate-tests.sh                    | 2 +-\n t/t4200-rerere.sh                      | 2 +-\n t/t9114-git-svn-dcommit-merge.sh       | 2 +-\n t/t9500-gitweb-standalone-no-errors.sh | 2 +-\n 4 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/t/annotate-tests.sh b/t/annotate-tests.sh\nindex 99caa42..c9d105d 100644\n--- a/t/annotate-tests.sh\n+++ b/t/annotate-tests.sh\n@@ -92,7 +92,7 @@ test_expect_success 'blame 2 authors + 1 branch2 author' '\n '\n \n test_expect_success 'merge branch1 & branch2' '\n-\tgit pull . branch1\n+\tgit merge branch1\n '\n \n test_expect_success 'blame 2 authors + 2 merged-in authors' '\ndiff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh\nindex 7f6666f..cf19eb7 100755\n--- a/t/t4200-rerere.sh\n+++ b/t/t4200-rerere.sh\n@@ -172,7 +172,7 @@ test_expect_success 'first postimage wins' '\n \tgit show second^:a1 | sed \"s/To die: t/To die! T/\" >a1 &&\n \tgit commit -q -a -m third &&\n \n-\ttest_must_fail git pull . first &&\n+\ttest_must_fail git merge first &&\n \t# rerere kicked in\n \t! grep \"^=======\\$\" a1 &&\n \ttest_cmp expect a1\ndiff --git a/t/t9114-git-svn-dcommit-merge.sh b/t/t9114-git-svn-dcommit-merge.sh\nindex f524d2f..d33d714 100755\n--- a/t/t9114-git-svn-dcommit-merge.sh\n+++ b/t/t9114-git-svn-dcommit-merge.sh\n@@ -62,7 +62,7 @@ test_expect_success 'setup git mirror and merge' '\n \techo friend > README &&\n \tcat tmp >> README &&\n \tgit commit -a -m \"friend\" &&\n-\tgit pull . merge\n+\tgit merge merge\n \t'\n \n test_debug 'gitk --all & sleep 1'\ndiff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh\nindex 718014d..e74b9ab 100755\n--- a/t/t9500-gitweb-standalone-no-errors.sh\n+++ b/t/t9500-gitweb-standalone-no-errors.sh\n@@ -328,7 +328,7 @@ test_expect_success \\\n \t git add b &&\n \t git commit -a -m \"On branch\" &&\n \t git checkout master &&\n-\t git pull . b &&\n+\t git merge b &&\n \t git tag merge_commit'\n \n test_expect_success \\\n-- \n1.8.4.2+fc1\n"},{"id":"229881","messageId":"1383211547-9145-4-git-send-email-felipe.contreras@gmail.com","threadId":"35252","inReplyTo":"1383211547-9145-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 03/16] pull: cleanup documentation","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-31T09:25:34Z","receivedAt":"2013-10-31T09:25:34Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"'origin/master' is very clear, no need to specify the 'remotes/' prefix,\nor babysit the user.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n Documentation/git-pull.txt | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt\nindex beea10b..03a39bc 100644\n--- a/Documentation/git-pull.txt\n+++ b/Documentation/git-pull.txt\n@@ -39,7 +39,7 @@ Assume the following history exists and the current branch is\n \"`master`\":\n \n ------------\n-\t  A---B---C master on origin\n+\t  A---B---C origin/master\n \t /\n     D---E---F---G master\n ------------\n@@ -51,7 +51,7 @@ result in a new commit along with the names of the two parent commits\n and a log message from the user describing the changes.\n \n ------------\n-\t  A---B---C remotes/origin/master\n+\t  A---B---C origin/master\n \t /         \\\n     D---E---F---G---H master\n ------------\n-- \n1.8.4.2+fc1\n"},{"id":"229884","messageId":"1383211547-9145-5-git-send-email-felipe.contreras@gmail.com","threadId":"35252","inReplyTo":"1383211547-9145-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 04/16] fetch: add missing documentation","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-31T09:25:35Z","receivedAt":"2013-10-31T09:25:35Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"There's no mention of the 'origin' default, or the fact that the\nupstream tracking branch remote is used.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n Documentation/git-fetch.txt | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt\nindex e08a028..7e75dc4 100644\n--- a/Documentation/git-fetch.txt\n+++ b/Documentation/git-fetch.txt\n@@ -37,6 +37,9 @@ or from several repositories at once if <group> is given and\n there is a remotes.<group> entry in the configuration file.\n (See linkgit:git-config[1]).\n \n+When no remote is specified, by the default the `origin` remote will be used,\n+unless there's an upstream branch configured for the current branch.\n+\n OPTIONS\n -------\n include::fetch-options.txt[]\n-- \n1.8.4.2+fc1\n"},{"id":"229883","messageId":"1383211547-9145-6-git-send-email-felipe.contreras@gmail.com","threadId":"35252","inReplyTo":"1383211547-9145-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 05/16] revision: add missing include","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-31T09:25:36Z","receivedAt":"2013-10-31T09:25:36Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Otherwise we might not have 'struct diff_options'.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n revision.h | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/revision.h b/revision.h\nindex e7f1d21..89132df 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -5,6 +5,7 @@\n #include \"grep.h\"\n #include \"notes.h\"\n #include \"commit.h\"\n+#include \"diff.h\"\n \n #define SEEN\t\t(1u<<0)\n #define UNINTERESTING   (1u<<1)\n-- \n1.8.4.2+fc1\n"},{"id":"229882","messageId":"1383211547-9145-7-git-send-email-felipe.contreras@gmail.com","threadId":"35252","inReplyTo":"1383211547-9145-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 06/16] shortlog: add missing declaration","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-31T09:25:37Z","receivedAt":"2013-10-31T09:25:37Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Otherwise we would have to include commit.h.\n\nSigned-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n shortlog.h | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/shortlog.h b/shortlog.h\nindex de4f86f..54bc07c 100644\n--- a/shortlog.h\n+++ b/shortlog.h\n@@ -19,6 +19,8 @@ struct shortlog {\n \tstruct string_list mailmap;\n };\n \n+struct commit;\n+\n void shortlog_init(struct shortlog *log);\n \n void shortlog_add_commit(struct shortlog *log, struct commit *commit);\n-- \n1.8.4.2+fc1\n"},{"id":"229885","messageId":"1383211547-9145-8-git-send-email-felipe.contreras@gmail.com","threadId":"35252","inReplyTo":"1383211547-9145-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 07/16] branch: trivial style fix","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-31T09:25:38Z","receivedAt":"2013-10-31T09:25:38Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n builtin/branch.c | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex ad0f86d..5696cf0 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -975,9 +975,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \t\t\tdie(_(\"no such branch '%s'\"), argv[0]);\n \t\t}\n \n-\t\tif (!branch_has_merge_config(branch)) {\n+\t\tif (!branch_has_merge_config(branch))\n \t\t\tdie(_(\"Branch '%s' has no upstream information\"), branch->name);\n-\t\t}\n \n \t\tstrbuf_addf(&buf, \"branch.%s.remote\", branch->name);\n \t\tgit_config_set_multivar(buf.buf, NULL, NULL, 1);\n-- \n1.8.4.2+fc1\n"},{"id":"229886","messageId":"1383211547-9145-9-git-send-email-felipe.contreras@gmail.com","threadId":"35252","inReplyTo":"1383211547-9145-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 08/16] sha1-name: trivial style cleanup","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-31T09:25:39Z","receivedAt":"2013-10-31T09:25:39Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n sha1_name.c | 1 -\n 1 file changed, 1 deletion(-)\n\ndiff --git a/sha1_name.c b/sha1_name.c\nindex 0e5fe7f..e9c2999 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -343,7 +343,6 @@ static int get_short_sha1(const char *name, int len, unsigned char *sha1,\n \treturn status;\n }\n \n-\n int for_each_abbrev(const char *prefix, each_abbrev_fn fn, void *cb_data)\n {\n \tchar hex_pfx[40];\n-- \n1.8.4.2+fc1\n"},{"id":"229887","messageId":"1383211547-9145-10-git-send-email-felipe.contreras@gmail.com","threadId":"35252","inReplyTo":"1383211547-9145-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 09/16] transport-helper: trivial style fix","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-31T09:25:40Z","receivedAt":"2013-10-31T09:25:40Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n transport-helper.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/transport-helper.c b/transport-helper.c\nindex b32e2d6..673b7c2 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -269,6 +269,7 @@ static const char *unsupported_options[] = {\n \tTRANS_OPT_THIN,\n \tTRANS_OPT_KEEP\n \t};\n+\n static const char *boolean_options[] = {\n \tTRANS_OPT_THIN,\n \tTRANS_OPT_KEEP,\n-- \n1.8.4.2+fc1\n"},{"id":"229890","messageId":"1383211547-9145-11-git-send-email-felipe.contreras@gmail.com","threadId":"35252","inReplyTo":"1383211547-9145-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 10/16] describe: trivial style fixes","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-31T09:25:41Z","receivedAt":"2013-10-31T09:25:41Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n builtin/describe.c | 7 +++----\n 1 file changed, 3 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/describe.c b/builtin/describe.c\nindex b9d3603..6f62109 100644\n--- a/builtin/describe.c\n+++ b/builtin/describe.c\n@@ -9,7 +9,7 @@\n #include \"hash.h\"\n #include \"argv-array.h\"\n \n-#define SEEN\t\t(1u<<0)\n+#define SEEN\t\t(1u << 0)\n #define MAX_TAGS\t(FLAG_BITS - 1)\n \n static const char * const describe_usage[] = {\n@@ -36,7 +36,6 @@ static const char *diff_index_args[] = {\n \t\"diff-index\", \"--quiet\", \"HEAD\", \"--\", NULL\n };\n \n-\n struct commit_name {\n \tstruct commit_name *next;\n \tunsigned char peeled[20];\n@@ -46,6 +45,7 @@ struct commit_name {\n \tunsigned char sha1[20];\n \tchar *path;\n };\n+\n static const char *prio_names[] = {\n \t\"head\", \"lightweight\", \"annotated\",\n };\n@@ -488,9 +488,8 @@ int cmd_describe(int argc, const char **argv, const char *prefix)\n \t} else if (dirty) {\n \t\tdie(_(\"--dirty is incompatible with commit-ishes\"));\n \t} else {\n-\t\twhile (argc-- > 0) {\n+\t\twhile (argc-- > 0)\n \t\t\tdescribe(*argv++, argc == 0);\n-\t\t}\n \t}\n \treturn 0;\n }\n-- \n1.8.4.2+fc1\n"},{"id":"229889","messageId":"1383211547-9145-12-git-send-email-felipe.contreras@gmail.com","threadId":"35252","inReplyTo":"1383211547-9145-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 11/16] pretty: trivial style fix","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-31T09:25:42Z","receivedAt":"2013-10-31T09:25:42Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n pretty.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/pretty.c b/pretty.c\nindex b4e32b7..962e82b 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -497,7 +497,7 @@ void pp_user_info(struct pretty_print_context *pp,\n static int is_empty_line(const char *line, int *len_p)\n {\n \tint len = *len_p;\n-\twhile (len && isspace(line[len-1]))\n+\twhile (len && isspace(line[len - 1]))\n \t\tlen--;\n \t*len_p = len;\n \treturn !len;\n-- \n1.8.4.2+fc1\n"},{"id":"229888","messageId":"1383211547-9145-13-git-send-email-felipe.contreras@gmail.com","threadId":"35252","inReplyTo":"1383211547-9145-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 12/16] revision: trivial style fixes","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-31T09:25:43Z","receivedAt":"2013-10-31T09:25:43Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n revision.c | 14 ++++++--------\n 1 file changed, 6 insertions(+), 8 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex 3fdea51..956040c 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1519,7 +1519,7 @@ struct cmdline_pathspec {\n static void append_prune_data(struct cmdline_pathspec *prune, const char **av)\n {\n \twhile (*av) {\n-\t\tALLOC_GROW(prune->path, prune->nr+1, prune->alloc);\n+\t\tALLOC_GROW(prune->path, prune->nr + 1, prune->alloc);\n \t\tprune->path[prune->nr++] = *(av++);\n \t}\n }\n@@ -1531,7 +1531,7 @@ static void read_pathspec_from_stdin(struct rev_info *revs, struct strbuf *sb,\n \t\tint len = sb->len;\n \t\tif (len && sb->buf[len - 1] == '\\n')\n \t\t\tsb->buf[--len] = '\\0';\n-\t\tALLOC_GROW(prune->path, prune->nr+1, prune->alloc);\n+\t\tALLOC_GROW(prune->path, prune->nr + 1, prune->alloc);\n \t\tprune->path[prune->nr++] = xstrdup(sb->buf);\n \t}\n }\n@@ -2134,7 +2134,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \t\t *\tcall init_pathspec() to set revs->prune_data here.\n \t\t * }\n \t\t */\n-\t\tALLOC_GROW(prune_data.path, prune_data.nr+1, prune_data.alloc);\n+\t\tALLOC_GROW(prune_data.path, prune_data.nr + 1, prune_data.alloc);\n \t\tprune_data.path[prune_data.nr++] = NULL;\n \t\tparse_pathspec(&revs->prune_data, 0, 0,\n \t\t\t       revs->prefix, prune_data.path);\n@@ -2987,7 +2987,7 @@ static struct commit *get_revision_internal(struct rev_info *revs)\n \tif (revs->max_count) {\n \t\tc = get_revision_1(revs);\n \t\tif (c) {\n-\t\t\twhile (0 < revs->skip_count) {\n+\t\t\twhile (revs->skip_count > 0) {\n \t\t\t\trevs->skip_count--;\n \t\t\t\tc = get_revision_1(revs);\n \t\t\t\tif (!c)\n@@ -3002,9 +3002,8 @@ static struct commit *get_revision_internal(struct rev_info *revs)\n \tif (c)\n \t\tc->object.flags |= SHOWN;\n \n-\tif (!revs->boundary) {\n+\tif (!revs->boundary)\n \t\treturn c;\n-\t}\n \n \tif (!c) {\n \t\t/*\n@@ -3050,9 +3049,8 @@ struct commit *get_revision(struct rev_info *revs)\n \n \tif (revs->reverse) {\n \t\treversed = NULL;\n-\t\twhile ((c = get_revision_internal(revs))) {\n+\t\twhile ((c = get_revision_internal(revs)))\n \t\t\tcommit_list_insert(c, &reversed);\n-\t\t}\n \t\trevs->commits = reversed;\n \t\trevs->reverse = 0;\n \t\trevs->reverse_output_stage = 1;\n-- \n1.8.4.2+fc1\n"},{"id":"229891","messageId":"1383211547-9145-14-git-send-email-felipe.contreras@gmail.com","threadId":"35252","inReplyTo":"1383211547-9145-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 13/16] diff: trivial style fix","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-31T09:25:44Z","receivedAt":"2013-10-31T09:25:44Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n builtin/diff.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/diff.c b/builtin/diff.c\nindex 2fb8c5d..adb93a9 100644\n--- a/builtin/diff.c\n+++ b/builtin/diff.c\n@@ -169,7 +169,7 @@ static int builtin_diff_tree(struct rev_info *revs,\n \tif (ent1->item->flags & UNINTERESTING)\n \t\tswap = 1;\n \tsha1[swap] = ent0->item->sha1;\n-\tsha1[1-swap] = ent1->item->sha1;\n+\tsha1[1 - swap] = ent1->item->sha1;\n \tdiff_tree_sha1(sha1[0], sha1[1], \"\", &revs->diffopt);\n \tlog_tree_diff_flush(revs);\n \treturn 0;\n-- \n1.8.4.2+fc1\n"},{"id":"229894","messageId":"1383211547-9145-15-git-send-email-felipe.contreras@gmail.com","threadId":"35252","inReplyTo":"1383211547-9145-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 14/16] run-command: trivial style fixes","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-31T09:25:45Z","receivedAt":"2013-10-31T09:25:45Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n run-command.c | 13 +++++--------\n 1 file changed, 5 insertions(+), 8 deletions(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex 1b7f88e..3914d9c 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -406,13 +406,12 @@ fail_pipe:\n \t\t\t\t\tunsetenv(*cmd->env);\n \t\t\t}\n \t\t}\n-\t\tif (cmd->git_cmd) {\n+\t\tif (cmd->git_cmd)\n \t\t\texecv_git_cmd(cmd->argv);\n-\t\t} else if (cmd->use_shell) {\n+\t\telse if (cmd->use_shell)\n \t\t\texecv_shell_cmd(cmd->argv);\n-\t\t} else {\n+\t\telse\n \t\t\tsane_execvp(cmd->argv[0], (char *const*) cmd->argv);\n-\t\t}\n \t\tif (errno == ENOENT) {\n \t\t\tif (!cmd->silent_exec_failure)\n \t\t\t\terror(\"cannot run %s: %s\", cmd->argv[0],\n@@ -446,7 +445,6 @@ fail_pipe:\n \t\tcmd->pid = -1;\n \t}\n \tclose(notify_pipe[0]);\n-\n }\n #else\n {\n@@ -480,11 +478,10 @@ fail_pipe:\n \tif (cmd->env)\n \t\tenv = make_augmented_environ(cmd->env);\n \n-\tif (cmd->git_cmd) {\n+\tif (cmd->git_cmd)\n \t\tcmd->argv = prepare_git_cmd(cmd->argv);\n-\t} else if (cmd->use_shell) {\n+\telse if (cmd->use_shell)\n \t\tcmd->argv = prepare_shell_cmd(cmd->argv);\n-\t}\n \n \tcmd->pid = mingw_spawnvpe(cmd->argv[0], cmd->argv, env, cmd->dir,\n \t\t\t\t  fhin, fhout, fherr);\n-- \n1.8.4.2+fc1\n"},{"id":"229893","messageId":"1383211547-9145-16-git-send-email-felipe.contreras@gmail.com","threadId":"35252","inReplyTo":"1383211547-9145-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 15/16] setup: trivial style fixes","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-31T09:25:46Z","receivedAt":"2013-10-31T09:25:46Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n setup.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/setup.c b/setup.c\nindex dbf4138..5432a31 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -563,7 +563,7 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)\n {\n \tconst char *env_ceiling_dirs = getenv(CEILING_DIRECTORIES_ENVIRONMENT);\n \tstruct string_list ceiling_dirs = STRING_LIST_INIT_DUP;\n-\tstatic char cwd[PATH_MAX+1];\n+\tstatic char cwd[PATH_MAX + 1];\n \tconst char *gitdirenv, *ret;\n \tchar *gitfile;\n \tint len, offset, offset_parent, ceil_offset = -1;\n@@ -578,7 +578,7 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)\n \tif (nongit_ok)\n \t\t*nongit_ok = 0;\n \n-\tif (!getcwd(cwd, sizeof(cwd)-1))\n+\tif (!getcwd(cwd, sizeof(cwd) - 1))\n \t\tdie_errno(\"Unable to read current working directory\");\n \toffset = len = strlen(cwd);\n \n-- \n1.8.4.2+fc1\n"},{"id":"229892","messageId":"1383211547-9145-17-git-send-email-felipe.contreras@gmail.com","threadId":"35252","inReplyTo":"1383211547-9145-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 16/16] add: avoid yoda conditions","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-31T09:25:47Z","receivedAt":"2013-10-31T09:25:47Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n---\n builtin/add.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 226f758..9b30356 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -429,7 +429,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \targc--;\n \targv++;\n \n-\tif (0 <= addremove_explicit)\n+\tif (addremove_explicit >= 0)\n \t\taddremove = addremove_explicit;\n \telse if (take_worktree_changes && ADDREMOVE_DEFAULT)\n \t\taddremove = 0; /* \"-u\" was given but not \"-A\" */\n-- \n1.8.4.2+fc1\n"},{"id":"229928","messageId":"87E9F831-E3BB-43AE-BC9F-C32C46908CBB@quendi.de","threadId":"35252","inReplyTo":"1383211547-9145-1-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH 00/16] Trivial patches","fromName":"Max Horn","fromEmail":"max@quendi.de","sentAt":"2013-10-31T18:03:27Z","receivedAt":"2013-10-31T18:03:27Z","isPatch":true,"sender":{"key":"max@quendi.de","avatar":"https://avatars.githubusercontent.com/u/241512?v=4"},"body":"\nOn 31.10.2013, at 10:25, Felipe Contreras <felipe.contreras@gmail.com> wrote:\n\n> Most of these have been sent before, but were not applied for one reason or\n> another.\n\nAll of these look fine and sensible to me. Some of the latter patches in the series might be a bit subjective (e.g. I personally don't mind \"yoda\" conditions at all), but none do harm, and most are a clear improvement. So I am all for applying this.\n\nCheers,\nMax\n\n> \n> Felipe Contreras (16):\n>  merge: simplify ff-only option\n>  t: replace pulls with merges\n>  pull: cleanup documentation\n>  fetch: add missing documentation\n>  revision: add missing include\n>  shortlog: add missing declaration\n>  branch: trivial style fix\n>  sha1-name: trivial style cleanup\n>  transport-helper: trivial style fix\n>  describe: trivial style fixes\n>  pretty: trivial style fix\n>  revision: trivial style fixes\n>  diff: trivial style fix\n>  run-command: trivial style fixes\n>  setup: trivial style fixes\n>  add: avoid yoda conditions\n> \n> Documentation/git-fetch.txt            |  3 +++\n> Documentation/git-pull.txt             |  4 ++--\n> builtin/add.c                          |  2 +-\n> builtin/branch.c                       |  3 +--\n> builtin/describe.c                     |  7 +++----\n> builtin/diff.c                         |  2 +-\n> builtin/merge.c                        | 11 ++---------\n> pretty.c                               |  2 +-\n> revision.c                             | 14 ++++++--------\n> revision.h                             |  1 +\n> run-command.c                          | 13 +++++--------\n> setup.c                                |  4 ++--\n> sha1_name.c                            |  1 -\n> shortlog.h                             |  2 ++\n> t/annotate-tests.sh                    |  2 +-\n> t/t4200-rerere.sh                      |  2 +-\n> t/t9114-git-svn-dcommit-merge.sh       |  2 +-\n> t/t9500-gitweb-standalone-no-errors.sh |  2 +-\n> transport-helper.c                     |  1 +\n> 19 files changed, 35 insertions(+), 43 deletions(-)\n> \n> -- \n> 1.8.4.2+fc1\n> \n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n> \n\n"},{"id":"229929","messageId":"xmqqhabx8ggs.fsf@gitster.dls.corp.google.com","threadId":"35252","inReplyTo":"1383211547-9145-2-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH 01/16] merge: simplify ff-only option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-31T18:04:19Z","receivedAt":"2013-10-31T18:04:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n> No functional changes.\n>\n> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n> ---\n>  builtin/merge.c | 11 ++---------\n>  1 file changed, 2 insertions(+), 9 deletions(-)\n>\n> diff --git a/builtin/merge.c b/builtin/merge.c\n> index 02a69c1..41fb66d 100644\n> --- a/builtin/merge.c\n> +++ b/builtin/merge.c\n> @@ -186,13 +186,6 @@ static int option_parse_n(const struct option *opt,\n>  \treturn 0;\n>  }\n>  \n> -static int option_parse_ff_only(const struct option *opt,\n> -\t\t\t  const char *arg, int unset)\n> -{\n> -\tfast_forward = FF_ONLY;\n> -\treturn 0;\n> -}\n> -\n>  static struct option builtin_merge_options[] = {\n>  \t{ OPTION_CALLBACK, 'n', NULL, NULL, NULL,\n>  \t\tN_(\"do not show a diffstat at the end of the merge\"),\n> @@ -210,9 +203,9 @@ static struct option builtin_merge_options[] = {\n>  \tOPT_BOOL('e', \"edit\", &option_edit,\n>  \t\tN_(\"edit message before committing\")),\n>  \tOPT_SET_INT(0, \"ff\", &fast_forward, N_(\"allow fast-forward (default)\"), FF_ALLOW),\n> -\t{ OPTION_CALLBACK, 0, \"ff-only\", NULL, NULL,\n> +\t{ OPTION_SET_INT, 0, \"ff-only\", &fast_forward, NULL,\n>  \t\tN_(\"abort if fast-forward is not possible\"),\n> -\t\tPARSE_OPT_NOARG | PARSE_OPT_NONEG, option_parse_ff_only },\n> +\t\tPARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, FF_ONLY },\n>  \tOPT_RERERE_AUTOUPDATE(&allow_rerere_auto),\n>  \tOPT_BOOL(0, \"verify-signatures\", &verify_signatures,\n>  \t\tN_(\"Verify that the named commit has a valid GPG signature\")),\n\nLooks good; thanks.\n"},{"id":"229933","messageId":"xmqqwqkt71l8.fsf@gitster.dls.corp.google.com","threadId":"35252","inReplyTo":"1383211547-9145-5-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH 04/16] fetch: add missing documentation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-31T18:10:59Z","receivedAt":"2013-10-31T18:10:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n> There's no mention of the 'origin' default, or the fact that the\n> upstream tracking branch remote is used.\n>\n> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n> ---\n>  Documentation/git-fetch.txt | 3 +++\n>  1 file changed, 3 insertions(+)\n>\n> diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt\n> index e08a028..7e75dc4 100644\n> --- a/Documentation/git-fetch.txt\n> +++ b/Documentation/git-fetch.txt\n> @@ -37,6 +37,9 @@ or from several repositories at once if <group> is given and\n>  there is a remotes.<group> entry in the configuration file.\n>  (See linkgit:git-config[1]).\n>  \n> +When no remote is specified, by the default the `origin` remote will be used,\n\nI recall there were typofix comments on this line.\n\n> +unless there's an upstream branch configured for the current branch.\n\nAlso there was a phrasing comment on this one, I think.\n\nResending without rerolling is not very much appreciated.\n\n\n> +\n>  OPTIONS\n>  -------\n>  include::fetch-options.txt[]\n"},{"id":"229934","messageId":"xmqqppql71l0.fsf@gitster.dls.corp.google.com","threadId":"35252","inReplyTo":"1383211547-9145-6-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH 05/16] revision: add missing include","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-31T18:11:07Z","receivedAt":"2013-10-31T18:11:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n> Otherwise we might not have 'struct diff_options'.\n>\n> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n> ---\n>  revision.h | 1 +\n>  1 file changed, 1 insertion(+)\n>\n> diff --git a/revision.h b/revision.h\n> index e7f1d21..89132df 100644\n> --- a/revision.h\n> +++ b/revision.h\n> @@ -5,6 +5,7 @@\n>  #include \"grep.h\"\n>  #include \"notes.h\"\n>  #include \"commit.h\"\n> +#include \"diff.h\"\n>  \n>  #define SEEN\t\t(1u<<0)\n>  #define UNINTERESTING   (1u<<1)\n\nThis is a step in the right direction to change the contract between\nthis header file and its consumers, but I think it falls short of\ndoing a good job at it.\n\nThe rule used to be that \"if you use a declaration in revision.h,\nyou must include diff.h before including it, even if you do not use\nany declaration made in diff.h yourself\". \n\nThe new rule this patch introduces is \"if you use a declaration in\nfoo.h, include foo.h, period---foo.h should handle its requirement\non its own internally and consumers should not have to care\", which\nis much saner.\n\nBut the patch needs to also remove '#include \"diff.h\"' from existing\nconsumers that themselves do not use any declaration from \"diff.h\"\n(e.g. bundle.c; there are others), while keeping the inclusion in\nthose that do (e.g. builtin/commit.c). That can be a separate patch\nthat immediately follow this one, or a part of the same patch.\n\nThanks.\n"},{"id":"229935","messageId":"xmqqiowd71kv.fsf@gitster.dls.corp.google.com","threadId":"35252","inReplyTo":"1383211547-9145-4-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH 03/16] pull: cleanup documentation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-31T18:11:12Z","receivedAt":"2013-10-31T18:11:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n> 'origin/master' is very clear, no need to specify the 'remotes/' prefix,\n> or babysit the user.\n>\n> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n> ---\n>  Documentation/git-pull.txt | 4 ++--\n>  1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt\n> index beea10b..03a39bc 100644\n> --- a/Documentation/git-pull.txt\n> +++ b/Documentation/git-pull.txt\n> @@ -39,7 +39,7 @@ Assume the following history exists and the current branch is\n>  \"`master`\":\n>  \n>  ------------\n> -\t  A---B---C master on origin\n> +\t  A---B---C origin/master\n>  \t /\n>      D---E---F---G master\n>  ------------\n\nThis change is wrong; the illustration depicts the distributed world\n(i.e. a fetch has not happened yet).  The next sentence after this\npicture reads:\n\n    Then \"`git pull`\" will fetch and replay the changes from the remote\n    `master` branch since it diverged from the local `master` \n\nIn other words, your (remotes/)origin/master has _not_ caught up to\nthe reality.\n\n> @@ -51,7 +51,7 @@ result in a new commit along with the names of the two parent commits\n>  and a log message from the user describing the changes.\n>  \n>  ------------\n> -\t  A---B---C remotes/origin/master\n> +\t  A---B---C origin/master\n>  \t /         \\\n>      D---E---F---G---H master\n>  ------------\n\nThis is a good change, especially in today's world.\n\nThanks.\n"},{"id":"229943","messageId":"CAMP44s3AKnZUJy0O0Hg+G=ER2bRn1nHMBeoJQ=ugCzO4d2WZ-g@mail.gmail.com","threadId":"35252","inReplyTo":"xmqqiowd71kv.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 03/16] pull: cleanup documentation","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-31T18:37:24Z","receivedAt":"2013-10-31T18:37:24Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Thu, Oct 31, 2013 at 12:11 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n>> --- a/Documentation/git-pull.txt\n>> +++ b/Documentation/git-pull.txt\n>> @@ -39,7 +39,7 @@ Assume the following history exists and the current branch is\n>>  \"`master`\":\n>>\n>>  ------------\n>> -       A---B---C master on origin\n>> +       A---B---C origin/master\n>>        /\n>>      D---E---F---G master\n>>  ------------\n>\n> This change is wrong; the illustration depicts the distributed world\n> (i.e. a fetch has not happened yet).\n\nThat is an irrelevant implementation detail, specially at this high\nlevel. In the user's mind origin/master means master on origin.\n\nIf you want to be pedantic, this is the \"reality\":\n\n  ------------\n      D---E---F---G master\n  ------------\n\n-- \nFelipe Contreras\n"},{"id":"229951","messageId":"xmqqob655kqi.fsf@gitster.dls.corp.google.com","threadId":"35252","inReplyTo":"CAMP44s3AKnZUJy0O0Hg+G=ER2bRn1nHMBeoJQ=ugCzO4d2WZ-g@mail.gmail.com","subject":"Re: [PATCH 03/16] pull: cleanup documentation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-31T19:00:21Z","receivedAt":"2013-10-31T19:00:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n> On Thu, Oct 31, 2013 at 12:11 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>\n>>> --- a/Documentation/git-pull.txt\n>>> +++ b/Documentation/git-pull.txt\n>>> @@ -39,7 +39,7 @@ Assume the following history exists and the current branch is\n>>>  \"`master`\":\n>>>\n>>>  ------------\n>>> -       A---B---C master on origin\n>>> +       A---B---C origin/master\n>>>        /\n>>>      D---E---F---G master\n>>>  ------------\n>>\n>> This change is wrong; the illustration depicts the distributed world\n>> (i.e. a fetch has not happened yet).\n>\n> That is an irrelevant implementation detail, specially at this high\n> level. In the user's mind origin/master means master on origin.\n\nYou are wrong.  In the user's mind, origin/master means the commit\nthat used to be at master on origin, and the point of this\nillustration is to make them understand that they live in a\ndistributed world, where their last observation will go stale over\ntime.\n\n>\n> If you want to be pedantic, this is the \"reality\":\n>\n>   ------------\n>       D---E---F---G master\n>   ------------\n\nYou are wrong again.  The \"reality\" is more like this:\n\n      origin/master in your repository\n      |\n      v\n      A---B---C master at origin\n     /\n    D---E---F---G master in your repository\n\nif you really want to write origin/master somewhere in this\nillustration.\n"},{"id":"229952","messageId":"xmqqk3gt5kht.fsf@gitster.dls.corp.google.com","threadId":"35252","inReplyTo":"1383211547-9145-7-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH 06/16] shortlog: add missing declaration","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-31T19:05:34Z","receivedAt":"2013-10-31T19:05:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n> Otherwise we would have to include commit.h.\n\nWas there a reason why commit.h is not included here, just like\nrevision.h would include diff.h, so that users of shortlog.h do not\nhave to worry about including commit.h themselves?\n\nNote: not requesting the patch to be changed; just inquiring the\nreasoning behind a different approach to solve related/same problem.\n\n> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n> ---\n>  shortlog.h | 2 ++\n>  1 file changed, 2 insertions(+)\n>\n> diff --git a/shortlog.h b/shortlog.h\n> index de4f86f..54bc07c 100644\n> --- a/shortlog.h\n> +++ b/shortlog.h\n> @@ -19,6 +19,8 @@ struct shortlog {\n>  \tstruct string_list mailmap;\n>  };\n>  \n> +struct commit;\n> +\n>  void shortlog_init(struct shortlog *log);\n>  \n>  void shortlog_add_commit(struct shortlog *log, struct commit *commit);\n"},{"id":"229954","messageId":"CAMP44s13kADwM0MudcEx=4Pk_rG0_tom2jfsEh2bz9_QXWAK5Q@mail.gmail.com","threadId":"35252","inReplyTo":"xmqqwqkt71l8.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 04/16] fetch: add missing documentation","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-31T19:08:31Z","receivedAt":"2013-10-31T19:08:31Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Thu, Oct 31, 2013 at 12:10 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n>> diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt\n>> index e08a028..7e75dc4 100644\n>> --- a/Documentation/git-fetch.txt\n>> +++ b/Documentation/git-fetch.txt\n>> @@ -37,6 +37,9 @@ or from several repositories at once if <group> is given and\n>>  there is a remotes.<group> entry in the configuration file.\n>>  (See linkgit:git-config[1]).\n>>\n>> +When no remote is specified, by the default the `origin` remote will be used,\n>\n> I recall there were typofix comments on this line.\n\n--- a/Documentation/git-fetch.txt\n+++ b/Documentation/git-fetch.txt\n@@ -37,7 +37,7 @@ or from several repositories at once if <group> is given and\n there is a remotes.<group> entry in the configuration file.\n (See linkgit:git-config[1]).\n\n-When no remote is specified, by the default the `origin` remote will be used,\n+When no remote is specified, by default the `origin` remote will be used,\n unless there's an upstream branch configured for the current branch.\n\n OPTIONS\n\n>> +unless there's an upstream branch configured for the current branch.\n>\n> Also there was a phrasing comment on this one, I think.\n\nThere was no constructive comment, no alternative was proposed.\n\nThe conclusion of the discussion (at least mine) is that the phrasing is fine.\n\n> Resending without rerolling is not very much appreciated.\n\nI missed a valid comment in one of my 160 pending patches. Sue me.\n\n-- \nFelipe Contreras\n"},{"id":"229955","messageId":"ADB546B4-992E-4B54-953E-B721BECD19B2@quendi.de","threadId":"35252","inReplyTo":"xmqqob655kqi.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 03/16] pull: cleanup documentation","fromName":"Max Horn","fromEmail":"max@quendi.de","sentAt":"2013-10-31T19:27:19Z","receivedAt":"2013-10-31T19:27:19Z","isPatch":true,"sender":{"key":"max@quendi.de","avatar":"https://avatars.githubusercontent.com/u/241512?v=4"},"body":"\nOn 31.10.2013, at 20:00, Junio C Hamano <gitster@pobox.com> wrote:\n\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n[...]\n\n> \n>> \n>> If you want to be pedantic, this is the \"reality\":\n>> \n>>  ------------\n>>      D---E---F---G master\n>>  ------------\n> \n> You are wrong again.  The \"reality\" is more like this:\n> \n>      origin/master in your repository\n>      |\n>      v\n>      A---B---C master at origin\n>     /\n>    D---E---F---G master in your repository\n> \n> if you really want to write origin/master somewhere in this\n> illustration.\n\nActually, I kind of like that. After just reading the existing phrasing in git-pull.txt, I doubt that a newbie would catch the difference between \"origin/master\" and \"master at origin\". With this illustration, it's very clearly conveyed that there is a difference.\n\n"},{"id":"229958","messageId":"CAMP44s2zTFKeHxp0G1imLXdHfzao_DSY+mkm7awYedwYafFZ+w@mail.gmail.com","threadId":"35252","inReplyTo":"xmqqk3gt5kht.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 06/16] shortlog: add missing declaration","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-31T19:33:34Z","receivedAt":"2013-10-31T19:33:34Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Thu, Oct 31, 2013 at 1:05 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>\n>> Otherwise we would have to include commit.h.\n>\n> Was there a reason why commit.h is not included here, just like\n> revision.h would include diff.h, so that users of shortlog.h do not\n> have to worry about including commit.h themselves?\n\nBecause you can't do:\n\nstruct diff_options;\nstruct diff_options diffopt;\n\nThe storage size is not known, but you can do:\n\nstruct diff_options;\nstruct diff_options *diffopt;\n\n-- \nFelipe Contreras\n"},{"id":"229962","messageId":"xmqqzjpp447x.fsf@gitster.dls.corp.google.com","threadId":"35252","inReplyTo":"ADB546B4-992E-4B54-953E-B721BECD19B2@quendi.de","subject":"Re: [PATCH 03/16] pull: cleanup documentation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-31T19:42:26Z","receivedAt":"2013-10-31T19:42:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Max Horn <max@quendi.de> writes:\n\n>> ... The \"reality\" is more like this:\n>> \n>>      origin/master in your repository\n>>      |\n>>      v\n>>      A---B---C master at origin\n>>     /\n>>    D---E---F---G master in your repository\n>> \n>> if you really want to write origin/master somewhere in this\n>> illustration.\n>\n> Actually, I kind of like that. After just reading the existing\n> phrasing in git-pull.txt, I doubt that a newbie would catch the\n> difference between \"origin/master\" and \"master at origin\". With this\n> illustration, it's very clearly conveyed that there is a difference.\n\nYeah, after re-reading the part of the documentation with the\nillustration replaced with the above, I was coming to the same\nconclusion.\n"},{"id":"229964","messageId":"CANiSa6i5z8Z9HPzsUWTh8U2HXc9p6MPgQjJ7K6KSDw8FXtFyww@mail.gmail.com","threadId":"35252","inReplyTo":"1383211547-9145-17-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH 16/16] add: avoid yoda conditions","fromName":"Martin von Zweigbergk","fromEmail":"martinvonz@gmail.com","sentAt":"2013-10-31T19:48:57Z","receivedAt":"2013-10-31T19:48:57Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"I was recently confused by the yoda condition in this block of code from [1]\n\n+ for (i = 0; i < revs.nr; i++)\n+ if (&bases->item->object == &revs.commit[i]->object)\n+ break; /* found */\n+ if (revs.nr <= i)\n\nI think I was particularly surprised because it came so soon after the\n\"i < revs.nr\". I didn't bother commenting because it seemed too\nsubjective and the code base has tons of these. Something as simple as\n\n  git grep '[0-9] [<>]' *.c\n\nfinds a bunch (probably with lots of false positives and negatives).\n\nI guess what I'm trying to say is that either we accept them and get\nused to reading them without being surprised, or we can change a bit\nmore than one at a time perhaps? I understand that this was an\noccurrence you just happened to run into, and I'm not saying that a\npatch has to deal with _all_ occurrences. I'm more just wondering if\nwe want mention our position, whatever it is, in CodingGuidelines.\n\nMartin\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/236252/focus=236716\n\nOn Thu, Oct 31, 2013 at 2:25 AM, Felipe Contreras\n<felipe.contreras@gmail.com> wrote:\n> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>\n> ---\n>  builtin/add.c | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/builtin/add.c b/builtin/add.c\n> index 226f758..9b30356 100644\n> --- a/builtin/add.c\n> +++ b/builtin/add.c\n> @@ -429,7 +429,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n>         argc--;\n>         argv++;\n>\n> -       if (0 <= addremove_explicit)\n> +       if (addremove_explicit >= 0)\n>                 addremove = addremove_explicit;\n>         else if (take_worktree_changes && ADDREMOVE_DEFAULT)\n>                 addremove = 0; /* \"-u\" was given but not \"-A\" */\n> --\n> 1.8.4.2+fc1\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n"},{"id":"229965","messageId":"CAMP44s1-pVDoLrEC9m0J+fRCxnvMb+P0ANMxS7vzBhFub_Yv6Q@mail.gmail.com","threadId":"35252","inReplyTo":"xmqqob655kqi.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 03/16] pull: cleanup documentation","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-31T19:51:42Z","receivedAt":"2013-10-31T19:51:42Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Thu, Oct 31, 2013 at 1:00 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>\n>> On Thu, Oct 31, 2013 at 12:11 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>>> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>>\n>>>> --- a/Documentation/git-pull.txt\n>>>> +++ b/Documentation/git-pull.txt\n>>>> @@ -39,7 +39,7 @@ Assume the following history exists and the current branch is\n>>>>  \"`master`\":\n>>>>\n>>>>  ------------\n>>>> -       A---B---C master on origin\n>>>> +       A---B---C origin/master\n>>>>        /\n>>>>      D---E---F---G master\n>>>>  ------------\n>>>\n>>> This change is wrong; the illustration depicts the distributed world\n>>> (i.e. a fetch has not happened yet).\n>>\n>> That is an irrelevant implementation detail, specially at this high\n>> level. In the user's mind origin/master means master on origin.\n>\n> You are wrong.  In the user's mind, origin/master means the commit\n> that used to be at master on origin, and the point of this\n> illustration is to make them understand that they live in a\n> distributed world, where their last observation will go stale over\n> time.\n\nWrong. That would make sense in 'git fetch', but here the point of the\nillustration is to make them understand what 'git pull' will do,\nnamely a merge.\n\nWhich refs point to C at which points in time irrelevant information,\nthe user wants to know that 'git pull' will create a merge.\n\n>> If you want to be pedantic, this is the \"reality\":\n>>\n>>   ------------\n>>       D---E---F---G master\n>>   ------------\n>\n> You are wrong again.  The \"reality\" is more like this:\n>\n>       origin/master in your repository\n>       |\n>       v\n>       A---B---C master at origin\n>      /\n>     D---E---F---G master in your repository\n>\n> if you really want to write origin/master somewhere in this\n> illustration.\n\nWrong. You probably mean:\n\n------------\n          A---B---C master on origin\n         /\n    D---E origin/master\n         \\\n          F---G master\n------------\n\nBut 'master on origin' doesn't exist in \"reality\" according to you, so:\n\n------------\n    D---E origin/master\n         \\\n          F---G master\n------------\n\n-- \nFelipe Contreras\n"},{"id":"229967","messageId":"CAMP44s3adv0MKeuYU1D1B-kdtWvyN4Dr9-ee_xrtdnAOYaHhGw@mail.gmail.com","threadId":"35252","inReplyTo":"CANiSa6i5z8Z9HPzsUWTh8U2HXc9p6MPgQjJ7K6KSDw8FXtFyww@mail.gmail.com","subject":"Re: [PATCH 16/16] add: avoid yoda conditions","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-31T19:56:35Z","receivedAt":"2013-10-31T19:56:35Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Thu, Oct 31, 2013 at 1:48 PM, Martin von Zweigbergk\n<martinvonz@gmail.com> wrote:\n\n> I guess what I'm trying to say is that either we accept them and get\n> used to reading them without being surprised, or we can change a bit\n> more than one at a time perhaps? I understand that this was an\n> occurrence you just happened to run into, and I'm not saying that a\n> patch has to deal with _all_ occurrences. I'm more just wondering if\n> we want mention our position, whatever it is, in CodingGuidelines.\n\nYes, I'm all in favor of updating CodingGuidelines with that.\n\n-- \nFelipe Contreras\n"},{"id":"229969","messageId":"xmqqvc0d432e.fsf@gitster.dls.corp.google.com","threadId":"35252","inReplyTo":"CAMP44s2zTFKeHxp0G1imLXdHfzao_DSY+mkm7awYedwYafFZ+w@mail.gmail.com","subject":"Re: [PATCH 06/16] shortlog: add missing declaration","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-31T20:07:21Z","receivedAt":"2013-10-31T20:07:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n> On Thu, Oct 31, 2013 at 1:05 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>>\n>>> Otherwise we would have to include commit.h.\n>>\n>> Was there a reason why commit.h is not included here, just like\n>> revision.h would include diff.h, so that users of shortlog.h do not\n>> have to worry about including commit.h themselves?\n>\n> Because you can't do:\n>\n> struct diff_options;\n> struct diff_options diffopt;\n>\n> The storage size is not known, but you can do:\n>\n> struct diff_options;\n> struct diff_options *diffopt;\n\nBut so can you do\n\n\tstruct diff_options *diffopt;\n\nwithout the declaration, no?  That is:\n\n\t$ cat >x.c <<\\EOF\n        struct foo {\n        \tstruct bar *ptr;\n\t};\n\n        int foo_is_null(struct foo *foo)\n        {\n        \treturn foo == 0;\n\t}\n\tEOF\n        $ gcc -Wall -c x.c\n"},{"id":"229970","messageId":"xmqqr4b1425c.fsf@gitster.dls.corp.google.com","threadId":"35252","inReplyTo":"CAMP44s1-pVDoLrEC9m0J+fRCxnvMb+P0ANMxS7vzBhFub_Yv6Q@mail.gmail.com","subject":"Re: [PATCH 03/16] pull: cleanup documentation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-31T20:27:11Z","receivedAt":"2013-10-31T20:27:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n> On Thu, Oct 31, 2013 at 1:00 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>>\n>>> On Thu, Oct 31, 2013 at 12:11 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>>>> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>>>\n>>>>> --- a/Documentation/git-pull.txt\n>>>>> +++ b/Documentation/git-pull.txt\n>>>>> @@ -39,7 +39,7 @@ Assume the following history exists and the current branch is\n>>>>>  \"`master`\":\n>>>>>\n>>>>>  ------------\n>>>>> -       A---B---C master on origin\n>>>>> +       A---B---C origin/master\n>>>>>        /\n>>>>>      D---E---F---G master\n>>>>>  ------------\n>>>>\n>>>> This change is wrong; the illustration depicts the distributed world\n>>>> (i.e. a fetch has not happened yet).\n>>>\n>>> That is an irrelevant implementation detail, specially at this high\n>>> level. In the user's mind origin/master means master on origin.\n>>\n>> You are wrong.  In the user's mind, origin/master means the commit\n>> that used to be at master on origin, and the point of this\n>> illustration is to make them understand that they live in a\n>> distributed world, where their last observation will go stale over\n>> time.\n>\n> Wrong. That would make sense in 'git fetch', but here the point of the\n> illustration is to make them understand what 'git pull' will do,\n> namely a merge.\n>\n> Which refs point to C at which points in time irrelevant information,\n> the user wants to know that 'git pull' will create a merge.\n\nMerge with what, and how do the users know what will be merged?\n\nThink.\n\nThe users need to learn that origin/master they were told to use\nwith \"git log origin/master..\" etc. trails reality, \"git fetch\" is\nhow they can get them in sync, and the reason they do not need to\nrun \"git fetch\" separately when they \"git pull\" is because it is run\nfor them internally.\n\nThat is what the illustration and the paragraph that follows teach\nthem.\n\nI've said enough on this.\n"},{"id":"229971","messageId":"xmqqmwlp41xd.fsf@gitster.dls.corp.google.com","threadId":"35252","inReplyTo":"CANiSa6i5z8Z9HPzsUWTh8U2HXc9p6MPgQjJ7K6KSDw8FXtFyww@mail.gmail.com","subject":"Re: [PATCH 16/16] add: avoid yoda conditions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-31T20:31:58Z","receivedAt":"2013-10-31T20:31:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin von Zweigbergk <martinvonz@gmail.com> writes:\n\n> I was recently confused by the yoda condition in this block of code from [1]\n>\n> + for (i = 0; i < revs.nr; i++)\n> + if (&bases->item->object == &revs.commit[i]->object)\n> + break; /* found */\n> + if (revs.nr <= i)\n>\n> I think I was particularly surprised because it came so soon after the\n> \"i < revs.nr\". I didn't bother commenting because it seemed too\n> subjective and the code base has tons of these.\n\nThat follows \"visual/textual order should follow the actual\nordering\" principle.  Think of a number-line you learn in elementary\nschool arithmetic class, and try to place revs.nr and i on it.\n\nI agree that there is no justification to write \"if 0 == something\",\nwhen \"if something == 0\" suffices.  The latter reads better and that\nis why the phrase \"yoda condition\" was invented.\n\nBut the situation is different when both sides are not constants,\nand especially when \"<\" and \"<=\" are involved..\n"},{"id":"229972","messageId":"CAMP44s3CnoLAUu=R4FCFWGyJpn58oTZgzQKgF7wE1hUnutv84w@mail.gmail.com","threadId":"35252","inReplyTo":"xmqqmwlp41xd.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 16/16] add: avoid yoda conditions","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-31T20:42:42Z","receivedAt":"2013-10-31T20:42:42Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Thu, Oct 31, 2013 at 2:31 PM, Junio C Hamano <gitster@pobox.com> wrote:\n\n> I agree that there is no justification to write \"if 0 == something\",\n> when \"if something == 0\" suffices.  The latter reads better and that\n> is why the phrase \"yoda condition\" was invented.\n>\n> But the situation is different when both sides are not constants,\n> and especially when \"<\" and \"<=\" are involved..\n\nTo me revs.nr is virtually a constant, I'm comparing i to revs.nr, not\nthe other way around.\n\nI believe I explained this already, but here it goes again:\n\nif (1.60 < john.size)\n\nThis makes no sense, \"if 1.69 is smaller than john\"?\n\nThe situation doesn't change when you use a variable:\n\nif (size_limit < john.size)\n\nTranslates to \"if size limit is smaller than john\", and still makes no sense.\n\n-- \nFelipe Contreras\n"},{"id":"229973","messageId":"xmqqfvrh41e0.fsf@gitster.dls.corp.google.com","threadId":"35252","inReplyTo":"xmqqzjpp447x.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 03/16] pull: cleanup documentation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-31T20:43:35Z","receivedAt":"2013-10-31T20:43:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Max Horn <max@quendi.de> writes:\n>\n>>> ... The \"reality\" is more like this:\n>>> \n>>>      origin/master in your repository\n>>>      |\n>>>      v\n>>>      A---B---C master at origin\n>>>     /\n>>>    D---E---F---G master in your repository\n>>> \n>>> if you really want to write origin/master somewhere in this\n>>> illustration.\n>>\n>> Actually, I kind of like that. After just reading the existing\n>> phrasing in git-pull.txt, I doubt that a newbie would catch the\n>> difference between \"origin/master\" and \"master at origin\". With this\n>> illustration, it's very clearly conveyed that there is a difference.\n>\n> Yeah, after re-reading the part of the documentation with the\n> illustration replaced with the above, I was coming to the same\n> conclusion.\n\nAs Felipe spotted in his response, \"A\" is not something you already\nhave, so the picture need to look like the amended patch below.\n\nThe other reason the original did not say \"origin/master\" is because\nthis holds true even if you do not have such a remote-tracking\nbranch for the master at the origin, but the illustration that shows\nthe history after \"git pull\" finishes spells remotes/origin/master\nout, so I think it would be an improvement to make the two pictures\nconsistent by drawing where the origin/master is before this \"git\npull\" is run.\n\nSomething like this, perhaps.  Note that I think \"on\" sounds funny\nand it should probably be \"at\" instead, but this weatherbaloon patch\ndoes not change it.\n\n-- >8 --\nSubject: doc/pull: clarify the illustrations\n\nThe second illustration that shows the history after \"git pull\"\nspelled the remote-tracking branch with \"remotes/\" prefix, which\nis not necessary.  Drop it.\n\nTo match the assumption that a remote-tracking branch is used to\nkeep track of the advancement of the master at the origin, update\nthe first illustration that shows the history before \"git pull\"\nto show the distinction between the master currently at origin and\nthe stale origin/master remote-tracking branch.\n\nNoticed-by: Felipe Contreras <felipe.contreras@gmail.com>\nHelped-by: Max Horn <max@quendi.de>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/git-pull.txt | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt\nindex beea10b..e83f3ce 100644\n--- a/Documentation/git-pull.txt\n+++ b/Documentation/git-pull.txt\n@@ -42,6 +42,8 @@ Assume the following history exists and the current branch is\n \t  A---B---C master on origin\n \t /\n     D---E---F---G master\n+        ^\n+\torigin/master in your repository\n ------------\n \n Then \"`git pull`\" will fetch and replay the changes from the remote\n@@ -51,7 +53,7 @@ result in a new commit along with the names of the two parent commits\n and a log message from the user describing the changes.\n \n ------------\n-\t  A---B---C remotes/origin/master\n+\t  A---B---C origin/master\n \t /         \\\n     D---E---F---G---H master\n ------------\n"},{"id":"229974","messageId":"xmqqbo25418j.fsf@gitster.dls.corp.google.com","threadId":"35252","inReplyTo":"1383211547-9145-8-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH 07/16] branch: trivial style fix","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-31T20:46:52Z","receivedAt":"2013-10-31T20:46:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Looks good; thanks.\n"},{"id":"229975","messageId":"xmqq7gct4152.fsf@gitster.dls.corp.google.com","threadId":"35252","inReplyTo":"1383211547-9145-16-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH 15/16] setup: trivial style fixes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-31T20:48:57Z","receivedAt":"2013-10-31T20:48:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patches 7 through this one looks good; thanks.\n"},{"id":"229977","messageId":"CAMP44s0VTfz5OpVqLbyTSSWVtOkTV5hVn0ofDcA16qDjWFfyhw@mail.gmail.com","threadId":"35252","inReplyTo":"xmqqr4b1425c.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 03/16] pull: cleanup documentation","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-31T21:15:24Z","receivedAt":"2013-10-31T21:15:24Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Thu, Oct 31, 2013 at 2:27 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>\n>> On Thu, Oct 31, 2013 at 1:00 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>>> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>>>\n>>>> On Thu, Oct 31, 2013 at 12:11 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>>>>> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>>>>\n>>>>>> --- a/Documentation/git-pull.txt\n>>>>>> +++ b/Documentation/git-pull.txt\n>>>>>> @@ -39,7 +39,7 @@ Assume the following history exists and the current branch is\n>>>>>>  \"`master`\":\n>>>>>>\n>>>>>>  ------------\n>>>>>> -       A---B---C master on origin\n>>>>>> +       A---B---C origin/master\n>>>>>>        /\n>>>>>>      D---E---F---G master\n>>>>>>  ------------\n>>>>>\n>>>>> This change is wrong; the illustration depicts the distributed world\n>>>>> (i.e. a fetch has not happened yet).\n>>>>\n>>>> That is an irrelevant implementation detail, specially at this high\n>>>> level. In the user's mind origin/master means master on origin.\n>>>\n>>> You are wrong.  In the user's mind, origin/master means the commit\n>>> that used to be at master on origin, and the point of this\n>>> illustration is to make them understand that they live in a\n>>> distributed world, where their last observation will go stale over\n>>> time.\n>>\n>> Wrong. That would make sense in 'git fetch', but here the point of the\n>> illustration is to make them understand what 'git pull' will do,\n>> namely a merge.\n>>\n>> Which refs point to C at which points in time irrelevant information,\n>> the user wants to know that 'git pull' will create a merge.\n>\n> Merge with what,\n\nMerge with C.\n\n> and how do the users know what will be merged?\n\nThey don't, not after they run 'git pull' anyway.\n\n> The users need to learn that origin/master they were told to use\n> with \"git log origin/master..\" etc. trails reality,\n\nYes, but they don't *need* to learn it *right now*. All they need to\nlearn is that 'git pull' will do a merge with 'master' from 'origin',\nAKA 'origin/master'.\n\n-- \nFelipe Contreras\n"},{"id":"229978","messageId":"CAMP44s14_GbN0nEm4Eu2Jdb5cO+jVaTVOnDqzwv0GK4_jfD93w@mail.gmail.com","threadId":"35252","inReplyTo":"xmqqfvrh41e0.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 03/16] pull: cleanup documentation","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-31T21:16:57Z","receivedAt":"2013-10-31T21:16:57Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Thu, Oct 31, 2013 at 2:43 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Junio C Hamano <gitster@pobox.com> writes:\n\n> The other reason the original did not say \"origin/master\" is because\n> this holds true even if you do not have such a remote-tracking\n> branch for the master at the origin, but the illustration that shows\n> the history after \"git pull\" finishes spells remotes/origin/master\n> out, so I think it would be an improvement to make the two pictures\n> consistent by drawing where the origin/master is before this \"git\n> pull\" is run.\n\nSo you care about \"reality\" when it fits your argument, but not when\nit doesn't. Got it.\n\n-- \nFelipe Contreras\n"},{"id":"229979","messageId":"CAMP44s2xYRP9rmpoQzDcdSWiYpU31FXEPW32Yqsn2DZjP4kD_A@mail.gmail.com","threadId":"35252","inReplyTo":"xmqqvc0d432e.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 06/16] shortlog: add missing declaration","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-10-31T21:17:23Z","receivedAt":"2013-10-31T21:17:23Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Thu, Oct 31, 2013 at 2:07 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>\n>> On Thu, Oct 31, 2013 at 1:05 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>>> Felipe Contreras <felipe.contreras@gmail.com> writes:\n>>>\n>>>> Otherwise we would have to include commit.h.\n>>>\n>>> Was there a reason why commit.h is not included here, just like\n>>> revision.h would include diff.h, so that users of shortlog.h do not\n>>> have to worry about including commit.h themselves?\n>>\n>> Because you can't do:\n>>\n>> struct diff_options;\n>> struct diff_options diffopt;\n>>\n>> The storage size is not known, but you can do:\n>>\n>> struct diff_options;\n>> struct diff_options *diffopt;\n>\n> But so can you do\n>\n>         struct diff_options *diffopt;\n>\n> without the declaration, no?  That is:\n\nYes.\n\n-- \nFelipe Contreras\n"},{"id":"229987","messageId":"20131031234048.GA41400@gmail.com","threadId":"35252","inReplyTo":"CAMP44s14_GbN0nEm4Eu2Jdb5cO+jVaTVOnDqzwv0GK4_jfD93w@mail.gmail.com","subject":"Re: [PATCH 03/16] pull: cleanup documentation","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2013-10-31T23:40:49Z","receivedAt":"2013-10-31T23:40:49Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Thu, Oct 31, 2013 at 03:16:57PM -0600, Felipe Contreras wrote:\n> On Thu, Oct 31, 2013 at 2:43 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> > Junio C Hamano <gitster@pobox.com> writes:\n> \n> > The other reason the original did not say \"origin/master\" is because\n> > this holds true even if you do not have such a remote-tracking\n> > branch for the master at the origin, but the illustration that shows\n> > the history after \"git pull\" finishes spells remotes/origin/master\n> > out, so I think it would be an improvement to make the two pictures\n> > consistent by drawing where the origin/master is before this \"git\n> > pull\" is run.\n> \n> So you care about \"reality\" when it fits your argument, but not when\n> it doesn't. Got it.\n\nWhat exactly do these flippant remarks accomplish?\nKeep these to yourself.  No one deserves this treatment nor does\nanyone care to read it.\n-- \nDavid\n"},{"id":"229992","messageId":"CAMP44s0rtJyJTW36iYX-RRu6NcQ1yF0NUUTXQoCUpMkWVg2Zmg@mail.gmail.com","threadId":"35252","inReplyTo":"20131031234048.GA41400@gmail.com","subject":"Re: [PATCH 03/16] pull: cleanup documentation","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-11-01T01:56:03Z","receivedAt":"2013-11-01T01:56:03Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Thu, Oct 31, 2013 at 5:40 PM, David Aguilar <davvid@gmail.com> wrote:\n> On Thu, Oct 31, 2013 at 03:16:57PM -0600, Felipe Contreras wrote:\n>> On Thu, Oct 31, 2013 at 2:43 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> > Junio C Hamano <gitster@pobox.com> writes:\n>>\n>> > The other reason the original did not say \"origin/master\" is because\n>> > this holds true even if you do not have such a remote-tracking\n>> > branch for the master at the origin, but the illustration that shows\n>> > the history after \"git pull\" finishes spells remotes/origin/master\n>> > out, so I think it would be an improvement to make the two pictures\n>> > consistent by drawing where the origin/master is before this \"git\n>> > pull\" is run.\n>>\n>> So you care about \"reality\" when it fits your argument, but not when\n>> it doesn't. Got it.\n>\n> What exactly do these flippant remarks accomplish?\n> Keep these to yourself.  No one deserves this treatment nor does\n> anyone care to read it.\n\nNobody is forcing you to read them.\n\nHowever, they happen to be true. Junio used as an argument that\n'origin/master' is not better than 'master on origin' because the\n\"real\" 'origin/master' might be pointing to something else, however,\nwhen he himself realized that 'origin/master' might not exist at all,\nthen suddenly the \"real\" 'origin/master' is not so important.\n\nIf this was a rational discussion I would ask you to point out where\nexactly the previous explanation is incorrect, but alas, if experience\nserves, this is not one of those.\n\nThe facts are very simple, Junio's proposal:\n\n------------\n          A---B---C master on origin\n         /\n    D---E---F---G master\n        ^\n       origin/master in your repository\n------------\n\nAssumes that:\n\n1) The user did 'git pull origin', not 'git pull $url', or any other remote\n2) 'origin/master' points to E, which might not be true, the user\nmight have issued a 'git fetch', or a previous pull might have been\ncancelled\n\nBoth issues are overridden by the comment \"Assume the following\nhistory exists\", so one has to assume that origin/master points to E,\nbut if one assumes that, then my proposal is also correct:\n\n------------\n         A---B---C origin/master\n        /\n    D---E---F---G master\n------------\n\nBecause one has to assume that 'origin/master' points to C, which is\nentirely possible. But more importantly, for the purposes of\nexplaining 'git pull' it lightens the mental load needed by the user.\n\nEither Junio's argument applies to both proposals, or none, but\nselectively cherry-picking where the argument applies is akin to a\nfeminist that argues men and women are equal, but men should pick the\ncheck in a restaurant.\n\n-- \nFelipe Contreras\n"},{"id":"229993","messageId":"20131101024821.GB41400@gmail.com","threadId":"35252","inReplyTo":"CAMP44s0rtJyJTW36iYX-RRu6NcQ1yF0NUUTXQoCUpMkWVg2Zmg@mail.gmail.com","subject":"Re: [PATCH 03/16] pull: cleanup documentation","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2013-11-01T02:48:22Z","receivedAt":"2013-11-01T02:48:22Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Thu, Oct 31, 2013 at 07:56:03PM -0600, Felipe Contreras wrote:\n> On Thu, Oct 31, 2013 at 5:40 PM, David Aguilar <davvid@gmail.com> wrote:\n> > On Thu, Oct 31, 2013 at 03:16:57PM -0600, Felipe Contreras wrote:\n> >> On Thu, Oct 31, 2013 at 2:43 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> >> > Junio C Hamano <gitster@pobox.com> writes:\n> >>\n> >> > The other reason the original did not say \"origin/master\" is because\n> >> > this holds true even if you do not have such a remote-tracking\n> >> > branch for the master at the origin, but the illustration that shows\n> >> > the history after \"git pull\" finishes spells remotes/origin/master\n> >> > out, so I think it would be an improvement to make the two pictures\n> >> > consistent by drawing where the origin/master is before this \"git\n> >> > pull\" is run.\n> >>\n> >> So you care about \"reality\" when it fits your argument, but not when\n> >> it doesn't. Got it.\n> >\n> > What exactly do these flippant remarks accomplish?\n> > Keep these to yourself.  No one deserves this treatment nor does\n> > anyone care to read it.\n> \n> Nobody is forcing you to read them.\n\nYou're missing the *key* point.\nNo one wants to interact with a rude arrogant loudmouth.\n\nYou could have explained this without being sarcastic and annoying.\nIt does not help your cause.\n\n\nSure, these are \"subjective\" people skills (that don't \"matter\"\nto the code) but this is a social activity.  If your defense is\nto whine and say, \"well, people on LKML are rude and flippant\nall the time!\" then that's a lame argument that's not\neven true.\n\nYour patch may in fact be a good one, but your anti-discussions\nhurt them.  Discussion are good, not an insane attack fest.\nWhat fun is that?  It's soooo lame.\n\nThat's all I'm saying.  You can defend it but no one cares.\n\nAnyways, I'm out to have fun at a party with friends.\nTry and get out, have fun, and come back when you're not looking\nfor someone to abuse.\n-- \nDavid\n"},{"id":"229994","messageId":"CAMP44s1_KW-HJ0aGbv3AAXFXXrvGscQTXXgsoK6WPNjTHmOiMQ@mail.gmail.com","threadId":"35252","inReplyTo":"20131101024821.GB41400@gmail.com","subject":"Re: [PATCH 03/16] pull: cleanup documentation","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-11-01T03:50:51Z","receivedAt":"2013-11-01T03:50:51Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Thu, Oct 31, 2013 at 8:48 PM, David Aguilar <davvid@gmail.com> wrote:\n> On Thu, Oct 31, 2013 at 07:56:03PM -0600, Felipe Contreras wrote:\n\n>> Nobody is forcing you to read them.\n>\n> You're missing the *key* point.\n> No one wants to interact with a rude arrogant loudmouth.\n\nThen don't. Problem solved.\n\n-- \nFelipe Contreras\n"},{"id":"230000","messageId":"20131101110539.GA32076@goldbirke","threadId":"35252","inReplyTo":"CAMP44s1_KW-HJ0aGbv3AAXFXXrvGscQTXXgsoK6WPNjTHmOiMQ@mail.gmail.com","subject":"Re: [PATCH 03/16] pull: cleanup documentation","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2013-11-01T11:05:39Z","receivedAt":"2013-11-01T11:05:39Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Thu, Oct 31, 2013 at 09:50:51PM -0600, Felipe Contreras wrote:\n> On Thu, Oct 31, 2013 at 8:48 PM, David Aguilar <davvid@gmail.com> wrote:\n> > On Thu, Oct 31, 2013 at 07:56:03PM -0600, Felipe Contreras wrote:\n> \n> >> Nobody is forcing you to read them.\n> >\n> > You're missing the *key* point.\n> > No one wants to interact with a rude arrogant loudmouth.\n> \n> Then don't. Problem solved.\n\nNope, it just recreates another old problem.  We've been there before,\nnot long ago:\n\nOn Sat, Oct 12, 2013 at 02:24:50AM -0500, Felipe Contreras wrote:\n> Clearly, a lot of my patches have not been reviewed properly [...]\n\n\nBest,\nGábor\n"},{"id":"230001","messageId":"CAMP44s1xRJN_Tmq6rwXebER2g=_wazub-7-775GZkyLSb8e-bA@mail.gmail.com","threadId":"35252","inReplyTo":"20131101110539.GA32076@goldbirke","subject":"Re: [PATCH 03/16] pull: cleanup documentation","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-11-01T12:20:24Z","receivedAt":"2013-11-01T12:20:24Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Fri, Nov 1, 2013 at 5:05 AM, SZEDER Gábor <szeder@ira.uka.de> wrote:\n> On Thu, Oct 31, 2013 at 09:50:51PM -0600, Felipe Contreras wrote:\n>> On Thu, Oct 31, 2013 at 8:48 PM, David Aguilar <davvid@gmail.com> wrote:\n>> > On Thu, Oct 31, 2013 at 07:56:03PM -0600, Felipe Contreras wrote:\n>>\n>> >> Nobody is forcing you to read them.\n>> >\n>> > You're missing the *key* point.\n>> > No one wants to interact with a rude arrogant loudmouth.\n>>\n>> Then don't. Problem solved.\n>\n> Nope, it just recreates another old problem.  We've been there before,\n> not long ago:\n\nDon't worry. That was the last time I sent those patches.\n\nI will only resend the patch series that receive attention.\n\nHappy?\n\n-- \nFelipe Contreras\n"}]}