{"thread":{"id":"10698","subject":"[PATCH] Make git-clean a builtin","startedAt":"2007-11-07T05:18:51Z","lastAt":"2007-11-10T22:43:37Z","messageCount":13,"participants":["Shawn Bohrer","Johannes Schindelin","Bill Lear","Matthieu Moy","Jon Loeliger","Junio C Hamano","Brian Downing","Miles Bader"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"58635","messageId":"11944127311587-git-send-email-shawn.bohrer@gmail.com","threadId":"10698","inReplyTo":null,"subject":"[PATCH] Make git-clean a builtin","fromName":"Shawn Bohrer","fromEmail":"shawn.bohrer@gmail.com","sentAt":"2007-11-07T05:18:51Z","receivedAt":"2007-11-07T05:18:51Z","isPatch":true,"sender":{"key":"shawn.bohrer@gmail.com","avatar":"https://gravatar.com/avatar/6eb093ef7d276306d18366254e0c95ff6a5db58231ac7e82fe78c2800aaae1b6?d=mp&s=160"},"body":"This replaces git-clean.sh with builtin-clean.c, and moves\ngit-clean.sh to the examples.\n\nThis also introduces a change in behavior where the -d parameter is\nrequired to remove an entire directory of untracked files even when\nthe directory is passed as a path. For example:\n\n git clean dir/\n\nNow requires\n\n git clean -d dir/\n\nif 'dir' only contains untracked files. This is consistent with the\nold behavior when two or more paths were specified.\n\nThanks to Johannes Schindelin for the conversion to using the\nparse-options API.\n\nSigned-off-by: Shawn Bohrer \n---\n Makefile | 3 +-\n builtin-clean.c | 134 +++++++++++++++++++++++++\n builtin.h | 1 +\n git-clean.sh => contrib/examples/git-clean.sh | 0 \n git.c | 1 +\n 5 files changed, 138 insertions(+), 1 deletions(-)\n create mode 100644 builtin-clean.c\n rename git-clean.sh => contrib/examples/git-clean.sh (100%)\n\ndiff --git a/Makefile b/Makefile\nindex c427fee..932ff08 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -209,7 +209,7 @@ BASIC_LDFLAGS =\n \n SCRIPT_SH = \\\n \tgit-bisect.sh git-checkout.sh \\\n-\tgit-clean.sh git-clone.sh git-commit.sh \\\n+\tgit-clone.sh git-commit.sh \\\n \tgit-merge-one-file.sh git-mergetool.sh git-parse-remote.sh \\\n \tgit-pull.sh git-rebase.sh git-rebase--interactive.sh \\\n \tgit-repack.sh git-request-pull.sh \\\n@@ -325,6 +325,7 @@ BUILTIN_OBJS = \\\n \tbuiltin-check-attr.o \\\n \tbuiltin-checkout-index.o \\\n \tbuiltin-check-ref-format.o \\\n+\tbuiltin-clean.o \\\n \tbuiltin-commit-tree.o \\\n \tbuiltin-count-objects.o \\\n \tbuiltin-describe.o \\\ndiff --git a/builtin-clean.c b/builtin-clean.c\nnew file mode 100644\nindex 0000000..fb2feb5\n--- /dev/null\n+++ b/builtin-clean.c\n@@ -0,0 +1,134 @@\n+/*\n+ * \"git clean\" builtin command\n+ *\n+ * Copyright (C) 2007 Shawn Bohrer\n+ *\n+ * Based on git-clean.sh by Pavel Roskin\n+ */\n+\n+#include \"builtin.h\"\n+#include \"cache.h\"\n+#include \"dir.h\"\n+#include \"parse-options.h\"\n+\n+static int force;\n+\n+static const char *const builtin_clean_usage[] = {\n+\t\"git-clean [-d] [-f] [-n] [-q] [-x | -X] [--] ...\",\n+\tNULL\n+};\n+\n+static int git_clean_config(const char *var, const char *value)\n+{\n+\tif (!strcmp(var, \"clean.requireforce\")) {\n+\t\tforce = !git_config_bool(var, value);\n+\t}\n+\treturn 0;\n+}\n+\n+int cmd_clean(int argc, const char **argv, const char *prefix)\n+{\n+\tint j;\n+\tint show_only = 0, remove_directories = 0, quiet = 0, ignored = 0;\n+\tint ignored_only = 0, baselen = 0;\n+\tstruct strbuf directory;\n+\tstruct dir_struct dir;\n+\tconst char *path = \".\";\n+\tconst char *base = \"\";\n+\tstatic const char **pathspec;\n+\tstruct option options[] = {\n+\t\tOPT__QUIET(&quiet),\n+\t\tOPT__DRY_RUN(&show_only),\n+\t\tOPT_BOOLEAN('f', NULL, &force, \"force\"),\n+\t\tOPT_BOOLEAN('d', NULL, &remove_directories,\n+\t\t\t\t\"remove whole directories\"),\n+\t\tOPT_BOOLEAN('x', NULL, &ignored, \"remove ignored files, too\"),\n+\t\tOPT_BOOLEAN('X', NULL, &ignored_only,\n+\t\t\t\t\"remove only ignored files\"),\n+\t\tOPT_END()\n+\t};\n+\n+\tgit_config(git_clean_config);\n+\targc = parse_options(argc, argv, options, builtin_clean_usage, 0);\n+\n+\tmemset(&dir, 0, sizeof(dir));\n+\tif (ignored_only) {\n+\t\tdir.show_ignored =1;\n+\t\tdir.exclude_per_dir = \".gitignore\";\n+\t}\n+\n+\tif (ignored && ignored_only)\n+\t\tdie(\"-x and -X cannot be used together\");\n+\n+\tif (!show_only && !force)\n+\t\tdie(\"clean.requireForce set and -n or -f not given; refusing to clean\");\n+\n+\tdir.show_other_directories = 1;\n+\n+\tif (!ignored) {\n+\t\tdir.exclude_per_dir = \".gitignore\";\n+\t\tif (!access(git_path(\"info/exclude\"), F_OK)) {\n+\t\t\tchar *exclude_path = git_path(\"info/exclude\");\n+\t\t\tadd_excludes_from_file(&dir, exclude_path);\n+\t\t}\n+\t}\n+\n+\tpathspec = get_pathspec(prefix, argv);\n+\tread_cache();\n+\tread_directory(&dir, path, base, baselen, pathspec);\n+\tstrbuf_init(&directory, 0);\n+\n+\tfor (j = 0; j < dir.nr; ++j) {\n+\t\tstruct dir_entry *ent = dir.entries[j];\n+\t\tint len, pos;\n+\t\tstruct cache_entry *ce;\n+\t\tstruct stat st;\n+\n+\t\t/*\n+\t\t * Remove the '/' at the end that directory\n+\t\t * walking adds for directory entries.\n+\t\t */\n+\t\tlen = ent->len;\n+\t\tif (len && ent->name[len-1] == '/')\n+\t\t\tlen--;\n+\t\tpos = cache_name_pos(ent->name, len);\n+\t\tif (0 <= pos)\n+\t\t\tcontinue;\t/* exact match */\n+\t\tpos = -pos - 1;\n+\t\tif (pos < active_nr) {\n+\t\t\tce = active_cache[pos];\n+\t\t\tif (ce_namelen(ce) == len &&\n+\t\t\t !memcmp(ce->name, ent->name, len))\n+\t\t\t\tcontinue; /* Yup, this one exists unmerged */\n+\t\t}\n+\n+\t\t/* remove the files */\n+\t\tif (!lstat(ent->name, &st) && (S_ISDIR(st.st_mode))) {\n+\t\t\tstrbuf_addstr(&directory, ent->name);\n+\t\t\tif (show_only && remove_directories) {\n+\t\t\t\tprintf(\"Would remove %s\\n\", directory.buf);\n+\t\t\t} else if (quiet && remove_directories) {\n+\t\t\t\tremove_dir_recursively(&directory, 0);\n+\t\t\t} else if (remove_directories) {\n+\t\t\t\tprintf(\"Removing %s\\n\", ent->name);\n+\t\t\t\tremove_dir_recursively(&directory, 0);\n+\t\t\t} else if (show_only) {\n+\t\t\t\tprintf(\"Would not remove %s\\n\", directory.buf);\n+\t\t\t} else {\n+\t\t\t\tprintf(\"Not removing %s\\n\", directory.buf);\n+\t\t\t}\n+\t\t\tstrbuf_reset(&directory);\n+\t\t} else {\n+\t\t\tif (show_only) {\n+\t\t\t\tprintf(\"Would remove %s\\n\", ent->name);\n+\t\t\t\tcontinue;\n+\t\t\t} else if (!quiet) {\n+\t\t\t\tprintf(\"Removing %s\\n\", ent->name);\n+\t\t\t}\n+\t\t\tunlink(ent->name);\n+\t\t}\n+\t}\n+\n+\tstrbuf_release(&directory);\n+\treturn 0;\n+}\ndiff --git a/builtin.h b/builtin.h\nindex 525107f..5476a92 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -24,6 +24,7 @@ extern int cmd_check_attr(int argc, const char **argv, const char *prefix);\n extern int cmd_check_ref_format(int argc, const char **argv, const char *prefix);\n extern int cmd_cherry(int argc, const char **argv, const char *prefix);\n extern int cmd_cherry_pick(int argc, const char **argv, const char *prefix);\n+extern int cmd_clean(int argc, const char **argv, const char *prefix);\n extern int cmd_commit_tree(int argc, const char **argv, const char *prefix);\n extern int cmd_count_objects(int argc, const char **argv, const char *prefix);\n extern int cmd_describe(int argc, const char **argv, const char *prefix);\ndiff --git a/git-clean.sh b/contrib/examples/git-clean.sh\nsimilarity index 100%\nrename from git-clean.sh\nrename to contrib/examples/git-clean.sh\ndiff --git a/git.c b/git.c\nindex 204a6f7..3fa8e4d 100644\n--- a/git.c\n+++ b/git.c\n@@ -293,6 +293,7 @@ static void handle_internal_command(int argc, const char **argv)\n \t\t{ \"check-attr\", cmd_check_attr, RUN_SETUP | NEED_WORK_TREE },\n \t\t{ \"cherry\", cmd_cherry, RUN_SETUP },\n \t\t{ \"cherry-pick\", cmd_cherry_pick, RUN_SETUP | NEED_WORK_TREE },\n+\t\t{ \"clean\", cmd_clean, RUN_SETUP | NEED_WORK_TREE },\n \t\t{ \"commit-tree\", cmd_commit_tree, RUN_SETUP },\n \t\t{ \"config\", cmd_config },\n \t\t{ \"count-objects\", cmd_count_objects, RUN_SETUP },\n-- \n1.5.3.GIT\n"},{"id":"58669","messageId":"Pine.LNX.4.64.0711071110040.4362@racer.site","threadId":"10698","inReplyTo":"11944127311587-git-send-email-shawn.bohrer@gmail.com","subject":"Re: [PATCH] Make git-clean a builtin","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-07T11:10:45Z","receivedAt":"2007-11-07T11:10:45Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nyou still have quite a number of instances where you wrap just one line \ninto curly brackets:\n\n\tif (bla) {\n\t\t[just one line]\n\t}\n\nCiao,\nDscho\n"},{"id":"58670","messageId":"18225.48553.44088.269677@lisa.zopyra.com","threadId":"10698","inReplyTo":"Pine.LNX.4.64.0711071110040.4362@racer.site","subject":"Re: [PATCH] Make git-clean a builtin","fromName":"Bill Lear","fromEmail":"rael@zopyra.com","sentAt":"2007-11-07T13:29:13Z","receivedAt":"2007-11-07T13:29:13Z","isPatch":true,"sender":{"key":"rael@zopyra.com","avatar":"https://gravatar.com/avatar/c4f2d2790ca3828d3b4e7dfebabf61d2fe94fd82fa49cdac2a5295dd2d46a874?d=mp&s=160"},"body":"On Wednesday, November 7, 2007 at 11:10:45 (+0000) Johannes Schindelin writes:\n>Hi,\n>\n>you still have quite a number of instances where you wrap just one line \n>into curly brackets:\n>\n>\tif (bla) {\n>\t\t[just one line]\n>\t}\n\nI've always found this a thoughtful practice. It helps ensure nobody writes:\n\n if (bla)\n just_one_line();\n /* perhaps a comment, other stuff ... */\n just_another_line();\n\nwhich I've seen happen countless times. It also is nice for others who\ncome along and extend the branch from just one line to multiple ones,\nas the brackets are already in place.\n\nWhy do you find it objectionable?\n\n\nBill\n"},{"id":"58672","messageId":"Pine.LNX.4.64.0711071414140.4362@racer.site","threadId":"10698","inReplyTo":"18225.48553.44088.269677@lisa.zopyra.com","subject":"Re: [PATCH] Make git-clean a builtin","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-07T14:17:00Z","receivedAt":"2007-11-07T14:17:00Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 7 Nov 2007, Bill Lear wrote:\n\n> On Wednesday, November 7, 2007 at 11:10:45 (+0000) Johannes Schindelin writes:\n>\n> > you still have quite a number of instances where you wrap just one \n> > line into curly brackets:\n> >\n> >\tif (bla) {\n> >\t\t[just one line]\n> >\t}\n> \n> I've always found this a thoughtful practice. It helps ensure nobody \n> writes:\n> \n> if (bla)\n> just_one_line();\n> /* perhaps a comment, other stuff ... */\n> just_another_line();\n\nBut if there is only one line and you fail to add curly brackets when \nadding a second line, well, uhm, then I cannot help you with anything.\n\nBTW I was talking about _one_ line, not a line and another one with a \ncomment.\n\n> It also is nice for others who come along and extend the branch from \n> just one line to multiple ones, as the brackets are already in place.\n\nThe fact is: these lines will stay single lines most likely for eternity.\n\n> Why do you find it objectionable?\n\nIt distracts. It's ugly. It's unnecessary.\n\nCiao,\nDscho\n"},{"id":"58674","messageId":"vpq7ikudsi9.fsf@bauges.imag.fr","threadId":"10698","inReplyTo":"18225.48553.44088.269677@lisa.zopyra.com","subject":"Re: [PATCH] Make git-clean a builtin","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2007-11-07T14:45:02Z","receivedAt":"2007-11-07T14:45:02Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Bill Lear writes:\n\n> I've always found this a thoughtful practice. It helps ensure nobody writes:\n>\n> if (bla)\n> just_one_line();\n> /* perhaps a comment, other stuff ... */\n> just_another_line();\n\nIt also simplify patches for cases like\n\n \tif (bla) {\n \t\tjust_one_line();\n+\t\tanother_added_line();\n \t}\n\ninstead of\n\n- \tif (bla)\n+ \tif (bla) {\n \t\tjust_one_line();\n+\t\tanother_added_line();\n+\t}\n\nBut it seems people here prefer not putting the braces in this case.\n\n-- \nMatthieu\n"},{"id":"58676","messageId":"20071107145434.GB6768@mediacenter.austin.rr.com","threadId":"10698","inReplyTo":"Pine.LNX.4.64.0711071110040.4362@racer.site","subject":"Re: [PATCH] Make git-clean a builtin","fromName":"Shawn Bohrer","fromEmail":"shawn.bohrer@gmail.com","sentAt":"2007-11-07T14:54:34Z","receivedAt":"2007-11-07T14:54:34Z","isPatch":true,"sender":{"key":"shawn.bohrer@gmail.com","avatar":"https://gravatar.com/avatar/6eb093ef7d276306d18366254e0c95ff6a5db58231ac7e82fe78c2800aaae1b6?d=mp&s=160"},"body":"On Wed, Nov 07, 2007 at 11:10:45AM +0000, Johannes Schindelin wrote:\n> \n> you still have quite a number of instances where you wrap just one line \n> into curly brackets:\n> \n> \tif (bla) {\n> \t\t[just one line]\n> \t}\n\nCrap. OK I count one instance unless you count:\n\n\tif (foo) {\n\t\tone_line();\n\t} else if (bar) {\n\t\tone_line();\n\t\ttwo_lines();\n\t} else {\n\t\tsomething_else();\n\t}\n\nNow I suppose I can get rid of the curly braces here as well but I\npersonally find that strange and ugly. So is there an official guideline\non if else statements?\n\nOf course I'll fix the other one I missed and send a new patch.\n"},{"id":"58681","messageId":"Pine.LNX.4.64.0711071501270.4362@racer.site","threadId":"10698","inReplyTo":"20071107145434.GB6768@mediacenter.austin.rr.com","subject":"Re: [PATCH] Make git-clean a builtin","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-07T15:04:52Z","receivedAt":"2007-11-07T15:04:52Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 7 Nov 2007, Shawn Bohrer wrote:\n\n> On Wed, Nov 07, 2007 at 11:10:45AM +0000, Johannes Schindelin wrote:\n> > \n> > you still have quite a number of instances where you wrap just one line \n> > into curly brackets:\n> > \n> > \tif (bla) {\n> > \t\t[just one line]\n> > \t}\n> \n> Crap. OK I count one instance unless you count:\n> \n> \tif (foo) {\n> \t\tone_line();\n> \t} else if (bar) {\n> \t\tone_line();\n> \t\ttwo_lines();\n> \t} else {\n> \t\tsomething_else();\n> \t}\n\nI do count them. Personally, I find it highly distracting and ugly. \nBesides, we have the convention of putting the \"}\" not into the same line \nas \"else\". (See keyword \"uncuddling\" in the list archives.)\n\nWhile it may be true that some parts of the code follow these rules less \nstrictly, it does not mean that we should introduce more of that kind.\n\nBTW there are plenty of examples in the existing code which illustrate our \nimplicit coding conventions.\n\n> Now I suppose I can get rid of the curly braces here as well but I \n> personally find that strange and ugly. So is there an official \n> guideline on if else statements?\n\nNot yet ;-) I can add it to the tentative v3 of Documentation/CodingStyle \nor CodingConventions or however the list would like to name it.\n\nCiao,\nDscho\n"},{"id":"58711","messageId":"1194464770.14978.11.camel@ld0161-tx32","threadId":"10698","inReplyTo":"18225.48553.44088.269677@lisa.zopyra.com","subject":"Re: [PATCH] Make git-clean a builtin","fromName":"Jon Loeliger","fromEmail":"jdl@freescale.com","sentAt":"2007-11-07T19:46:10Z","receivedAt":"2007-11-07T19:46:10Z","isPatch":true,"sender":{"key":"jdl@jdl.com","avatar":"https://gravatar.com/avatar/75ce9a10b151acd2c28ec4ab2136dba7b2ff1634530bd04b155981a749d08a64?d=mp&s=160"},"body":"On Wed, 2007-11-07 at 07:29, Bill Lear wrote:\n> On Wednesday, November 7, 2007 at 11:10:45 (+0000) Johannes Schindelin writes:\n> >Hi,\n> >\n> >you still have quite a number of instances where you wrap just one line \n> >into curly brackets:\n> >\n> >\tif (bla) {\n> >\t\t[just one line]\n> >\t}\n> \n> I've always found this a thoughtful practice. It helps ensure nobody writes:\n> \n> if (bla)\n> just_one_line();\n> /* perhaps a comment, other stuff ... */\n> just_another_line();\n> \n> which I've seen happen countless times. It also is nice for others who\n> come along and extend the branch from just one line to multiple ones,\n> as the brackets are already in place.\n> \n> Why do you find it objectionable?\n\nI _totally_ agree with Bill.\n\njdl\n"},{"id":"58717","messageId":"7vabppbxef.fsf@gitster.siamese.dyndns.org","threadId":"10698","inReplyTo":"11944127311587-git-send-email-shawn.bohrer@gmail.com","subject":"Re: [PATCH] Make git-clean a builtin","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-07T20:42:16Z","receivedAt":"2007-11-07T20:42:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Shawn Bohrer writes:\n\n> This replaces git-clean.sh with builtin-clean.c, and moves\n> git-clean.sh to the examples.\n>\n> This also introduces a change in behavior where the -d parameter is\n> required to remove an entire directory of untracked files even when\n> the directory is passed as a path.\n\nThe updated behaviour may be better, but this description at the\nfirst read makes one wonder if it is describing a regression as\nif it is a feature.\n\n> ... For example ...\n> ...\n> if 'dir' only contains untracked files. This is consistent with the\n> old behavior when two or more paths were specified.\n\nI think what you fixed are two inconsistencies in the original\nimplementation. If you spelled out the existing inconsistency\nand described what your implementation does differently, the\nproposal would start looking like a real improvement, like this:\n\n 1. When dir has only untracked files, these two behave differently:\n\n $ git clean -n dir\n $ git clean -n dir/\n\n the former says \"Would not remove dir/\", while the latter would\n say \"Would remove dir/untracked\" for all paths under it.\n\n With -d, the former would stop refusing, but the difference in\n reporting is still there. The latter lists all paths under the\n directory.\n\n 2. When there are more parameters, the latter behave differently:\n\n $ git clean -n dir/ foo\n\n refuses to remove dir/. This is inconsistent.\n\n My reimplementation changes the behaviour by always\n requiring the -d option with or without the trailing slash.\n\nHaving said that, I do not particularly agree with the way the\nnew implementation resolves the existing inconsistencies. \n\nWouldn't it be better to remove \"dir\" when the user explicitly\ntold you to clean \"dir\", with or without the trailing slash?\nThat's what the user asked you to do, isn't it?\n"},{"id":"58723","messageId":"20071107205101.GE6212@lavos.net","threadId":"10698","inReplyTo":"Pine.LNX.4.64.0711071501270.4362@racer.site","subject":"Re: [PATCH] Make git-clean a builtin","fromName":"Brian Downing","fromEmail":"bdowning@lavos.net","sentAt":"2007-11-07T20:51:01Z","receivedAt":"2007-11-07T20:51:01Z","isPatch":true,"sender":{"key":"bdowning@lavos.net","avatar":"https://avatars.githubusercontent.com/u/366426?v=4"},"body":"On Wed, Nov 07, 2007 at 03:04:52PM +0000, Johannes Schindelin wrote:\n> I do count them. Personally, I find it highly distracting and ugly. \n> Besides, we have the convention of putting the \"}\" not into the same line \n> as \"else\". (See keyword \"uncuddling\" in the list archives.)\n\nI was under the impression that Git followed the kernel coding standards,\nwhich seem to want \"cuddled\" else statements:\n\n136 Note that the closing brace is empty on a line of its own, _except_ in\n137 the cases where it is followed by a continuation of the same statement,\n138 ie a \"while\" in a do-statement or an \"else\" in an if-statement, like\n139 this:\n140 \n141 do {\n142 body of do-loop\n143 } while (condition);\n144 \n145 and\n146 \n147 if (x == y) {\n148 ..\n149 } else if (x > y) {\n150 ...\n151 } else {\n152 ....\n153 }\n154 \n155 Rationale: K&R.\n\nSearching the MARC list archives for \"uncuddling\" only yields the message\nI am replying to.\n\nIn addition, the kernel style seems to want braces for all branches of\na conditional if any branch needs it:\n\n163 Do not unnecessarily use braces where a single statement will do.\n164 \n165 if (condition)\n166 action();\n167 \n168 This does not apply if one branch of a conditional statement is a single\n169 statement. Use braces in both branches.\n170 \n171 if (condition) {\n172 do_this();\n173 do_that();\n174 } else {\n175 otherwise();\n176 }\n\nThis makes sense (to me), as at most you're only adding one extra line\nfor the final closing brace, and it makes the whole conditional look more\n\"balanced\", IMHO.\n\nBut regardless, whatever the actual style for Git should be followed.\nLife's too short for arguments about coding style (even if divergence\nfrom K&R brace style is just plain wrong. :)\n\n-bcd\n"},{"id":"58730","messageId":"7v7iktafq2.fsf@gitster.siamese.dyndns.org","threadId":"10698","inReplyTo":"20071107205101.GE6212@lavos.net","subject":"Re: [PATCH] Make git-clean a builtin","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-07T21:49:25Z","receivedAt":"2007-11-07T21:49:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"bdowning@lavos.net (Brian Downing) writes:\n\n> This makes sense (to me), as at most you're only adding one extra line\n> for the final closing brace, and it makes the whole conditional look more\n> \"balanced\", IMHO.\n>\n> But regardless, whatever the actual style for Git should be followed.\n> Life's too short for arguments about coding style (even if divergence\n> from K&R brace style is just plain wrong. :)\n\nOk. We do not have any particularly strong technical reason to\ndeviate from the kernel style. Let's follow that.\n"},{"id":"58797","messageId":"20071108053750.GC6768@mediacenter.austin.rr.com","threadId":"10698","inReplyTo":"7vabppbxef.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Make git-clean a builtin","fromName":"Shawn Bohrer","fromEmail":"shawn.bohrer@gmail.com","sentAt":"2007-11-08T05:37:50Z","receivedAt":"2007-11-08T05:37:50Z","isPatch":true,"sender":{"key":"shawn.bohrer@gmail.com","avatar":"https://gravatar.com/avatar/6eb093ef7d276306d18366254e0c95ff6a5db58231ac7e82fe78c2800aaae1b6?d=mp&s=160"},"body":"On Wed, Nov 07, 2007 at 12:42:16PM -0800, Junio C Hamano wrote:\n> \n> Having said that, I do not particularly agree with the way the\n> new implementation resolves the existing inconsistencies. \n> \n> Wouldn't it be better to remove \"dir\" when the user explicitly\n> told you to clean \"dir\", with or without the trailing slash?\n> That's what the user asked you to do, isn't it?\n\nYes I suppose I agree. Of course I need to spend some more time staring\nat the code to figure out how to do so. Perhaps I can figure out what\nis causing the original inconsistency in git-ls-files while I'm at it.\n"},{"id":"59248","messageId":"87ir49afhi.fsf@catnip.gol.com","threadId":"10698","inReplyTo":"18225.48553.44088.269677@lisa.zopyra.com","subject":"Re: [PATCH] Make git-clean a builtin","fromName":"Miles Bader","fromEmail":"miles@gnu.org","sentAt":"2007-11-10T22:43:37Z","receivedAt":"2007-11-10T22:43:37Z","isPatch":true,"sender":{"key":"miles@gnu.org","avatar":"https://gravatar.com/avatar/01069b69593af7bff28e2f97afeb3644ae6fe2f5f56cb3a8cf34c5fb8c36efe5?d=mp&s=160"},"body":"Bill Lear writes:\n> Why do you find it objectionable?\n\nIt bloats the code, and makes it less readable.\n\n[My conjecture is that the latter happens because braces are so visually\nstriking that they attract the eye; for _long_ blocks, this property of\nbraces helps, because it makes it easier to find them amongst the rest\nof the code, but for _short_ blocks, it hurts, because it draws the eye\naway from the actual code, and emphasizes structure at a time when\nstructure doesn't need emphasizing (because it's utterly obvious already\nwith such short blocks).]\n\n-Miles\n\n-- \nRun away! Run away!\n"}]}