{"thread":{"id":"43392","subject":"Re: [PATCH 1/2] Allow '-' in config variable names","startedAt":"2006-10-30T16:25:36Z","lastAt":"2006-10-31T09:49:54Z","messageCount":9,"participants":["Junio C Hamano","Johannes Schindelin","J. Bruce Fields","Linus Torvalds"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"298564","messageId":"Pine.LNX.4.64.0610300823250.25218@g5.osdl.org","threadId":"43392","inReplyTo":null,"subject":"[PATCH 1/2] Allow '-' in config variable names","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-10-30T16:25:36Z","receivedAt":"2006-10-30T16:25:36Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nI need this in order to allow aliases of the same form as \"ls-tree\", \n\"rev-parse\" etc, so that I can use\n\n\t[alias]\n\t\tmy-cat=--paginate cat-file -p\n\nto add a \"git my-cat\" command.\n\nSigned-off-by: Linus Torvalds \n---\n\nThis will be followed by another patch to \"builtin-push.c\" to allow me to \nmake a useful (to me) \"push-all\" alias.\n\ndiff --git a/config.c b/config.c\nindex e8f0caf..3cae390 100644\n--- a/config.c\n+++ b/config.c\n@@ -103,6 +103,11 @@ static char *parse_value(void)\n \t}\n }\n \n+static inline int iskeychar(int c)\n+{\n+\treturn isalnum(c) || c == '-';\n+}\n+\n static int get_value(config_fn_t fn, char *name, unsigned int len)\n {\n \tint c;\n@@ -113,7 +118,7 @@ static int get_value(config_fn_t fn, cha\n \t\tc = get_next_char();\n \t\tif (c == EOF)\n \t\t\tbreak;\n-\t\tif (!isalnum(c))\n+\t\tif (!iskeychar(c))\n \t\t\tbreak;\n \t\tname[len++] = tolower(c);\n \t\tif (len >= MAXNAME)\n@@ -181,7 +186,7 @@ static int get_base_var(char *name)\n \t\t\treturn baselen;\n \t\tif (isspace(c))\n \t\t\treturn get_extended_base_var(name, baselen, c);\n-\t\tif (!isalnum(c) && c != '.')\n+\t\tif (!iskeychar(c) && c != '.')\n \t\t\treturn -1;\n \t\tif (baselen > MAXNAME / 2)\n \t\t\treturn -1;\n@@ -573,7 +578,7 @@ int git_config_set_multivar(const char*\n \t\t\tdot = 1;\n \t\t/* Leave the extended basename untouched.. */\n \t\tif (!dot || i > store.baselen) {\n-\t\t\tif (!isalnum(c) || (i == store.baselen+1 && !isalpha(c))) {\n+\t\t\tif (!iskeychar(c) || (i == store.baselen+1 && !isalpha(c))) {\n \t\t\t\tfprintf(stderr, \"invalid key: %s\\n\", key);\n \t\t\t\tfree(store.key);\n"},{"id":"298316","messageId":"Pine.LNX.4.64.0610300825470.25218@g5.osdl.org","threadId":"43392","inReplyTo":"Pine.LNX.4.64.0610300823250.25218@g5.osdl.org","subject":"[PATCH 2/2] git push: add verbose flag and allow overriding of default target repository","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-10-30T16:28:59Z","receivedAt":"2006-10-30T16:28:59Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nThis adds a command line flag \"-v\" to enable a more verbose mode, and \n\"--repo=\" to override the default target repository for \"git push\" (which \notherwise always defaults to \"origin\").\n\nThis, together with the patch to allow dashes in config variable names, \nallows me to do\n\n\t[alias]\n\t\tpush-all = push -v --repo=all\n\nin my user-global config file, and then I can (for any project I maintain) \nadd to the project-local config file\n\n\t[remote \"all\"]\n\t\turl=one.target.repo:/directory\n\t\turl=another.target:/pub/somewhere/else\n\nand now \"git push-all\" just updates all the target repositories, and shows \nme what it does - regardless of which repo I am in.\n\nSigned-off-by: Linus Torvalds \n---\n\nMaybe this is just useful to me? I dunno. I have long had a per-repository \n\"push-all\" script that I have in the root directory of the repos I \nmaintain, but this is much more practical. With this, I can do things like\n\n\tgit push-all --tags\n\nand it will push all the new tags to all the public repositories I \nmaintain for that particular repository.\n\nSpecial Linus-only behaviour? Maybe. On the other hand, I think this is a \npretty clean patch regardless.\n\ndiff --git a/builtin-push.c b/builtin-push.c\nindex f5150ed..3151fb7 100644\n--- a/builtin-push.c\n+++ b/builtin-push.c\n@@ -10,7 +10,7 @@\n \n static const char push_usage[] = \"git-push [--all] [--tags] [-f | --force] [...]\";\n \n-static int all, tags, force, thin = 1;\n+static int all, tags, force, thin = 1, verbose;\n static const char *execute;\n \n #define BUF_SIZE (2084)\n@@ -248,6 +248,8 @@ static int do_push(const char *repo)\n \t\twhile (dest_refspec_nr--)\n \t\t\targv[dest_argc++] = *dest_refspec++;\n \t\targv[dest_argc] = NULL;\n+\t\tif (verbose)\n+\t\t\tfprintf(stderr, \"Pushing to %s\\n\", dest);\n \t\terr = run_command_v(argc, argv);\n \t\tif (!err)\n \t\t\tcontinue;\n@@ -281,6 +283,14 @@ int cmd_push(int argc, const char **argv\n \t\t\ti++;\n \t\t\tbreak;\n \t\t}\n+\t\tif (!strcmp(arg, \"-v\")) {\n+\t\t\tverbose=1;\n+\t\t\tcontinue;\n+\t\t}\n+\t\tif (!strncmp(arg, \"--repo=\", 7)) {\n+\t\t\trepo = arg+7;\n+\t\t\tcontinue;\n+\t\t}\n \t\tif (!strcmp(arg, \"--all\")) {\n \t\t\tall = 1;\n"},{"id":"297918","messageId":"7vodrtv2wy.fsf@assigned-by-dhcp.cox.net","threadId":"43392","inReplyTo":"Pine.LNX.4.64.0610300823250.25218@g5.osdl.org","subject":"Re: [PATCH 1/2] Allow '-' in config variable names","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-10-30T22:45:17Z","receivedAt":"2006-10-30T22:45:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> I need this in order to allow aliases of the same form as \"ls-tree\", \n> \"rev-parse\" etc, so that I can use\n>\n> \t[alias]\n> \t\tmy-cat=--paginate cat-file -p\n>\n> to add a \"git my-cat\" command.\n\nI do not have problem with this (and would perhaps also want to\nadd '_' to keychar set), but people who envisioned parsing\nconfig from scripts (i.e. Perly git) might prefer if we stayed\nwithin alnum, since I'd suspect then they may be able to reuse\nexisting .ini parsers. I do not much care about that myself,\nbut I am bringing it up just in case other people might.\n\nOther than that, this sounds nice.\n\nBy the way, everybody seems to do \"alias.xxx = -p cat-file -p\"\n(I have it as \"git less\"). Maybe we would want to make a\nbuilt-in alias for that?\n"},{"id":"296457","messageId":"7vy7qxtnkg.fsf@assigned-by-dhcp.cox.net","threadId":"43392","inReplyTo":"Pine.LNX.4.64.0610300825470.25218@g5.osdl.org","subject":"Re: [PATCH 2/2] git push: add verbose flag and allow overriding of default target repository","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-10-30T23:02:07Z","receivedAt":"2006-10-30T23:02:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> This adds a command line flag \"-v\" to enable a more verbose mode, and \n> \"--repo=\" to override the default target repository for \"git push\" (which \n> otherwise always defaults to \"origin\").\n>\n> This, together with the patch to allow dashes in config variable names, \n> allows me to do\n>\n> \t[alias]\n> \t\tpush-all = push -v --repo=all\n>\n> in my user-global config file, and then I can (for any project I maintain) \n> add to the project-local config file\n>\n> \t[remote \"all\"]\n> \t\turl=one.target.repo:/directory\n> \t\turl=another.target:/pub/somewhere/else\n>\n> and now \"git push-all\" just updates all the target repositories, and shows \n> me what it does - regardless of which repo I am in.\n>\n> Signed-off-by: Linus Torvalds \n> ---\n>\n> Maybe this is just useful to me? I dunno.\n\nI would say if something is useful to you it is useful to your\nsubsystem people and anybody who has a public tree and more than\none machines to verify the tips of his branches on, including\nme.\n\n"},{"id":"294602","messageId":"7vfyd5rxvg.fsf@assigned-by-dhcp.cox.net","threadId":"43392","inReplyTo":"7vodrtv2wy.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 1/2] Allow '-' in config variable names","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-10-31T03:02:27Z","receivedAt":"2006-10-31T03:02:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Linus Torvalds writes:\n>\n>> I need this in order to allow aliases of the same form as \"ls-tree\", \n>> \"rev-parse\" etc, so that I can use\n>>\n>> \t[alias]\n>> \t\tmy-cat=--paginate cat-file -p\n>>\n>> to add a \"git my-cat\" command.\n>\n> I do not have problem with this (and would perhaps also want to\n> add '_' to keychar set), but people who envisioned parsing\n> config from scripts (i.e. Perly git) might prefer if we stayed\n> within alnum, since I'd suspect then they may be able to reuse\n> existing .ini parsers. I do not much care about that myself,\n> but I am bringing it up just in case other people might.\n>\n> Other than that, this sounds nice.\n\nOne thing I forgot to add. Just like we downcase what user has\nwritten in config file, it might make sense to also remove '-'\n(and if we add '_' to keychar set, that one as well) to when\ncanonicalizing the key value. That way, somewhat awkward long\nconfiguration variables we currently have can be written more\nreadably, e.g. repack.use-delta-base-offset\n\nLikes, dislikes? It is not strictly needed, since we can do\nCamelCase as well in the configuration file.\n\n> By the way, everybody seems to do \"alias.xxx = -p cat-file -p\"\n> (I have it as \"git less\"). Maybe we would want to make a\n> built-in alias for that?\n\nSeconds?\n"},{"id":"297934","messageId":"20061031030556.GB16445@fieldses.org","threadId":"43392","inReplyTo":"7vfyd5rxvg.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 1/2] Allow '-' in config variable names","fromName":"J. Bruce Fields","fromEmail":"bfields@fieldses.org","sentAt":"2006-10-31T03:05:56Z","receivedAt":"2006-10-31T03:05:56Z","isPatch":true,"sender":{"key":"bfields@citi.umich.edu","avatar":null},"body":"On Mon, Oct 30, 2006 at 07:02:27PM -0800, Junio C Hamano wrote:\n> Junio C Hamano writes:\n> > By the way, everybody seems to do \"alias.xxx = -p cat-file -p\"\n> > (I have it as \"git less\"). Maybe we would want to make a\n> > built-in alias for that?\n> \n> Seconds?\n\nYes!\n\nI'd vote for calling it just \"git cat\".\n\n"},{"id":"296847","messageId":"7vbqntrxj8.fsf@assigned-by-dhcp.cox.net","threadId":"43392","inReplyTo":"Pine.LNX.4.64.0610300825470.25218@g5.osdl.org","subject":"Re: [PATCH 2/2] git push: add verbose flag and allow overriding of default target repository","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-10-31T03:09:47Z","receivedAt":"2006-10-31T03:09:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> diff --git a/builtin-push.c b/builtin-push.c\n> index f5150ed..3151fb7 100644\n> --- a/builtin-push.c\n> +++ b/builtin-push.c\n> @@ -10,7 +10,7 @@\n> \n> static const char push_usage[] = \"git-push [--all] [--tags] [-f | --force] [...]\";\n> \n> -static int all, tags, force, thin = 1;\n> +static int all, tags, force, thin = 1, verbose;\n> static const char *execute;\n> \n> #define BUF_SIZE (2084)\n\nBy the way, has anybody noticed we use a funny value for the\nbuffer size?\n"},{"id":"298079","messageId":"Pine.LNX.4.64.0610301922020.25218@g5.osdl.org","threadId":"43392","inReplyTo":"7vfyd5rxvg.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 1/2] Allow '-' in config variable names","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-10-31T03:23:20Z","receivedAt":"2006-10-31T03:23:20Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Mon, 30 Oct 2006, Junio C Hamano wrote:\n> \n> One thing I forgot to add. Just like we downcase what user has\n> written in config file, it might make sense to also remove '-'\n> (and if we add '_' to keychar set, that one as well) to when\n> canonicalizing the key value. That way, somewhat awkward long\n> configuration variables we currently have can be written more\n> readably, e.g. repack.use-delta-base-offset\n\nWell, then we need to teach \"git \" that \"git \" is the same \nthing as \"alias\".\n\n> > By the way, everybody seems to do \"alias.xxx = -p cat-file -p\"\n> > (I have it as \"git less\"). Maybe we would want to make a\n> > built-in alias for that?\n> \n> Seconds?\n\nI like it mainly as a great example of how to use aliases, not because I \nactually much -use- the thing, so I don't personally care.\n\n"},{"id":"297406","messageId":"Pine.LNX.4.63.0610311047250.26682@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"43392","inReplyTo":"7vfyd5rxvg.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 1/2] Allow '-' in config variable names","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-10-31T09:49:54Z","receivedAt":"2006-10-31T09:49:54Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 30 Oct 2006, Junio C Hamano wrote:\n\n> Junio C Hamano writes:\n> \n> > Linus Torvalds writes:\n> >\n> >> I need this in order to allow aliases of the same form as \"ls-tree\", \n> >> \"rev-parse\" etc, so that I can use\n> >>\n> >> \t[alias]\n> >> \t\tmy-cat=--paginate cat-file -p\n> >>\n> >> to add a \"git my-cat\" command.\n> >\n> Seconds?\n\nI personally dislike any name with \"_\" or \"-\" in it, since I am stuck with \ndifferent keyboard layouts and keep mistyping them. I even often find \nmyself hitting just to complete a _single_ \"-\", since the position \nof the key is not wildly jumping around between different keyboard \nlayouts.\n\nSo I can live without it.\n\nCiao,\nDscho\n"}]}