{"thread":{"id":"54749","subject":"[PATCH] style: indent multiline \"if\" conditions by 4 spaces","startedAt":"2020-12-03T08:01:16Z","lastAt":"2020-12-03T18:52:39Z","messageCount":5,"participants":["Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"411209","messageId":"X8ibEpkel8OMS/hV@coredump.intra.peff.net","threadId":"54749","inReplyTo":null,"subject":"[PATCH] style: indent multiline \"if\" conditions by 4 spaces","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-12-03T08:00:18Z","receivedAt":"2020-12-03T08:01:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Commit 6dc905d974 (config: split repo scope to local and worktree,\n2020-02-10) made some \"if\" statements multiline, but didn't indent the\nsecond lines in our usual way.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nI just happened to be reading nearby code and saw this.\n\n remote.c      | 2 +-\n upload-pack.c | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex 8a6dbbb903..8b9d090ae5 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -355,7 +355,7 @@ static int handle_config(const char *key, const char *value, void *cb)\n \tremote = make_remote(name, namelen);\n \tremote->origin = REMOTE_CONFIG;\n \tif (current_config_scope() == CONFIG_SCOPE_LOCAL ||\n-\tcurrent_config_scope() == CONFIG_SCOPE_WORKTREE)\n+\t    current_config_scope() == CONFIG_SCOPE_WORKTREE)\n \t\tremote->configured_in_repo = 1;\n \tif (!strcmp(subkey, \"mirror\"))\n \t\tremote->mirror = git_config_bool(key, value);\ndiff --git a/upload-pack.c b/upload-pack.c\nindex 5dc8e1f844..a85d4ec502 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -1302,7 +1302,7 @@ static int upload_pack_config(const char *var, const char *value, void *cb_data)\n \t}\n \n \tif (current_config_scope() != CONFIG_SCOPE_LOCAL &&\n-\tcurrent_config_scope() != CONFIG_SCOPE_WORKTREE) {\n+\t    current_config_scope() != CONFIG_SCOPE_WORKTREE) {\n \t\tif (!strcmp(\"uploadpack.packobjectshook\", var))\n \t\t\treturn git_config_string(&data->pack_objects_hook, var, value);\n \t}\n-- \n2.29.2.896.g080220a959\n"},{"id":"411263","messageId":"xmqqv9dihhqs.fsf@gitster.c.googlers.com","threadId":"54749","inReplyTo":"X8ibEpkel8OMS/hV@coredump.intra.peff.net","subject":"Re: [PATCH] style: indent multiline \"if\" conditions by 4 spaces","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-12-03T18:22:35Z","receivedAt":"2020-12-03T18:23:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Commit 6dc905d974 (config: split repo scope to local and worktree,\n> 2020-02-10) made some \"if\" statements multiline, but didn't indent the\n> second lines in our usual way.\n>\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n> I just happened to be reading nearby code and saw this.\n\nCan we rephrase \"by 4 spaces\" to avoid misleading new developers,\nthough?  It's not like our rule is to indent continued expression by\n4 spaces---it happens to be 4 in this case only because the second\nline is aligned with the column inside \"if (\".\n\nIf the expression were\n\n\tif ((A == X && B == Y &&\n\t     boolean_expression_on_the_second_line()) ||\n\t    C == Z)\n\nwe would have indented the second line to align with the inside of\nthe opening parenthesis, which may end up with \"by 5 spaces\", but\nfour vs five is not the important part of the equation.\n\nThanks.  \n\n> diff --git a/remote.c b/remote.c\n> index 8a6dbbb903..8b9d090ae5 100644\n> --- a/remote.c\n> +++ b/remote.c\n> @@ -355,7 +355,7 @@ static int handle_config(const char *key, const char *value, void *cb)\n>  \tremote = make_remote(name, namelen);\n>  \tremote->origin = REMOTE_CONFIG;\n>  \tif (current_config_scope() == CONFIG_SCOPE_LOCAL ||\n> -\tcurrent_config_scope() == CONFIG_SCOPE_WORKTREE)\n> +\t    current_config_scope() == CONFIG_SCOPE_WORKTREE)\n>  \t\tremote->configured_in_repo = 1;\n>  \tif (!strcmp(subkey, \"mirror\"))\n>  \t\tremote->mirror = git_config_bool(key, value);\n> diff --git a/upload-pack.c b/upload-pack.c\n> index 5dc8e1f844..a85d4ec502 100644\n> --- a/upload-pack.c\n> +++ b/upload-pack.c\n> @@ -1302,7 +1302,7 @@ static int upload_pack_config(const char *var, const char *value, void *cb_data)\n>  \t}\n>  \n>  \tif (current_config_scope() != CONFIG_SCOPE_LOCAL &&\n> -\tcurrent_config_scope() != CONFIG_SCOPE_WORKTREE) {\n> +\t    current_config_scope() != CONFIG_SCOPE_WORKTREE) {\n>  \t\tif (!strcmp(\"uploadpack.packobjectshook\", var))\n>  \t\t\treturn git_config_string(&data->pack_objects_hook, var, value);\n>  \t}\n"},{"id":"411264","messageId":"X8kvJRnOtSEYziRK@coredump.intra.peff.net","threadId":"54749","inReplyTo":"xmqqv9dihhqs.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] style: indent multiline \"if\" conditions by 4 spaces","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-12-03T18:32:05Z","receivedAt":"2020-12-03T18:32:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Dec 03, 2020 at 10:22:35AM -0800, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > Commit 6dc905d974 (config: split repo scope to local and worktree,\n> > 2020-02-10) made some \"if\" statements multiline, but didn't indent the\n> > second lines in our usual way.\n> >\n> > Signed-off-by: Jeff King <peff@peff.net>\n> > ---\n> > I just happened to be reading nearby code and saw this.\n> \n> Can we rephrase \"by 4 spaces\" to avoid misleading new developers,\n> though?  It's not like our rule is to indent continued expression by\n> 4 spaces---it happens to be 4 in this case only because the second\n> line is aligned with the column inside \"if (\".\n> \n> If the expression were\n> \n> \tif ((A == X && B == Y &&\n> \t     boolean_expression_on_the_second_line()) ||\n> \t    C == Z)\n> \n> we would have indented the second line to align with the inside of\n> the opening parenthesis, which may end up with \"by 5 spaces\", but\n> four vs five is not the important part of the equation.\n\nYeah, the goal is to align it, but I had trouble thinking of a succinct\nway to say that. Maybe just:\n\n  style: align indent of multiline \"if\" conditions\n\n?\n\n-Peff\n"},{"id":"411265","messageId":"xmqqr1o6hh09.fsf@gitster.c.googlers.com","threadId":"54749","inReplyTo":"X8kvJRnOtSEYziRK@coredump.intra.peff.net","subject":"Re: [PATCH] style: indent multiline \"if\" conditions by 4 spaces","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-12-03T18:38:30Z","receivedAt":"2020-12-03T18:39:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Yeah, the goal is to align it, but I had trouble thinking of a succinct\n> way to say that. Maybe just:\n>\n>   style: align indent of multiline \"if\" conditions\n\nI couldn't come up with a good description, and \njust did s/by 4 spaces/to align/, i.e.\n\n    style: indent multiline \"if\" conditions to align\n\nwhile queuing.  I dunno.\n\n"},{"id":"411267","messageId":"X8kzrFd2XaVJ+UU6@coredump.intra.peff.net","threadId":"54749","inReplyTo":"xmqqr1o6hh09.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] style: indent multiline \"if\" conditions by 4 spaces","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-12-03T18:51:24Z","receivedAt":"2020-12-03T18:52:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Dec 03, 2020 at 10:38:30AM -0800, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > Yeah, the goal is to align it, but I had trouble thinking of a succinct\n> > way to say that. Maybe just:\n> >\n> >   style: align indent of multiline \"if\" conditions\n> \n> I couldn't come up with a good description, and \n> just did s/by 4 spaces/to align/, i.e.\n> \n>     style: indent multiline \"if\" conditions to align\n> \n> while queuing.  I dunno.\n\nThat sounds perfectly reasonable to me.\n\n-Peff\n"}]}