Ho Jeff,
The code you are going to touch is quite complicated since it has to
handle different special cases, eg:
- when the *whole* app is quitting (it may have many windows, e.g. at
session shutdown)
- when a window is closed
- when all tabs in a window are closed, but not the window itself
Besides, closing the window triggers the unsaved docs confirmation
dialog, which can lead to cancellation of the closing or lead to saving
some docs, which is in turn async and can trigger saving errors which
once again implicitly cancel the closing.
I glanced at the whole thread and I my feeling is that the cleanest way
would be to add a new state. Ideally this new state should also be used
in the gedit code itself - if needed - to remove other hacks.
Maybe the state should probably represent that state of a window *after*
the confirmation has been shown and the docs have been saved, but before
removing the tabs, howver I am not sure this is possible right now since
I seem to recall that we save and remove each tab while looping.
Anyway, my point is: I do not have a definitive answer on the "right
way", this needs quite a bit of investigation and thought and we are
very happy that you would like to work on it! :)
The best place to discuss things interactively is irc, you can find us
on #gedit on irc.gnome.org.
The other right place to discuss large design changes is bugzilla: you
open a bug and describe the issue and the solution so that it is stored
for future reference.
We also encourage contributors to take advantage of git: create your own
tree in github or gitorius and start experimenting away, when you have
something working ask us to review and merge your branch.
Ciao
Paolo
On Tue, 2010-09-07 at 21:26 -0500, Jeff Johnston wrote:
> So these are my initial findings...
>
> 1) Introduce a new GeditWindowState
>
> If I go the approach of introducing a new GeditWindowState variable
> for when gedit is quiting it seems like I could swap out what the
> GEDIT_IS_QUITTING code is doing. This would make sense because
> GEDIT_IS_QUITTING is essentially the only thing that explicitly tells
> other methods that gedit is closing down and feels like a valid window
> "state" to me. However, it seems that the GeditWindowState is very
> closely tied to what is happening to the tab states. Or really it
> looks like the tab states drive the window state. This can be seen in
> the gedit-window.c analyze_tab_state() method.
>
> It seems that introducing a new GeditWindowState is relatively complex
> because right now there is a lot of code that keeps checking the known
> states. Plus it seems like it would be easy for other code to flip the
> state to something else anyway.
>
> 2) New close (quit) event
>
> If I go the route of creating a new close event for when gedit is
> shutting down would the patch be accepted? This is a bigger change to
> the API and there is no reason to go down this path if it is too big a
> change. I still feel like it would be nice to have an event handler
> for just before gedit shuts down, and just after gedit is fully
> initialized.
>
> 3) Put GEDIT_IS_QUITTING on the header file
>
> Of course this is the easiest thing to do. The problem is this may be
> something that is in the code that you do not want exposed at the
> public API level.
>
>
> What approach do you think I should take?
>
>
> Is the best place to have discussions like this? I hope to become an
> active contributor to gedit as I am trying to replace my existing code
> environment to gedit.
>
>
> -Jeff
>
>
>
>
> On Mon, Sep 6, 2010 at 3:42 PM, Jeff Johnston
> <jeff johnston mn gmail com> wrote:
> Great! Which approach should I pursue? The event is the
> cleanest for my needs, but the window state may be less
> intrusive. The window state is the one I have looked at the
> least but when you look at the states it does seem like a
> valid state to have.
>
> Can I write it against the 2.30 release or should it be with
> the trunk code?
>
> -Jeff
>
>
>
>
> On Mon, Sep 6, 2010 at 2:17 PM, Nacho <nacho resa gmail com>
> wrote:
> I guess if you want something done you should file a
> bug and attach a patch there that we can review.
>
>
>
> On Mon, Sep 6, 2010 at 9:14 PM, Jeff Johnston
> <jeff johnston mn gmail com> wrote:
>
>
> Yes. That is ultimately the problem. I never
> really know that gedit is shutting down...at
> least not in anyway in which the tabs are
> still intact. I think there are three ways I
> can do it. Either we create an event that
> gedit calls before shutting down, change the
> window state to GEDIT_WINDOW_STATE_CLOSING
> (maybe), or expose the GEDIT_IS_QUITTING
> variable on the window object in the header
> file. This seems to me the order of how clean
> the approach is as well.
>
> -Jeff
>
>
>
>
>
> On Mon, Sep 6, 2010 at 1:24 PM, Nacho
> <nacho resa gmail com> wrote:
>
>
> On Mon, Sep 6, 2010 at 8:23
> PM, Nacho
> <nacho resa gmail com> wrote:
> hey,
>
>
> why don't you listen
> to tab-added/removed
> signals and save each
> time one of this
> signals happen?
> oh! well you would get the
> tab-removed when the window is
> closing. So forget this.
>
>
>
> Regards.
>
>
> On Mon, Sep 6, 2010 at
> 6:22 PM, Jeff Johnston
> <jeff johnston mn gmail com> wrote:
>
>
> That is the
> problem
> though. The
> tabs are all
> closed down
> before the
> window delete
> event is
> called. Going
> through the
> code I can see
> that a lot of
> the gedit
> environment is
> shut down by
> the time the
> my delete
> event callback
> is invoked.
>
>
>
> gedit-commands-file.c (1766)
>
>
> if
> (unsaved_docs
> == NULL)
> {
> /*
> There is no
> document to
> save -> close
> all tabs */
>
> gedit_window_close_all_tabs (window);
>
> if
> (is_quitting)
>
> gtk_widget_destroy (GTK_WIDGET (window));
>
>
> return;
> }
>
>
>
>
>
>
> On Mon, Sep 6,
> 2010 at 11:18
> AM, Sébastien
> Wilmet
> <sebastien wilmet gmail com> wrote:
>
> On
> Mon,
> 2010-09-06 at 10:51 -0500, Jeff Johnston wrote:
> >
> Looking at this even further I can see that there is no way to tell if
> >
> gedit
> is
> closing and have a way to get at the tabs. This seems like a
> >
> reasonable thing that we should be able to find out. If a new event
> >
> seems
> too
> much
> maybe
> we
> could
> just
> put
> the
> GEDIT_IS_QUITTING on the
> >
> header
> file
> and
> make
> it
> part
> of the
> API?
> That
> or
> have a
> new
> >
> GeditWindowState that says gedit is closing
> >
> (GEDIT_WINDOW_STATE_CLOSING).
> >
> > For
> now I
> do not
> see
> any
> other
> way
> around
> this
> so I
> am
> going
> to
> check
> > the
> GEDIT_IS_QUITTING (but hardcoded in my app) on the window object.
> >
> Keeping track of the tabs is not good enough (and error prone at
> >
> best).
> >
> >
> Whatever the solution I would like to do the work once I know what
> >
> direction to take...
> >
> >
> >
> -Jeff
> Johnston
> >
> >
> >
> > On
> Sun,
> Sep 5,
> 2010
> at
> 9:29
> PM,
> Jeff
> Johnston
> >
> <jeff johnston mn gmail com> wrote:
> >
> >
> I am
> wondering if anyone else would find it useful to have a
> >
> gedit
> close
> event?
> >
> >
> What I
> would
> like
> to do
> is
> save
> the
> tabs
> that
> are
> opened
> when
> >
> gedit
> shuts
> down.
> The
> problem is that gedit closes all the
> >
> tabs
> and
> destroys the window before I have a chance to do
> >
> anything. What would be really useful is to have an event that
> >
> is
> invoked when gedit is shut down, but fires before any
> >
> cleanup is done.
> >
> >
> I saw
> that
> gedit
> sets a
> variable on the window object that
> >
> says
> the
> app is
> quiting. I started to write a 'tab-removed'
> >
> event
> that
> took
> advantage of that, but it felt very hacky. I
> >
> could
> just
> persist the state of the tabs as they are opened
> >
> and
> closed, but that leads to other issues. Ideally I could
> >
> just
> take a
> snapshot of the tabs just before gedit closes
> >
> down.
> >
> >
> On a
> related note this is the flip side of the question I had
> >
> about
> loading tabs on startup. The solution there was to use a
> >
> "realize" event on the window. However, for both startup and
> >
> shutdown I could see it being very useful to have an event
> >
> that
> gets
> invoked when gedit is all the way up and just before
> >
> any
> cleanup is done. That would give custom plugins a last
> >
> chance
> to do
> something with full access to gedit's api while
> >
> it is
> fully
> intact.
> >
> >
> I
> would
> be
> more
> than
> willing to do the work! I have been
> >
> digging into the gedit source code and I think it is very easy
> >
> to
> follow.
> >
> >
> >
> This
> is the
> code I
> found
> (2.30.3) that clearly shows that the
> >
> tabs
> are
> all
> closed
> and
> then
> the
> window
> is
> destroyed.
> >
> >
> gedit-commands-file.c (1766)
> >
> >
> g_object_set_data (G_OBJECT (window),
> >
>
>
>
> GEDIT_IS_QUITTING,
> >
>
>
>
> GBOOLEAN_TO_POINTER (is_quitting));
> >
> >
>
> ...
> >
> >
> if
> (unsaved_docs == NULL)
> >
> {
> >
>
>
> /*
> There
> is no
> document to save -> close all tabs */
> >
> gedit_window_close_all_tabs (window);
> >
> >
> if
> (is_quitting)
> >
> gtk_widget_destroy (GTK_WIDGET (window));
> >
> >
> return;
> >
>
> }
> >
>
>
> Maybe
> it's
> possible to use the "delete-event" signal of the window. But
> your
> handler must be called before _gedit_cmd_file_quit () (i.e. before
> the
> tabs
> are
> closed).
>
>
>
>
> _______________________________________________
> gedit-list
> mailing list
> gedit-list gnome org
> http://mail.gnome.org/mailman/listinfo/gedit-list
>
>
>
>
>
>
>
>
>
> _______________________________________________
> gedit-list mailing list
> gedit-list gnome org
> http://mail.gnome.org/mailman/listinfo/gedit-list
>
>
>
>
>
>
> _______________________________________________
> gedit-list mailing list
> gedit-list gnome org
> http://mail.gnome.org/mailman/listinfo/gedit-list