* [PATCH 1/2] lib: Add back the synchronization of 'T' flag with deleted tag. @ 2011-07-17 3:56 Antoine Beaupré 2011-07-17 3:56 ` [PATCH 2/2] lib: add 'safe' setting for flags Antoine Beaupré 2012-01-06 22:37 ` [PATCH 1/2] lib: Add back the synchronization of 'T' flag with deleted tag Jani Nikula 0 siblings, 2 replies; 5+ messages in thread From: Antoine Beaupré @ 2011-07-17 3:56 UTC (permalink / raw) To: notmuch; +Cc: Antoine Beaupré This adds a special configuration, off by default, that allows notmuch to synchronize the T flag again. The configuration is named maildir_reckless_trash and quite clearly indicates that it could be dangerous to use in the context described in commit 2c26204, which I could actually reproduce. In contexts where notmuch is the only mail client used, this is actually safe to use. Besides, (T)rashed messages are not necessarily immediately expunged from the Maildir by the client or the IMAP server. Signed-off-by: Antoine Beaupré <anarcat@koumbit.org> --- lib/message.cc | 14 +++++++++++++- lib/notmuch.h | 4 ++++ notmuch-client.h | 7 +++++++ notmuch-config.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++--- 4 files changed, 71 insertions(+), 4 deletions(-) diff --git a/lib/message.cc b/lib/message.cc index d993cde..f633887 100644 --- a/lib/message.cc +++ b/lib/message.cc @@ -58,7 +58,8 @@ static struct maildir_flag_tag flag2tag[] = { { 'F', "flagged", false}, { 'P', "passed", false}, { 'R', "replied", false}, - { 'S', "unread", true } + { 'S', "unread", true }, + { 'T', "deleted", false}, }; /* We end up having to call the destructor explicitly because we had @@ -993,6 +994,7 @@ notmuch_message_maildir_flags_to_tags (notmuch_message_t *message) char *combined_flags = talloc_strdup (message, ""); unsigned i; int seen_maildir_info = 0; + notmuch_bool_t reckless_trash; for (filenames = notmuch_message_get_filenames (message); notmuch_filenames_valid (filenames); @@ -1020,7 +1022,17 @@ notmuch_message_maildir_flags_to_tags (notmuch_message_t *message) if (status) return status; + // TODO: this should probably be moved up in the stack to avoid + // opening the config file on every message (!) + config = notmuch_config_open (ctx, NULL, NULL); + if (config == NULL) + return 1; + reckless_trash = notmuch_config_get_maildir_reckless_trash (config); + for (i = 0; i < ARRAY_SIZE(flag2tag); i++) { + if (flag2tag[i].flag == 'T' && !reckless_trash) { + continue; + } if ((strchr (combined_flags, flag2tag[i].flag) != NULL) ^ flag2tag[i].inverse) diff --git a/lib/notmuch.h b/lib/notmuch.h index 974be8d..f0c1b67 100644 --- a/lib/notmuch.h +++ b/lib/notmuch.h @@ -922,6 +922,8 @@ notmuch_message_remove_all_tags (notmuch_message_t *message); * 'P' Adds the "passed" tag to the message * 'R' Adds the "replied" tag to the message * 'S' Removes the "unread" tag from the message + * 'T' Adds the "deleted" tag to the message and + * state->reckless_trash is TRUE. * * For each flag that is not present, the opposite action (add/remove) * is performed for the corresponding tags. @@ -962,6 +964,8 @@ notmuch_message_maildir_flags_to_tags (notmuch_message_t *message); * 'P' iff the message has the "passed" tag * 'R' iff the message has the "replied" tag * 'S' iff the message does not have the "unread" tag + * 'T' iff the message has the "trashed" tag and + * state->reckless_trash is TRUE. * * Any existing flags unmentioned in the list above will be preserved * in the renaming. diff --git a/notmuch-client.h b/notmuch-client.h index 63be337..62d1e0e 100644 --- a/notmuch-client.h +++ b/notmuch-client.h @@ -235,6 +235,13 @@ notmuch_config_set_maildir_synchronize_flags (notmuch_config_t *config, notmuch_bool_t synchronize_flags); notmuch_bool_t +notmuch_config_get_maildir_reckless_trash (notmuch_config_t *config); + +void +notmuch_config_set_maildir_reckless_trash (notmuch_config_t *config, + notmuch_bool_t reckless_trash); + +notmuch_bool_t debugger_is_active (void); #endif diff --git a/notmuch-config.c b/notmuch-config.c index 485fa72..613fefc 100644 --- a/notmuch-config.c +++ b/notmuch-config.c @@ -67,9 +67,11 @@ static const char maildir_config_comment[] = " The following option is supported here:\n" "\n" "\tsynchronize_flags Valid values are true and false.\n" + "\treckless_trash Valid values are true and false.\n" "\n" - "\tIf true, then the following maildir flags (in message filenames)\n" - "\twill be synchronized with the corresponding notmuch tags:\n" + "\tIf synchronize_flags is true, then the following maildir flags\n" + "\t(in message filenames) will be synchronized with the corresponding\n" + "\tnotmuch tags:\n" "\n" "\t\tFlag Tag\n" "\t\t---- -------\n" @@ -78,10 +80,26 @@ static const char maildir_config_comment[] = "\t\tP passed\n" "\t\tR replied\n" "\t\tS unread (added when 'S' flag is not present)\n" + "\t\tT trashed (only if maildir_reckless_trash is enabled)\n" "\n" "\tThe \"notmuch new\" command will notice flag changes in filenames\n" "\tand update tags, while the \"notmuch tag\" and \"notmuch restore\"\n" - "\tcommands will notice tag changes and update flags in filenames\n"; + "\tcommands will notice tag changes and update flags in filenames\n" + "\n" + "\tBy default the maildir synchronization code doesn't look at the\n" + "\t'trashed' (T) flag on messages. The reason for this behaviour is\n" + "\tit can be dangerous if another mail client is trying to delete\n" + "\tduplicates of a message with the same message ID.\n" + "\n" + "\tA workaround for this issue is to expunge the duplicate messages\n" + "\twith the other client before running notmuch new.\n" + "\n" + "\tIf you are using only notmuch to handle your emails or are never\n" + "\tdoing such operations, enabling reckless_trash should be safe,\n" + "\tbut otherwise it is safer to keep this disabled and delete mails\n" + "\tmanually with a shell command like:\n" + "\n" + "\tnotmuch search --format=files tag:deleted | xargs rm\n"; struct _notmuch_config { char *filename; @@ -95,6 +113,7 @@ struct _notmuch_config { const char **new_tags; size_t new_tags_length; notmuch_bool_t maildir_synchronize_flags; + notmuch_bool_t maildir_reckless_trash; }; static int @@ -251,6 +270,7 @@ notmuch_config_open (void *ctx, config->new_tags = NULL; config->new_tags_length = 0; config->maildir_synchronize_flags = TRUE; + config->maildir_reckless_trash = FALSE; if (! g_key_file_load_from_file (config->key_file, config->filename, @@ -353,6 +373,15 @@ notmuch_config_open (void *ctx, g_error_free (error); } + error = NULL; + config->maildir_reckless_trash = + g_key_file_get_boolean (config->key_file, + "maildir", "reckless_trash", &error); + if (error) { + notmuch_config_set_maildir_reckless_trash (config, FALSE); + g_error_free (error); + } + /* Whenever we know of configuration sections that don't appear in * the configuration file, we add some comments to help the user * understand what can be done. */ @@ -764,3 +793,18 @@ notmuch_config_set_maildir_synchronize_flags (notmuch_config_t *config, "maildir", "synchronize_flags", synchronize_flags); config->maildir_synchronize_flags = synchronize_flags; } + +notmuch_bool_t +notmuch_config_get_maildir_reckless_trash (notmuch_config_t *config) +{ + return config->maildir_reckless_trash; +} + +void +notmuch_config_set_maildir_reckless_trash (notmuch_config_t *config, + notmuch_bool_t reckless_trash) +{ + g_key_file_set_boolean (config->key_file, + "maildir", "reckless_trash", reckless_trash); + config->maildir_reckless_trash = reckless_trash; +} -- 1.7.2.5 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] lib: add 'safe' setting for flags 2011-07-17 3:56 [PATCH 1/2] lib: Add back the synchronization of 'T' flag with deleted tag Antoine Beaupré @ 2011-07-17 3:56 ` Antoine Beaupré 2012-01-06 22:45 ` Jani Nikula 2012-01-06 22:37 ` [PATCH 1/2] lib: Add back the synchronization of 'T' flag with deleted tag Jani Nikula 1 sibling, 1 reply; 5+ messages in thread From: Antoine Beaupré @ 2011-07-17 3:56 UTC (permalink / raw) To: notmuch; +Cc: Antoine Beaupré the 'safe' setting needs to be 'true' for flags to be manipulated by notmuch new/tag/restore. for now, only the (T)rash tag is configurable and set to false (by default) but this could be extended to allow the user to configure which flags are allowed to be synchronized. the reason why only T is configurable is because (a) it's the the only one that is actually dangerous and (b) I couldn't figure out how to properly configure multiple settings like this. --- lib/message.cc | 40 +++++++++++++++++++++++++--------------- lib/notmuch.h | 20 ++++++++++++++++---- notmuch-new.c | 1 + notmuch-restore.c | 1 + notmuch-tag.c | 1 + 5 files changed, 44 insertions(+), 19 deletions(-) diff --git a/lib/message.cc b/lib/message.cc index f633887..f812648 100644 --- a/lib/message.cc +++ b/lib/message.cc @@ -50,16 +50,17 @@ struct maildir_flag_tag { char flag; const char *tag; bool inverse; + bool safe; }; /* ASCII ordered table of Maildir flags and associated tags */ static struct maildir_flag_tag flag2tag[] = { - { 'D', "draft", false}, - { 'F', "flagged", false}, - { 'P', "passed", false}, - { 'R', "replied", false}, - { 'S', "unread", true }, - { 'T', "deleted", false}, + { 'D', "draft", false, true}, + { 'F', "flagged", false, true}, + { 'P', "passed", false, true}, + { 'R', "replied", false, true}, + { 'S', "unread", true , true}, + { 'T', "deleted", false, false}, }; /* We end up having to call the destructor explicitly because we had @@ -994,7 +995,6 @@ notmuch_message_maildir_flags_to_tags (notmuch_message_t *message) char *combined_flags = talloc_strdup (message, ""); unsigned i; int seen_maildir_info = 0; - notmuch_bool_t reckless_trash; for (filenames = notmuch_message_get_filenames (message); notmuch_filenames_valid (filenames); @@ -1022,15 +1022,8 @@ notmuch_message_maildir_flags_to_tags (notmuch_message_t *message) if (status) return status; - // TODO: this should probably be moved up in the stack to avoid - // opening the config file on every message (!) - config = notmuch_config_open (ctx, NULL, NULL); - if (config == NULL) - return 1; - reckless_trash = notmuch_config_get_maildir_reckless_trash (config); - for (i = 0; i < ARRAY_SIZE(flag2tag); i++) { - if (flag2tag[i].flag == 'T' && !reckless_trash) { + if (!flag2tag[i].safe) { continue; } if ((strchr (combined_flags, flag2tag[i].flag) != NULL) @@ -1119,6 +1112,9 @@ _get_maildir_flag_actions (notmuch_message_t *message, tag = notmuch_tags_get (tags); for (i = 0; i < ARRAY_SIZE (flag2tag); i++) { + if (!flag2tag[i].safe) { + continue; + } if (strcmp (tag, flag2tag[i].tag) == 0) { if (flag2tag[i].inverse) to_clear = talloc_asprintf_append (to_clear, @@ -1134,6 +1130,9 @@ _get_maildir_flag_actions (notmuch_message_t *message, /* Then, find the flags for all tags not present. */ for (i = 0; i < ARRAY_SIZE (flag2tag); i++) { + if (!flag2tag[i].safe) { + continue; + } if (flag2tag[i].inverse) { if (strchr (to_clear, flag2tag[i].flag) == NULL) to_set = talloc_asprintf_append (to_set, "%c", flag2tag[i].flag); @@ -1256,6 +1255,17 @@ _new_maildir_filename (void *ctx, return filename_new; } +void +notmuch_message_set_flag_safety (char flag, notmuch_bool_t safe) +{ + unsigned i; + for (i = 0; i < ARRAY_SIZE (flag2tag); i++) { + if (flag2tag[i].flag == flag) { + flag2tag[i].safe = safe; + } + } +} + notmuch_status_t notmuch_message_tags_to_maildir_flags (notmuch_message_t *message) { diff --git a/lib/notmuch.h b/lib/notmuch.h index f0c1b67..475e75a 100644 --- a/lib/notmuch.h +++ b/lib/notmuch.h @@ -922,8 +922,7 @@ notmuch_message_remove_all_tags (notmuch_message_t *message); * 'P' Adds the "passed" tag to the message * 'R' Adds the "replied" tag to the message * 'S' Removes the "unread" tag from the message - * 'T' Adds the "deleted" tag to the message and - * state->reckless_trash is TRUE. + * 'T' Adds the "deleted" tag to the message * * For each flag that is not present, the opposite action (add/remove) * is performed for the corresponding tags. @@ -941,6 +940,9 @@ notmuch_message_remove_all_tags (notmuch_message_t *message); * notmuch_database_add_message. See also * notmuch_message_tags_to_maildir_flags for synchronizing tag changes * back to maildir flags. + * + * Actions are performed only if the tag is marked as "safe" in the + * configuration (only used by maildir_reckless_trash for now). */ notmuch_status_t notmuch_message_maildir_flags_to_tags (notmuch_message_t *message); @@ -964,8 +966,7 @@ notmuch_message_maildir_flags_to_tags (notmuch_message_t *message); * 'P' iff the message has the "passed" tag * 'R' iff the message has the "replied" tag * 'S' iff the message does not have the "unread" tag - * 'T' iff the message has the "trashed" tag and - * state->reckless_trash is TRUE. + * 'T' iff the message has the "trashed" tag * * Any existing flags unmentioned in the list above will be preserved * in the renaming. @@ -979,10 +980,21 @@ notmuch_message_maildir_flags_to_tags (notmuch_message_t *message); * notmuch_message_remove_tag, or notmuch_message_freeze/ * notmuch_message_thaw). See also notmuch_message_maildir_flags_to_tags * for synchronizing maildir flag changes back to tags. + * + * Actions are performed only if the tag is marked as "safe" in the + * configuration (only used by maildir_reckless_trash for now). */ notmuch_status_t notmuch_message_tags_to_maildir_flags (notmuch_message_t *message); +/* Set the 'safe' setting on the given flag + * + * The flag is the one-character IMAP flag, for example for Trashed + * messages, it's the char 'T'. + */ +void +notmuch_message_set_flag_safety(char flag, notmuch_bool_t safe); + /* Freeze the current state of 'message' within the database. * * This means that changes to the message state, (via diff --git a/notmuch-new.c b/notmuch-new.c index 7d17793..1502a70 100644 --- a/notmuch-new.c +++ b/notmuch-new.c @@ -801,6 +801,7 @@ notmuch_new_command (void *ctx, int argc, char *argv[]) add_files_state.new_tags = notmuch_config_get_new_tags (config, &add_files_state.new_tags_length); add_files_state.synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config); + notmuch_message_set_flag_safety('T', notmuch_config_get_maildir_reckless_trash (config)); add_files_state.message_ids_to_sync = _filename_list_create (ctx); db_path = notmuch_config_get_database_path (config); diff --git a/notmuch-restore.c b/notmuch-restore.c index f095f64..1f3622e 100644 --- a/notmuch-restore.c +++ b/notmuch-restore.c @@ -43,6 +43,7 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) return 1; synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config); + notmuch_message_set_flag_safety('T', notmuch_config_get_maildir_reckless_trash (config)); if (argc) { input = fopen (argv[0], "r"); diff --git a/notmuch-tag.c b/notmuch-tag.c index 6204ae3..e2f7cb8 100644 --- a/notmuch-tag.c +++ b/notmuch-tag.c @@ -101,6 +101,7 @@ notmuch_tag_command (void *ctx, unused (int argc), unused (char *argv[])) return 1; synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config); + notmuch_message_set_flag_safety('T', notmuch_config_get_maildir_reckless_trash (config)); query = notmuch_query_create (notmuch, query_string); if (query == NULL) { -- 1.7.2.5 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] lib: add 'safe' setting for flags 2011-07-17 3:56 ` [PATCH 2/2] lib: add 'safe' setting for flags Antoine Beaupré @ 2012-01-06 22:45 ` Jani Nikula 0 siblings, 0 replies; 5+ messages in thread From: Jani Nikula @ 2012-01-06 22:45 UTC (permalink / raw) To: Antoine Beaupré, notmuch; +Cc: Antoine Beaupré On Sat, 16 Jul 2011 23:56:13 -0400, Antoine Beaupré <anarcat@koumbit.org> wrote: > the 'safe' setting needs to be 'true' for flags to be manipulated by > notmuch new/tag/restore. > > for now, only the (T)rash tag is configurable and set to false (by > default) but this could be extended to allow the user to configure which > flags are allowed to be synchronized. > > the reason why only T is configurable is because (a) it's the the only > one that is actually dangerous and (b) I couldn't figure out how to > properly configure multiple settings like this. > --- > lib/message.cc | 40 +++++++++++++++++++++++++--------------- > lib/notmuch.h | 20 ++++++++++++++++---- > notmuch-new.c | 1 + > notmuch-restore.c | 1 + > notmuch-tag.c | 1 + > 5 files changed, 44 insertions(+), 19 deletions(-) > > diff --git a/lib/message.cc b/lib/message.cc > index f633887..f812648 100644 > --- a/lib/message.cc > +++ b/lib/message.cc > @@ -50,16 +50,17 @@ struct maildir_flag_tag { > char flag; > const char *tag; > bool inverse; > + bool safe; > }; > > /* ASCII ordered table of Maildir flags and associated tags */ > static struct maildir_flag_tag flag2tag[] = { > - { 'D', "draft", false}, > - { 'F', "flagged", false}, > - { 'P', "passed", false}, > - { 'R', "replied", false}, > - { 'S', "unread", true }, > - { 'T', "deleted", false}, > + { 'D', "draft", false, true}, > + { 'F', "flagged", false, true}, > + { 'P', "passed", false, true}, > + { 'R', "replied", false, true}, > + { 'S', "unread", true , true}, > + { 'T', "deleted", false, false}, > }; > > /* We end up having to call the destructor explicitly because we had > @@ -994,7 +995,6 @@ notmuch_message_maildir_flags_to_tags (notmuch_message_t *message) > char *combined_flags = talloc_strdup (message, ""); > unsigned i; > int seen_maildir_info = 0; > - notmuch_bool_t reckless_trash; > > for (filenames = notmuch_message_get_filenames (message); > notmuch_filenames_valid (filenames); > @@ -1022,15 +1022,8 @@ notmuch_message_maildir_flags_to_tags (notmuch_message_t *message) > if (status) > return status; > > - // TODO: this should probably be moved up in the stack to avoid > - // opening the config file on every message (!) > - config = notmuch_config_open (ctx, NULL, NULL); > - if (config == NULL) > - return 1; > - reckless_trash = notmuch_config_get_maildir_reckless_trash (config); > - Antoine - You do *not* send a patch 1/2 that adds features and then 2/2 that takes them away. I feel like I totally wasted my time reviewing the first. Jani. > for (i = 0; i < ARRAY_SIZE(flag2tag); i++) { > - if (flag2tag[i].flag == 'T' && !reckless_trash) { > + if (!flag2tag[i].safe) { > continue; > } > if ((strchr (combined_flags, flag2tag[i].flag) != NULL) > @@ -1119,6 +1112,9 @@ _get_maildir_flag_actions (notmuch_message_t *message, > tag = notmuch_tags_get (tags); > > for (i = 0; i < ARRAY_SIZE (flag2tag); i++) { > + if (!flag2tag[i].safe) { > + continue; > + } > if (strcmp (tag, flag2tag[i].tag) == 0) { > if (flag2tag[i].inverse) > to_clear = talloc_asprintf_append (to_clear, > @@ -1134,6 +1130,9 @@ _get_maildir_flag_actions (notmuch_message_t *message, > > /* Then, find the flags for all tags not present. */ > for (i = 0; i < ARRAY_SIZE (flag2tag); i++) { > + if (!flag2tag[i].safe) { > + continue; > + } > if (flag2tag[i].inverse) { > if (strchr (to_clear, flag2tag[i].flag) == NULL) > to_set = talloc_asprintf_append (to_set, "%c", flag2tag[i].flag); > @@ -1256,6 +1255,17 @@ _new_maildir_filename (void *ctx, > return filename_new; > } > > +void > +notmuch_message_set_flag_safety (char flag, notmuch_bool_t safe) > +{ > + unsigned i; > + for (i = 0; i < ARRAY_SIZE (flag2tag); i++) { > + if (flag2tag[i].flag == flag) { > + flag2tag[i].safe = safe; > + } > + } > +} > + > notmuch_status_t > notmuch_message_tags_to_maildir_flags (notmuch_message_t *message) > { > diff --git a/lib/notmuch.h b/lib/notmuch.h > index f0c1b67..475e75a 100644 > --- a/lib/notmuch.h > +++ b/lib/notmuch.h > @@ -922,8 +922,7 @@ notmuch_message_remove_all_tags (notmuch_message_t *message); > * 'P' Adds the "passed" tag to the message > * 'R' Adds the "replied" tag to the message > * 'S' Removes the "unread" tag from the message > - * 'T' Adds the "deleted" tag to the message and > - * state->reckless_trash is TRUE. > + * 'T' Adds the "deleted" tag to the message > * > * For each flag that is not present, the opposite action (add/remove) > * is performed for the corresponding tags. > @@ -941,6 +940,9 @@ notmuch_message_remove_all_tags (notmuch_message_t *message); > * notmuch_database_add_message. See also > * notmuch_message_tags_to_maildir_flags for synchronizing tag changes > * back to maildir flags. > + * > + * Actions are performed only if the tag is marked as "safe" in the > + * configuration (only used by maildir_reckless_trash for now). > */ > notmuch_status_t > notmuch_message_maildir_flags_to_tags (notmuch_message_t *message); > @@ -964,8 +966,7 @@ notmuch_message_maildir_flags_to_tags (notmuch_message_t *message); > * 'P' iff the message has the "passed" tag > * 'R' iff the message has the "replied" tag > * 'S' iff the message does not have the "unread" tag > - * 'T' iff the message has the "trashed" tag and > - * state->reckless_trash is TRUE. > + * 'T' iff the message has the "trashed" tag > * > * Any existing flags unmentioned in the list above will be preserved > * in the renaming. > @@ -979,10 +980,21 @@ notmuch_message_maildir_flags_to_tags (notmuch_message_t *message); > * notmuch_message_remove_tag, or notmuch_message_freeze/ > * notmuch_message_thaw). See also notmuch_message_maildir_flags_to_tags > * for synchronizing maildir flag changes back to tags. > + * > + * Actions are performed only if the tag is marked as "safe" in the > + * configuration (only used by maildir_reckless_trash for now). > */ > notmuch_status_t > notmuch_message_tags_to_maildir_flags (notmuch_message_t *message); > > +/* Set the 'safe' setting on the given flag > + * > + * The flag is the one-character IMAP flag, for example for Trashed > + * messages, it's the char 'T'. > + */ > +void > +notmuch_message_set_flag_safety(char flag, notmuch_bool_t safe); > + > /* Freeze the current state of 'message' within the database. > * > * This means that changes to the message state, (via > diff --git a/notmuch-new.c b/notmuch-new.c > index 7d17793..1502a70 100644 > --- a/notmuch-new.c > +++ b/notmuch-new.c > @@ -801,6 +801,7 @@ notmuch_new_command (void *ctx, int argc, char *argv[]) > > add_files_state.new_tags = notmuch_config_get_new_tags (config, &add_files_state.new_tags_length); > add_files_state.synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config); > + notmuch_message_set_flag_safety('T', notmuch_config_get_maildir_reckless_trash (config)); > add_files_state.message_ids_to_sync = _filename_list_create (ctx); > db_path = notmuch_config_get_database_path (config); > > diff --git a/notmuch-restore.c b/notmuch-restore.c > index f095f64..1f3622e 100644 > --- a/notmuch-restore.c > +++ b/notmuch-restore.c > @@ -43,6 +43,7 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) > return 1; > > synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config); > + notmuch_message_set_flag_safety('T', notmuch_config_get_maildir_reckless_trash (config)); > > if (argc) { > input = fopen (argv[0], "r"); > diff --git a/notmuch-tag.c b/notmuch-tag.c > index 6204ae3..e2f7cb8 100644 > --- a/notmuch-tag.c > +++ b/notmuch-tag.c > @@ -101,6 +101,7 @@ notmuch_tag_command (void *ctx, unused (int argc), unused (char *argv[])) > return 1; > > synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config); > + notmuch_message_set_flag_safety('T', notmuch_config_get_maildir_reckless_trash (config)); > > query = notmuch_query_create (notmuch, query_string); > if (query == NULL) { > -- > 1.7.2.5 > > _______________________________________________ > notmuch mailing list > notmuch@notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] lib: Add back the synchronization of 'T' flag with deleted tag. 2011-07-17 3:56 [PATCH 1/2] lib: Add back the synchronization of 'T' flag with deleted tag Antoine Beaupré 2011-07-17 3:56 ` [PATCH 2/2] lib: add 'safe' setting for flags Antoine Beaupré @ 2012-01-06 22:37 ` Jani Nikula 2012-01-17 5:12 ` Antoine Beaupré 1 sibling, 1 reply; 5+ messages in thread From: Jani Nikula @ 2012-01-06 22:37 UTC (permalink / raw) To: Antoine Beaupré, notmuch; +Cc: Antoine Beaupré On Sat, 16 Jul 2011 23:56:12 -0400, Antoine Beaupré <anarcat@koumbit.org> wrote: > This adds a special configuration, off by default, that allows notmuch > to synchronize the T flag again. The configuration is named > maildir_reckless_trash and quite clearly indicates that it could be > dangerous to use in the context described in commit 2c26204, which I > could actually reproduce. Thanks for the commit reference. Please find some comments below. > In contexts where notmuch is the only mail client used, this is actually > safe to use. Besides, (T)rashed messages are not necessarily immediately > expunged from the Maildir by the client or the IMAP server. > > Signed-off-by: Antoine Beaupré <anarcat@koumbit.org> > --- > lib/message.cc | 14 +++++++++++++- > lib/notmuch.h | 4 ++++ > notmuch-client.h | 7 +++++++ > notmuch-config.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++--- > 4 files changed, 71 insertions(+), 4 deletions(-) > > diff --git a/lib/message.cc b/lib/message.cc > index d993cde..f633887 100644 > --- a/lib/message.cc > +++ b/lib/message.cc > @@ -58,7 +58,8 @@ static struct maildir_flag_tag flag2tag[] = { > { 'F', "flagged", false}, > { 'P', "passed", false}, > { 'R', "replied", false}, > - { 'S', "unread", true } > + { 'S', "unread", true }, > + { 'T', "deleted", false}, > }; > > /* We end up having to call the destructor explicitly because we had > @@ -993,6 +994,7 @@ notmuch_message_maildir_flags_to_tags (notmuch_message_t *message) > char *combined_flags = talloc_strdup (message, ""); > unsigned i; > int seen_maildir_info = 0; > + notmuch_bool_t reckless_trash; > > for (filenames = notmuch_message_get_filenames (message); > notmuch_filenames_valid (filenames); > @@ -1020,7 +1022,17 @@ notmuch_message_maildir_flags_to_tags (notmuch_message_t *message) > if (status) > return status; > > + // TODO: this should probably be moved up in the stack to avoid > + // opening the config file on every message (!) > + config = notmuch_config_open (ctx, NULL, NULL); The config file is for notmuch the command line tool, *not* for the lib. You can't call the cli from from the lib. The config (or command line argument) should be passed as argument, but that would require changing the lib interface. > + if (config == NULL) > + return 1; > + reckless_trash = notmuch_config_get_maildir_reckless_trash (config); > + > for (i = 0; i < ARRAY_SIZE(flag2tag); i++) { > + if (flag2tag[i].flag == 'T' && !reckless_trash) { > + continue; > + } > if ((strchr (combined_flags, flag2tag[i].flag) != NULL) > ^ > flag2tag[i].inverse) > diff --git a/lib/notmuch.h b/lib/notmuch.h > index 974be8d..f0c1b67 100644 > --- a/lib/notmuch.h > +++ b/lib/notmuch.h > @@ -922,6 +922,8 @@ notmuch_message_remove_all_tags (notmuch_message_t *message); > * 'P' Adds the "passed" tag to the message > * 'R' Adds the "replied" tag to the message > * 'S' Removes the "unread" tag from the message > + * 'T' Adds the "deleted" tag to the message and > + * state->reckless_trash is TRUE. > * > * For each flag that is not present, the opposite action (add/remove) > * is performed for the corresponding tags. > @@ -962,6 +964,8 @@ notmuch_message_maildir_flags_to_tags (notmuch_message_t *message); > * 'P' iff the message has the "passed" tag > * 'R' iff the message has the "replied" tag > * 'S' iff the message does not have the "unread" tag > + * 'T' iff the message has the "trashed" tag and > + * state->reckless_trash is TRUE. "trashed" tag? The comment (and the commit message) is incorrect. You only check for reckless_trash in maildir_flags_to_tags, not tags_to_maildir_flags. With this patch, one-way syncing from tags to flags would be done unconditionally. And if I understand the problem correctly, you're fixing the less critical one of the two! I am wondering (but I'm too tired to check) if the original problem could be avoided by simply refusing to sync "deleted" tag to 'T' flag if there are more than one file for that message. This is a dangerous feature, which is why it was originally disabled. Accidentally deleting mail is not something people take lightly. They'll be amused by "reckless trash" - until it recklessly deletes an important mail. However, something like this might be a useful feature to have for people who want to delete mail. It would need good tests to accompany it, though. BR, Jani. > * > * Any existing flags unmentioned in the list above will be preserved > * in the renaming. > diff --git a/notmuch-client.h b/notmuch-client.h > index 63be337..62d1e0e 100644 > --- a/notmuch-client.h > +++ b/notmuch-client.h > @@ -235,6 +235,13 @@ notmuch_config_set_maildir_synchronize_flags (notmuch_config_t *config, > notmuch_bool_t synchronize_flags); > > notmuch_bool_t > +notmuch_config_get_maildir_reckless_trash (notmuch_config_t *config); > + > +void > +notmuch_config_set_maildir_reckless_trash (notmuch_config_t *config, > + notmuch_bool_t reckless_trash); > + > +notmuch_bool_t > debugger_is_active (void); > > #endif > diff --git a/notmuch-config.c b/notmuch-config.c > index 485fa72..613fefc 100644 > --- a/notmuch-config.c > +++ b/notmuch-config.c > @@ -67,9 +67,11 @@ static const char maildir_config_comment[] = > " The following option is supported here:\n" > "\n" > "\tsynchronize_flags Valid values are true and false.\n" > + "\treckless_trash Valid values are true and false.\n" > "\n" > - "\tIf true, then the following maildir flags (in message filenames)\n" > - "\twill be synchronized with the corresponding notmuch tags:\n" > + "\tIf synchronize_flags is true, then the following maildir flags\n" > + "\t(in message filenames) will be synchronized with the corresponding\n" > + "\tnotmuch tags:\n" > "\n" > "\t\tFlag Tag\n" > "\t\t---- -------\n" > @@ -78,10 +80,26 @@ static const char maildir_config_comment[] = > "\t\tP passed\n" > "\t\tR replied\n" > "\t\tS unread (added when 'S' flag is not present)\n" > + "\t\tT trashed (only if maildir_reckless_trash is enabled)\n" > "\n" > "\tThe \"notmuch new\" command will notice flag changes in filenames\n" > "\tand update tags, while the \"notmuch tag\" and \"notmuch restore\"\n" > - "\tcommands will notice tag changes and update flags in filenames\n"; > + "\tcommands will notice tag changes and update flags in filenames\n" > + "\n" > + "\tBy default the maildir synchronization code doesn't look at the\n" > + "\t'trashed' (T) flag on messages. The reason for this behaviour is\n" > + "\tit can be dangerous if another mail client is trying to delete\n" > + "\tduplicates of a message with the same message ID.\n" > + "\n" > + "\tA workaround for this issue is to expunge the duplicate messages\n" > + "\twith the other client before running notmuch new.\n" > + "\n" > + "\tIf you are using only notmuch to handle your emails or are never\n" > + "\tdoing such operations, enabling reckless_trash should be safe,\n" > + "\tbut otherwise it is safer to keep this disabled and delete mails\n" > + "\tmanually with a shell command like:\n" > + "\n" > + "\tnotmuch search --format=files tag:deleted | xargs rm\n"; > > struct _notmuch_config { > char *filename; > @@ -95,6 +113,7 @@ struct _notmuch_config { > const char **new_tags; > size_t new_tags_length; > notmuch_bool_t maildir_synchronize_flags; > + notmuch_bool_t maildir_reckless_trash; > }; > > static int > @@ -251,6 +270,7 @@ notmuch_config_open (void *ctx, > config->new_tags = NULL; > config->new_tags_length = 0; > config->maildir_synchronize_flags = TRUE; > + config->maildir_reckless_trash = FALSE; > > if (! g_key_file_load_from_file (config->key_file, > config->filename, > @@ -353,6 +373,15 @@ notmuch_config_open (void *ctx, > g_error_free (error); > } > > + error = NULL; > + config->maildir_reckless_trash = > + g_key_file_get_boolean (config->key_file, > + "maildir", "reckless_trash", &error); > + if (error) { > + notmuch_config_set_maildir_reckless_trash (config, FALSE); > + g_error_free (error); > + } > + > /* Whenever we know of configuration sections that don't appear in > * the configuration file, we add some comments to help the user > * understand what can be done. */ > @@ -764,3 +793,18 @@ notmuch_config_set_maildir_synchronize_flags (notmuch_config_t *config, > "maildir", "synchronize_flags", synchronize_flags); > config->maildir_synchronize_flags = synchronize_flags; > } > + > +notmuch_bool_t > +notmuch_config_get_maildir_reckless_trash (notmuch_config_t *config) > +{ > + return config->maildir_reckless_trash; > +} > + > +void > +notmuch_config_set_maildir_reckless_trash (notmuch_config_t *config, > + notmuch_bool_t reckless_trash) > +{ > + g_key_file_set_boolean (config->key_file, > + "maildir", "reckless_trash", reckless_trash); > + config->maildir_reckless_trash = reckless_trash; > +} > -- > 1.7.2.5 > > _______________________________________________ > notmuch mailing list > notmuch@notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] lib: Add back the synchronization of 'T' flag with deleted tag. 2012-01-06 22:37 ` [PATCH 1/2] lib: Add back the synchronization of 'T' flag with deleted tag Jani Nikula @ 2012-01-17 5:12 ` Antoine Beaupré 0 siblings, 0 replies; 5+ messages in thread From: Antoine Beaupré @ 2012-01-17 5:12 UTC (permalink / raw) To: Jani Nikula, notmuch [-- Attachment #1: Type: text/plain, Size: 2789 bytes --] On Sat, 07 Jan 2012 00:37:07 +0200, Jani Nikula <jani@nikula.org> wrote: > On Sat, 16 Jul 2011 23:56:12 -0400, Antoine Beaupré <anarcat@koumbit.org> wrote: > > + // TODO: this should probably be moved up in the stack to avoid > > + // opening the config file on every message (!) > > + config = notmuch_config_open (ctx, NULL, NULL); > > The config file is for notmuch the command line tool, *not* for the > lib. You can't call the cli from from the lib. The config (or command > line argument) should be passed as argument, but that would require > changing the lib interface. I see. I wasn't aware of that. > > + * 'T' iff the message has the "trashed" tag and > > + * state->reckless_trash is TRUE. > > "trashed" tag? That should probably be "deleted". > The comment (and the commit message) is incorrect. You only check for > reckless_trash in maildir_flags_to_tags, not tags_to_maildir_flags. > With this patch, one-way syncing from tags to flags would be done > unconditionally. And if I understand the problem correctly, you're > fixing the less critical one of the two! Indeed! What an oversight... > I am wondering (but I'm too tired to check) if the original problem > could be avoided by simply refusing to sync "deleted" tag to 'T' flag if > there are more than one file for that message. That would be a great idea. > This is a dangerous feature, which is why it was originally > disabled. Accidentally deleting mail is not something people take > lightly. They'll be amused by "reckless trash" - until it recklessly > deletes an important mail. Yes, I understand this. > However, something like this might be a useful feature to have for > people who want to delete mail. It would need good tests to accompany > it, though. And to be honest, that's where I got off the boat. :) It just got too hard, and anyways I use a custom script that deletes mails from notmuch search tag:deleted, so syncing that flag isn't so important for me. I guess this got everything covered for me. I would be ready to accept this patch being dropped from the queue, although I think it's a key step in having a more general tag to maildir flags synchronisation strategy that would allow to run notmuch from multiple clients, without having to sync databases around. Thanks for the review, this patch is indeed not ready, and I am not sure when I will have time to push it further. Cheers, A. -- Modern man has a kind of poverty of the spirit which stands in great contrast to his remarkable scientific and technological achievements. We've learned to walk in outer space and yet we haven't learned to walk to earth as brothers and sisters. - Dr. Martin Luther King, Jr. [-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-01-17 5:12 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-07-17 3:56 [PATCH 1/2] lib: Add back the synchronization of 'T' flag with deleted tag Antoine Beaupré 2011-07-17 3:56 ` [PATCH 2/2] lib: add 'safe' setting for flags Antoine Beaupré 2012-01-06 22:45 ` Jani Nikula 2012-01-06 22:37 ` [PATCH 1/2] lib: Add back the synchronization of 'T' flag with deleted tag Jani Nikula 2012-01-17 5:12 ` Antoine Beaupré
Code repositories for project(s) associated with this public inbox https://yhetil.org/notmuch.git/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).