From: Tomi Ollila Date: Wed, 25 May 2016 15:55:43 +0000 (+0300) Subject: Re: [PATCH v3] config: Expand ~ to $HOME X-Git-Url: http://git.tremily.us/?a=commitdiff_plain;h=6e67ecda05173491ee04cb59fd40ae9679951e28;p=notmuch-archives.git Re: [PATCH v3] config: Expand ~ to $HOME --- diff --git a/62/63b063103425169230e583f73952831b70eadd b/62/63b063103425169230e583f73952831b70eadd new file mode 100644 index 000000000..0614245ff --- /dev/null +++ b/62/63b063103425169230e583f73952831b70eadd @@ -0,0 +1,231 @@ +Return-Path: +X-Original-To: notmuch@notmuchmail.org +Delivered-To: notmuch@notmuchmail.org +Received: from localhost (localhost [127.0.0.1]) + by arlo.cworth.org (Postfix) with ESMTP id EF2116DE01F7 + for ; Wed, 25 May 2016 08:55:54 -0700 (PDT) +X-Virus-Scanned: Debian amavisd-new at cworth.org +X-Spam-Flag: NO +X-Spam-Score: 0.589 +X-Spam-Level: +X-Spam-Status: No, score=0.589 tagged_above=-999 required=5 tests=[AWL=-0.063, + SPF_NEUTRAL=0.652] autolearn=disabled +Received: from arlo.cworth.org ([127.0.0.1]) + by localhost (arlo.cworth.org [127.0.0.1]) (amavisd-new, port 10024) + with ESMTP id 9CsfeJYB6O32 for ; + Wed, 25 May 2016 08:55:46 -0700 (PDT) +Received: from guru.guru-group.fi (guru.guru-group.fi [46.183.73.34]) + by arlo.cworth.org (Postfix) with ESMTP id 46E4F6DE00EB + for ; Wed, 25 May 2016 08:55:45 -0700 (PDT) +Received: from guru.guru-group.fi (localhost [IPv6:::1]) + by guru.guru-group.fi (Postfix) with ESMTP id ADDFA100090; + Wed, 25 May 2016 18:55:43 +0300 (EEST) +From: Tomi Ollila +To: Bijan Chokoufe Nejad , notmuch@notmuchmail.org +Subject: Re: [PATCH v3] config: Expand ~ to $HOME +In-Reply-To: <1463238949-22612-1-git-send-email-bijan@chokoufe.com> +References: <1463238949-22612-1-git-send-email-bijan@chokoufe.com> +User-Agent: Notmuch/0.22+26~g2adc7d1 (http://notmuchmail.org) Emacs/24.5.1 + (x86_64-unknown-linux-gnu) +X-Face: HhBM'cA~ +MIME-Version: 1.0 +Content-Type: text/plain +X-BeenThere: notmuch@notmuchmail.org +X-Mailman-Version: 2.1.20 +Precedence: list +List-Id: "Use and development of the notmuch mail system." + +List-Unsubscribe: , + +List-Archive: +List-Post: +List-Help: +List-Subscribe: , + +X-List-Received-Date: Wed, 25 May 2016 15:55:55 -0000 + +On Sat, May 14 2016, Bijan Chokoufe Nejad wrote: + +> Very useful in case you want to keep your .notmuch-config synchronized across +> machines where you have different user names. + + +Good progress! There are quite a few issues still to resolve... + +First, commit message (1st line got a bit outdated). More comments inline + +> +> --- +> v2: Check for '~/' instead of checking for '~' and fix memory issue. +> Unit test is still pending +> v3: Extended implementation handling also '~foo/' and '~foo' +> --- +> notmuch-config.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ +> 1 file changed, 46 insertions(+) +> +> diff --git a/notmuch-config.c b/notmuch-config.c +> index 01bb185..25668a6 100644 +> --- a/notmuch-config.c +> +++ b/notmuch-config.c +> @@ -602,9 +602,55 @@ _config_set_list (notmuch_config_t *config, +> *config_var = NULL; +> } +> +> +static char * +> +_config_get_user_folder (notmuch_config_t *config, const char *path) { + +Naming, but that will go away via other future changes. + +> + char* tmp = talloc_strdup (config, path + 1); +> + const char* user; +> + if (strpbrk(tmp, "/") != NULL) { +> + user = strtok (tmp, "/"); + +Above (sh|c)ould be done like: + + char *user = talloc_strdup (config, path + 1); // XXX path + 1 + char *tmp = strchr(user, '/'); + if (tmp) *tmp = '\0'; + +> + } +> + else { +> + user = tmp; +> + } +> + struct passwd * pwd = getpwnam (user); +> + +> + if (pwd == NULL) { +> + printf("Config: could not expand %s.\n", path); +> + printf("'%s' doesn't seem to be a valid user on this system.\n", user); +> + exit(EXIT_FAILURE); +> + } + +Unable to expand ~ expression is not failure. it is just used unexpanded. +This is the way e.g. shell and emacs does it. + +> + +> + tmp = talloc_asprintf (config, "%s/%s", pwd->pw_dir, path + strlen (user) + 2); +> + return tmp; +> +} +> + +> +static void +> +_config_expand_path (notmuch_config_t *config) { +> + char* path = (char*) _config_get (config, &config->database_path, +> + "database", "path"); + +_config_get done twice in all cases, which doesn't sound like a good idea. + +> + if (path != NULL && path[0] == '~') { +> + char *new_path; +> + if (strlen (path) == 1 || path[1] == '/') { + +strlen(path) scans the whole string and then the return value is just +compared with 1 +better do path[1] == '\0' (after it was checked that path[0] != '\0' above) + +> + const char *home_path = getenv("HOME"); +> + if (strlen (path) == 1) + +ditto + +> + new_path = talloc_asprintf (config, "%s%s", home_path, path + 1); +> + else if (path[1] == '/') +> + new_path = talloc_asprintf (config, "%s/%s", home_path, path + 2); +> + } +> + else { +> + new_path = _config_get_user_folder (config, path); +> + } +> + notmuch_config_set_database_path (config, new_path); + +I think this was my suggestion (or something similar) .. but that is not a +good idea -- some (unrelated) config setting will (or may) expand ~ and +write it to the config file. + +> + talloc_free (new_path); +> + } +> +} +> + +> const char * +> notmuch_config_get_database_path (notmuch_config_t *config) +> { +> + // ideally a get_database_path should only get the database path and this +> + // call to _config_expand_path would be done in a setup phase +> + _config_expand_path (config); +> return _config_get (config, &config->database_path, "database", "path"); +> } +> +> -- +> 2.7.4 + +I think there are at least these 2 ways that might get this done... + +... both uses the expand_tilde WIP i did post earlier -- there is still +something to be fixed there (in addition to header file) + +;; This buffer is for notes you don't want to save, and for Lisp evaluation. +;; If you want to create a file, visit that file with C-x C-f, +;; then enter the text in that file's own buffer. + +--------------- either just with changes in notmuch_config_get_database_path + +const char * +notmuch_config_get_database_path (notmuch_config_t *config) +{ + if (config->database_path != NULL) + return config->database_path; + + (void)_config_get (config, &config->database_path, "database", "path"); + + if (!!config->database_path && config->database_path[0] == '~') { + const char * rest; + const char * path = expand_tilde (config->database_path, &rest); + if (path != NULL) { + talloc_free (config->database_path); + //config->database_path = [path stringByAppendingString rest]; + config->database_path = talloc_asprintf ("%s%s", path, rest); + } + } + return config->database_path; +} + +--------------- or with new helper _config_get_expand_tilde and using that... + +const char * +notmuch_config_get_database_path (notmuch_config_t *config) +{ + return _config_get_expand_tilde (config, &config->database_path, + "database", "path"); +} + + +--- this is to be next to _config_get and the changes of these two needs to +--- be tracked carefully (if there is ever need to change); alternatively +--- support function and flag variable to decice when to expand and when not + +static const char * +_config_get_expand_tilde (notmuch_config_t *config, char **field, + const char *group, const char *key) +{ + /* read from config file and cache value, if not cached already */ + if (*field == NULL) { + char *value; + value = g_key_file_get_string (config->key_file, group, key, NULL); + if (value) { + const char *path, *rest; + if (value[0] == '~' && (path = expand_tilde (value, &rest)) != NULL) + *field = talloc_asprintf ("%s%s", path, rest); + else + *field = talloc_strdup (config, value); + free (value); + } + } + return *field; +} + + + +Tomi +