On Mon, 2011-09-05 at 21:55 +0100, Peter Clifton wrote: > Looking at where I hooked up my code, I have hooked it up in a stupid > place - the file monitoring is cancelled and re-wired every time PCB > handles a menu action! > > It probably needs to hook up so it is only called upon the PCBChanged" > action. Gah - this causes fun and games. I turns out that we only missed our own save action modifications to the PCB file because of the stupid place I hooked up the code. (It cancelled and re-attached the file-monitor right after any menu action - including File->Save of course). I've pushed a few preliminaries (HID API) to git HEAD to allow the core to notify us around a SavePCB() call, and to tell us if the filename of the PCB has changed (e.g. File->SaveAs). Unfortunately, I can't make it work for the SaveAs case. I'm sure I must be missing something simple, but I just can't get the file monitors to ignore changes to the new PCB file despite tracing the fact that we delete the old file monitor, save the PCB (in the new filename), THEN hook up the new file change monitor. I'm _guessing_ that either the saved changes don't hit the disk immediately, or GLib / GIO is helpfully caching the directory state from before I hooked up the file monitor - then falsely detecting a change (as we hook up the new file-monitor right after we wrote to the file). Anyway, I'm not quite sure how to fix it. It might come down to not disconnecting the file-change monitor, but adding a flag somewhere to make the callback ignore the next change we get notified about (after a save). Anyway - if you fancy figuring out just what I'm doing wrong, the patch (work in progress) against git HEAD is attached. > We could consider splitting that action into "PCBChanged" and > "PCBReverted" actions (or add arguments to the former), so the GUI can > do different levels of UI resetting for load and revert. I added a "revert" parameter to the PCBChanged action, so that should be a good start for you looking into this. It appears there will be some special casing in the HID to do, and possibly a bit of work in the core to make sure it doesn't wipe out more state than it needs as it reloads the board. Obviously we need to be careful that we don't get PCB into an inconsistent state reloading a changed board from disk whilst trying to keep "some" view settings. Remember that we could notionally change _anything_ in the PCB file we revert to. Best wishes, -- Peter Clifton Electrical Engineering Division, Engineering Department, University of Cambridge, 9, JJ Thomson Avenue, Cambridge CB3 0FA Tel: +44 (0)7729 980173 - (No signal in the lab!) Tel: +44 (0)1223 748328 - (Shared lab phone, ask for me)
From b3bf8f798e7daf4d9bbf4dac9d0c5725ac6ac911 Mon Sep 17 00:00:00 2001
From: Peter Clifton <pcjc2@xxxxxxxxx>
Date: Mon, 5 Sep 2011 22:18:16 +0100
Subject: [PATCH] hid/gtk: Connect up the file-change monitor in a more
sensible place
I must have had a brain-fail when I hooked it up in the call which sets
the window title. NB: That also gets called after every menu operation!
Hook up the connection in the ghid_sync_with_new_layout() function,
which looks to be a much more appropriate place, however changing this
alone revealed another issue - we now get notified for changes WE make
to the files.
We were avoiding those events as the file-monitor was being reset before
it could pop up, at the end of the menu action which invoked the save).
Hook up to the new HID API (added in the previous commit) which allows
us to temporarily disconnect the file-monitor when PCB's core is saving
to the PCB file.
---
src/file.c | 1 +
src/hid/gtk/gtkhid-main.c | 3 +
src/hid/gtk/gui-top-window.c | 271 ++++++++++++++++++++++++------------------
src/hid/gtk/gui.h | 2 +
4 files changed, 159 insertions(+), 118 deletions(-)
diff --git a/src/file.c b/src/file.c
index be7f6a5..d31b157 100644
--- a/src/file.c
+++ b/src/file.c
@@ -358,6 +358,7 @@ SavePCB (char *file)
gui->notify_save_pcb (file, false);
retcode = WritePipe (file, true);
+ printf ("File changes hitting disk now\n");
gui->notify_save_pcb (file, true);
return retcode;
diff --git a/src/hid/gtk/gtkhid-main.c b/src/hid/gtk/gtkhid-main.c
index 89d50be..a855aa7 100644
--- a/src/hid/gtk/gtkhid-main.c
+++ b/src/hid/gtk/gtkhid-main.c
@@ -2170,6 +2170,9 @@ hid_gtk_init ()
ghid_hid.flush_debug_draw = ghid_flush_debug_draw;
ghid_hid.finish_debug_draw = ghid_finish_debug_draw;
+ ghid_hid.notify_save_pcb = ghid_notify_save_pcb;
+ ghid_hid.notify_filename_changed = ghid_notify_filename_changed;
+
hid_register_hid (&ghid_hid);
#include "gtk_lists.h"
}
diff --git a/src/hid/gtk/gui-top-window.c b/src/hid/gtk/gui-top-window.c
index 73bada5..1277da0 100644
--- a/src/hid/gtk/gui-top-window.c
+++ b/src/hid/gtk/gui-top-window.c
@@ -262,6 +262,130 @@ void ghid_hotkey_cb (int which)
(gpointer) ghid_hotkey_actions[which].node);
}
+static void
+info_bar_response_cb (GtkInfoBar *info_bar,
+ gint response_id,
+ gpointer user_data)
+{
+ GhidGui *_gui = (GhidGui *)user_data;
+
+ gtk_widget_destroy (_gui->info_bar);
+ _gui->info_bar = NULL;
+
+ if (response_id == GTK_RESPONSE_ACCEPT)
+ hid_actionl ("LoadFrom", "revert", "none", NULL);
+}
+
+static void
+file_changed_cb (GFileMonitor *monitor,
+ GFile *file,
+ GFile *other_file,
+ GFileMonitorEvent event_type,
+ gpointer user_data)
+{
+ GhidGui *_gui = (GhidGui *)user_data;
+ GtkWidget *icon;
+ GtkWidget *label;
+ GtkWidget *content_area;
+ char *file_path;
+ char *file_path_utf8;
+ char *markup;
+
+ if (event_type != G_FILE_MONITOR_EVENT_CHANGED)
+ return;
+
+ /* File has changed on disk */
+
+ if (_gui->info_bar)
+ gtk_widget_destroy (_gui->info_bar);
+
+ _gui->info_bar = gtk_info_bar_new_with_buttons (_("Reload"),
+ GTK_RESPONSE_ACCEPT,
+ GTK_STOCK_CANCEL,
+ GTK_RESPONSE_CANCEL,
+ NULL);
+ gtk_box_pack_start (GTK_BOX (_gui->vbox_middle),
+ _gui->info_bar, FALSE, FALSE, 0);
+ gtk_box_reorder_child (GTK_BOX (_gui->vbox_middle), _gui->info_bar, 0);
+
+ gtk_info_bar_set_message_type (GTK_INFO_BAR (_gui->info_bar),
+ GTK_MESSAGE_WARNING);
+
+ g_signal_connect (_gui->info_bar, "response",
+ G_CALLBACK (info_bar_response_cb), _gui);
+
+ file_path = g_file_get_path (file);
+ printf ("File %s has changed on disk (monitor %p)\n", file_path, monitor);
+ file_path_utf8 = g_filename_to_utf8 (file_path, -1, NULL, NULL, NULL);
+ g_free (file_path);
+ markup =
+ g_markup_printf_escaped (_("<b>The file %s has changed on disk</b>\n"
+ "\n"
+ "Do you want to reload the file?"),
+ file_path_utf8);
+ g_free (file_path_utf8);
+
+ content_area =
+ gtk_info_bar_get_content_area (GTK_INFO_BAR (_gui->info_bar));
+
+ icon = gtk_image_new_from_stock (GTK_STOCK_DIALOG_WARNING,
+ GTK_ICON_SIZE_DIALOG);
+ gtk_box_pack_start (GTK_BOX (content_area),
+ icon, FALSE, FALSE, 0);
+
+ label = gtk_label_new ("");
+ gtk_box_pack_start (GTK_BOX (content_area),
+ label, TRUE, TRUE, 6);
+
+ gtk_label_set_use_markup (GTK_LABEL (label), TRUE);
+ gtk_label_set_markup (GTK_LABEL (label), markup);
+ g_free (markup);
+
+ gtk_misc_set_alignment (GTK_MISC (label), 0., 0.5);
+
+ gtk_widget_show_all (_gui->info_bar);
+}
+
+static void
+disconnect_file_change_monitor (GhidGui *_gui)
+{
+ printf ("Disconnect monitor %p\n", _gui->file_monitor);
+
+ if (_gui->file_monitor != NULL)
+ g_object_unref (_gui->file_monitor);
+ _gui->file_monitor = NULL;
+
+ if (_gui->info_bar != NULL)
+ gtk_widget_destroy (_gui->info_bar);
+ _gui->info_bar = NULL;
+}
+
+static void
+connect_file_change_monitor (GhidGui *_gui)
+{
+ GFile *file;
+
+ /* Ensure any existing monitor is disconnected */
+ disconnect_file_change_monitor (_gui);
+
+ if (PCB->Filename == NULL ||
+ *PCB->Filename == '\0')
+ return;
+
+ file = g_file_new_for_path (PCB->Filename);
+
+ /* XXX: Could hook up more error handling for g_file_monitor_file */
+ _gui->file_monitor = g_file_monitor_file (file,
+ G_FILE_MONITOR_NONE,
+ NULL,
+ NULL);
+ g_object_unref (file);
+
+ printf ("Connecting for %s new monitor is %p\n", PCB->Filename, _gui->file_monitor);
+ g_signal_connect (_gui->file_monitor, "changed",
+ G_CALLBACK (file_changed_cb), _gui);
+}
+
/* Sync toggle states that were saved with the layout and notify the
| config code to update Settings values it manages.
*/
@@ -277,6 +401,35 @@ ghid_sync_with_new_layout (void)
ghid_window_set_name_label (PCB->Name);
ghid_set_status_line_label ();
+ connect_file_change_monitor (ghidgui);
+}
+
+void
+ghid_notify_save_pcb (const char *filename, bool done)
+{
+ /* Do nothing if it is not the active PCB file we're watching
+ * that is being saved.
+ */
+ if (strcmp (filename, PCB->Filename) != 0)
+ {
+ printf ("Being notified about save - ignoring save to %s\n", filename);
+ return;
+ }
+
+ printf ("Being notified about save to %s\n", filename);
+
+ if (!done)
+ disconnect_file_change_monitor (ghidgui);
+ else
+ connect_file_change_monitor (ghidgui);
+}
+
+void
+ghid_notify_filename_changed (void)
+{
+ printf ("Notified the PCB file is now under a new name\n");
+ disconnect_file_change_monitor (ghidgui);
+ connect_file_change_monitor (ghidgui);
}
/* ---------------------------------------------------------------------------
@@ -467,119 +620,6 @@ ghid_remove_accel_groups (GtkWindow *window, GhidGui *gui)
(GHID_ROUTE_STYLE_SELECTOR (gui->route_style_selector)));
}
-static void
-info_bar_response_cb (GtkInfoBar *info_bar,
- gint response_id,
- gpointer user_data)
-{
- GhidGui *_gui = (GhidGui *)user_data;
-
- gtk_widget_destroy (_gui->info_bar);
- _gui->info_bar = NULL;
-
- if (response_id == GTK_RESPONSE_ACCEPT)
- hid_actionl ("LoadFrom", "revert", "none", NULL);
-}
-
-static void
-file_changed_cb (GFileMonitor *monitor,
- GFile *file,
- GFile *other_file,
- GFileMonitorEvent event_type,
- gpointer user_data)
-{
- GhidGui *_gui = (GhidGui *)user_data;
- GtkWidget *icon;
- GtkWidget *label;
- GtkWidget *content_area;
- char *file_path;
- char *file_path_utf8;
- char *markup;
-
- if (event_type != G_FILE_MONITOR_EVENT_CHANGED)
- return;
-
- /* File has changed on disk */
-
- if (_gui->info_bar)
- gtk_widget_destroy (_gui->info_bar);
-
- _gui->info_bar = gtk_info_bar_new_with_buttons (_("Reload"),
- GTK_RESPONSE_ACCEPT,
- GTK_STOCK_CANCEL,
- GTK_RESPONSE_CANCEL,
- NULL);
- gtk_box_pack_start (GTK_BOX (_gui->vbox_middle),
- _gui->info_bar, FALSE, FALSE, 0);
- gtk_box_reorder_child (GTK_BOX (_gui->vbox_middle), _gui->info_bar, 0);
-
- gtk_info_bar_set_message_type (GTK_INFO_BAR (_gui->info_bar),
- GTK_MESSAGE_WARNING);
-
- g_signal_connect (_gui->info_bar, "response",
- G_CALLBACK (info_bar_response_cb), _gui);
-
- file_path = g_file_get_path (file);
- file_path_utf8 = g_filename_to_utf8 (file_path, -1, NULL, NULL, NULL);
- g_free (file_path);
- markup =
- g_markup_printf_escaped (_("<b>The file %s has changed on disk</b>\n"
- "\n"
- "Do you want to reload the file?"),
- file_path_utf8);
- g_free (file_path_utf8);
-
- content_area =
- gtk_info_bar_get_content_area (GTK_INFO_BAR (_gui->info_bar));
-
- icon = gtk_image_new_from_stock (GTK_STOCK_DIALOG_WARNING,
- GTK_ICON_SIZE_DIALOG);
- gtk_box_pack_start (GTK_BOX (content_area),
- icon, FALSE, FALSE, 0);
-
- label = gtk_label_new ("");
- gtk_box_pack_start (GTK_BOX (content_area),
- label, TRUE, TRUE, 6);
-
- gtk_label_set_use_markup (GTK_LABEL (label), TRUE);
- gtk_label_set_markup (GTK_LABEL (label), markup);
- g_free (markup);
-
- gtk_misc_set_alignment (GTK_MISC (label), 0., 0.5);
-
- gtk_widget_show_all (_gui->info_bar);
-}
-
-static void
-connect_file_change_monitor (GhidGui *_gui, char *filename)
-{
- GFile *file;
-
- if (_gui->file_monitor != NULL)
- g_object_unref (_gui->file_monitor);
- _gui->file_monitor = NULL;
-
- if (_gui->info_bar != NULL)
- gtk_widget_destroy (_gui->info_bar);
- _gui->info_bar = NULL;
-
- if (filename == NULL)
- return;
-
- file = g_file_new_for_path (filename);
-
- /* XXX: Could hook up more error handling for g_file_monitor_file */
- _gui->file_monitor = g_file_monitor_file (file,
- G_FILE_MONITOR_NONE,
- NULL,
- NULL);
- g_object_unref (file);
-
- g_signal_connect (_gui->file_monitor, "changed",
- G_CALLBACK (file_changed_cb), _gui);
-}
-
-
/* Refreshes the window title bar and sets the PCB name to the
* window title bar or to a seperate label
*/
@@ -608,11 +648,6 @@ ghid_window_set_name_label (gchar * name)
gtk_window_set_title (GTK_WINDOW (gport->top_window), str);
g_free (str);
g_free (filename);
-
- if (PCB->Filename && *PCB->Filename)
- connect_file_change_monitor (ghidgui, PCB->Filename);
- else
- connect_file_change_monitor (ghidgui, NULL);
}
static void
diff --git a/src/hid/gtk/gui.h b/src/hid/gtk/gui.h
index dfed2c6..af59121 100644
--- a/src/hid/gtk/gui.h
+++ b/src/hid/gtk/gui.h
@@ -338,6 +338,8 @@ int ghid_drc_window_throw_dialog (void);
/* In gui-top-window.c */
void ghid_update_toggle_flags (void);
+void ghid_notify_save_pcb (const char *file, bool done);
+void ghid_notify_filename_changed (void);
void ghid_install_accel_groups (GtkWindow *window, GhidGui *gui);
void ghid_remove_accel_groups (GtkWindow *window, GhidGui *gui);
--
1.7.5.4
Attachment:
signature.asc
Description: This is a digitally signed message part
_______________________________________________ geda-user mailing list geda-user@xxxxxxxxxxxxxx http://www.seul.org/cgi-bin/mailman/listinfo/geda-user