I can't +1 this enough. I have no use gnome-todo until (if) this is implemented.

Created attachment 342502 [details] [review]
Todo.txt: Develop a plugin to handle tasks stored in Text file Author:Rohit Kaushik <kaushikrohit325@gmail.com>

This patch contains a addition of a Todo-txt plugin that can read and save tasks
from text files with format similar to todo.txt.The plugin also has a FileMonitor
that tracks changes in the text file and updates the Todo tasks accordingly.

Bugzilla link:

Review of attachment 342502 [details] [review]:

Overall, the code looks good but still needs some work. Mostly about style, but some changes should be discussed. For example, one way to improve the design of the plugins is by moving the parsing functions inside a GtdTodoTxtParser class, that takes care of all the parsing and only throws back the tasks and tasklists. What do you think?

Besides that, there are some unrelated changes here. For example, the diff in provider-popover.ui, GtdProviderSelector, GtdListSelectorGridItem and GtdListSelectorPanel should be in a different patch. GtdProviderEds changes must be removed from this patch too. The .gitignore and Makefile.in files should not be committed at all. Please remove them from this patch.

Thanks for the great work!

::: configure.ac
@@ +93,3 @@
 GNOME_TODO_ADD_PLUGIN([today-panel], [Today Panel], [yes])
 GNOME_TODO_ADD_PLUGIN([unscheduled-panel], [Unscheduled Tasks Panel], [yes])
+GNOME_TODO_ADD_PLUGIN([todo-txt], [Todo Txt], [yes])

Rename to "Todo.txt" if possible

@@ +143,3 @@
         Today panel ............. ${enable_today_panel_plugin}
         Unscheduled panel ....... ${enable_unscheduled_panel_plugin}
+	Todo txt ................ ${enable_todo_txt_plugin}

Here too.

::: data/todo.gresource.xml
@@ +44,3 @@
   </gresource>
+
+  <!--Todo Txt-->

Here too.

::: plugins/todo-txt/gtd-plugin-todo-txt.c
@@ +1,3 @@
+/* gtd-plugin-todo-txt.c
+ *
+ * Copyright (C) 2015 Rohit Kaushik <kaushikrohit325@gmail.com>

2016

@@ +107,3 @@
+    {
+      gtd_provider_todo_txt_set_monitor (self->providers->data,
+                                         self->monitor);

Put parameters at the same line

@@ +122,3 @@
+  self = GTD_PLUGIN_TODO_TXT (activatable);
+
+    if (self->source)

Wrong indentation. Also, use an early return here (i.e. "if (!self->source)\n return;" please)

::: plugins/todo-txt/gtd-plugin-todo-txt.h
@@ +1,3 @@
+/* gtd-todo-txt-plugin.h
+ *
+ * Copyright (C) 2015 Rohit Kaushik <kaushikrohit325@gmail.com>

Jump a line below. And today is 2016.

@@ +17,3 @@
+
+#ifndef GTD_TODO_TXT_PLUGIN_H
+#define GTD_TODO_TXT_PLUGIN_H

GTD_PLUGIN_TODO_TXT_H

@@ +35,3 @@
+                                    GFile             *second,
+                                    GFileMonitorEvent  event,
+                                    gpointer           data);

Doesn't follow GNOME To Do coding style.

Function name in the same line starting at col 22, first parenthesis at col 66,

::: plugins/todo-txt/gtd-provider-todo-txt.c
@@ +1,3 @@
+/* gtd-provider-todo-txt.c
+ *
+ * Copyright (C) 2015 Rohit Kaushik <kaushikrohit325@gmail.com>

2017

@@ +26,3 @@
+#include <glib/gi18n.h>
+
+gint no_of_lines = 0;

Please, let's avoid global vars. If this is shared between multiple functions, put it inside the GtdProviderTodoTxt struct.

@@ +71,3 @@
+};
+
+typedef struct _TaskData

This can be an anonymous structure.

@@ +82,3 @@
+  GDateTime *creation_date;
+  GDateTime *completion_date;
+  GDateTime *due_date;

Wrong style. The '*' char starts at column 22.

@@ +86,3 @@
+  gboolean is_subtask;
+  gboolean is_task_completed;
+  gint priority;

Ditto about wrong style.

@@ +143,3 @@
+  tdata->is_subtask = FALSE;
+  tdata->is_task_completed = FALSE;
+  tdata->priority = 0;

You don't need to set to FALSE nor 0. 'g_new0()' already does that.

@@ +148,3 @@
+
+static gint
+gtd_provider_todo_txt_get_priority (char *token)

gchar

@@ +165,3 @@
+    }
+  return 0;
+}

Jump lines below "break"s and above "return".

@@ +204,3 @@
+
+  return date;
+}

Why not use the GDate API to parse the date? "g_date_set_parse()" parses dates.

@@ +232,3 @@
+    return FALSE;
+
+  return TRUE;

Ditto about GDate API.

@@ +242,3 @@
+  for (pos = 0; pos < strlen (token); pos++)
+    {
+      if (!isalnum (token[pos]))

g_unichar_isalnum()

@@ +250,3 @@
+
+static gint
+gtd_provider_todo_txt_get_token_id (char *token,

gchar

@@ +303,3 @@
+
+static TaskData*
+gtd_provider_todo_txt_parse_tokens (GList *tk, GtdProviderTodoTxt *provider)

Second param in a line of its own.

@@ +316,3 @@
+  last_read_token = TASK_COMPLETE;
+
+  for (it = tk; it !=NULL; it = it->next)

Space after '!='

@@ +405,3 @@
+    {
+      token_id = gtd_provider_todo_txt_get_token_id (it->data,
+                                                     last_read);

Function params in the same line.

@@ +416,3 @@
+              gtd_manager_emit_error_message (gtd_manager_get_default (),
+                                  _("Task Completion token x should be at the start of line"),
+                                   "Skipping this line");

Maybe just ignore these errors. Polluting the UI with potentially hundreds of error messages is not a good idea.

@@ +429,3 @@
+              gtd_manager_emit_error_message (gtd_manager_get_default (),
+                                  _("Task Prirority should be at the start of line"),
+                                   "Skipping this line");

Ditto about skipping.

@@ +469,3 @@
+              gtd_manager_emit_error_message (gtd_manager_get_default (),
+                                             _("Incorrect Due Date"),
+                                               "Please make sure the due date in todo.txt is valid.Tasks with invalid date are not loaded");

Space after the period. Use _() to localize those strings.

@@ +477,3 @@
+          gtd_manager_emit_error_message (gtd_manager_get_default (),
+                                       _("Unrecognised Token in Todo.Txt line"),
+                                          "Todo cannot recognise some tag in your todo.txt file.Some task may not be loaded");

Typos ("To Do", "Some tasks", space after '.'). Use _() to localize those strings.

@@ +487,3 @@
+      gtd_manager_emit_error_message (gtd_manager_get_default (),
+                                    _("No Tasklist found for some tasks"),
+                                      "Some of tasks in your todo.txt file donot have tasklist.Todo supports taks with Tasklist.Please add list to all your tasks");

Typo ("do not"). Use _() to localize those strings.

@@ +495,3 @@
+
+static GList*
+gtd_provider_todo_txt_tokenize (char *line, GtdProviderTodoTxt *provider)

'provider' comes first. Use gchar.

@@ +505,3 @@
+  {
+    g_strstrip (*token);
+    tokens = g_list_append (tokens, g_strdup(*token));

This is a potentially quadratic code. Either you can simply use the GStrv returned by g_strsplit(), or use g_list_prepend() + g_list_reverse() to avoid the quadratic code.

@@ +516,3 @@
+static GList*
+gtd_provider_todo_txt_get_list_updated_token (gchar *line,
+                                              GtdTaskList *list)

Ditto about param order.

@@ +529,3 @@
+  {
+    g_strstrip (*token);
+    tokens = g_list_append (tokens, g_strdup(*token));

Ditto about quadratic code. Why don't you use gtd_provider_todo_txt_tokenize() here?

@@ +538,3 @@
+
+      token_id = gtd_provider_todo_txt_get_token_id (it->data,
+                                                     last_read_token);

Params in the same line.

@@ +605,3 @@
+
+static GtdTask*
+gtd_provider_todo_txt_create_new_task ()

Should be 'gtd_provider_todo_txt_create_new_task (void)'

@@ +721,3 @@
+static void
+gtd_provider_todo_txt_create_empty_list (gchar *line,
+                                         GtdProviderTodoTxt *self)

Ditto about param order.

@@ +723,3 @@
+                                         GtdProviderTodoTxt *self)
+{
+  if (!g_hash_table_contains (self->lists, line))

Use an early return to lower indentation.

Created attachment 343730 [details] [review]
provider-popover.ui: Set change_location_button to sensitive

Created attachment 343732 [details] [review]
gtd-provider-selector.c:Intialize widget-template before adding providers to provider-selector

Created attachment 343735 [details] [review]
gtd-list-selector-panel.c: save task-list on rename task-list

Created attachment 343737 [details] [review]
gtd-list-selector-grid-item: check if list is valid before adding it to grid

Created attachment 343904 [details] [review]
Todo.txt: Gnome-Todo plugin to manage todo.txt task files

Review of attachment 343730 [details] [review]:

LGTM

Review of attachment 343732 [details] [review]:

Good catch

Review of attachment 343735 [details] [review]:

Good catch again

Review of attachment 343737 [details] [review]:

I'm not sure this is correct. Ideally, we'd never add a NULL list. This is the symptom of another issue, and I think it's better to find out the original cause of this error.

Review of attachment 343904 [details] [review]:

This is a big improvement overall. There are a few issues still. First, I think the plugin can be much smarter wrt selecting the todo.txt file. I had problems when the todo.txt file didn't exist. I propose the following behavior:
 - Initially, when there is no file set, assume the ~/Documents/todo.txt file.
   - Create this file if it doesn't exist.
   - If there is already a ~/Documents/todo.txt file, pick it's tasks.
 - When the user changes the file...
   - Create a new file if it doesn't exist.
   - If the new file exist AND is a valid todo.txt file, pick it
   - If the new file exist AND is ~not~ a valid todo.txt file, refuse to overwrite it - for safety reasons. We might change that later, but no data loss is accepted.

::: plugins/todo-txt/gtd-plugin-todo-txt.c
@@ +92,3 @@
+  GError *file_monitor = NULL;
+
+  self->monitor =g_file_monitor_file (self->source_file,

Style issue.

If self->monitor != NULL, it should drop the ref to it and close it.

@@ +102,3 @@
+                                     _("Error opening file monitor.Todo.txt will not be monitored."),
+                                      file_monitor->message);
+      g_error_free (file_monitor);

g_clear_error()

@@ +275,3 @@
+{
+
+  self->settings = g_settings_new ("org.gnome.todo.txt");

"org.gnome.todo.plugins.todo-txt"

@@ +276,3 @@
+
+  self->settings = g_settings_new ("org.gnome.todo.txt");
+  self->preferences = gtk_file_chooser_widget_new (GTK_FILE_CHOOSER_ACTION_SAVE);

The preferences panel could care some love. I think it's better to have a filechooser button in the middle of the page, that way I can see where the current todo.txt file is. Let's poke the designers about that.

@@ +277,3 @@
+  self->settings = g_settings_new ("org.gnome.todo.txt");
+  self->preferences = gtk_file_chooser_widget_new (GTK_FILE_CHOOSER_ACTION_SAVE);
+  self->source = g_settings_get_string (self->settings, "todo-txt-source");

Settings name: "file"

::: plugins/todo-txt/gtd-todo-txt-parser.c
@@ +20,3 @@
+#include <gtd-todo-txt-parser.h>
+
+struct _GtdTodoTxtParser{

Style issue

::: plugins/todo-txt/gtd-todo-txt-parser.h
@@ +45,3 @@
+  gint                     priority;
+
+} TaskData;

If you have accessor methods, you don't need to expose this struct here. This way, we can fix the parser without breaking any code that depends on it. Move it to the .c file.

@@ +69,3 @@
+GList*        gtd_todo_txt_parser_get_task_line            (GtdTask     *task);
+
+TaskData*     task_data_new                                (void);

Is it used outside this file?

::: plugins/todo-txt/org.gnome.todo.txt.gschema.xml
@@ +1,3 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<schemalist gettext-domain="gnome-todo">
+    <schema id="org.gnome.todo.txt" path="/org/gnome/todo/plugins/todo-txt/">

id="org.gnome.todo.plugins.todo-txt"

@@ +2,3 @@
+<schemalist gettext-domain="gnome-todo">
+    <schema id="org.gnome.todo.txt" path="/org/gnome/todo/plugins/todo-txt/">
+        <key name="todo-txt-source" type="s">

name="file"

::: plugins/todo-txt/todo-txt.plugin.in
@@ +1,2 @@
+[Plugin]
+Name = Todo Txt

Name=Todo.txt

Review of attachment 343730 [details] [review]:

LGTM

Created attachment 344572 [details] [review]
Todo.txt: Gnome-Todo plugin to manage todo.txt task files

Created attachment 344573 [details] [review]
Todo.txt: Gnome-Todo plugin to manage todo.txt task files

Created attachment 344635 [details] [review]
Todo.txt: Gnome-Todo plugin to manage todo.txt task files

Comment on attachment 343730 [details] [review]
provider-popover.ui: Set change_location_button to sensitive

Commited the first patch with improvements on the commit message.

Comment on attachment 343732 [details] [review]
gtd-provider-selector.c:Intialize widget-template before adding providers to provider-selector

Commited with improvements on the commit message.

Comment on attachment 343735 [details] [review]
gtd-list-selector-panel.c: save task-list on rename task-list

Commited with improvements on the commit message.

Thanks for all your patches. This is a great addition to GNOME To Do. Let's test this work and fill more bugs about it.