@fireclawthefox commented on this pull request.


> @@ -45,6 +45,19 @@
 GeanyPlugin *geany_plugin;
 GeanyData *geany_data;
 
+
+/* Callback function for document activate */
+static void plugin_workbench_on_doc_activate(G_GNUC_UNUSED GObject * obj, 
GeanyDocument * doc,
+                                                                               
  G_GNUC_UNUSED gpointer user_data)
+{
+       /* it seems odd to assert this here,
+        * as an untitled doc (new one) would have no filename.
+        */
+       g_return_if_fail(doc != NULL && doc->file_name != NULL);

Your comment states uncertainty if this should be here or not. Has this been 
clarified yet? If there is actual reason to check for the file_name, it should 
probably be specified in the comment as for why this is done. Otherwise, if it 
works without this filename check, maybe removing it is the better option to 
not cause more confusion.

> @@ -1076,6 +1076,32 @@ gboolean 
> wb_project_dir_file_is_included(WB_PROJECT_DIR *dir, const gchar *filen
 }
 
 
+/** Is @a filename included in the project directory?

shouldn't the @ be in front of 'filename' rather than the 'a'?

-- 
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany-plugins/pull/1576#pullrequestreview-4536579447
You are receiving this because you are subscribed to this thread.

Message ID: <geany/geany-plugins/pull/1576/review/[email protected]>

Reply via email to